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

Don't unconditionally enable bevy_render or bevy_assets if mutli-threaded feature is enabled #11726

Merged
merged 2 commits into from
Feb 6, 2024
Merged

Conversation

AxiomaticSemantics
Copy link
Contributor

Objective

bevy_render has been set to be automatically enabled if mutlti-threaded feature is

Solution

make it conditional

@@ -77,7 +77,7 @@ serialize = [
multi-threaded = [
"bevy_asset/multi-threaded",
"bevy_ecs/multi-threaded",
"bevy_render/multi-threaded",
"bevy_render?/multi-threaded",
Copy link
Member

Choose a reason for hiding this comment

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

Neat, I hadn't seen this syntax before. Do you have docs to link for this?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Aha: https://doc.rust-lang.org/cargo/reference/features.html

The "package-name/feature-name" syntax will also enable package-name if it is an optional dependency. Often this is not what you want. You can add a ? as in "package-name?/feature-name" which will only enable the given feature if something else enables the optional dependency.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Core Common functionality for all bevy apps labels Feb 5, 2024
@alice-i-cecile alice-i-cecile added this to the 0.13 milestone Feb 5, 2024
@AxiomaticSemantics AxiomaticSemantics changed the title Don't unconditionally enable bevy_render Don't unconditionally enable bevy_render or bevy_assets if mutli-threaded feature is enabled Feb 5, 2024
@james7132 james7132 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Feb 6, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 6, 2024
Merged via the queue into bevyengine:main with commit f2cb155 Feb 6, 2024
27 of 28 checks passed
@AxiomaticSemantics AxiomaticSemantics deleted the upstream branch February 7, 2024 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Core Common functionality for all bevy apps C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants