-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Add compiler_error!() description for error when multi_threaded feature is enabled but async_executor is not #19334
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?
Add compiler_error!() description for error when multi_threaded feature is enabled but async_executor is not #19334
Conversation
occuring due to missing feature.
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
P.S. Do let me know if I should use a specific PR name when making PRs, since this was my first PR 😅. This is a temporary fix as the original author(s) would need to make this update and push the changes with a new version onto crates.io. If it's too tedious and feels unnecessary, feel free to close the PR rather than merging it. This is just to get familiar with PR process within Bevy Ecosystem and building up more contextual awareness. |
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.
Actually. on second thought, I think that just improving the way the features depend on each other is a much better fix.
We're pretty flexible on this sort of thing: the main thing to do is communicate what was fixed so you can quickly scan the git commit history. I'll go ahead and tweak the title to show you how it might have been better here. |
I think the nicer fix is to just add |
Objective
Fixes #19051.
Solution
The
compile_error!()
is added tobevy_tasks/src/lib.rs
. It triggers specifically when themulti_threaded
feature is active, butasync_executor
is not.The error message is user-friendly, detailing:
Cargo.toml
snippet for a fix.DefaultPlugins
usually handles this automatically.Testing
These changes were tested using a minimal, external project designed to specifically trigger the new
compile_error!()
.How it was tested:
compile_fail_tasks
binary project was set up in an external directory.Cargo.toml
was configured to depend on the localbevy_tasks
, explicitly disabling default features and enabling onlymulti_threaded
.src/main.rs
referenced abevy_tasks
component (bevy_tasks::ComputeTaskPool::new();
) to ensure compilation paths were hit.cargo build -p compile_fail_tasks
from the Bevy root.Results:
The build failed as expected, prominently displaying the custom
compile_error!()
message. While otherSend
/Sync
errors also appeared (confirming the underlying problem this fix addresses), the primary goal of providing a clear compile-time error was achieved.How to test:
Reviewers can easily verify this by:
tests/compile_fail_tasks/Cargo.toml
with:cargo build -p compile_fail_tasks
or simplycargo build
within the current directory.Platforms tested:
macOS (aarch64). As a
#[cfg]
based compile-time check, behavior should be consistent across platforms.Tagging @bushrat011899 for review.