Generalised ECS reactivity with Observers #10839

james-j-obrien · 2023-12-02T07:31:46Z

Objective

Provide an expressive way to register dynamic behavior in response to ECS changes that is consistent with existing bevy types and traits as to provide a smooth user experience.
Provide a mechanism for immediate changes in response to events during command application in order to facilitate improved query caching on the path to relations.

Solution

A new fundamental ECS construct, the Observer; inspired by flec's observers but adapted to better fit bevy's access patterns and rust's type system.

Examples

There are 2 main ways to register observers. The first is a "component observer" that looks like this:

world.observer(|observer: Observer<OnAdd, CompA>, query: Query<&CompA>| {
    // ...
});

The above code will spawn a new entity representing the observer that will run it's callback whenever CompA is added to an entity. This is a system-like function that supports dependency injection for all the standard bevy types: Query, Res, Commands etc. It also has an Observer parameter that provides information about the event such as the source entity. Importantly these systems run during command application which is key for their future use to keep ECS internals up to date. There are similar events for OnInsert and OnRemove, this will be expanded with things such as ArchetypeCreated, TableEmpty etc. in follow up PRs.

The other way to register an observer is an "entity observer" that looks like this:

world.entity_mut(entity).observe(|observer: Observer<Resize>| {
    // ...
});

Entity observers trigger whenever an event of their type is targeted at that specific entity. This type of observer will de-spawn itself if the entity it is observing is ever de-spawned so as to not leave dangling observers.

Entity observers can also be spawned from deferred contexts such as other observers, systems, or hooks using commands:

commands.entity(entity).observe(|observer: Observer<Resize>| {
    // ...
});

Observers are not limited to in built event types, they can be used with any event type that implements Component (this could be split into it's own event type but eventually they should all become entities). This means events can also carry data:

#[derive(Component)]
struct Resize { x: u32, y: u32 }

commands.entity(entity).observe(|observer: Observer<Resize>, query: Query<&mut Size>| {
    let data = observer.data();
    // ...
});

// Will trigger the observer when commands are applied.
commands.event(Resize { x: 10, y: 10 }).target(entity).emit();

For more advanced use cases there is the ObserverBuilder API that allows more control over the types of events that an Observer will listen to.

world.observer_builder()
    // Listen to events targeting A or B
    .components::<(A, B)>()
    // Add multiple event types, this prevents accessing typed event data but allows Observers
    // to listen to any number of events (and still allows untyped access if required)
    .on_event::<OnAdd>()
    .on_event::<OnInsert>()
    .run(|observer: Observer<_>| {
        // Runs on add or insert of components A or B 
    });

Dynamic components and event types are also fully supported allowing for runtime defined event types.

Design Questions

Currently observers are implemented as entities which I believe to be ideal in the mid to long-term, however this does mean they aren't really usable in the render world. Alternatively I could implement them with their own IDs for now but would then need to port them back to entities at a later date.
The ObserverSystem trait is not strictly necessary, I'm using it currently as a marker for systems that don't expect apply to be run on it's SystemParam since queue is run for observers instead, with clear documentation I don't think that's required in the long run.

Possible Follow-ups

Deprecate RemovedComponents, observers should fulfill all use cases while being more flexible and performant.
Queries as entities: Swap queries to entities and begin using observers listening to archetype creation events to keep their caches in sync, this allows unification of ObserverState and QueryState as well as unlocking several API improvements for Query and the management of QueryState.
Event bubbling: For some UI use cases in particular users are likely to want some form of event bubbling for entity observers, this is trivial to implement naively but ideally this includes an acceleration structure to cache hierarchy traversals.
All kinds of other in-built event types.
Optimization; in order to not bloat the complexity of the PR I have kept the implementation straightforward, there are several areas where performance can be improved. The focus for this PR is to get the behavior implemented and not incur a performance cost for users who don't use observers.

I am leaving each of these to follow up PR's in order to keep each of them reviewable as this already includes significant changes.

MiniaczQ · 2024-03-03T16:27:45Z

Is this ready for review? It's still marked as draft

james-j-obrien · 2024-03-03T22:26:45Z

The PR is broadly functional and the API is pretty close to what I expect the final API to look like.

Before this would be ready to be merged there are some optimizations I would like to implement as well as cleaning up some of the internal implementation. I also don't have any tests or a particularly compelling example.

I'm not sure at exactly what point it should come out of draft but I want to take at least one more cleanup pass and add a few tests.

alice-i-cecile · 2024-03-03T23:56:21Z

Could we try constructing and synchronizing simple hashmap-based index as the example here? That seems like a useful application.

james-j-obrien · 2024-03-08T07:29:22Z

I have added a modest set of tests, fixed several bugs and revamped the example to be more complex.

I'm going to take this out of draft now that I am more confident in it's robustness, however there are still areas where the PR is lacking; documentation is sparse and doc tests are non-existent.

# Objective - Provide a reliable and performant mechanism to allows users to keep components synchronized with external sources: closing/opening sockets, updating indexes, debugging etc. - Implement a generic mechanism to provide mutable access to the world without allowing structural changes; this will not only be used here but is a foundational piece for observers, which are key for a performant implementation of relations. ## Solution - Implement a new type `DeferredWorld` (naming is not important, `StaticWorld` is also suitable) that wraps a world pointer and prevents user code from making any structural changes to the ECS; spawning entities, creating components, initializing resources etc. - Add component lifecycle hooks `on_add`, `on_insert` and `on_remove` that can be assigned callbacks in user code. --- ## Changelog - Add new `DeferredWorld` type. - Add new world methods: `register_component::<T>` and `register_component_with_descriptor`. These differ from `init_component` in that they provide mutable access to the created `ComponentInfo` but will panic if the component is already in any archetypes. These restrictions serve two purposes: 1. Prevent users from defining hooks for components that may already have associated hooks provided in another plugin. (a use case better served by observers) 2. Ensure that when an `Archetype` is created it gets the appropriate flags to early-out when triggering hooks. - Add methods to `ComponentInfo`: `on_add`, `on_insert` and `on_remove` to be used to register hooks of the form `fn(DeferredWorld, Entity, ComponentId)` - Modify `BundleInserter`, `BundleSpawner` and `EntityWorldMut` to trigger component hooks when appropriate. - Add bit flags to `Archetype` indicating whether or not any contained components have each type of hook, this can be expanded for other flags as needed. - Add `component_hooks` example to illustrate usage. Try it out! It's fun to mash keys. ## Safety The changes to component insertion, removal and deletion involve a large amount of unsafe code and it's fair for that to raise some concern. I have attempted to document it as clearly as possible and have confirmed that all the hooks examples are accepted by `cargo miri` as not causing any undefined behavior. The largest issue is in ensuring there are no outstanding references when passing a `DeferredWorld` to the hooks which requires some use of raw pointers (as was already happening to some degree in those places) and I have taken some time to ensure that is the case but feel free to let me know if I've missed anything. ## Performance These changes come with a small but measurable performance cost of between 1-5% on `add_remove` benchmarks and between 1-3% on `insert` benchmarks. One consideration to be made is the existence of the current `RemovedComponents` which is on average more costly than the addition of `on_remove` hooks due to the early-out, however hooks doesn't completely remove the need for `RemovedComponents` as there is a chance you want to respond to the removal of a component that already has an `on_remove` hook defined in another plugin, so I have not removed it here. I do intend to deprecate it with the introduction of observers in a follow up PR. ## Discussion Questions - Currently `DeferredWorld` implements `Deref` to `&World` which makes sense conceptually, however it does cause some issues with rust-analyzer providing autocomplete for `&mut World` references which is annoying. There are alternative implementations that may address this but involve more code churn so I have attempted them here. The other alternative is to not implement `Deref` at all but that leads to a large amount of API duplication. - `DeferredWorld`, `StaticWorld`, something else? - In adding support for hooks to `EntityWorldMut` I encountered some unfortunate difficulties with my desired API. If commands are flushed after each call i.e. `world.spawn() // flush commands .insert(A) // flush commands` the entity may be despawned while `EntityWorldMut` still exists which is invalid. An alternative was then to add `self.world.flush_commands()` to the drop implementation for `EntityWorldMut` but that runs into other problems for implementing functions like `into_unsafe_entity_cell`. For now I have implemented a `.flush()` which will flush the commands and consume `EntityWorldMut` or users can manually run `world.flush_commands()` after using `EntityWorldMut`. - In order to allowing querying on a deferred world we need implementations of `WorldQuery` to not break our guarantees of no structural changes through their `UnsafeWorldCell`. All our implementations do this, but there isn't currently any safety documentation specifying what is or isn't allowed for an implementation, just for the caller, (they also shouldn't be aliasing components they didn't specify access for etc.) is that something we should start doing? (see 10752) Please check out the example `component_hooks` or the tests in `bundle.rs` for usage examples. I will continue to expand this description as I go. See bevyengine#10839 for a more ergonomic API built on top of this one that isn't subject to the same restrictions and supports `SystemParam` dependency injection.

MiniaczQ

I looked through the example and some user-sided code, I'll try to review the rest soon

examples/ecs/observers.rs

MiniaczQ · 2024-03-10T16:38:35Z

examples/ecs/observers.rs

+                    let Some(mut entity) = commands.get_entity(source) else {
+                        return;
+                    };


If the targeted observer runs, shouldn't this entity be guaranteed to exist?

I had an entity validity check in the original dispatch code, but it got removed in a refactor, happy to add it back if we want to ensure that all event sources are alive. Currently two different mines can fire explode events at a specific entity and without this check it will try and create commands for itself after it's been despawned.

I see this as more of a symptom of the way we are currently applying commands generated by hooks/observers, if we changed it to only apply new commands after the entire queue is flushed we wouldn't have this issue and the state would generally be more consistent. I think this is a discussion we should have before shipping this.

Fix here: #13249

examples/ecs/observers.rs

crates/bevy_ecs/src/observer/builder.rs

iiYese · 2024-03-11T23:29:35Z

crates/bevy_ecs/src/observer/builder.rs

+    }
+
+    /// Add [`ComponentId`] in `T` to the list of components listened to by this observer.
+    pub fn components<B: Bundle>(&mut self) -> &mut Self {


It's unclear from this documentation whether components listened to by the observer behave in a similar way to events where you lose access to the data if you listen to multiple. Especially because events are components.

You don't get access to any data about the component other than it's ID so the component set is mostly internal. If you want to actually access the component you need to do it via a world reference or a query (providing a pointer gets gnarly for aliasing), but you're right this should be more clear, I think there are many places where the docs need fleshing out. There's also currently no way to get all the component ids if you have an observer that listens for OnAdd on multiple components and then all of them are added simultaneously (after the first match for an event observers aren't fired again).

The whole events being components thing is a bit unfortunate as it's confusing to communicate when the "component" isn't actually ever attached to any entity, but building out an entirely new API for events when both are just "type registered to the ECS" seems overkill. We could hide this better like we do for resources, it just bloats out the PR more.

It's not fully clear to me how components work, I haven't ran into any direct explanation.

If I understand correctly, components allows us to target OnAdd, OnRemove events on specific components (A or B or C...), but how do they work with custom events?

When OnAdd is triggered it is triggered for a specific component on a specific entity i.e. entity X has component A added. So when creating an event you need some way to choose which components you are targeting (if any), that's what .components does.

A given custom event doesn't need to be targeted at components, you can just target entities and any observers that are listening for that event will trigger, unless they are only listening for events that target a specific component.

This is a deviation from the flecs API in that flecs always requires you to target a component and all observers have to be listening to events targeting a specific set of components, we could do that as well in order to enforce the consistency of always having component targets, but on the other hand many users won't care about the component they are targeting and just want to send an event at an entity.

This is to avoid mixing terminology with Bevy Observers: bevyengine/bevy#10839

MiniaczQ

Looked through most of the code. I will need to revisit the observer folder again, I didn't grasp how it all works yet.

crates/bevy_ecs/src/bundle.rs

MiniaczQ · 2024-05-06T13:55:29Z

crates/bevy_ecs/src/bundle.rs

+        unsafe {
+            deferred_world.trigger_on_add(archetype, entity, bundle_info.iter_components());
+            if archetype.has_add_observer() {
+                deferred_world.trigger_observers(
+                    ON_ADD,
+                    Some(entity),
+                    bundle_info.iter_components(),
+                );
+            }
+            deferred_world.trigger_on_insert(archetype, entity, bundle_info.iter_components());
+            if archetype.has_insert_observer() {
+                deferred_world.trigger_observers(
+                    ON_INSERT,
+                    Some(entity),
+                    bundle_info.iter_components(),
+                );
+            }
+        };


Duplicate unsafe code, the only difference is bundle_info access in "add" section instead of add_bundle access.
Probably worth separating it so we only maintain one unsafe section.

On experimentation I remembered why I didn't factor this out in the first place. You can't just pass the bundle_info down since sometimes you are using the add_bundle instead and you can't just pass the iterator down because you need to consume it twice to trigger hooks and then observers. It's a bit annoying.

MiniaczQ · 2024-05-06T14:01:46Z

crates/bevy_ecs/src/archetype.rs

+        flags: ArchetypeFlags,
+        set: bool,
+    ) {
+        // TODO: Refactor component index to speed this up.


Are all TODOs here meant for followup PRs?

The component index is going to be another significant PR so I don't have it included here.

crates/bevy_ecs/src/world/component_constants.rs

crates/bevy_ecs/src/observer/runner.rs

crates/bevy_ecs/src/observer/mod.rs

crates/bevy_ecs/src/observer/builder.rs

MiniaczQ · 2024-05-06T16:30:20Z

crates/bevy_ecs/src/observer/builder.rs

+    }
+
+    /// Add [`ComponentId`] in `T` to the list of components listened to by this observer.
+    pub fn components<B: Bundle>(&mut self) -> &mut Self {


It's not fully clear to me how components work, I haven't ran into any direct explanation.

If I understand correctly, components allows us to target OnAdd, OnRemove events on specific components (A or B or C...), but how do they work with custom events?

crates/bevy_ecs/src/observer/mod.rs

cBournhonesque · 2024-05-06T21:50:01Z

crates/bevy_ecs/src/observer/mod.rs

+#[derive(Default, Debug)]
+pub struct Observers {
+    // Cached ECS observers to save a lookup most common events.
+    on_add: CachedObservers,


These are not CachedComponentObservers because the OnAdd event can look at multiple components at once?

I considered putting CachedComponentObservers here given that registering an observer to listen to all OnAdd events is a pretty big performance footgun. Could still do that and just panic if you try and register that kind of observer.

crates/bevy_ecs/src/observer/mod.rs

cBournhonesque · 2024-05-06T23:39:52Z

crates/bevy_ecs/src/observer/mod.rs

+        world.init_component::<A>();
+        world.init_component::<B>();
+
+        world.observer(|_: Observer<OnAdd, (A, B)>, mut res: ResMut<R>| res.0 += 1);


So this listens for events targeting A or B.
Is it possible to observe events where both A and B are added to an entity at the same time?
i.e. if the entire bundle gets added to the entity?

No there's no current way to listen to only events where they are both added, if you wanted that behavior you can just listen to one and check it against a query.

You mean listen to Observer<OnAdd, A> and then in the observer system run a query such as Query<Entity, Added<B>>?

You wouldn't be able to ensure they were both just added, and I don't think that would generally be a desirable use case given that whether you add 2 components at the same time or not doesn't seem like it should matter.

I am considering "merging" queries and observers again like I had in the initial implementation to make expressing this easier such that you could at least be sure that the entity had both components.

The problem is the of existence of user implementations of WorldQuery make it really unclear which components you are listening to based on a QueryData function signature. I feel like I'll have to leave that until after a hypothetical query v2.

How come I wouldn't be able to ensure that they were both just added? The Added query doesn't work in side the observer system?

Sorry I keep replying before your message shows up for me which strangely interleaves the conversation and looks like I'm trying to respond to your questions. Added theoretically could work in an observer system but that would only signify that the component was added since the last time the observer was run, not that the component was added as part of this event.

I see; thanks.
It doesn't have to be part of the v1, but reacting on the Insertion of multiple components together is definitely something that could be useful.
I was actually thinking of switching to it in lightyear; instead of creating an event ReceivedComponent<C> for each component that got replicated over the network, I could let users directly add observers Observers<OnAdd, (Replicate, C)> or even Observers<OnAdd, (Replicate, C1, C2, ...)> to be sure to target the correct entity

crates/bevy_ecs/src/observer/mod.rs

crates/bevy_ecs/src/system/observer_system.rs

crates/bevy_ecs/src/system/system_param.rs

james-j-obrien added 30 commits November 26, 2023 17:01

Initial hooks implementation

001f900

Perf improvements, archetype flags

26edb73

Refactoring

4eb9a80

Improve command application logic

e174cd8

Simplify command application to get recursive ordering

abc728b

First docs and safety comment pass

06d4db6

Miri pass

dec4cec

Add missing cfg(test)

6589b9f

Tidy iterator chains

43d7c74

Cleanup

3d55037

CI pass

9754ae9

Re-generate README.md

d237d0b

Add missing safety comment

79bbe67

Simplify asserts

ce371fc

Relax restrictions on register_component

bbecffa

Update example comments

3600bee

Initial implementation

1ffe76c

Initial hooks implementation

779eabe

Perf improvements, archetype flags

6ae319e

Refactoring

5c47028

Improve command application logic

cdd1ee0

Simplify command application to get recursive ordering

21cd650

First docs and safety comment pass

4e08655

Miri pass

b620001

Add missing cfg(test)

9b76944

Tidy iterator chains

924fb5d

Cleanup

d23d1b2

CI pass

9b490ad

Re-generate README.md

b7e5462

Add missing safety comment

a4d164a

alice-i-cecile mentioned this pull request Mar 7, 2024

Upstream and use bevy_mod_picking #12365

Open

22 tasks

james-j-obrien added 6 commits March 7, 2024 16:46

Merge main

de77347

Cleanup unsafe code, simplify types and trait definitions

238aa90

CI Fixes

8db24ac

Fix bugs, add tests

5faba35

Revamp example

19e55c0

Merge branch 'main' into observers

4678743

james-j-obrien marked this pull request as ready for review March 8, 2024 07:29

james-j-obrien added 3 commits March 7, 2024 23:42

CI Fixes

09fc6b7

Slightly tidy up example

b025837

Minor example tidying

4ddbb94

alice-i-cecile mentioned this pull request Mar 9, 2024

Portable Callback objects #10582

Closed

MiniaczQ reviewed Mar 10, 2024

View reviewed changes

alice-i-cecile added this to the 0.14 milestone Mar 11, 2024

iiYese suggested changes Mar 11, 2024

View reviewed changes

maniwani mentioned this pull request Apr 14, 2024

Add Disabled marker #12928

Closed

james-j-obrien added 3 commits April 26, 2024 18:27

Minor cleanup

50c8c71

Merge main

df1f0b8

Satisfy MIRI at the cost of performance

a6732fe

afonsolage mentioned this pull request Apr 29, 2024

My StyleSheets aren't cascading to new children afonsolage/bevy_ecss#61

Open

Zeenobit added a commit to Zeenobit/moonshine_view that referenced this pull request May 3, 2024

Rename Observer to Model

cfe9b81

This is to avoid mixing terminology with Bevy Observers: bevyengine/bevy#10839

MiniaczQ reviewed May 6, 2024

View reviewed changes

cBournhonesque reviewed May 7, 2024

View reviewed changes

james-j-obrien added 2 commits May 6, 2024 18:22

Merge branch 'main' into observers

4e41128

Merge main, incorporate some feedback

ae56005

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generalised ECS reactivity with Observers #10839

Generalised ECS reactivity with Observers #10839

james-j-obrien commented Dec 2, 2023 •

edited

MiniaczQ commented Mar 3, 2024

james-j-obrien commented Mar 3, 2024

alice-i-cecile commented Mar 3, 2024

james-j-obrien commented Mar 8, 2024

MiniaczQ left a comment

MiniaczQ Mar 10, 2024

james-j-obrien Mar 10, 2024

james-j-obrien Apr 27, 2024 •

edited

james-j-obrien May 5, 2024

iiYese Mar 11, 2024

james-j-obrien Mar 11, 2024

MiniaczQ May 6, 2024 •

edited

james-j-obrien May 6, 2024

MiniaczQ left a comment

MiniaczQ May 6, 2024

james-j-obrien May 7, 2024

MiniaczQ May 6, 2024

james-j-obrien May 6, 2024

MiniaczQ May 6, 2024 •

edited

cBournhonesque May 6, 2024

james-j-obrien May 7, 2024

cBournhonesque May 6, 2024

james-j-obrien May 7, 2024

cBournhonesque May 7, 2024

james-j-obrien May 7, 2024 •

edited

cBournhonesque May 7, 2024

james-j-obrien May 7, 2024

cBournhonesque May 7, 2024

Generalised ECS reactivity with Observers #10839

Are you sure you want to change the base?

Generalised ECS reactivity with Observers #10839

Conversation

james-j-obrien commented Dec 2, 2023 • edited

Objective

Solution

Examples

Design Questions

Possible Follow-ups

MiniaczQ commented Mar 3, 2024

james-j-obrien commented Mar 3, 2024

alice-i-cecile commented Mar 3, 2024

james-j-obrien commented Mar 8, 2024

MiniaczQ left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

james-j-obrien Apr 27, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MiniaczQ May 6, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MiniaczQ left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MiniaczQ May 6, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

james-j-obrien May 7, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

james-j-obrien commented Dec 2, 2023 •

edited

james-j-obrien Apr 27, 2024 •

edited

MiniaczQ May 6, 2024 •

edited

MiniaczQ May 6, 2024 •

edited

james-j-obrien May 7, 2024 •

edited