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

Add no_std support to bevy_app #16874

Merged
merged 7 commits into from
Dec 18, 2024

Conversation

bushrat011899
Copy link
Contributor

@bushrat011899 bushrat011899 commented Dec 17, 2024

Objective

Solution

  • Added the following features:
    • std (default)
    • bevy_tasks (default)
    • downcast (default)
    • portable-atomic
    • critical-section
  • downcast and bevy_tasks are now optional dependencies for bevy_app.

Testing

  • CI
  • Personal UEFI and Raspberry Pi Pico demo applications compile and run against this branch

Draft Release Notes

Bevy's application framework now supports no_std platforms.

Following up on bevy_ecs gaining no_std support, bevy_app extends the functionality available on these targets to include the powerful App and Plugin abstractions. With this, library authors now have the option of making their plugins no_std compatible, or even offering plugins specifically to improve Bevy on certain embedded platforms!

To start making a no_std compatible plugin, simply disable default features when including bevy_app:

[dependencies]
bevy_app = { version = "0.16", default-features = false }

We encourage library authors to do this anyway, as it can also help with compile times and binary size on all platforms.

Keep an eye out for future no_std updates as we continue to improve the parity between std and no_std. We look forward to seeing what kinds of applications are now possible with Bevy!

Notes

  • downcast-rs is optional as it isn't compatible with portable-atomic. I will investigate making a PR upstream to add support for this functionality, as it should be very straightforward.
  • In line with the bevy_ecs no-std-ification, I've added documentation to all features, and grouped them as well.
  • Creating this PR in draft while CI runs and so I can polish before review.

@bushrat011899 bushrat011899 added C-Feature A new feature, making something new possible A-App Bevy apps and plugins M-Needs-Release-Note Work that should be called out in the blog due to impact X-Contentious There are nontrivial implications that should be thought through D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Dec 17, 2024
"concurrent-queue/portable-atomic",
"spin/portable_atomic",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes to bevy_ecs/Cargo.toml could be split into their own PR, but for now I'm just including them here for brevity. I'm happy to split it out if anyone has any concerns.

commands.push(PreparedCommand::new::<Self>(
cmd!(
sh,
"cargo check -p bevy_app --no-default-features --features bevy_reflect --target {target}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that bevy_app is compatible, we can start looking into replacing/augmenting this compile_check_no_std CI task with something more robust and tangible, like a no_std example crate.

Ok(Some(delay)) => std::thread::sleep(delay),
Ok(Some(_delay)) => {
#[cfg(feature = "std")]
std::thread::sleep(_delay);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without access to std::thread::sleep, no_std apps will run as-fast-as-possible by default. This isn't ideal, but I believe a reasonable compromise.

Comment on lines +8 to +9
#[cfg(any(target_arch = "wasm32", feature = "std"))]
use bevy_utils::Instant;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of how we bring this Instant in. I think it'd be worth considering some kind of bevy_platform_support crate which moves things like spin, portable-atomic, critical-section, wasm-time, etc. into a single spot. It'd be more focussed than bevy_utils, and possibly clean up some of the cfg(...) noise we have across Bevy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please!

Comment on lines +7 to +12
#[cfg(not(feature = "downcast"))]
#[doc(hidden)]
pub trait Downcast {}

#[cfg(not(feature = "downcast"))]
impl<T: ?Sized> Downcast for T {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

downcast-rs isn't compatible with portablie-atomic (though it could be with a PR I believe!), so I've included a little dummy trait so the Plugin: Downcast bound can remain.

Comment on lines +23 to +24
#[cfg(feature = "trace")]
use tracing::info_span;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with bevy_ecs, I'm flattening our relationship with bevy_utils::tracing and also relying on log for non-span logging. It's already been mentioned that we should probably move things like log and tracing into workspace dependencies, but I'm just maintaining status quo for now.

@@ -23,7 +24,7 @@ mod plugin;
mod plugin_group;
mod schedule_runner;
mod sub_app;
#[cfg(not(target_arch = "wasm32"))]
#[cfg(all(not(target_arch = "wasm32"), feature = "std"))]
mod terminal_ctrl_c_handler;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't use Control-C in the terminal if your platform doesn't have a terminal...or a keyboard...

@@ -332,6 +333,7 @@ impl SubApp {
}

/// See [`App::get_added_plugins`].
#[cfg(feature = "downcast")]
pub fn get_added_plugins<T>(&self) -> Vec<&T>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only real consequence of losing downcast-rs support on certain no_std targets (not all!). I would like to bring feature parity regardless, but it is fairly minor IMO.

@bushrat011899 bushrat011899 marked this pull request as ready for review December 18, 2024 00:27
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Dec 18, 2024
@alice-i-cecile
Copy link
Member

No serious problems here; let me know when this is ready.

bushrat011899 and others added 2 commits December 18, 2024 11:38
Co-Authored-By: Alice Cecile <alice.i.cecile@gmail.com>
@bushrat011899 bushrat011899 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Dec 18, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 18, 2024
Merged via the queue into bevyengine:main with commit f45e78e Dec 18, 2024
33 checks passed
@TimJentzsch TimJentzsch added the O-Embedded Weird hardware and no_std platforms label Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-App Bevy apps and plugins C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes M-Needs-Release-Note Work that should be called out in the blog due to impact O-Embedded Weird hardware and no_std platforms S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants