Contributor guide for feature flag usage

[atlv24]

#20714 triggered some discussion about how we should approach features in bevy. There was some disagreement, and ive been thinking more about it and now believe we can reach an agreement. Proposal for a path forward:

• bevy_* crates always only enable the crate they are named after and any transitive bevy_* crate dependency features. This means enabling bevy_mesh will enable bevy_image, because it has a non-optional dependency on it. This means that if you have a no-default-features bevy with only bevy_mesh feature enabled, you can still import bevy::image, even though bevy_image is not enabled. I think this makes sense, because bevy_image is needed to talk about bevy_mesh atm, so having it easily importable is good.
• cross-crate features like gltf animations, which need both bevy_gltf and bevy_animation to work, never enable crate features. They are declared in feature_name = ["bevy_crate?/feature_name"] form, and propagated down to all relevant crates. This means we can have them all in default-features, and when a user wants to "remove" a default crate they can easily copy paste the feature list and just delete the crate-name features bevy_* which they do not want, without worrying about which other features are enabling these.
• cross-crate features never start with bevy_. (i.e. no bevy_ui_picking_backend feature, as thats not an actual crate)
• cross-crate features between bevy_crate1 and bevy_crate2 are named crate1_crate2, in the order that most makes sense, when possible. (lets call it ui_picking instead of bevy_ui_picking_backend, and gltf_animation instead of animation)
• cross-crate features are granular if they can work in isolation: we can make feature groups at a higher level out of more granular features, but not the other way around. (i.e. no picking feature, but yes ui_picking, and we can have a reflect feature which enables a group of more granular reflect_ecs, reflect_camera etc. features, each of which do not force-enable any crates besides bevy_reflect thanks to ? feature syntax. note there is no bevy_reflect feature currently, it is always enabled)
• cross-crate features need not propagate to identically-named features: for example reflect_ecs enables bevy_ecs?/bevy_reflect. There is no reflect_ecs feature on bevy_ecs, as that would be redundant and hurt single-crate UX.

Adopting this pattern means we need to rename a few features and add a few features:

• animation becomes gltf_animation
• bevy_ui_picking_backend becomes ui_picking
• bevy_mesh_picking_backend becomes mesh_picking
• bevy_sprite_picking_backend becomes sprite_picking
• we add sprite_text, sprite_render_text, ui_text, ui_render_text, for gating bevy_text support on each of bevy_sprite, bevy_sprite_render, bevy_ui, bevy_ui_render
• pbr_gizmos, sprite_gizmos features for gating bevy_gizmos support on bevy_pbr and bevy_sprite
• potentially a few more such as discussed in make bevy_mesh optional for bevy_animate #20742
• a new reflect feature which enables a feature group of reflect_* features such as reflect_ecs = ["bevy_ecs?/bevy_reflect"], reflect_app = ["bevy_app?/bevy_reflect"] etc.

[atlv24]

A note on "cross-crate features ... never enable crate features". This has implications: say you have no-default-features bevy with bevy_gltf enabled. This enables a bunch of things transitively, but not bevy_animation. Now say you enable gltf_animation. If we dont enable bevy_animation dependency here, then we either need to gate out all the relevant code behind all(feature = "gltf_animation", feature = "bevy_animation") or accept a compilation error. The alternative, if we decide to force enable bevy_animation, is that then bevy_animation is not accessible via bevy::animation import path, but is in tree.

Open to suggestions here

[atlv24]

I thought about it some more and i think gltf_animation should enable bevy_animation if bevy_gltf is enabled. It makes sense for the reflection case too: if we enable reflect and it enabled all the bevy_*/bevy_reflects then it is effectively enabling bevy_reflect. This is better UX, even if bevy::animation is not available.

[atlv24]

@mockersf need your input here

[mockersf]

Mostly OK for me, except for one part where I don't feel very strongly for (just a bit strongly).

• bevy_* crates always only enable the crate they are named after and any transitive bevy_* crate dependency features. This means enabling bevy_mesh will enable bevy_image, because it has a non-optional dependency on it. This means that if you have a no-default-features bevy with only bevy_mesh feature enabled, you can still import bevy::image, even though bevy_image is not enabled. I think this makes sense, because bevy_image is needed to talk about bevy_mesh atm, so having it easily importable is good.

I don't think they should enable transitive bevy_* crate dependency features. It's the crate responsibility to depend on its dependencies, it shouldn't be encoded in features that are harder to keep in sync, and which potentially enable different features themselves.

If bevy_a depends on bevy_b with default features disabled, then the top level bevy_a feature shouldn't enable bevy_b with default features.

It should be bevy_a responsibility to re-export bevy_b if it can't be used without it. Keeping each crate self-dependant could be also useful for third parties crates that try to keep their (direct) dependencies minimal, and rely on Bevy to pull the exact minimum needed.

---

Another thing that I think we should cleanup about features is the use of default features. I'm starting to think that all bevy_* crates should have 0 default features, and leave it up to its dependencies to enable what they need. Doing this would make it more acceptable for me to have bevy_a enable bevy_b

[atlv24]

I thought a while on this, and while I was initially in favor, I dont think we should get rid of the feature dep tree duplication. If we dont, then DefaultPlugins wont initialize necessary plugins even if they are depended on transitively, leading to runtime errors.

The alternative gets really complicated: we would need to be checking in every crate plugin to see that all its direct dependencies have had their plugins added, but that makes plugin initialization ordering very hard to reconcile, as they aren't always supposed to be added in dependency order. Additionally, they wouldn't be configurable at the top level by DefaultPlugins.set().

I agree with making them all default-features=false where possible though!

[mockersf]

I agree with making them all default-features=false where possible though!

I meant making all our crates except Bevy have 0 default features, so

[features]
default = []

[atlv24]

I don't see why thats better than default-features=false, but im not strongly opposed to that either.

[atlv24]

Hmm. thinking on it some more, its nice because it makes it easy to do things the "right" way (minimize dependency) but it also can hurt direct crate dependency ux a bit? i guess if you're doing that you probably want fine grained control anyways though.

This would also mean that every single crate feature would need to be forwarded through bevy Cargo.toml