Mutually exclusive components

[eugineerd]

Objective

Implement mutually exclusive invariant from #1481, allowing enum-like components that cannot co-exist on the same archetype.

Goal: #23569

Solution

Add World::register_mutually_exclusive_components that allows to register which components are incompatible with each other and would be removed if any of the others are inserted.

    world.register_mutually_exclusive_components::<(CompA, CompB)>();

    let e = world.spawn(CompA).id();
    world.entity_mut(e).insert(CompB);
    assert!(!world.entity(e).contains::<CompA>());
    world.entity_mut(e).insert(CompA);
    assert!(!world.entity(e).contains::<CompB>());

    world.spawn((CompA, CompB)); // panic

This is a minimal implementation which just makes this pattern integrated with the ecs - World::register_mutually_exclusive_components and ComponentInfo::mutually_exclusive are the only public interfaces featured in this PR.

Possible extensions that are left to future PRs:

• #[derive(Component)] shortcut to define mutually exclusive components more ergonomically.
• Allow to define groups to make it possible to add components to be mutually exclusive with multiple others without having to list them all.
• Make Query filters aware of mutually exclusive components so that entities with different mutually exclusive components don't cause conflicts.
• Enforce ownership semantics of LINKED_SPAWN relationships.
• Return Result from World::spawn* and Commands::spawn* with an error if a bundle contains mutually exclusive components instead of a panic. Or log an error/warning and resolve conflicts by choosing last (or first?) mutually exclusive component.

Testing

bundle/tests.rs contains new tests for basic functionality, miri passes.

[github-actions]

It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note.

Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes.

[Jondolf]

I like this a lot! This has been something that I'd really like for Avian, as we've been planning to split the RigidBody enum into mutually exclusive DynamicBody, KinematicBody, and StaticBody components. This is already possible to hack together via hooks, but it's not ideal, and the lack of a first-party "mutual exclusiveness" invariant has kept me wary of using the pattern.

I did an initial cursory review of the code, and it looks pretty solid already. I like the tests! I'm not too familiar with the archetype code though, so I'll defer checking its correctness to the ECS wizards :)

Some general initial thoughts:

• I like starting minimal here, and a World API is enough for my purposes. More user-facing derive shorthands or Query integration would be nice, but can totally be left to future PRs IMO.
• I like the default behavior of removing existing components that are incompatible with a new insertion, and panicking when adding two exclusive components simultaneously. I can see others maybe wanting different behavior in some cases, like not inserting an incompatible component instead of replacing an existing one, or emitting a warning instead of panicking, but I think the behavior in this PR is the right default. If other options are strongly desired, we can consider adding them in other PRs.
• Some usage docs and examples could be nice! Though right now, this is not as user-facing as it's only a World API.

I'll try to do a more thorough review later

[urben1680]

I don't feel comfortable reviewing all this table and insertion stuff, but I pointed out a potential memory issue with this on Discord I just want to repeat here.

If you have 100 components exclusive to each others, 100 ComponentInfo will contain each a vector with 99 ids. On Discord a user already mentioned to have a use case of 50 components and growing. So this is can grow squared.

I gave this some more thought and a very space efficient but probably horribly performing solution would to not store a vector here but an Option<ComponentId> pointing to the next component this is exclusive to. This should be a circle so you can iterate all of them no matter at which you start. You just need to keep checking when it repeats. Adding to the group or fusing groups would require iterating all to find the "previous" component to make it point to the new component/group, if that bothers you the you would have to store a Option<(ComponentId, ComponentId)> instead so you can iterate backwards too.

Again, this is just a suggestion, assuming initial registrations and setting query states is just a one-time cost that could be okay to be slower with mutually-exclusive components.

With that bevy could join the glorious history of linked lists in Rust.

[eugineerd]

100 components all being mutually exclusive should take 100 * 99 * 8 bytes < 80KB (not accounting for vec reservation), which is not too critical, I think. There are ways to make it more memory-efficient (introduction of groups would be a good candidate for deduplication), but I don't see a reason to do that just yet.

[eugineerd]

Thinking about your suggested approach, it also can't represent configurations like this:

A  xor B
C  xor B
A !xor C

(...although I'm not sure where this would actually be useful)

[urben1680]

I think that is true. Let's stick with simple vectors for now.

[urben1680]

Possible extensions that are left to future PRs:

• Make Query filters aware of mutually exclusive components so that entities with different mutually exclusive components don't cause conflicts.

I wonder if this turns out tricky if we want that conflict check to stay at WorldQuery::init_state but the exclusiveness of two components has not yet been registered. If that turns out to not be working well we might not want to make the registration public here yet in case we want it to be a Component derive thing only that queries can see even if the component is not registered yet.

[eugineerd]

I wonder if this turns out tricky if we want that conflict check to stay at WorldQuery::init_state but the exclusiveness of two components has not yet been registered.

Since mutually exclusive components can only relax conflict checks, adding new mutually exclusive components shouldn't affect any existing queries and so no new validation panics should happen. This would probably behave similar to DefaultQueryFilters in that case.

[chescock]

It would be exciting to see this land! Cutting the scope by ignoring #[derive(Component)] and Query in the first version is a good approach, although it might be good to have some concrete plans on how to integrate with Query to prove that this is a good data model.

[chescock]

This check ensures that no archetypes exist, but you check the data when BundleInfo is constructed. If someone does

world.register_bundle::<(A, B)>();
world.register_mutually_exclusive_components::<(A, B)>();
world.spawn((A, B));

The the BundleInfo for (A, B) will be successfully registered and cached by register_bundle, then register_mutually_exclusive_components will succeed because there are no archetypes with either component, but then spawn will succeed by using the cached BundleInfo.

... Oh, wait, but this is the same pattern that required components use. I think that might be an existing bug, then? ... Yup, I can trigger required components being ignored. Let me spin that out into an issue... #22829. EDIT: There was already an issue for that bug: #18212.

[eugineerd]

Yeah, that's a problem. Should probably be fixed before this can move forward. A linear BundleInfo scan might not be too problematic since at the point where mutually exclusive components are registered there shouldn't be too many bundles registered, but I'm not sure.

[eugineerd]

Another solution that is maybe worth considering is to move this check to the edge caching step in BundleInfo::insert_bundle_into_archetype instead so the "no archetypes with these components exist" rule will be enough to uphold the invariant.

[eugineerd]

Went ahead and implemented it that way. One nice thing about moving the check from bundle registration is that removing a bundle of mutually exclusive components doesn't panic anymore.

[eugineerd]

some concrete plans on how to integrate with Query to prove that this is a good data model.

My plan was to just copy whatever DefaultQueryFilters does and add all components that are mutually exclusive with components inside With to Without.

[Jondolf]

I'll leave reviewing the correctness of the ECS internals to people who are more familiar with them, but the code overall looks pretty good, and I approve of the feature and direction as well as the API for an initial PR; we can add e.g. a possible macro attribute API in a follow-up.

This feature is strongly desired for Avian, so I hope this gets attention soon :)

[JaySpruce]

As someone allegedly familiar with the ECS internals, the insertion logic looks good to me.

Just a thought, maybe "incompatible components" could be the feature's name? Less of a mouthful at least

[micttyoid]

What about something like the opposite of require?

[eugineerd]

Just a thought, maybe "incompatible components" could be the feature's name? Less of a mouthful at least

Maybe, although I feel like "mutually exclusive" is a more precise definition for this type of behavior. Another alternative from #1481 is "disjoint", although it feels a bit less clear imo. I don't feel too strongly about that though.

[natepiano]

I'm a big fan of mutually exclusive - it precisely defines what is going on and I'm pretty sure it's commonly known. I'm looking forward to this feature dropping so i can get rid of some boilerplate in my project!

[eugineerd]

@alice-i-cecile, what kind of goal should this feature be a part of? I understand that mutual exclusivity is considered an archetype invariant in general, so supposedly it should be a part of some "Archetype Invariants" goal, but this particular feature is isolated in implementation and can be used standalone. Or should there be a "Mutually exclusive components" goal instead that'll detail which cases this feature is useful for (and differences from fragmenting value components since both have overlapping usages)?

[alice-i-cecile]

@alice-i-cecile, what kind of goal should this feature be a part of? I understand that mutual exclusivity is considered an archetype invariant in general, so supposedly it should be a part of some "Archetype Invariants" goal, but this particular feature is isolated in implementation and can be used standalone. Or should there be a "Mutually exclusive components" goal instead that'll detail which cases this feature is useful for (and differences from fragmenting value components since both have overlapping usages)?

Either of these would be fine. I just want to make sure we have a high-level view of our plans here, and can evaluate the architecture and API clearly.

I do really want this feature, to be clear, but there's a reason that archetype invariants work has been repeatedly attempted and abandoned.