Observer EventMatchers

[ItsDoot]

Objective

On<E, B>'s B type parameter is the single most commonly confusing construct in bevy_ecs. The predominate use case is for filtering lifecycle event observers:

world.add_observer(|on: On<Add, A>| {
    // gets fired whenever component `A` is added to an entity
});

For most other use cases, like picking or custom user events, this type parameter is wholly unnecessary.

Therefore, it should be removed from view for the cases where it is not needed.

Solution

Introduce a tiny layer above Event called EventMatcher, and update On's bounds:

pub trait EventMatcher: 'static {
    type Event: Event;
    type Components: Bundle;
}

pub struct On<'w, 't, E: EventMatcher> {
    observer: Entity,
    event: &'w mut E::Event,
    trigger: &'w mut EventMatcherTrigger<'t, E>,
    trigger_context: &'w TriggerContext,
}

This allows us to lift the B type parameter in On<E, B> into the E type parameter. To ensure other events like picking continue to work, we introduce a blanket implementation for all Events:

// All events are implicitly EventMatchers, with no additional components.
impl<E: Event> EventMatcher for E {
    type Event = Self;
    type Components = ();
}

To facilitate lifecycle events, we first rename them as follows:

• Add -> AddEvent
• Insert -> InsertEvent
• Discard -> DiscardEvent
• Remove -> RemoveEvent
• Despawn -> DespawnEvent

Then, we introduce new generic types in place of those old identifiers that are EventMatchers:

pub struct Add<B: Bundle>(PhantomData<B>);

impl<B: Bundle> EventMatcher for Add<B> {
    type Event = AddEvent;
    type Components = B;
}

pub struct Insert<B: Bundle>(PhantomData<B>);

impl<B: Bundle> EventMatcher for Insert<B> {
    type Event = InsertEvent;
    type Components = B;
}

pub struct Discard<B: Bundle>(PhantomData<B>);

impl<B: Bundle> EventMatcher for Discard<B> {
    type Event = DiscardEvent;
    type Components = B;
}

pub struct Remove<B: Bundle>(PhantomData<B>);

impl<B: Bundle> EventMatcher for Remove<B> {
    type Event = RemoveEvent;
    type Components = B;
}

pub struct Despawn<B: Bundle>(PhantomData<B>);

impl<B: Bundle> EventMatcher for Despawn<B> {
    type Event = DespawnEvent;
    type Components = B;
}

This results in a slightly modified observer setup:

world.add_observer(|on: On<Add<A>>| {
    // gets fired whenever component `A` is added to an entity
});

Testing

No new tests have been added, we have decent coverage of observer tests as is.

[james-j-obrien]

Cool approach! Looks good.

[Diddykonga]

LGTM

Abstracts the E: Event and B: Bundle from On<E, B> into On<E> where E: EventMatcher
This allows the syntax to be like Add<C>, while the actual semantics use the Event and Components associated types on the EventMatcher. Similiar-ish to an custom WorldQuery.

[chescock]

Nice, this is an elegant way to solve this! I like how a bunch of B generics disappeared from places that don't care about them.

[chescock]

Should E be generalized from Event to EventMatcher here, like the other uses of On? Or is the idea that EntityEvents never use components?

[chescock]

Making sure I understand: We never actually create values of these types, and they just exist to be used as type parameters. Right?

[ItsDoot]

Correct.

[chescock]

(Same question about EntityEvent for the uses in this file.)

[chescock]

Ooh, once we have a separation between the type used to trigger the event and the type used to observe it, I bet we'll be able to use this for other things, like #14649.

[chescock]

It's also possible to use On<AddEvent>, right? But our recommendation is On<Add<()>> for consistency?

[ItsDoot]

Correct.

[chescock]

Ah, so the idea behind the "matcher" metaphor is that there is a stream of AddEvents going by and the Add<C> matcher will only match ones with a C? I was a little confused by the name at first. Matcher is an uncommon term, and I was thinking of these more like sets of events than like filters.

Bikeshedding a bit: How about something like EventPattern? These types seem to fill the role of the pattern in a match expression. And a "pattern" also sounds like a way to generate a set of events to observe.

[chescock]

Not really related to this PR, but: Are we planning to leave these aliases in place forever? I thought they were mostly to help migrate in the release where we renamed them.

[alice-i-cecile]

Normally I would just remove the doc aliases / deprecation after a cycle, but with the rise of the LLM I'm questioning that policy. Lots of reports of "my LLM got confused by a migration" for up to a couple years after it was done.

[Diddykonga]

Do we need to worry about someones tool breaking on migrations, when that tool hallucinates/breaks on its own?

[alice-i-cecile]

I'm curious about the answers to @chescock's questions :)

FYI, I'm planning to hold off merging this until after we cut the 0.19-rc, so we can land this together with a F: QueryFilter generic in 0.20.