Loading AudioFile from another asset appears to be the cause of broken spatialization

[mgi388]

Summary

rustc 1.78.0 (9b00956e5 2024-04-29)
Bevy version: 0.13.0
`bevy-kira-components` version: main

I created a slightly modified version of the spatial example here: https://github.com/SolarLiner/bevy-kira-components/compare/main...mgi388:bevy-kira-components:spatial-asset-example?expand=1

However, the difference is that instead of source coming from AssetServer.load directly, I have another asset drums.custom file, and that asset embeds the audio file name. This is the contents of drums.custom:

drums.ogg

Then, I have a custom asset loader which loads .custom files and simply takes the contents of it to be a path to the audio file to load, and I keep a handle to it. Then, I use that handle as source once it's loaded. These lines show where the entity is created.

However, also note that if you try and use the same asset name embedded in drums.custom, the issue remains. See my comment in the code, also repeated here:

Passing a direct source to the emitter works as well, but only if it is a different asset path to the one embedded in custom asset. Try changing this to just drums.ogg and notice that spatialization does not work either. I.e. it seems that when something else "owns" the handle spatialization does not work, but if source gets its own "unique" handle, it works.

Expected behavior

Spatialization should still work. In the branch, you can uncomment the direct source and see how it originally works.

Actual behavior

Spatialization does not work, but the audio still plays.

I can't see anything obviously wrong with my use of Bevy Assets, but that's not an impossibility. FYI: I originally discovered this because I had hard-coded audio paths in my WIP project, and when I changed my project to start loading audio files from game asset files, I noticed that spatialization stopped working.

Reproduction steps

• Change into the new example and cargo run
• Just listen to the sound not change at all as the sphere goes around
• Note, I've tested this on main which was 81b696a at the time.

[mgi388]

The bug is that in this block of code: https://github.com/SolarLiner/bevy-kira-components/blob/81b696a334c33a56f78bcb63c8de7d62fd67e931/src/sources/mod.rs#L138-L145

It's possible that Option<&SpatialEmitterHandle> is none by the time this system runs and then we get a main output_destination. I only briefly looked but I did notice that system sets and .before(AudioSourceSetup) was being used so I'm surprised the handle hasn't been added by the time audio_added system runs 🤔

This is not a real fix, but changing the system to the following proved the "embedded" asset load works again:

fn audio_added(
    mut commands: Commands,
    mut audio_world: ResMut<AudioWorld>,
    asset_server: Res<AssetServer>,
    assets: Res<Assets<T>>,
    q_added: Query<
        (
            Entity,
            &Handle<T>,
            &T::Settings,
            Option<&SpatialEmitterHandle>,
            &OutputDestination,
        ),
        Without<AudioHandle<T::Handle>>,
    >,
) {
    for (entity, source, settings, spatial_emitter, output_destination) in &q_added {
        // ---------------- Added this check ----------------
        let Some(_emitter) = spatial_emitter else {
            info!("No spatial emitter, continuing and waiting for next frame");
            continue;
        };

        let output_destination = if let Some(emitter) = spatial_emitter {
            kira::OutputDestination::Emitter(emitter.0.id())
        } else {
            let output_handle = match output_destination {
                OutputDestination::MainOutput => &*audio_world.audio_manager.main_track(),
            };
            kira::OutputDestination::Track(output_handle.id())
        };
        // ...
    }
}

It's not a real fix because this would never end up using main track.

[mgi388]

OK this might be more of a real fix:

fn audio_added(
    mut commands: Commands,
    mut audio_world: ResMut<AudioWorld>,
    asset_server: Res<AssetServer>,
    assets: Res<Assets<T>>,
    q_added: Query<
        (
            Entity,
            &Handle<T>,
            &T::Settings,
            Option<&SpatialEmitter>,
            Option<&SpatialEmitterHandle>,
            &OutputDestination,
        ),
        Without<AudioHandle<T::Handle>>,
    >,
) {
    for (
        entity,
        source,
        settings,
        spatial_emitter,
        spatial_emitter_handle,
        output_destination,
    ) in &q_added
    {
        // ---------------- Added this check ----------------
        // SpatialEmitterHandle is only added in a system which listens to
        // Added<InternalAudioMarker>, and so this system may run before
        // InternalAudioMarker has been added. If there is a SpatialEmitter
        // but no SpatialEmitterHandle, we're not ready to create the audio
        // handle, so bail out and wait for the next frame.
        if spatial_emitter.is_some() && spatial_emitter_handle.is_none() {
            trace!("No spatial emitter handle yet, waiting for next frame");
            continue;
        }

        let output_destination = if let Some(emitter) = spatial_emitter_handle {
            kira::OutputDestination::Emitter(emitter.0.id())
        } else {
            let output_handle = match output_destination {
                OutputDestination::MainOutput => &*audio_world.audio_manager.main_track(),
            };
            kira::OutputDestination::Track(output_handle.id())
        };
        // ...
    }
}

@SolarLiner take a look when you get a chance. If it sounds like the right idea I'll make a PR if you don't get to it first.

[SolarLiner]

If this is a system ordering issue, could this be solved by adding ordering rules instead? I'd rather that than these kinds of checks, but I guess if there aren't any other options you can go for the PR.

[SolarLiner]

Ok so it turns out this is already the case, I'm using system sets to place the add_emitters spatial audio system before the AudioSourcePlugin::audio_added system, so it should always add the spatial emitter handle first, then run the audio source added system, at which point the spatial handle should already be available?

The fact that it isn't is very surprising to me. I guess the only fix is to check for it manually as suggested above, but it feels like something is off in Bevy here.

[mgi388]

Yeah I noticed the system sets and ordering and am surprised as well especially since it works with the direct asset. I have lost the context I had when debugging and I can’t quite remember if I had any more useful info to add yet. I would guess the ordering is right but maybe the query in add_emitter doesn’t match on the frame but the query in audio_added does match thereby causing the issue where that gets run but spatial emitter handle isn’t there yet.

[SolarLiner]

Maybe this is due to a lack of automatic command flush issue, and adding an explicit flush would solve the issue?