update documentation on AudioSink

[rdrpenguin04]

Objective

When an AudioSink is removed from an entity, the audio player will automatically start any AudioSource still attached, which normally is the one used to start playback in the first place.

Solution

Long story short, the default behavior is restarting the audio, and this commit documents that.

---

Changelog

Fixed documentation on AudioSink to clarify removal behavior.

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[Selene-Amanita]

What does "unchanged" mean? When it's already playing, or when it was playing but paused in the middle?

Why does this happen? Shouldn't the audio stop? Isn't this a bug? Is a new AudioSink re-added?

Does it also happen if there is no PlaybackSettings on this entity, what about if PlaybackSettings::paused is true?

To be honest I'm not sure why the API is this way, I feel like to be consistent with most other Bevy APIs the AudioSink should be part of AudioSourceBundle not added after, and no audio would be played if an entity doesn't have all the components of AudioSourceBundle including AudioSink, but there's maybe something I don't know/am missing.

[rdrpenguin04]

AudioSink is created by the audio engine dynamically. This may be a bug, but it's difficult to change, and it's genuinely useful. There's a discussion on the Discord about this in the #audio-dev channel (Reproducer and following discussion: https://canary.discord.com/channels/691052431525675048/749430447326625812/1135716276132642878).

If the AudioSource that was used to start the audio hasn't been removed, and the AudioSink is removed, it's recreated from the same AudioSource, thus being an extremely easy way to restart music. Likewise, if the source is swapped out for another one, this provides a clean way to swap from one song to another instantly.

[Selene-Amanita]

It makes sense that that's what's happening.

That's a personal opinion and a bit out of scope for this PR but this API (if it's intended to have to remove the AudioSink to restart an audio track or switch track) feels kinda hacky to me.
I would expect:

• either AudioSink to be added in AudioSourceBundle and play_queued_audio_system would query newly-added AudioSink, or play_queued_audio_system would create AudioSink only for newly added Handle<Source> and PlaybackSettings,
• a system would check for changed Handle<Source> to recreate the AudioSink if the track changed
• a method on either AudioSink or PlaybackSettings would allow the track to restart,
• I would also expect that either PlaybackSettings is deleted when AudioSink is created, or that modifying its fields after would effectively do something

There might be a very good reason for most of it though like not having a one-frame lag in the changes or something.

Anyway, for this PR, I think this comment should clearly mention what's happening, something like:

/// If this component is removed from an entity, and the components from `[`AudioBundle`] are still
/// attached to that entity, a new [`AudioSink`] will be recreated. This will effectively:
/// - reset the playback settings to the values in [`PlaybackSettings`]
/// - restart the audio source from the begining
/// - change the audio source if [`Handle<Source>`](Source) changed

[rdrpenguin04]

The idea is not to explain the implementation details but to explain the effect of them; other parts of the documentation don't explain why the engine does what it does with certain things, so why should this?

Also, the more I think about it, the more natural this API feels. It's odd, but not as odd as it could be.

[Selene-Amanita]

I think when components are added/removed from entities automatically should be documented.

I'm not saying it should be mentioned in the doc of AudioBundle for example, but this is the doc of this component, how it behaves is important.