Optional override for global spatial scale

[rparrett]

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 to default_spatial_scale.
• SpatialScale is no longer a resource and is wrapped by DefaultSpatialScale.
• Added an optional spatial_scale to PlaybackSettings.

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()
}));

[Ixentus]

Ran the changed example and it worked as expected.

The changes makes sense and the code looks good. LGTM

[rparrett]

I will try to rebase this soon.

[rparrett]

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?

[Ixentus]

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?

[rparrett]

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.

[rparrett]

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.

[Ixentus]

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.

[rparrett]

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?

[Ixentus]

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.

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.