Lifecycle event observers for queries

[chescock]

What problem does this solve or what need does it fill?

Lifecycle event observers are hard to use for anything but the simplest cases.

Let's consider some cases where they should be useful:

Maintaining an index

Suppose we want to maintain an index that lets us look up the set of entities with a given component value. Not everything needs to be in the index, so we also define a marker component that should exclude entities from being indexed. Our first attempt will be a system that rebuilds the index:

#[derive(Component, Eq, PartialEq, Hash, Copy, Clone)]
#[component(immutable)]
struct Value(usize);

#[derive(Component)]
struct ExcludeFromIndex;

#[derive(Resource, Default)]
struct Index(HashMap<Value, EntityHashSet>);

fn rebuild_index(
    mut index: ResMut<Index>,
    query: Query<(Entity, &Value), Without<ExcludeFromIndex>>,
) {
    index.0.clear();
    for (entity, value) in query {
        index.0.entry(*value).or_default().insert(entity);
    }
}

Computed values

bevy_feathers wants to calculate the background color for a button based on the values and presence of other components. The actual code has two Has inputs and two &C inputs, but to make a cleaner example I'm using one of each plus an Option<&C>:

fn update_color(query: Query<(Option<&Hovered>, Has<Pressed>, &ButtonVariant, &mut BackgroundColor)>) {
    for (hovered, disabled, pressed, variant, mut color) in query {
        *color = calculate_color(hovered, pressed, variant);
    }
}

We need observers!

The problem with using ordinary systems here is that the code both runs too often but also not often enough! It runs too often because it has to evaluate every entity, even if they don't change. And it runs not often enough because the index or background color will be out-of date between when changes were made and when the system runs.

What alternative(s) have you considered?

Lifecycle events are intended to solve exactly this use case. Let's try to use them! First, maintaining the index:

fn add_to_index_when_value_is_added(
    event: On<Insert, Value>,
    mut index: ResMut<Index>,
    query: Query<(Entity, &Value), Without<ExcludeFromIndex>>,
) {
    if let Ok((entity, value)) = query.get(event.target()) {
        index.0.entry(*value).or_default().insert(entity);
    }
}

fn add_to_index_when_exclude_is_removed(
    event: On<Remove, ExcludeFromIndex>,
    mut index: ResMut<Index>,
    query: Query<(Entity, &Value)>,
    value_component_id: ComponentIdFor<Value>,
) {
    if !event.components().contains(&value_component_id) // Wait, what?
        && let Ok((entity, value)) = query.get(event.target())
    {
        index.0.entry(*value).or_default().insert(entity);
    }
}

fn remove_from_index_when_value_is_removed(
    event: On<Replace, Value>,
    mut index: ResMut<Index>,
    query: Query<(Entity, &Value), Without<ExcludeFromIndex>>,
) {
    if let Ok((entity, value)) = query.get(event.target()) {
        index.0.entry(*value).or_default().remove(&entity);
    }
}

fn remove_from_index_when_exclude_is_added(
    event: On<Add, ExcludeFromIndex>,
    mut index: ResMut<Index>,
    query: Query<(Entity, &Value)>,
) {
    if let Ok((entity, value)) = query.get(event.target()) {
        index.0.entry(*value).or_default().remove(&entity);
    }
}

That's a lot more code! We have separate observers to add to the index and remove from it, which is reasonable. But we also need separate observers to handle adding to the index because Value was added and adding to the index because ExcludeFromIndex was removed.

Worse, they need slightly different signatures: add_to_index_when_value_is_added needs the Without<ExcludeFromIndex> clause in order to respect ExcludeFromIndex, but add_to_index_when_exclude_is_removed can't have that clause, since it runs while ExcludeFromIndex is still present!

And it's especially hard to correctly handle the case where both components are removed at once, which notably includes despawning an entity that has both. The problem is that the observer for removing ExcludeFromIndex runs before the archetype change, so it sees that Value exists and thinks we need to add the value to the index. The only way I could think of to correctly handle it is to check event.components() to see whether Value is being removed at the same time. (And if #17634 were done then I don't think it would be possible at all.)

Well, what about the computed values case? What we want is something like

fn update_color(
    event: On<?????>,
    mut query: Query<(Option<&Hovered>, Has<Pressed>, &ButtonVariant, &mut BackgroundColor)>,
) {
    if let Ok((hovered, pressed, variant, mut color)) = query.get_mut(event.target()) {
        *color = calculate_color(hovered, pressed, variant);
    }
}

But we need to update the value if Hovered or Pressed are removed, and the Remove and Replace triggers fire before the archetype change, so Option<&Hovered> and Has<Pressed> would return the old value. I think it's possible to handle that correctly, but it would be a lot of code so I'm not going to try.

Before<E> and After<E> lifecycle events

The issues so far could be fixed by adding Before and After variants of each lifecycle event, as described in #20729. Let's pretend we have those, and try again. First, maintaining the index:

fn add_to_index_when_value_is_added(
    event: On<After<Insert>, Value>,
    mut index: ResMut<Index>,
    query: Query<(Entity, &Value), Without<ExcludeFromIndex>>,
) {
    if let Ok((entity, value)) = query.get(event.target()) {
        index.0.entry(*value).or_default().insert(entity);
    }
}

fn add_to_index_when_exclude_is_removed(
    event: On<After<Remove>, ExcludeFromIndex>,
    mut index: ResMut<Index>,
    query: Query<(Entity, &Value), Without<ExcludeFromIndex>>,
) {
    if let Ok((entity, value)) = query.get(event.target()) {
        index.0.entry(*value).or_default().insert(entity);
    }
}

fn remove_from_index_when_value_is_removed(
    event: On<Before<Replace>, Value>,
    mut index: ResMut<Index>,
    query: Query<(Entity, &Value), Without<ExcludeFromIndex>>,
) {
    if let Ok((entity, value)) = query.get(event.target()) {
        index.0.entry(*value).or_default().remove(&entity);
    }
}

fn remove_from_index_when_exclude_is_added(
    event: On<Before<Add>, ExcludeFromIndex>,
    mut index: ResMut<Index>,
    query: Query<(Entity, &Value), Without<ExcludeFromIndex>>,
) {
    if let Ok((entity, value)) = query.get(event.target()) {
        index.0.entry(*value).or_default().remove(&entity);
    }
}

That's nicer! We still need two observers, but they have the same signature and implementation so we can at worst just copy-paste them.

Now, for computed values, we can write the update_color observer as above. We still need multiple copies for the different trigger types, but they are:

    event: On<After<Insert>, (Hovered, ButtonVariant, BackgroundColor)>,
    event: On<After<Add>, Pressed>,
    event: On<After<Remove>, (Hovered, Pressed)>,

The Insert events ensure we run when the query starts matching an entity, or when the value of Hovered or ButtonVariant is replaced. (And when BackgroundColor is replaced, since we need to repopulate it!)

We use Add instead of Insert for Pressed, since replacing the value wouldn't change the result of Has.

And we need Remove events for Hovered and Pressed since they change the result of Option and Has.

More problems

There are still some problems with this solution:

Default query filters

One major problem is that it doesn't handle default query filters like Disabled correctly. If we add a Disabled component to an entity in our index, it won't be removed. Worse, if we later change or remove the value, the entity won't match the queries we use to remove it from the index! So if we disable an entity and then despawn it without re-enabling it, it will remain in the index forever.

If the observer author remembers the Disabled component, it's possible to handle it explicitly the way we did with our ExcludeFromIndex components. But other code may add other default query filters, and there is no way to know about all of them.

One option here is to fire Remove and Replace triggers for all components on an entity when Disabled is added, and Add and Insert when it's removed. That would fix the problem at the cost of some complexity, although we'd need some other solution to handle cases that use Allow<Disabled> filters.

Separate observers for each event

We had to write two copies of each function for the index code, and three for the computed value. There are ways to manage that, either by extracting the body of the observer to a separate function, or by making the function generic in the event type and doing world.add_observer(update_color::<After<Add>, Pressed>).

I believe this is the problem that #14649 describes, and there are some possible solutions there.

Observers are invoked unnecessarily

Many of the invocations of these observers will do nothing because the query doesn't match. The add_to_index_when_value_is_added observer still gets invoked for entities with the ExcludeFromIndex component. The update_color observer gets invoked every time Hovered changes, even on entities that aren't buttons. It's not a huge expense to to query.get and return, but it's still an unavoidable overhead.

Hard to figure out exactly what events to observe

With the introduction of Before and After, there are eight different lifecycle events we could potentially observe. And the Add/Insert and Replace/Remove pairs are similar enough that it can be hard to remember which is which.

And once we get it right, it's still a maintenance problem. If we modify the query in update_color to consider a new value, we need to make sure to add it to the appropriate lists of observed components. It would be easy to forget to do that, and it can be hard to detect bugs where an observer sometimes fails to run when expected. And conversely, if we modify the query to stop considering a value, it's easy to forget to remove it from the lists of observed components, resulting in wasted calculation that's hard to discover.

What solution would you like?

We tried using the current lifecycle events, but had to add several new features, and still had something that was hard to get right. Instead, let's forget everything we know about them and start from scratch!

We started with systems that ran queries, so let's try to write something that looks like that. Instead of taking a Query<D, F> and iterating over it, we'll take an On<Something<D, F>> with a single value. And, of course, the index will need to remove entities one-by-one instead of doing clear().

Here is what I want to write:

fn add_to_index(
    event: On<DataAdded<(Entity, &Value), Without<ExcludeFromIndex>>>,
    mut index: ResMut<Index>,
) {
    let (entity, value) = event.data();
    index.0.entry(*value).or_default().insert(entity);
}

fn remove_from_index(
    event: On<DataRemoving<(Entity, &Value), Without<ExcludeFromIndex>>>,
    mut index: ResMut<Index>,
) {
    let (entity, value) = event.data();
    index.0.entry(*value).or_default().remove(&entity);
}

fn update_color(
    event: On<DataAdded<(Option<&Hovered>, Has<Pressed>, &ButtonVariant, &mut BackgroundColor)>>,
) {
    let (hovered, pressed, variant, mut color) = event.data();
    *color = calculate_color(hovered, pressed, variant);
}

Those observers were much easier to write, and much more clear in what they intend to do. So, if that's what we want to write, what do the semantics of DataAdded and DataRemoving need to be to get the behavior we want?

They need to trigger when the data being returned by the query changes. Note that data changes aren't just when the set of entities changes! When Value changes for an existing entity, we need to remove it at the old value and add it at the new value. And when a button has a Pressed component added we need to recalculate its background color.

So we trigger DataAdded after an insert, remove, or spawn of an entity that now matches the query, where either it didn't match before the change, or where a component it has read access to was replaced or removed, or a component it has archetypal (Has) access to was added or removed.

And we trigger DataRemoving before an insert, remove, or despawn of an entity that currently matches the query, where either it won't match after the change, or where a component it has read access to will be replaced or removed, or a component it has archetypal (Has) access to will be added or removed.

How do we implement that?

The most straightforward option is to keep the infrastructure for the existing lifecycle events and translate the DataAdded or DataRemoving observers to those events. The translation can be done relatively simply using the query access, and it's no worse than what users have to do manually today.

But giving the engine more information opens up the possibility of reducing the number of triggers. We're no longer required to trigger observers when Value is inserted on an entity with ExcludeFromIndex, or when Hovered changes on a non-button. This opens up some options for different performance tradeoffs.

For example, we could cache the set of queries that match each archetype. That's no more space than the current cache of archetypes for each query. It may be faster to find the query observers to trigger by starting with ones that match the archetype instead of ones that match the changed components, particularly for Disabled, which will trigger almost every observer.

We could even go farther and cache the set of triggers for each edge in the archetype graph. That would increase memory usage and would be tricky to update when observers were spawned or despawned, but it would let the per-entity cost be a simple Vec iteration, which could be even faster than the current implementation in the hot path.

What about the existing lifecycle events?

We won't need them anymore! DataAdded and DataRemoving can handle all the cases that the existing lifecycle events handle:

• Add, C is DataAdded<(), With<C>>
• Insert, C is DataAdded<&C>
• Replace, C is DataRemoving<&C>
• Remove, C is DataRemoving<(), With<C>>

Multi-component observers don't have exact matches, but could be replaced by multiple single-component observers. In practice, I expect that users of multi-component observers actually want behavior closer to DataAdded<(), (With<A>, With<B>)> or DataAdded<(&A, &B)> and would be simpler in this model.

So we could remove the existing lifecycle events, at least from the public API. Instead of eight different subtly-different lifecycle event types, there would just be two!

[alice-i-cecile]

Very nice writeup! I need to chew on this more deeply, and contrast this to some alternative proposals: namely a directly flecs-inspired API, and the Before<Removed> generic lifecycle events.

My initial impressions are:

• I like the elegance of the design: leveraging query infrastructure is very expressive
• I agree that our current approach is inadequate and frustrating
• this decouples us from the hooks implementation. Is that good? Bad? Do we really need both?
• this needs to have a name that is distinct from "observers" when we ship it, both to reduce confusion with Bevy observers, and to reduce confusion with flecs

[james-j-obrien]

Generally a big fan of this direction, my initial observer implementation was closer to this and so the architecture was built with this in mind, it should be possible to derive triggers like this from existing events as you describe. I'm not sure if the proposed architectural changes will lead to better performance since the maintenance cost of the more discriminate indexing is high. I'm sure there are some gains to be made, but at the very least the shortest path is deriving the set of existing triggers that you need to listen to from a given query.

I would argue this doesn't need to be renamed to distinguish it from flecs' observers at the very least since this is if anything more similar to flecs' API, although some behavior of bevy's observers has diverged so you could say this is a big enough overhaul to have a new name if desirable.

[CorvusPrudens]

This API seems similar to Flecs Monitor observers. Although monitors merely fire when a query starts or stops matching, which is a bit less expressive.

Overall, I believe strongly in this approach. It will become especially powerful if we ever land relationship traversal for queries. I've started working on a straightforward implementation in user space, although I think a good implementation is somewhat blocked currently.

One thing I'm not so sure about is this:

The translation can be done relatively simply using the query access

Has<T> and Option<&T> seem rather tricky!

fn system(data: On<DataAdded<&A, Has<B>, Option<&C>>>) {}

We want to observe when the data changes, so we can't simply run system as a Remove observer for B or C directly -- the data hasn't changed yet! Instead, we could queue an execution of system, which would then run immediately after they're removed.

Obviously we can write a separate trait that handles these semantics (that's how I've implemented it currently by necessity), but it would be nice if we could get it "for free." And indeed, maybe there's already enough information to distinguish these. I'm not familiar enough with Access to know.

[CorvusPrudens]

To expand on the above, let's organize how we might translate lifecycle events for DataAdded:

Data	Add	Insert	Replace	Remove
&T	x	✓	x	x
Option<&T>	x	✓	x	✓ (queued)
Has<T>	✓	x	x	✓ (queued)

Is the reported access different between Option<&T> and Has<T> such that we could set up these observers? If not, then an additional trait may be required.

[chescock]

Is the reported access different between Option<&T> and Has<T> such that we could set up these observers? If not, then an additional trait may be required.

Yup! &T has read and required access, Option<&T> has read, and Has<T> has archetypal. So I think you can figure it out from QueryState::component_access.

My mental model here is that read access means you care about add/overwrite/remove, while archetypal access and filters mean you just care about add/remove. But then you can ignore remove if you have a With filter and add if you have a Without filter since you know the query won't match after those events. And then you combine add + overwrite = insert.

One twist is that Allows<T> shows up as archetypal access like Has<T>, so you'll get some false positives there.

There's also an interesting question of what the behavior should be if you query &Archetype. Arguably that should be expressed as "archetypal access for all components" and should trigger on any archetype change.

We want to observe when the data changes, so we can't simply run system as a Remove observer for B or C directly -- the data hasn't changed yet! Instead, we could queue an execution of system, which would then run immediately after they're removed.

Right, I was expecting to add the Before<Add> and After<Remove> events, which we don't have today.

Queuing the observer to handle After<Remove> is a clever idea! I don't see a way to handle Before<Add>, though, which we'd need for On<DataRemoved<D, Without<C>>>.

I had been thinking we'd want to avoid exposing those, since the proliferation of event types would make the API more difficult to consume. But building them first and then experimenting with the rest out-of-tree sounds like a better idea!