Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider making PlaybackSettings::ONCE not the default or removing it #12359

Open
rparrett opened this issue Mar 7, 2024 · 5 comments
Open
Labels
A-Audio Sounds playback and modification C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@rparrett
Copy link
Contributor

rparrett commented Mar 7, 2024

Motivation

Following up on a question posed during the "ecs-based audio" refactor in #8424 (comment) which I don't think was ever really addressed.

This begs the question: should PlaybackMode::Once even exist? Does it even make sense to retain audio-related components, if sound is no longer playing?

As well as this comment on PlaybackMode:

fn default() -> Self {
// TODO: what should the default be: ONCE/DESPAWN/REMOVE?
Self::ONCE
}

Motivated by a recent Discord discussion about "how to replay a sound."

What problem does this solve or what need does it fill?

PlaybackSettings currently defaults to PlaybackMode::Once, meaning the audio will play once, and then the audio entity and its components are left alone and not despawned or removed.

That sounds convenient at first. After the AudioSink is created, it can be re-used, saving you the CPU cycles of spawning, despawning, and creating the sink. Or archetype-moves adding/removing components.

Unfortunately, it really can't be reused. Once the audio in the sink's queue is played, it is gone, and there is no way for a user to refill the sink and start playback again. The only way I know of to restart audio is to remove the AudioSink component from the entity -- play_queued_audio_system will create a new one. That trick doesn't work for playing a different sound with the same sink. See #11862.

So why does this mode exist, and why is the default behavior to leak audio entities into the world and never clean them up?

What solution would you like?

Make the default behavior PlaybackMode::Remove, and add an internal system that cleans up "orphaned" entities when audio components are removed and the entity is otherwise "empty."

Leave PlaybackMode::Once around in case it is useful to someone. Add docs explaining why it's sort of useless?

What alternative(s) have you considered?

  1. Make the default behavior PlaybackMode::Despawn. This is might be what users want most often. Leave Once in case it's useful to someone.

  2. Same, but remove PlaybackMode::Once.

  3. Keep current default behavior, remove the TODO comment.

  4. Add some sort of machinery for reusing a sink by refilling it or filling it with different audio.

    I naively attempted to add append to the AudioSinkPlayback trait, but it required making the trait generic, and it seemed like a mess. It's probably possible, but that route would lead to a pretty inconvenient API that seems counter to the "ecs-based audio." Using change detection on Handle<Source> seems like it would be pretty funky too.

@rparrett rparrett added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Mar 7, 2024
@TrialDragon TrialDragon added A-Audio Sounds playback and modification C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! and removed S-Needs-Triage This issue needs to be labelled labels Mar 7, 2024
@AxiomaticSemantics
Copy link
Contributor

AxiomaticSemantics commented Mar 7, 2024

This is useful to me at least, though bevy_audio needs some patching before it is, but being able to keep a sink on an entity and later play another souncd/disallow playing another sound. At any rate I almost exlusively use ONCE as a policy. Using the other policies is basically asking for lots of archetype table moves. So I'm all fine with it not being the default policy, it is useful.

WRT 2: Add some sort of machinery for reusing a sink by refilling it or filling it with different audio.

See #12035 which is the start of an attempt to cover such a use-case. I'd like to get in shape this release, it's POC atm as I'd like to hear how bevy_audio SME if there is one would like to handle it, my attempt was just to show how I'd like to use it.

@rparrett
Copy link
Contributor Author

rparrett commented Mar 7, 2024

See #12035 which is the start of an attempt to cover such a use-case. I'd like to get in shape this release, it's POC atm as I'd like to hear how bevy_audio SME if there is one would like to handle it, my attempt was just to show how I'd like to use it.

I feel like exposing raw rodio types is probably a non-starter as a user-facing API, but I am definitely not a subject-matter expert.

@AxiomaticSemantics
Copy link
Contributor

See #12035 which is the start of an attempt to cover such a use-case. I'd like to get in shape this release, it's POC atm as I'd like to hear how bevy_audio SME if there is one would like to handle it, my attempt was just to show how I'd like to use it.

I feel like exposing raw rodio types is probably a non-starter as a user-facing API, but I am definitely not a subject-matter expert.

I'm agnostic to a particular implementation, but the behaviour of re-use is very wanted.

@SolarLiner
Copy link
Contributor

Entity commands could fit here, I think. The command would either queue the audio onto the existing Sink if already existing, therefore reusing it, or create a new AudioSink component otherwise.

@SolarLiner SolarLiner added S-Needs-Design This issue requires design work to think about how it would best be accomplished and removed S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! labels Mar 12, 2024
@Magnitus-
Copy link

Magnitus- commented Jun 5, 2024

Thanks for clarifying that for me. I was wondering if I could reuse an existing audio component that doesn't loop for some events (like button clicks) or if I should just re-recreate the component to play the sound each time the event occurs (which seems a little wasteful, hence my hesitation).

I guess it is the later then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Audio Sounds playback and modification C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
Status: Open
Development

No branches or pull requests

5 participants