-
-
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
Remove the ability to ignore global volume #11092
Remove the ability to ignore global volume #11092
Conversation
IMO, it might be worth leaving the option to play sound without a volume filter (direct sound output).
|
Opting out of other kinds of filters makes sense to me, but not volume control.
It doesn't, but I don't see any reason you would want to ignore the volume setting. Not even a niche one.
Keep the option but as a separate |
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 also can't find any case that would need to use absolute volume.
But it could be useful to implement (in another PR) an option for more global volumes (probably would need another name) so that we could have "volume categories" for things like sound effects, background music, etc. If someone had any good reason for absolute volume, this would make it possible to achieve the same effect in a different way.
Removing this makes sense to me. I've never seen audio software where you can ignore the global volume (usually called Master Volume). If you need functionality like this, it's simple to make. Just set a group volume and multiply it by the entities volume. |
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.
full agreement with this
I'm convinced too. This isn't useful enough to complicate a core bit of the API, even if I do love enums. |
Objective
The ability to ignore the global volume doesn't seem desirable and complicates the API.
#7706 added global volume and the ability to ignore it, but there was no further discussion about whether that's useful. Feel free to discuss here :)
Solution
Replace the
Volume
type's functionality with theVolumeLevel
. RemoveVolumeLevel
.I also removed
DerefMut
derive that effectively made the volumepub
and actually ensured that the volume isn't set below0
even in release builds.Migration Guide
The option to ignore the global volume using
Volume::Absolute
has been removed andVolume
now stores the volume level directly, removing the need for theVolumeLevel
struct.