bevy
bevy copied to clipboard
Optional override for global spatial scale
Objective
Fixes #10414.
That issue and its comments do a great job of laying out the case for this.
Solution
Added an optional spatial_scale
field to PlaybackSettings
, which overrides the default value set on AudioPlugin
.
Changelog
-
AudioPlugin::spatial_scale
has been renamed todefault_spatial_scale
. -
SpatialScale
is no longer a resource and is wrapped byDefaultSpatialScale
. - Added an optional
spatial_scale
toPlaybackSettings
.
Migration Guide
AudioPlugin::spatial_scale
has been renamed to default_spatial_scale
and the default spatial scale can now be overridden on individual audio sources with PlaybackSettings::spatial_scale
.
If you were modifying or reading SpatialScale
at run time, use DefaultSpatialScale
instead.
// before
app.add_plugins(DefaultPlugins.set(AudioPlugin {
spatial_scale: SpatialScale::new(AUDIO_SCALE),
..default()
}));
// after
app.add_plugins(DefaultPlugins.set(AudioPlugin {
default_spatial_scale: SpatialScale::new(AUDIO_SCALE),
..default()
}));
I will try to rebase this soon.
Maybe we also want to add a DefaultSpatialScale setting? I felt like doing this would be sort of ugly -- maybe requiring spatial_scale to be an Option<SpatialScale>, so I didn't do that.
Three months later, I feel like this might be worth the effort. Anyone have an opinion?
Maybe we also want to add a DefaultSpatialScale setting? I felt like doing this would be sort of ugly -- maybe requiring spatial_scale to be an Option, so I didn't do that.
Three months later, I feel like this might be worth the effort. Anyone have an opinion?
DefaultSpatialScale sounds like a good idea. Setting a ton of entities to the same value would be painful. Would it just be a Resource who's value would be used if .with_spatial(true)
is used without .with_spatial_scale()
? Where does the Option come in?
Yeah, I guess things would work more or less like before this PR for configuring the default scale, but PlaybackSettings
would need to use an Option<SpatialScale>
to make
used if
.with_spatial(true)
is used without.with_spatial_scale()
doable. But users wouldn't really have to deal with it, I guess. with_spatial_scale
could still take a SpatialScale
.
It was easier to just re-implement the changes than it was to fix merge conflicts, so I did that and added DefaultSpatialScale
. That wasn't a super small change, so I'd appreciate a re-review.
Much more convenient. Went through it again and it looks great. Let's get this merged!
You might want to update the opening post, especially the changelog and migration guide since they get included in the release info.
Thanks for the reminder. The PR description should be up-to-date now. While updating the migration guide I found an opportunity to make this slightly less breaking by taking a SpatialScale
instead of DefaultSpatialScale
in AudioPlugin
.
I wonder if the From<SpatialScale>
impl is still useful?
Thanks for the reminder. The PR description should be up-to-date now. While updating the migration guide I found an opportunity to make this slightly less breaking by taking a
SpatialScale
instead ofDefaultSpatialScale
inAudioPlugin
.
Very clean!
I wonder if the
From<SpatialScale>
impl is still useful?
Since users never need to construct a DefaultSpatialScale
it doesn't seem useful anymore.