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_window #17031

Merged
merged 4 commits into from
Jan 1, 2025

Conversation

bushrat011899
Copy link
Contributor

Objective

Solution

  • Added the following features:
    • std (default)
    • bevy_reflect (default)
    • libm

Testing

  • CI

Notes

  • bevy_reflect was previously always enabled, which isn't how most other crates handle reflection. I've brought this in line with how most crates gate bevy_reflect. This is where the majority of the changes come from in this PR.

@bushrat011899 bushrat011899 added C-Feature A new feature, making something new possible A-Windowing Platform-agnostic interface layer to run your app in X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 30, 2024
@@ -134,6 +134,14 @@ impl Prepare for CompileCheckNoStdCommand {
"Please fix compiler errors in output above for bevy_state no_std compatibility.",
));

commands.push(PreparedCommand::new::<Self>(
Copy link
Member

Choose a reason for hiding this comment

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

We should probably start deduplicating this logic soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah @BD103 and I had a chat about this earlier today. I like the idea of moving this out into a toml, csv or some other "plain data" file format.

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 also think we're rapidly reaching a critical mass of functionality where we could replace this CI task with just an example crate instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice if this could be rolled up into a loop, but the macro requires a string literal, which means no format string.

Copy link
Contributor

@TimJentzsch TimJentzsch left a comment

Choose a reason for hiding this comment

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

I like the improved features documentation!

@TimJentzsch TimJentzsch 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-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 30, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Dec 30, 2024
auto-merge was automatically disabled December 30, 2024 21:42

Head branch was pushed to by a user without write access

@bushrat011899
Copy link
Contributor Author

@alice-i-cecile just ran cargo fmt to fix that CI issue

@TimJentzsch TimJentzsch added the O-Embedded Weird hardware and no_std platforms label Dec 31, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 1, 2025
Merged via the queue into bevyengine:main with commit 10e113d Jan 1, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples 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-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants