Event Rearchitecture

[cart]

There is general consensus that our terminology for Events, "entity events", Observers, and BufferedEvents needs clarity. Additionally, many of us also agree that the current Observer system would benefit from additional static-ness: currently it is assumed that you can use events in pretty much any context, and they all go through the exact same code path.

Alice put forth a proposal to Overhaul Observers, and we have already partially implemented it for 0.17. I think it does a great job of outlining many of the issues at play, and it solves them reasonably well. But I also think the proposed solution isn't yet ideal. Given that it is already partially implemented for 0.17, it is a breaking change, and given that we have already broken the Observer API a number of times, I think we need to sort this out before the next release.

This is a big changeset, but it is largely just a reframing of what is already there. I haven't fundamentally changed the behaviors. I've just refined and constrained in a way that allows us to do what we are currently doing in a clearer, simpler, and more performant way.

First, I'll give some quick notes on Alice's proposal (which you all should read if you haven't yet!):

Notes on Alice's Proposal

• I like the move toward a more static API
• I think we've gone too far down the "separate terminology" path. The proposal introduces a zoo of apis, terms, and "subterms". I think we need to simplify our concepts and names to make this all easier to talk about and use in practice.
• BroadcastEvent feels like the wrong name. EntityEvent is also "broadcast" in the exact same way
• BufferedEvent is a completely different system than EntityEvent and BroadcastEvent. This muddles concepts too much. It needs its own standalone, single-word concept name.
• "Universal observers": I think this should be fully context driven, rather than needing encoding in the API.
• I agree we can't get rid of buffered events, and that merging them with "broadcast events" isn't helpful
• I'm not quite sure how we'd make the proposed PropagateEvent subtrait work transparently. This can't be "layered on top" as a trait. It needs to be baked in at more fundamental level.

• I don't like app.add_broadcast_observers(), app.add_universal_observers(), Observer::entity_observer, Observer::broadcast, etc. The On event should statically determine whether an observer is an "entity observer" or a "broadcast" Observer. This would already be encoded in the type system and is therefore something we can do on the developer's behalf. Likewise, any observer being registered at a top level is inherently not a specific entity observer. All of these variants serve to make users guess and poke around in a way that is unnecessary. I want simple one word concept names, single constructors, etc.

Proposed Principals

• Static-ness:
  • Events should only be usable in the context they were defined to be used.
  • When triggered, Observers should only have access to fields and behaviors that are relevant:
    • Dont return Option or PLACEHOLDER: the field or function shouldn't exist
    • Entity events that don't support propagation shouldn't expose that functionality
• Don't do unnecessary work at runtime
  • Event triggers shouldn't branch through every potential event code path
  • Don't clone potentially large lists of event context unnecessarily (Ex: we currently clone the component list for every observer invocation)
• Minimize codegen
  • Don't recompile things redundantly.
  • Don't compile unnecessary code paths.
• Clear and Simple
  • Minimize the number of concept names floating around, and lock each concept down heavily to a specific context
  • I'm convinced at this point that "buffered events" and "observer events" sharing concept names is wrong. We need two clean and clear terms, and I'm willing to give "buffered events" a slightly worse name if it means "observer events" can be nicer.
  • Don't throw the concept name "Event" out ... it is a very good name. Instead, constrain it to one specific thing.
  • Minimize our API surface
  • Events contain all context, including what used to be the "target". This lets people define the "target" name that makes the most sense for the context, and lets the documentation fully describe the context of that "target".

Concepts

• Event (the thing you "observe")
  • Rationale: "Event" is the clear choice for this concept. An "event" feels like something that happens in real time. "Event observers" are things that observe events when they occur (are triggered). Additionally, this is the concept that "propagates", and "event propagation" is a term people understand.
  • Trigger: (the verb that "causes" events to happen for targets). Events are Triggered. This can include additional context/ data that is passed to observers / informs the trigger behavior. Events have exactly one Trigger. If you want a different trigger behavior, define a new event. This makes the system more static, more predictable, and easier to understand and document. world.trigger_ref_with makes it possible to pass in mutable reference to your own Trigger data, making it possible to customize the input trigger data and read out the final trigger data.
  • Observer (the thing that "observes" events): An event's Trigger determines which observers will run.
  • Event Types: You can build any "type" of event. The concept of a "target" has been removed. Instead, define a Trigger that expects a specific kind of event (ex: E: EntityEvent).
    • EntityEvent We add a new EntityEvent trait, which defines an event.entity() accessor. This is used by the Trigger impls : EntityTrigger, PropagateEntityTrigger, and EntityComponentsTrigger.
• Message (the buffered thing you "read" and "write")
  • Message is a solid metaphor for what this is ... it is data that is written and then at some later point read by someone / something else. I expect existing consumers of "buffered events" to lament this name change, as "event" feels nicer. But having a separate name is within everyone's best interest.
  • MessageReader (the thing that reads messages)
  • MessageWriter (the thing that writes messages)

The Changes

• Event trait changes
  • Event is now used exclusively by Observers
  • Added Event::Trigger, which defines what trigger implementation this event will use
• Added the Trigger trait
  • All of the shared / hard-coded observer trigger logic has been broken out into individual context-specific Trigger traits.
• "Trigger Targets" have been removed.
  • Instead, Events, in combination with their Trigger impl, decide how they will be triggered. In general, this means that Events now include their "targets" as fields on the event.
  • APIs like trigger_targets have been replaced by trigger, which can now be used for any Event
• EntityEvent trait changes
  • Propagation config has been removed from the EntityEvent trait. It now lives on the Trigger trait (specifically the PropagateEntityTrigger trait).
  • EntityEvent now provides entity / entity_mut accessors for the Event it is implemented for
  • EntityEvent defaults to having no propagation (uses the simpler EntityTrigger)
  • #[entity_event(propagate)] enables the "default" propagation logic (uses ChildOf). The existing #[entity_event(traversal = X)] has been renamed to #[entity_event(propagate = X)
  • Deriving EntityEvent requires either a single MyEvent(Entity), the entity field name (MyEvent { entity: Entity}), or MyEvent { #[event_entity] custom: Entity }
• Animation event changes
  • Animation events now have their own AnimationEvent trait, which sets the AnimationEventTrigger. This allows developers to pass in events that dont include the Entity field (as this is set by the system). The custom trigger also opens the doors to cheaply passing in additional animation system context, accessible through On
• EntityComponentsTrigger
  • The built in Add/Remove/etc lifecycle events now use the EntityComponentsTrigger, which passes in the components as additional state. This significantly cuts down on clones, as it does a borrow rather than cloning the list into each observer execution.
  • Each event now has an entity field.
• Style changes
  • Prefer the event name for variables: explode: On<Explode> not event: On<Explode>
  • Prefer using the direct field name for the entity on entity events, rather than event.entity(). This allows us to use more specific names where appropriate, provides better / more contextual docs, and coaches developers to think of On<MyEvent> as the event itself.

Take a look at the changes to the examples and the built-in events to see what this looks like in practice.

Downsides

• Moving the "target" into the event adds some new constraints:
  • Triggering the same event for multiple entities requires multiple trigger calls. For "expensive" events (ex: lots of data attached to the event), this will be more awkward. Your options become:
    • Create multiple instances of the event, cloning the expensive data
    • Use trigger_ref, and mutate the event on each call to change the target.
    • Move the "expensive" shared data into the Trigger, and use `trigger_ref_with``
    • We could build a new EntityEvent method that abstracts over the "event mutation" behavior and provides something like the old trigger_target behavior.
    • Use a different EntityTargetTrigger (not currently provided by bevy, but we could), which brings back the old behavior. This would be used with trigger_with to replicate the old pattern: world.trigger_with(MyEvent, [e1, e2].into()) (or we could make the into() implicit)
  • Bubbling the event involves mutating the event to set the entity. This means that trigger_ref will result in the event's EntityEvent::entity() being the final bubbled entity instead of the initial entity.
  • Some APIs (trivially) benefit from the "target entity" being separate from the event. Specifically, this new API requires changes to the "Animation Event" system in AnimationPlayer. I think this is actually a good change set, as it allows us to:
    • Cheaply expose more animation state as part of a new AnimationEventTrigger impl
    • Move that "implict" entity target provided by the AnimationPlayer into the AnimationEventTrigger
    • Encode the "animation event trigger-ness" of the event into the type itself (by requiring #[event(trigger = AnimationEventTrigger)])
    • By not implementing Default for AnimationEventTrigger, we can block animation events from being fired manually by the user.

Draft TODO

• Fill in documentation and update existing docs
• Benchmark: I expect this impl to be significantly faster. There might also be tangible binary size improvements, as I've removed a lot of redundant codegen.
• Update release notes and migration guides

Next Steps

• The BufferedEvent -> Message rename was not included to keep the size down.

Fixes #19648

[cart]

Gotta resolve some conflicts with main.

[alice-i-cecile]

I think we need to sort this out before the next release.

Fully agree here :) Changes to internals can wait, but I would really like to nail down the fundamental user facing API and terminology. Really glad you're taking a look here.

I'm aligned with you on the core principles: less duplication, more staticness, more efficiency! I'll leave more detailed thoughts in review comments to allow them to be collapsed and threaded.

[alice-i-cecile]

Very helpful: thank you for filling in the safety comments. I'm happy with the clarity of the explanation there, and feel like I can (carefully) make and review changes here in the future.

I think that there's likely things that we could do to clean up the gnarliness of the unsafe code here, but I am fine to leave it for now.

Nit: there are 5 instances of "contrained" in the new safety docs. They're all collapsed though, so it will be easier for you to just grep locally.

[cart]

Nit: there are 5 instances of "contrained" in the new safety docs. They're all collapsed though, so it will be easier for you to just grep locally.

Hehe I fixed it before you comment was posted 😈

[cart]

I'm going forward with merging this, in the interest of keeping the 0.17 ball rolling.

[didi-peterk]

I just read this a couple of weeks agoo then got into the throes of getting our app's tuple(payload) events done over, at the same time as absorbing rust without getting to "crabby" about it :)

Our history of prior apps done in C++ and java and a hybrid of both were highly event/message based between the "nodes" (ie: entities).

I had there, made a pretty sharp divide between the "routing header" and the "payload". The "routing" being things like "initial_sender", and even a "hop history" of the thread through a graph traversal, max hops, timeout "reply_to" etc. You have "on.original_entity() and on.target".

This was handed by a "router" which your "world" (commands being access to it) seems to take the place of.

The main item for us to get up now is we use a different "bubbling" algorithm for hierarchy trees and interaction.

We use a two way "bubble" system where one starts at a node and then go down to the leaf that was "hit" or had the initial event posted (triggered) to it then if "propagated" back up. On thew way down, any ancestor can eat, or forward to the child below or reverse the direction of the traversal, then on the way back up a node can eat, forward (propagate) or abort. For UI inputs only for low volume "hailing" events like enter/exit and press, and "wheel", and a node then can attach "listeners" to a pointer and receive moves etc directly from the source and request exclusive "capture" or focus.

I saw some reference to "custom triggers" in discussions.

In 0.16 this spring, | looked at the code and found a loop where the "propagate" flag was checked to see if the current entity has a component of the type specified in the traversal and then continue, or abort.

It looks like much of the change is internal how to optimise and maintain caches for ECS triggered system functions ( those that can access the world via gueries ) And I really like using the ECS memory management plus thread managed triggered queries.

Anyway thoughts on this, how best to get what I want ( at least simulated but effective ), and what are you envisioning for the architecture ? Initially for the bubbling difference.

For the bigger aps ( muti-player, multi-core, server mirrored peer to peer etc ) events get dispatched locally in local context with entities as addressed nodes, and rust types and signatures as message types, for remotely we did it "over the wire" with a serialized version of the same structure through a local to remote node ID map.

[didi-peterk]

BTW congrats on getting 0.17 out by the end of the quarter. Much nicer in many ways :)

[didi-peterk]

Ok - I made some very small mods to my bevy 0.17.1 to add bits to "propagate" - I made the propagate bit the high bit ( so no shifting needed to access the low 31 bits as an integer) . My tests here work fine and do what I need for added flexibility with minimal mods to bevy_ecs/event/trigger.rs and bevy_ecs/observer/system_param.rs.

Not an ideal where an entity could be returned from a triggered observer to propagate to and avoid the whole call chain and setup to get to PropagateEntityTrigger::trigger(), but the minimal change enables and if I trigger to the "next entity" inside the observer that wished to propagate it will lose it's "original_event_target". But I likely want to have an expanded "hop history" anyway. But I do have to invoke the whole call chain from "commands" again for the next trigger if done that way.

Anyway thoughts ? Should I make a PR for this ?