World::register_required_components updates bundles

[urben1680]

Objective

Fixes #18212

Solution

This solution updates bundles containing the component that gets additionally required components registered via World::register_required_components & friends.

I also added a method to Bundles to iterate BundleInfo that contain a certain ComponentId. Needed for the implementation, might be useful for users.

Testing

Added a new test. Also ran insert_simple benchmark to bench bundle registering:

base spawns 10k entities at once, so the bundle registration has more impact than unbatched where the registration happens once for 10k spawns.

Maybe this is too hot and needs more optimization?

It could be improved by simply have no additional field in Bundles. I guess that would bring bundle registration performance to main. But that also means a much more expensive search to update bundles as I would need to iterate every BundleInfo and do a linear search on its contributed components.

[SkiFire13]

I have not reviewed this deep in detail, however I don't see any update to archetype moves, and this most likely makes this incomplete at best and unsound at worst.

Imagine for example a situation with bundle with just component A, and an archetype edge from {} to {A} with just that bundle. At this point make A require B: this would update the bundle to also include B, however the edge would still point to {A}. Then the next time that edge is followed you would try to insert a B in an archetype that doesn't contain it, which looks pretty nonsensical.

[urben1680]

@SkiFire13 The scope of this PR is to make the runtime-added required components that do not error today already to also update the bundles that are affected by that. This still is not updating archetype edges and needs a follow-up to support that.

The World::try_register_required_components + friends return an error (or panic for the non-try variants) if any archetype contains the component that is attempted to add a new requirement to it. This stays like that here, I even added another error case where it can fail.

Effectively, this only regards bundles that are merely registered, not used yet.

So this should all be sound from my point of view, but of course pointing out issues with the implementation is more than welcome.

[alice-i-cecile]

I don't understand this area of the ECS well enough to be able to review this in a timely fashion, sorry :(

[SkiFire13]

Nit for the test: I would also try to test the case where a C->D required component was added later, to ensure that the tracking metadata is properly updated.

[urben1680]

That exposed a bug! 😀

[urben1680]

@SkiFire13 The updated test does fail, but not because of the bundle side of things that I worked on here.

When I change the last bit of the test with this:

// check if another registration can be associated to the bundle using the previously registered component
world.register_required_components::<D, E>(); // note that D is part of the bundle
//let bundle = world.bundles().get(bundle_id).unwrap();
//let contributed: HashSet<_> = bundle.contributed_components().iter().copied().collect();
let contributed: HashSet<_> = world.components().get_info(a_id).unwrap().required_components().iter_ids().collect();

//assert!(contributed.contains(&a_id));
assert!(contributed.contains(&b_id));
assert!(contributed.contains(&c_id));
assert!(contributed.contains(&d_id));
assert!(contributed.contains(&e_id));

Then the last assertion fails.

[urben1680]

I can confirm your PR #20110 fixes this.

[urben1680]

Blocked by #20110 which fixes another bug exposed by a new test in this PR.