Skip to content

Workaround for import diagnostics failing to mention disabled features #19297

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

greeble-dev
Copy link
Contributor

Objective

Users often start a Bevy app by cut and pasting an example. But they can quickly run into errors if the example uses a feature that's disabled by default:

  | use bevy::dev_tools::fps_overlay::FpsOverlayPlugin;
  |           ^^^^^^^^^ could not find `dev_tools` in `bevy`

The fix is simple - enable the bevy_dev_tools feature. But it's hard for users to get from the compile error to the fix (see #19200 (comment))

In some situations the rust compiler will flag up missing features as a possible cause - this might not get users all the way there, but it would be a good start:

note: found an item that was configured out
  --> bevy\src\lib.rs:72:24
   |
   | pub use bevy_internal::dev_tools;
   |                        ^^^^^^^^^
note: the item is gated behind the `bevy_dev_tools` feature
  --> bevy\src\lib.rs:71:7
   |
   | #[cfg(feature = "bevy_dev_tools")]
   |       ^^^^^^^^^^^^^^^^^^^^^^^^^^

But that diagnostic doesn't trigger right now. I suspect it trips over the way the bevy module glob imports bevy_internal::*. I've filed a rustc bug here: rust-lang/rust#141256.

Solution

The workaround is to duplicate the bevy_internal re-exports and their feature gates. This is ugly but simple.

I also considered:

  1. Only import the modules behind feature gates. A bit less verbose but also less simple to update.
  2. Only import bevy_dev_tools and bevy_remote. These are the only features that are disabled by default, so they're likely to cover the majority of cases that users will encounter.
  3. Don't do anything yet, wait to see what happens with Diagnostic doesn't mention cfg-ed item referenced through glob import rust-lang/rust#141256.

The fix could go into 0.16.x if the reference to bevy_anti_aliasing is removed.

Testing

Simulate a user by editing the bevy/Cargo.toml section for example fps_overlay to comment out the required-features = ["bevy_dev_tools"]. Alternatively, copy and paste examples/dev_tools/fps_overlay.rs into a new crate.

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Cross-Cutting Impacts the entire engine X-Contentious There are nontrivial implications that should be thought through S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 19, 2025
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Not too much boilerplate, and the cost of screwing up maintenance isn't high. I don't hate this and think it's worth it on net, but I definitely feel that this is an upstream bug, rather than intended behavior.

@mockersf
Copy link
Member

could you try how it would work with the include_str macro to share a common file for both list of imports?

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 19, 2025
@greeble-dev
Copy link
Contributor Author

could you try how it would work with the include_str macro to share a common file for both list of imports?

I'm not sure how this would work. I don't think I can simply do include_str as the imports are different - bevy wants pub use bevy_internal::foo while bevy_internal wants pub use bevy_foo as foo.

Do you mean I should have a file that's simply a list of crate names and then bevy and bevy_internal should each have a macro that interprets it?

@alice-i-cecile alice-i-cecile added S-Needs-Help The author needs help finishing this PR. and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Cross-Cutting Impacts the entire engine C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Help The author needs help finishing this PR. X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants