-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Make bevy_audio optional for android_shared_stdcxx
#22245
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
base: main
Are you sure you want to change the base?
Conversation
It's a bit unexpected to require `bevy_audio` even when it's not used. I'd treat this feature similar to `std`. It just enables `android_shared_stdcxx` on `cpal` inside `bevy_audio`.
|
I don't remember the impact at the moment, but a shared stdcxx was needed for some things on android and we were relying on cpal to do it for us |
|
I think this makes sense. I can build for Android successfully when using other audio crate without relying on bevy audio and shared stdcxx. And after #20323, bevy audio no longer needs it anyway. |
As it's a shared library, I think testing needs to be actually playing sound on Android, not just building
Nice! |
But the whole point was to build for Android without requiring audio. And if it's enabled - enable |
|
Yup, that was a response to the other comment. #20323 would do this and more, and merging this first would conflict, so I would prefer to wait for it |
|
Ah, got it! Makes sense, but do you think #20323 could go into 0.18? |
In that case can we merge this PR for 0.18? |
|
And it's a single character, pretty sure it will be easy to resolve conflicts for #20323 🙂 |
It is technically a breaking change... but it's probably OK |
Objective
bevy_audioeven when it's not used withdefault_platform, which enablesandroid_shared_stdcxx.Solution
std. It just enablesandroid_shared_stdcxxoncpalinsidebevy_audio.