-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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.
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
Programmed soundtrack example #12774
Programmed soundtrack example #12774
Conversation
Create soundtrack example, added new assets, and updated credits.
The generated |
CREDITS.md
Outdated
@@ -28,6 +28,8 @@ | |||
* FiraMono by The Mozilla Foundation and Telefonica S.A (SIL Open Font License, Version 1.1: assets/fonts/FiraMono-LICENSE) | |||
* Barycentric from [mk_bary_gltf](https://github.com/komadori/mk_bary_gltf) (MIT OR Apache-2.0) | |||
* `MorphStressTest.gltf`, [MorphStressTest] ([CC-BY 4.0] by Analytical Graphics, Inc, Model and textures by Ed Mackey) | |||
* Mysterious accoustic guitar music sample from [florianreichelt](https://freesound.org/people/florianreichelt/sounds/412429/) (CC0 license) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can either fix this typo (which came from the link), or allow it in our typos config.
I think I'm in favor of fixing it: the link plus artist name should make this straightforward to discover the source anyways.
examples/audio/soundtrack.rs
Outdated
commands.insert_resource(GameStateTimer(Timer::from_seconds(10.0, TimerMode::Repeating))); | ||
|
||
// Create the track list | ||
let track_1 = asset_server.load::<AudioSource>("sounds/Mysterious accoustic guitar.ogg"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the way this propagates through is unfortunate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the best approach for me to fix this? Do I make a suggestion for every line of code that is affected, or do I "Request Changes" ? Still kind of new to contributing to projects, so unsure of how to proceed from here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the author, just push changes to your branch that fix the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a bit of cleanup :) Really glad this exists, and I'm happy with the approach taken to fade.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice example! Audio is an often overlooked element in game engines, to their detriment.
Glad my approach was satisfactory! |
The generated |
Apologies, I didn't realize if I pushed to my fork, it was going to reflect on this PR (still new to all this). My intention was not to pollute this PR with unnecessary commits. I fixed the typo issue and changed the run condition as suggested. However, I forgot to add the comment changes that were suggested to better reflect the purpose of this example. I also did not change to use keyboard input yet, was waiting for more feedback on it first. I will be more careful in the future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once you've regenerated the example text (CI will yell at you), this is good to merge.
The generated |
source: soundtrack_player.track_list.first().unwrap().clone(), | ||
settings: PlaybackSettings { | ||
mode: bevy::audio::PlaybackMode::Loop, | ||
volume: bevy::audio::Volume::ZERO, | ||
..default() | ||
}, | ||
}, | ||
FadeIn, | ||
)); | ||
} | ||
GameState::Battle => { | ||
commands.spawn(( | ||
AudioBundle { | ||
source: soundtrack_player.track_list.get(1).unwrap().clone(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the tracklist is not dynamic, I think having named fields in the SoundtrackPlayer
resource would make more sense, and remove the indexing accesses into the vec here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did consider this, but was worried that making it too "static" would not be good for the example, so I decided to go with a Vec
to show that this is flexible. I almost created a struct to hold not only the track, but a title and volume setting as well since the original issue mentioned configurable volume, but opted against this for simplicity. This is my first example so still not sure as to how in depth they need to be, or how flexible/dynamic they should be.
I also used unwrap()
statements instead of doing proper error handling since I wanted to keep the code simple and focused on the subject of the example, and if it cannot get the files, then there's not really any point in running the example and panicking should be fine. I also saw that other examples also uses unwrap()
statements.
If I do write other examples in the future, I will make sure to keep your feedback in mind. Any other feedback or pointers is also appreciated.
time: Res<Time>, | ||
) { | ||
for (audio, entity) in audio_sink.iter_mut() { | ||
audio.set_volume(audio.volume() + time.delta_seconds() / FADE_TIME); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing we can do at this stage for the example but this highlights the need for tweening in the audio engine -- this is going to result in sounding like stepwise motion, because it will only update as fast as the game renders (which is 60 or maybe even 120 Hz which is definitely not fast enough to perceive as gradual).
In this case because the fade is 2 seconds each increment is going to be gradual enough, but shorten the fade time and the problem manifests more clearly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also something I considered while programming this. Also, I just did a linear fade on the volume setting, but according to the docs, to decrease the perceived volume by half, you should multiply by 0.316. I figured it was good enough at illustrating the concept while still keeping things simple.
Created soundtrack example, fade-in and fade-out features, added new assets, and updated credits.
Objective
Solution
AudioBundle
is spawned and plays the appropriate track.Timer
for simplicity.GameState
resource.