Remove audio + assets modules in favor of raw Bevy APIs and thin LoadResource API

[cart]

I think we shouldn't encourage something like an assets module containing generic things like ImageHandles, SfxHandles, BgmHandles, etc. Mixing a "footstep" sound and a "button press" sound under the same categorization is (1) not scalable to large numbers of things and (2) a missed opportunity to organize things based on context. Put "button sounds" next button logic, inside a button module. Put "footstep sounds" next to footstep logic.

The whole audio module (PlaySfx, PlayBgm abstractions) should be removed ... too opinionated and too centralized. Triggering clips with their string name is a pattern we should not encourage. We should just use the Bevy APIs (load handles and spawn bundles to play audio). They are more than good enough, and I want people learning how to write Bevy code, not bevy-quickstart code.

To replace the load tracking logic, I've added a "load resource" system, which allows us to define and load Resources as Assets. This allows us to rely on Bevy's built-in asset tracking to determine when the Resource is ready to be inserted. Using this pattern is preferable because it is much closer to what we will encourage after "high level asset-driven state transitions" are implemented.

[janhohenheim]

Thanks! Some first thoughts:

• I have no big thoughts on the asset organization. Personally, I have enjoyed knowing exactly which file I needed to look at if I wanted to change an asset, but I'm sure I would be happy with your solution as well after playing around with it 🙂
• I don't have much personal preference for the audio, other than that I would prefer having a central bottleneck through which all SFX flows as I can then easily control things like "how many sound effects are allowed to play at once?". But such an abstraction is fairly easy to build yourself, so I wouldn't block on that. I however heavily suspect that @alice-i-cecile will disagree with the lack of audio abstraction here as this version comes directly from her design. In fact, since this API is coming from here PR to the Bevy examples, this is closer to Bevy code than to bevy_quickstart code 😅
• If the asset loading can be made to resemble the future official solution, I'm all for it! 😄

Approving the PR since I have nothing against it really. Will wait with merging until others have bikeshed it.

[alice-i-cecile]

Overall opinions:

1. I really like the LoadResource trait. I still wish we had first-party solutions for "wait for everything to load", but this is a really nice use of more sophisticated asset features.
2. I definitely dislike splitting music into multiple resources: you need to manage things like "what track is playing" and handle cross-fading in a single coherent spot (and the code duplication is eh).
3. Moving away from string labels is fine: ultimately you want this to either be enums or data-driven.
4. I have similar complaints about scattering sound effects throughout the codebase. Cohesively doing things like adding spatialization, setting the volume, blending and so on becomes much harder. I think that the assets should live with the content domain that they're part of (player sfx with player sprites for example), but more advanced features require looking at sound effects as a whole :)
5. Reducing complexities / abstractions is good!

[alice-i-cecile]

Overall I'm not opposed to this: it's simpler, and well-suited to a jam. I still think it's harder to polish up, but that's fine in this context.

[cart]

other than that I would prefer having a central bottleneck through which all SFX flows

In general you all are describing a Bus API: context-specific mixable audio targets (ex: A "background music" bus, a "sound effects" bus, etc). This is definitely something Bevy should support, but I will assert that pretty much any abstraction we try to build in bevy-quickstart will be:

1. Incomplete: You mention crossfading, but I'm not seeing any crossfading in the API. If you want to crossfade, you still need to drop down to the individual clips API, which leads me to the next problem.
2. Lossy: by abstracting out the Bevy Audio API in places like PlayBgm and PlaySfx (which serve kind of like pseudo-busses), you have removed Entity from the user-facing playback API. This makes it harder to do context-specific operations directly on the clip (ex: increase / decrease volume, adjust speed, etc). Now the user needs to somehow search for the Entity (aka know that these operations will implicitly spawn an entity, know that you've automatically inserted an IsBgm component, and then they need query for it and hope theres only one). play_bgm() also has implicit behaviors like despawning all currently playing clips. This fully prevents crossfade operations.

I will assert that this is not the platform to be designing such an API. The experts haven't been engaged and the oversight here is too low.

In general, I would like to (again) urge everyone to be conservative when it comes to abstraction building. We should favor doing things "the bevy way" and drive improvements upstream when things are lacking. Adding new abstractions to the template should be a last resort. They should be very scoped and augment the "bevy" way of doing things.

I do agree that it makes sense to be able to categorize and control playback of audio. I think we already have a good tool for that ... marker components (which yall are already making use of in PlayBgm!). This is the pattern we already recommend in our examples:

fn play(mut commands: Commands, player_assets: Res<PlayerAssets>) {
    commands.spawn((
        PlayerBundle {
            source: player_assets.step().clone(),
            settings: PlaybackSettings::DESPAWN,
        },
        SoundEffect,
    ))
}

fn adjust_sound_effect_volume(sinks: Query<&AudioSink, With<SoundEffect>) {
    for sink in &sinks {
        sink.set_volume(0.5);
    }
}

I think this is already significantly preferable to the proposed APIs.

I definitely dislike splitting music into multiple resources: you need to manage things like "what track is playing" and handle cross-fading in a single coherent spot (and the code duplication is eh).

This is going to happen no matter what in the context of scenes, which will each have their own audio that will be loaded and unloaded as the scene is loaded and unloaded. I also think that is "good design" generally (with or without scenes), as it allows people to build cohesive / standalone sets of functionality that nest into a larger whole.

I will assert that constructing a thing that knows every possible piece of music that could play ahead of time prevents modularization for very little benefit. We can answer questions like "what is playing" and "how do we cross-fade" without having an uber-struct. In fact, I consider those problems to be fully independent of "where do handles live". Finding what is currently playing is a question for the live data on the spawned entities, not an uber-handle-manager-resource. Any view you try to build there will be incomplete and risks being out of date. Ditto for cross fading (which includes answering the question "what is playing" and then adjusting the volume per-frame between old and new audio entities). If/when/how these things happen is context specific. Any abstraction you try to build over them has a high risk of not being the right fit. Hiding these mechanisms from users isn't doing them any favors.

Again, I think marker components and raw Bevy Audio APIs are the move here.

[cart]

I've added marker components, per the discussion above.

[janhohenheim]

That sounds fair enough. Thanks for the explanation. The marker components indeed would make something like "play only 25 SFX at the same time" or "generally crossfade between all BGM" easy to implement: just add an observer for Trigger<OnAdd, SoundEffect>.
I'll leave some minor comments, but I fully agree with the PR now :)

We actually already went from Sfx to SoundEffects and then back again to Sfx because we noticed that the extra wording ended up making the code more verbose but not more clear. This is the reason why we are using Bgm over Soundtrack: it's a bit less correct, but way smaller typographically. I would like to see one of the following two changes:

• Change SoundEffect to Sfx and BackgroundMusic to Bgm, or
• change BackgroundMusic to Soundtrack

[cart]

Change SoundEffect to Sfx and BackgroundMusic to Bgm or change BackgroundMusic to Soundtrack

I think we should name these whatever we would call them in the context of a theoretical "volume control screen" in a game. I haven't seen sfx or bgm in any volume control screen (to my knowledge). I do often see "sound effects" and "music" or "background music".

I think "soundtrack" implies a more linear / intentionally listenable collection of music. I consider it to be a "more specific category" that is a subset of "background music". Aka you buy "the soundtrack" for a game. Thats not necessarily the same thing as the category of audio playing that is generally music.

I'm actually most inclined to use SoundEffect and Music

Some prior art:

[janhohenheim]

@cart those comparisons are a very good point for using "music" over "soundtrack", you've convinced me.
My experience from having tested both writing variants leads me to still prefer the short variants for development, but I'm ultimately fine with the long versions as well.

[benfrankel]

IMO code and UI are different contexts with different requirements. My personal preference would be sfx and music.

[cart]

I've opted for SoundEffect and Music.

IMO code and UI are different contexts with different requirements. My personal preference would be sfx and music.

While I do agree, generally, I also see no reason to have two separate concept names that then later need to be coupled in these contexts. Bevy generally favors unshortened names for things in the interest of clarity (with some exceptions for uber-highly trafficked types like Res and Mut). I think it makes sense to follow that convention here. In the world of autocomplete, those extra characters don't cost much.

[cart]

I think this is merge-able now.

[cart]

Ah forgot the dependency comment.

[cart]

Also worth calling out that ultimately we'd like to have something like this:

#[derive(Resource, AssetCollection, Reflect, Clone)]
pub struct PlayerAssets {
    pub ducky: Handle<Image>,
    pub steps: Vec<Handle<AudioSource>>,
}

Where the dependency attribute is implicit. When that rolls out this code should change to use it.

(it would probably also support defining the paths inline as attributes)

[cart]

Comments have been resolved!