`World::register_required_components` can make bundles outdated

[urben1680]

Bevy version

main

What you did

Adding required components manually can make a Bundle outdated. This can cause diverging behavior depending on if you use bundles registered before or after manually requiring components in these bundles.

#[derive(Component)]
struct C1;
#[derive(Component, Default)]
struct C2;

let mut world = World::new();
world.register_bundle::<C1>();
world.register_required_components::<C1, C2>(); // succeeds, no C1 anywhere yet
assert!(world.spawn((C1,)).contains::<C2>()); // succeeds, registers new bundle after requiring C2
assert!(world.spawn(C1).contains::<C2>()); // fails, uses outdated bundle

Worth noting you can register bundles in many ways so a solution that minds only bundles registered by type is not enough.

[alice-i-cecile]

Definitely a real bug. I wonder how easy it is to hit by accident though, and I'm not sure what the right mitigation is.

[urben1680]

The easiest thing to do is to introduce a HashSet<ComponentId>/FixedBitSet into Bundles or Components where every contributed component in a new bundle is inserted into. Then register_required_components could assert that the component a requirement is added to is absent in that set.

[alice-i-cecile]

Hmm. That's plausible as a fix. I worry a bit about performance costs with dynamic components.

[urben1680]

Dynamic components in specific do not need to push themselves into the set, they are already contained in Bundles::dynamic_component_bundle_ids as keys that can be called contains on. Unless you mean the extra check at register_required_components.

EDIT: Thinking about it, dynamic components might need to fix themselves instead of trigger a panic. It is easily possible to not do assert!(!dynamic_component_bundle_ids.contains(id)) but to .get_mut(id) and extend the required components.

[urben1680]

Another idea could be to update each bundle on the first time it is used to insert components. But that would make BundleInfo::required_components returning an Option, no idea what contributed_components should do.