Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

radio #69

Merged
merged 4 commits into from Nov 8, 2019
Merged

radio #69

merged 4 commits into from Nov 8, 2019

Conversation

jaan242
Copy link
Contributor

@jaan242 jaan242 commented Nov 2, 2019

Implemented playing radio stations in vehicles. Sounds are played in the order they are stored in files. Only driver ped can change station using the same buttons which are used for switching weapons (E and Q on a PC, arrows at bottom of screen when using touch input). Selected radio station is not currently synced between players.

@in0finite
Copy link
Owner

in0finite commented Nov 2, 2019 •
edited

  • GameManager should not have anything with radio
  • Vehicles.Radio should not be singleton component. Actually, it can be completely removed. All radio-related code should be in Vehicle_Radio.cs.
  • Every vehicle should have it's own AudioSource component for playing radio. This way, you can play multiple radios at the same time, which is good if we want to hear radio sound in 3d.
  • Don't use m_ped == Ped.Instance, but use m_ped.IsControlledByLocalPlayer
  • You should not use AudioManager.PlayStream() which creates new game object every time, but instead use AudioManager.CreateAudioClipFromStream() and assign the created clip to AudioSource. Also, destroy this clip after it's finished playing, or otherwise you will have memory leak.
  • Problem with radio stations is that sounds are not in correct order, and that there are many intros and outros to a single song. So you can't just play them in the order they are stored. But for now, let's leave it like this. - this is already explained in Playing radio stations #29
  • You didn't understand what I meant regarding touch input. You don't have to change it. It already has buttons for switching to next/previous weapon, which can be used to switch radio stations when in vehicle. So, revert changes to TouchInput.cs and UI.prefab.
  • Starting/stopping radio : I think it's better to do this from Vehicle.Update(), because if ped dies, noone will turn off radio.

@jaan242
Copy link
Contributor Author

jaan242 commented Nov 3, 2019

Driver has different touch input, so no changes there, but improved other things.

@in0finite
Copy link
Owner

So, GameManager.prefab can be reverted now ?

in0finite
in0finite requested changes Nov 3, 2019
View reviewed changes
Assets/Scripts/Behaviours/Vehicles/Vehicle.cs Outdated
@@ -416,7 +417,18 @@ private void Update()
{
UpdateColors();
}


if (currentRadioStation != 0 && Ped.Instance.CurrentVehicle == this)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ped.Instance can be null.

Ped.Instance must be the driver.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why driver? passenger cant listen to radio?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, passenger should hear radio. But, this way, when ped enters a vehicle as pessenger, ContinueRadio() will be called which will switch the current song. Am I right ?

Copy link
Contributor Author

@jaan242 jaan242 Nov 6, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently radio station is not synced between players so I don't see a problem here. And it switches to next song only if last one has stopped playing.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it switches to the next song always, because radio.isPlaying will be false when ped exits the vehicle. Correct me if I am wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it will switch song when ped (driver or passenger) enters vehicle. I guess radio should be started in VehicleSittingState.EnterVehicleInternal then?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, you can do it from there. But make sure to restore previous time of the song. Don't restart the song every time when ped enters the vehicle.

Assets/Scripts/Behaviours/Vehicles/Vehicle_Radio.cs Outdated
}
else
{
currentRadioStationPos = Random.Range(radioStartPos, radioEndPos);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Station time should not be randomized every time you switch to that station. It should be randomized on game start.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where exactly is good place for that? Vehicle.Start ?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static field initializer or OnLoaderFinished() callback in some manager script

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Vehicle.Start because:

UnityException: RandomRangeInt is not allowed to be called from a MonoBehaviour constructor (or instance field initializer), call it in Awake or Start instead.

@in0finite in0finite added audio feature New feature labels Nov 3, 2019
@jaan242
Copy link
Contributor Author

jaan242 commented Nov 6, 2019

Reverted GameManager.prefab
Added not null check to Ped.Instance
Radio station and time randomized in Vehicle.Start

@in0finite
Copy link
Owner

Let me tell you how I imagined radio station times. Each vehicle has it's own active station. But, stations in all vehicles have the same current time (song and song's time). So, if ped changes vehicles, and plays the same station, he should hear the same song.

With your setup, each vehicle has it's own current time for station. Plus, all stations have the same current position. It should not be like this. It should mimic the real world.

@jaan242
Copy link
Contributor Author

jaan242 commented Nov 6, 2019 •
edited

Radio station playback position is kept across vehicles and station switches.

@in0finite
Copy link
Owner

Ok. Will test this tomorrow.

@in0finite in0finite merged commit 679dd46 into in0finite:dev Nov 8, 2019
in0finite pushed a commit that referenced this pull request May 31, 2020
@jaan242 @in0finite
radio (#69)
92bbc4b 
* radio

* radio 2

* radio 3

* radio 4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@in0finite in0finite in0finite requested changes

Assignees
No one assigned
Labels
audio feature New feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jaan242 @in0finite