-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
opt-out multi-threaded
feature flag
#9269
Conversation
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
Hmm. Why make this a non-default feature rather than making sure users can set a single top-level feature flag? Enabling multithreading seems like a defensible default behavior for |
Sorry, I don't quite get the if we enable default |
It looks like your PR is a breaking change, but you didn't provide a migration guide. Could you add some context on what users should update when this change get released in a new version of Bevy? |
Okay, I'm content with that justification. @james7132 and @mockersf will likely have opinions too on the best way to do this. |
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.
This definitely worsens the experience for people using bevy_ecs by itself, but I don't think we should negatively impact the experience of using it as a dependency for everyone else by requiring them to disable default features every time they use it.
We may need to explicitly document this as it does seem like most consumers of bevy_ecs, by default, want multi threaded execution.
This is also a breaking change and needs a migration guide.
@flisky can you please add a migration guide section to the PR description and then I'll merge this. Let us know if you'd like a hand with that! |
@alice-i-cecile thanks for the reminding, and any suggestion is appreciated. |
Improved wording for you! |
@alice-i-cecile Thank you! |
This should still be explicitly documented, either in Cargo.toml for bevy_ecs or in some README, but I don't think we should block this PR on this. |
Objective
Fixes #9113
Solution
disable
multi-threaded
default featureMigration Guide
The
multi-threaded
feature inbevy_ecs
andbevy_tasks
is no longer enabled by default. However, this remains a default feature for the umbrellabevy
crate. If you depend onbevy_ecs
orbevy_tasks
directly, you should consider enabling this to allow systems to run in parallel.