-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Application lifetime events (suspend audio on Android) #10158
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very useful fix, with good support for alternative audio backends. I like the straightforward lifetime events pattern, and I look forward to seeing how this evolves in the future.
Non-blocking, but does it make more sense to use a State
rather than and Event
for the Lifetime
information? It feels like users may want to know about the current state, rather than just the transitions.
On most platforms, this is done by callbacks which translates more easily to events. Adding a state later if there's need for it should be easy enough. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leinan's concerns about resuming audio paused for other reasons are blocking for me. Good catch!
the code resuming audio is in the example code, and is not general. it uses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I can live with that. We can improve it later.
examples/mobile/src/lib.rs
Outdated
) { | ||
for event in lifetime_events.read() { | ||
match event { | ||
Lifetime::Suspended => music_controller.single().pause(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we gate these only for Android builds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they will start to be useful on iOS with next version of winit, and they don't hurt for now
Co-Authored-By: Marco Buono <418473+coreh@users.noreply.github.com>
derive(serde::Serialize, serde::Deserialize), | ||
reflect(Serialize, Deserialize) | ||
)] | ||
pub enum ApplicationLifetime { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bikeshedding heavily here, but ApplicationLifetime
seems to suggest this is not an event but a config option of some kind? ApplicationLifetimeEvent
is more verbose, but more explicit in what it's denoting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to avoid events called ...Event
. The variants makes it clear enough what it is for me
) # Objective - Handle pausing audio when Android app is suspended ## Solution - This is the start of application lifetime events. They are mostly useful on mobile - Next version of winit should add a few more - When application is suspended, send an event to notify the application, and run the schedule one last time before actually suspending the app - Audio is now suspended too 🎉 https://github.com/bevyengine/bevy/assets/8672791/d74e2e09-ee29-4f40-adf2-36a0c064f94e --------- Co-authored-by: Marco Buono <418473+coreh@users.noreply.github.com>
) # Objective - Handle pausing audio when Android app is suspended ## Solution - This is the start of application lifetime events. They are mostly useful on mobile - Next version of winit should add a few more - When application is suspended, send an event to notify the application, and run the schedule one last time before actually suspending the app - Audio is now suspended too 🎉 https://github.com/bevyengine/bevy/assets/8672791/d74e2e09-ee29-4f40-adf2-36a0c064f94e --------- Co-authored-by: Marco Buono <418473+coreh@users.noreply.github.com>
Objective
Solution
audio.mp4