Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add OnMutate observer and demonstrate how to use it for UI reactivity #14520

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

alice-i-cecile
Copy link
Member

@alice-i-cecile alice-i-cecile commented Jul 29, 2024

Objective

OnMutate hooks and observers are a natural and much desired complement to the hooks and observers feature added in #10839. These were cut from that initial PR due to a) complexity and b) performance concerns, but are incredibly valuable as a general purpose tool for tracking and responding to changes.

P.S. As discussed in my Vision for Bevy UI and @cart's #14437, reactivity is a major open question for building complex UI in Bevy.

This PR will not cover their uses within UI to avoid getting bogged down in controversy, but my hope is to add a complex example or two demonstrating this in follow-up work.

Solution

Users can now watch specific components for mutations by calling app.generate_on_mutate::<MyComponent>>.

When this is enabled, OnMutate triggers will be sent whenever a component of that type is added or mutated, targeting the entity whose component has been changed.

This is a useful building block for reactive UI when combined with observers:

    commands
        .spawn(TextBundle { ..default() })
        .insert(CounterValue(0))
        .observe(
            |trigger: Trigger<OnMutate, CounterValue>,
             mut query: Query<(&CounterValue, &mut Text)>| {
                let (counter_value, mut text) = query.get_mut(trigger.entity()).unwrap();
                *text = Text::from_section(counter_value.0.to_string(), TextStyle::default());
            },
        )

To do

  • add an ECS example
  • cut the UI example
  • make the simple example less ugly
  • move the method on App into bevy_app
  • see if we can make an OnMutate hook too
  • move the OnMutate event generation into the ECS internals, and remove its generic
  • create an equivalent generate_change_events API on World

Follow-up work

To make this pattern broadly useful, we need:

  1. Resource equivalents to all of the lifecycle hooks (Add resource hooks (and observers) #12231).
  2. Asset change detection (Add change ticks to asset references #14444).
  3. Broader unification of the two forms of events, so observers can be triggered by batched events (sent with an EventWriter).

To make this great, I think we want:

  1. Archetype-level (Accelerate change detection by redundantly storing change ticks #5097) or query-level (Query-level change detection #14510) change tracking to make these updates fast.
  2. A way to automatically detect whether or not change detection events are needed based on the registered observers (related to Change Detection as Components / Opt-in or opt-out of Change Detection #4882).
  3. Better tools to gracefully exit observers when standard operations fail.
  4. Relations to make defining entity-links more robust and simpler.
  5. Nicer picking events to avoid having to use the naive OnMutate<Interaction> pattern (Upstream and use bevy_mod_picking #12365).
  6. Syntax sugar to fetch matching components from the triggered entity and its broader context using upwards-searching relational queries) in observers.

This is probably also very useful for indexes (#4513).

@alice-i-cecile alice-i-cecile added C-Enhancement A new feature A-ECS Entities, components, systems, and events A-UI Graphical user interfaces, styles, layouts, and widgets D-Complex Quite challenging from either a design or technical perspective. Ask for help! X-Controversial There is active debate or serious implications around merging this PR C-Needs-Release-Note Work that should be called out in the blog due to impact S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jul 29, 2024
@SanderMertens
Copy link

Prior art in flecs, which uses an on_set (insert) hook: https://github.com/SanderMertens/flecs/blob/master/src/addons/script/template.c#L392

@alice-i-cecile
Copy link
Member Author

alice-i-cecile commented Jul 29, 2024

For the reader, these are easier to implement in flecs, due to the ability to send these events directly when mutation occurs. If I understand correctly, there's no distinction between the OnInsert and OnMutate events, which is nice, because OnInsert's advantage over OnAdd is... quite artificial. Overwriting component values via insertion is really rare, and not what most users want to detect.

Bevy can't do that in any obvious way, because that would require mutable access to a shared queue for every mutation. In other words, a on_mutate_queue field inside of Mut and ResMut. Because this data is shared, it would block parallelism: both systems-level and things like Query::par_iter_mut.

As a result, my first design for implementing this is to implement a cleanup query between observers that just checks each entity for changes and sends a trigger event if so.

We might be able to improve performance by swapping to some sort of unordered, easily parallelized event queue data structure (probably just a channel), and actually moving this behavior to Mut / ResMut. I think that's more complex / risky though, so I'm going to punt it to follow-up, and ideally we can benchmark the approaches against each other.

@SanderMertens
Copy link

SanderMertens commented Jul 29, 2024

these are easier to implement in flecs, due to the ability to send these events directly when mutation occurs

This is not actually what happens. If you modify a value in flecs through an ECS operation this enqueues a Set or Modified command in the command queue. This is what causes hooks/observers to get invoked. Parallelization is not an issue because each thread has its own command queue.

Nit: I'd use a hook for reactivity, at least for things like props that are local to a template instance. It kind of "belongs" there architecturally because it's behavior associated with a component vs. a decoupled plugin reacting to something. It also guarantees that when observers run, incrementalization has finished for the template instance. It's also more efficient.

@alice-i-cecile
Copy link
Member Author

Thanks for the explanation :)

Nit: I'd use a hook for reactivity, at least for things like props that are local to a template instance. It kind of "belongs" there architecturally because it's behavior associated with a component vs. a decoupled plugin reacting to something. It also guarantees that when observers run, incrementalization has finished for the template instance. It's also more efficient.

And yeah, I'll see if I can get hooks working here too. If so then yeah, I think that hooks are right for "standard widget behavior", while observers are better for user-space customization.

@viridia
Copy link
Contributor

viridia commented Jul 31, 2024

I'm tempted to write a "counter example considered harmful" because I've seen the basic "counter" example used so many times for different frameworks - and the problem with it is that it's simplicity is misleading. The counter example is unrepresentative in a number of ways:

  • It represents a ui component with a single data dependency.
  • It represents a ui component whose data is declared, not only in the same place that it's consumed, but in the same place that it's updated.

While these two conditions hold true for many real-world ui components, they are not in the majority. As a teaching tool, it's fine - but as a basis for validating an architecture it leaves a lot out.

For example, many real-world ui components depend on multiple data sources. Those sources will often change at the same time - in other words, it will often be the case that you'll get a spurt of multiple update triggers, either from a single dependency or multiple dependencies. These triggers really ought to be batched/debounced so that you don't end up doing redundant work updating your UI. This is less important in a fine-grained reactive system because the presumption is that most reactions are small; but in a coarse-grained reactive system reactions tend to be larger and more computationally intensive, so you want to avoid them where possible. You don't want to do a VDOM diff 5 times in a single frame if you can help it.

In practice, this means that when you get an update signal, instead of modifying the display entities directly, you set some dirty bit, and then run the updates asynchronously later - either as a separate system or during a command flush, depending on how asynchronous you want to be.

Also, most UI widgets that display data are presenting data that originates from some external source - something that is defined either higher up in the UI hierarchy, or not in the UI hierarchy at all. While some widgets have purely local states like hover, or pressed, many do not. Most checkbox designs, for example, externalize their state - that is, the component which owns the "checked / unchecked" variable is outside of the checkbox (in React this is called a "controlled" component, meaning that it's up to the caller to supply the widget's state, and to listen to callbacks to update that state.)

fn watch_for_mutations<C: Component>(mut commands: Commands, query: Query<Entity, Changed<C>>) {
// Note that this is a linear time check, even when no mutations have occurred.
// To accelerate this properly, we need to implement archetype-level change tracking.
commands.trigger_targets(OnMutate::<C>::default(), query.iter().collect::<Vec<_>>());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collecting into a Vec seems like an arbitrary constraint here. Necessary in the context of commands.trigger_targets for obvious reasons, but I think this is worth optimizing via direct world access. Makes me want something like:

let mut resumable_iter = query_state.resumable_iter();

// notice the `world` argument in `next(world)`, which might (?!?) allows us to have full world access
// for each iteration of an Entity-only query
while let Some(entity) = resumable_iter.next(world) {
  world.trigger_targets(OnMutate::<C>::default(), entity);
}

Which would would allow us to cut out constructing the allocated vec with safe code.

}
}

fn watch_for_mutations<C: Component>(mut commands: Commands, query: Query<Entity, Changed<C>>) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be using / encouraging this approach. Doing this as a normal "single pass over changed" system means that we are driving reactions to completion over the course of (possibly) many frames. The number of frames it will take to resolve a single propagation of changes is significantly expanded by a number of factors:

  1. Each mutation that triggers another mutation within a given component type will have a frame of delay if that entity is "behind" this entity in the change query iteration.
  2. Each mutation that triggers another mutation across component types will have a frame of delay if the watch_for_mutations system of the triggered mutations runs before the watch_for_mutations of the originating mutation's type.
  3. We only check if a component has changed once per frame, which means if it changes again, that will not be observed until the next frame.

Given that changes propagate in roughly hierarchy order many levels deep across many types, I anticipate resolving a single propagation tree to take many frames (on the order of seconds of clock time).

The "perceived jank cost" of allowing people to do this is too high I think.

Copy link
Member

@cart cart Jul 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would not be a full solve, but if we did these checks in topo-sorted hierarchy order and checked changed components for each relevant type at each entity, that would allow most UI-related changes to propagate in a single frame via a single pass. From there, you could loop multiple times over the hierarchy until all changes have been fully propagated (which would be a "full" solve, at the cost of being overly expensive due to searching the whole hierarchy an unnecessarily high number of times per frame).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quill uses a "run to convergence" strategy: reactions are run in a loop within a single frame, but are required to converge to quiescence within a set number of iterations: https://github.com/viridia/quill/blob/main/crates/bevy_quill_core/src/view.rs#L508

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense!

@viridia
Copy link
Contributor

viridia commented Jul 31, 2024

Another wrinkle to take in to consideration is that a common pattern in UI involves a chain of dependencies. For example, if you are displaying information on a selected item, the widget needs to update whenever the id of the selected item changes; but it also needs to update if the properties within the selected item change. This gets even more elaborate if the selected item has properties which are ids that point to other data structures. In an ECS world of entity ids and asset handles, these kinds of structures are fairly standard.

In an implicitly reactive model, all of this happens automatically: dereferencing the id creates a dependency on the id; dereferencing the item's properties creates a dependency on the item (component or resource). In an explicit model, on the other hand, it's easy to miss a dependency, to forget that you need to subscribe to both the id and the thing it points to.

Now, I'm not claiming that the approach that I've taken is the best one. In fact, I'm pretty sure it's not. However, I'm laboring under a constraint, which is "build the best reactive framework you can without changing Bevy". If we're allowed to change Bevy, then I suspect many things will become possible that aren't now. However, I don't have a good sense of what's possible in this space.

@alice-i-cecile alice-i-cecile removed the A-UI Graphical user interfaces, styles, layouts, and widgets label Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Enhancement A new feature C-Needs-Release-Note Work that should be called out in the blog due to impact D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants