Make the old and new archetype available in lifecycle observers

[chescock]

Objective

Simplify observers that need to detect when multiple components are added or removed.

In particular, if a module needs to keep track of entities with some component C that are not Disabled, it will normally start tracking an entity on Add, C and Remove, Disabled and stop tracking it on Remove, C and Add, Disabled. But it should not start tracking the entity when C and Disabled are removed together, such as when an entity with both is despawned, and this is difficult to detect today.

Fixes #22700, although with the idea from #22700 (comment) rather than the solution in the issue description.

Solution

Make the old and new archetypes available in lifecycle observers. Wrap them in Option so that the old archetype during spawn and the new archetype during despawn can be None.

Showcase

fn on_remove_disable(
    on: On<Remove, Disabled>,
    mut cache: ResMut<EntitiesWithA>,
    a_component: ComponentIdFor<A>,
) {
    // The `A` component may have been removed at the same time as `Disabled`,
    // either due to a remove or despawn.  Only try to add this entity to our
    // cache if the `A` component is still in the new archetype.
    if on.trigger().new_archetype.is_some_and(|a| a.contains(*a_component)) {
        cache.0.insert(on.entity);
    }
}

Alternatives

Adding support for Remove observers that run after the change and Add observers that run before would solve this issue with a simpler user experience, as they would be able to check the current archetype of the entity using ordinary Query infrastructure. That would avoid the need for tricks like ComponentIdFor and E: for<'a> Event<Trigger<'a> = EntityComponentsTrigger<'a>>.

But I believe there are concerns about the performance impact of adding more observer types, so I expect a change like that to be controversial.

[Jondolf]

Looks good! I believe this should be enough to cover my current needs in Avian.

I'm fine with this over the more granular enum approach. Given that flecs also does something similar, I'm guessing that having access to the archetypes could be useful by itself too (though I'd be interested to hear more concrete use cases).

Adding support for Remove observers that run after the change and Add observers that run before would solve this issue with a simpler user experience, as they would be able to check the current archetype of the entity using ordinary Query infrastructure.

Would you still have access to the entity's components in the After<Remove> observer though, even in the despawn case? I would assume not, and if that is the case, then I think it wouldn't really work for me. I need to detect if the removal is a remove or despawn, but sometimes also query for some data to update data structures in resources accordingly.

[chescock]

Would you still have access to the entity's components in the After<Remove> observer though, even in the despawn case? I would assume not, and if that is the case, then I think it wouldn't really work for me. I need to detect if the removal is a remove or despawn, but sometimes also query for some data to update data structures in resources accordingly.

No, you wouldn't. I was imagining After<Remove> only being used for components like Disabled where you'd normally use a Without filter when querying.

So you'd have After<Insert>, Collider, After<Remove>, Disabled, and After<Remove>, ColliderDisabled for adding a new collider, and Before<Replace>, Collider, Before<Add>, Disabled, and Before<Add>, ColliderDisabled for removing an existing collider. And you could Query<&Collider, Without<ColliderDisabled>> (plus the implicit Without<Disabled>) in all six of them, ignoring any event where the query doesn't match.

(And my more ambitious proposal is #20817, where you'd just write On<DataAdded<&Collider, Without<ColliderDisabled>>> and not have to worry about the individual components or remember that Disabled exists.)

If there are cases that model can't handle, I'd appreciate hearing about them! And if they're cases that #20817 wouldn't handle, it would be good to document them over there!

[hymm]

Implementation is fairly straight forward and low impact. My one hesitation is about how much we should be exposing Archetypes in the public api. i.e. are archetypes considered an implementation detail or are they something that users should have to confront when they want to do lower level stuff? I think I lean towards the latter as we just merged #21984 which exposes how the data is actually stored. So at least for some cases, it's inevitable that the users needs to think of how data is being stored and accessed.

[Diddykonga]

With this change you should be able to detect the difference between removal and despawn, no? where despawn = new_archetype.is_none()
So is a separate on_despawn still necessary?