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.

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

added temporary functions to control audio volume #657

Closed
wants to merge 6 commits into from
Closed

added temporary functions to control audio volume #657

wants to merge 6 commits into from

Conversation

Dash-L
Copy link
Contributor

@Dash-L Dash-L commented Oct 11, 2020

This PR is in reference to #650. I should start off by saying I'm very new to GitHub, I don't really want to merge this into master, simply into a bevyengine/bevy better-audio branch so that more work can be done on the system before it actually gets used, however, I cannot find a way to do that so here we are. All I did was make it so audio can be played at specified volumes with play_volume() and play_source_volume(), this is not a breaking change as the other audio functions were, for all intents and purposes, unchanged and will work as they used to.

Signed-off-by: Dashiell Elliott <dashiell.elliott@yahoo.com>
@memoryruins memoryruins added A-Audio Sounds playback and modification C-Feature A new feature, making something new possible labels Oct 11, 2020
@Dash-L
Copy link
Contributor Author

Dash-L commented Oct 11, 2020

Something to note is that we should probably migrate to rodio 0.12 before we do anything more with audio, see #638 for discussion about that.

@Dash-L Dash-L changed the title added temporary functions to control audio volume bevyengine/bevy#650 added temporary functions to control audio volume #650 Oct 11, 2020
@Dash-L Dash-L changed the title added temporary functions to control audio volume #650 added temporary functions to control audio volume Oct 11, 2020
@cart
Copy link
Member

cart commented Oct 12, 2020

I like this idea! I created a "better-audio" branch for you. Feel free to change the base of the pr to that.

@Dash-L Dash-L changed the base branch from master to better-audio October 12, 2020 19:14
@Dash-L
Copy link
Contributor Author

Dash-L commented Oct 15, 2020

So I migrated to rodio 0.12 using thread local resources, but I may have done something wrong as seen by that last commit "Merge branch 'master' into better-audio." I had previously updated the master branch of my local fork and that was my way of updating the better-audio branch so I could use the thread local resources, not sure if I did it correctly though. I plan to add something like an AudioDescriptor struct that is used to configure audio and I think it would make sense to use events to control audio playing and stopping since it will all be running on the main thread now.

@cart
Copy link
Member

cart commented Oct 15, 2020

I'm starting to think that the "better-audio" branch isn't the best way to approach this. While experimenting, I think its probably better for you to maintain your own branch (and potentially a 3rd party audio plugin if you want people to be able to test it in their bevy games)

Double reviewing code (once for better-audio and once for master) doesn't sound very appealing to me and it limits your ability to experiment freely.

I also think we should do the rodio upgrade to 0.12 in a separate pr so we can merge that quickly.

@Dash-L
Copy link
Contributor Author

Dash-L commented Oct 16, 2020

Ok, Im going to close this PR and (if there isn't one already) open a PR for the rodio upgrade.

@Dash-L Dash-L closed this Oct 16, 2020
@Dash-L Dash-L deleted the better-audio branch October 16, 2020 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Audio Sounds playback and modification C-Feature A new feature, making something new possible
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants