#Observers Overhaul

1324 messages · Page 2 of 2 (latest)

feral tinsel
#

Clearly, we need to pick a foundation for reactions that supports building a signal abstraction on top.
Not sure if this follows from the preceding text. If using this approach:
or via some static analysis at compile time
Would you still need signals? If you can define everything statically up front, I don't think so

candid halo
#

The problem with static analysis is that it's strictly local

#

It's not a whole-program analysis, but only within a specific ui component or function

feral tinsel
#

Also a bit funny:

In a way, signals are the inverse of type erasure: they erase everything but the type!
and then later:
In this case, a "signal" is just a type-erased wrapper
:p

candid halo
#

I could probably have chosen a better adjective

feral tinsel
candid halo
# feral tinsel That sounds like a desirable property though, are there any practical downsides ...

Well, we were discussing the usefulness of signals. Signals can be used both locally, within a UI component, or globally to wire components together. There are plenty of alternative ways to connect logic - the classic one is just bubbled events with a parent widget doing orchestration - but the number of choices available diminishes as you start to talk about widgets that are widely separated conceptually. That being said, I've seen other approaches too, like react-query or redux which, although advertised as a global data store and network layer respectively, can effectively function as a signaling mechanism.

#

Here's a practical use case: in my game editor, there's a type of entity called a waypoint which is used for actor goals. The waypoint asset can be edited via a reflective property editor - you can type in the coordinates. But while you are doing this, a gizmo shows up in the 3D view, showing you where the waypoint is in the world. These two display entities live in completely different parts of the UI hierarchy, but they are subscribed to the same underlying data.

#

(Actually, that's probably not the best example, as there are other ways to do gizmos).

feral tinsel
#

What if in this case you'd use the ECS as the global store, and use regular observers to get notified when something changes?

#

Two UI components can depend on the same external ECS component

candid halo
feral tinsel
#

Is that a problem? Couldn't that be passed in as a prop?

candid halo
#

A common code smell in React apps is called "prop drilling", which is the case where you have some changing property needed by a widget deep in the hierarchy, but the prop is supplied by an ancestor that's many levels higher; each time that property changes, you have to re-evaluate all the widgets in the ancestor chain, "drilling down" until you get to the widget that actually needs the value. With a signal approach, you can pass down the signal and the leaf node can update itself without the intermediate ancestors being involved.

feral tinsel
#

Wouldn't that be the same here?

#

An entity is passed down to a child, but the entity value itself doesn't change

#

Only when the component of the entity (which is only accessed by the leaf) changes, will it get updated

#

Which doesn't trigger the ancestor chain

candid halo
#

That's all that a signal really is - a pointer to some data, which can be dereferenced. The only difference is that doing the dereference also subscribes you to changes.

#

As the consumer of a signal, you don't know whether you are reading a plain variable or the computed output of some function.

feral tinsel
#

Right, so this seems like something that can be done with a local/signal-less approach as well right? 🤔

#

Just that instead of a signal object you pass down the entity

#

Advantage of using an entity+component is that it works out of the box with everything else. You don't need a separate API

candid halo
#

The waypoint gizmo is more general than that. It doesn't display just waypoints, but any 3d point in the world. It doesn't know whether the coordinates it is showing are a property of a struct, stored in a component, in a resource, or is some virtual value.

#

It just reads from the stream

#

So it's completely modular and reusable

feral tinsel
#

yeah, any problem can be solved by introducing another level of abstraction :p

#

but is it worth it

candid halo
#

Well, let me ask: are Futures worth it?

#

Everything you can do with futures could be done without them.

feral tinsel
#

If your point is that an approach that uses a static/local analysis has worse UX or sth then I'm not sure if I agree 🤔

candid halo
#

It's more that there are some cases that a static/local analysis doesn't cover

#

For the cases that they do cover, they work fine

feral tinsel
#

Those cases being resources or virtual values?

candid halo
#

Specifically, cases that require global interconnections

#

Cutting across the hierarchy

feral tinsel
#

You can do that in the static approach as well:

parent_entity {
  child_entity {
    SomeComponent: {10, 20}
  }
}

template CompA {
  prop src = entity
  const v = src[SomeComponent]
}

template CompB {
  prop src = entity
  const v = src[SomeComponent]
}

CompA(parent_entity.child_entity)
CompB(parent_entity.child_entity)
candid halo
#

Think about unix/bash pipes: the reason that they are so versatile is because programs like more and cat are blind: they don't care where their input is coming from or where their output is going to.

#

If more required a specific filename as a parameter, it would be far less adaptable.

feral tinsel
#

Right, I understand. But the price of that is that you now need to express every reactive value in this new abstraction that nobody yet uses

candid halo
#

True

feral tinsel
#

You can also flip it, and require that every reactive value must be expressed as a component

#

It's not like there are no rules at all for reactivity in JS and that everything just works

candid halo
#

Yep, and in fact that that is how I've mutables and derived reactions in bevy_reactor, they are components. Signals are just an API, they aren't components by themselves.

#

The fact that it gives you a different API is an issue, especially in an established engine. It's easier in something like Solid which is signals all the way down.

candid halo
#

Redux is based around the idea of an immutable data store: every change to the data conforms to the pattern old state + action => new state. Structural sharing is applied to make this efficient.

#

Individual UI components subscribe to a "slice" of the global data store, and dispatch "actions" which produce an updated global state. This gives you things like "time-travel debugging" in which you can rewind the most recent changes to get to an earlier state.

feral tinsel
#

I'll take a look. I'm primarily interested in making reactivity work in a way that feels native to ECS though

#

So mostly curious about what issues I'm going to run into if I ignore signals

#

Your writeups are always very informative, definitely helps

oak cipher
ruby roost
#

Yes, that would be great! I didn't really understand the counter-arguments well enough to feel like I could explain them, though. I'm hoping that was in part because I had failed to articulate what I was actually proposing, and that writing it down like this will make it easier for others to explain the problems with it. (And I feel a little guilty repeatedly pinging @feral tinsel to explain the same things over and over again. Although I'm about to do it again anyway...)

#

Huh, I meant to say the same thing here that I said in #ecs-dev message , and I interpreted your reaction emoji there as general agreement. What are some of the ways that multi-component observers are being used that don't fit this model?

feral tinsel
#

Perhaps one thing that's worth considering though is splitting up the query observer part from the lifecycle event refactor parts since (as I understand it) they're different things

stiff basin
#

@candid halo I can help if u want thread local stuff, I had to do it in order to do my async ui things

jade hearth
#

Wouldn't thread locals kinda fall apart if you capture signals (or signal-like handles)? There's no guarantee the closure will be executed on the same thread (unless you control for that of course, but that seems awkward).

candid halo
# jade hearth Wouldn't thread locals kinda fall apart if you capture signals (or signal-like h...

I hadn't considered that problem, but I am not sure it is a problem. However, there's another problem: which World are we using? The reactive context contains a reference to a world, presumably the same world as was used to spawn the reaction in the first place, but if there are multiple worlds, then which one do we use? I guess that we need to initialize the thead-local variable to point to the world that is currently processing its reactions.

jade hearth
#

Assuming signals are just entities, basically.

candid halo
#

In bevy_reactor, signals looked like this:

#[derive(Copy)]
pub enum Signal<T> {
    /// A mutable variable that can be read and written to.
    Mutable(Mutable<T>),

    /// A readonly value that is computed from other signals.
    Derived(Derived<T>),

    /// A constant value, mainly useful for establishing defaults.
    Constant(T),
}

Where Mutable looks like this:

/// Contains a reference to a reactive mutable variable.
#[derive(PartialEq, Debug)]
pub struct Mutable<T> {
    /// The entity that holds the mutable value.
    pub(crate) cell: Entity,
    /// The component id for the mutable cell.
    pub(crate) component: ComponentId,

    /// Marker
    pub(crate) marker: std::marker::PhantomData<T>,
}
#

So in the case of a signal that connects to a Mutable, it's just an entity id and a component id.

jade hearth
#

Oh, sorry, my thoughts here were only half-formed.

I guess that we need to initialize the thread-local variable to point to the world that is currently processing its reactions.
This seems like it could work just fine.

#

You could use this to implicitly subscribe to signals as well when they're accessed in a particular system.

candid halo
#

I could maybe spend some time updating both bevy_reactor and thorium_ui to the latest Bevy.

#

I'd need to remove a bunch of stuff, like all the templating code

candid halo
#

I have a bunch of implicit conversions for signals, so that any function that accepts a signal, you can pass in a regular value and it turns into a Signal::Constant.

#

Unfortunately, because Signal in my implementation is an enum, you can't extend it to new kinds of reactions without changing the enum.

#

Making it truly extensible would require a dyn trait, which works against the copy / clone aspect of it.

#

I'm also thinking about what @feral tinsel said earlier; there is a case against signals. However, the question is what to replace them with. It's clear that a lot of the use cases for signals can be done in other ways, but this doesn't mean that all of the use cases for signals are so covered. Without actually writing a large complex app, it's hard to validate these ideas.

#

Solving the local connectivity problem has a lot more obvious solutions than solving the global connectivity problem.

jade hearth
#

I have a feeling most folks wouldn't be too excited about Clone signals (if the implicit cloning proposals are anything to go by).

feral tinsel
jade hearth
#

Although Clone signals would solve my lifetime problems -- they could just be reference counted.

Frameworks like Leptos can largely sidestep this lifetime issue by relying on the structure of the application; that is, UIs in Leptos are necessarily built as a tree of components. Trees have clear ownership semantics, so you can associate signal lifetimes with component lifetimes. As Leptos builds the tree, it pushes and pops signal owners to the implicit stack.

I don't think this is a reasonable model for Bevy. While UIs in Bevy may usually form a tree, structures in the ECS are generally not so neat. Reactivity is useful for far more than just UI, so constraining Bevy's reactivity to a tree structure would be severely limiting.

jade hearth
candid halo
#

So in bevy_reactor, mutables are entities; signals are just pointers to them.

#

Mutables use an OwnedBy relation to manage lifetimes - when the owner is despawned, so are any mutables it owns.

#

You can of course spawn a mutable with no owner, in which case it lives until manually despawned.

mental meadow
#

Hmm if you could refer to an Component on an Entity with an id would the Mutable need to be an Entity or does it need to be an Entity to add additional data/components.

candid halo
jade hearth
#

Please correct me if I'm off-base, but I get the sense that the majority of existing reactivity crates diverge from normal-looking Bevy code. Indeed, they often must diverge for the sake of their reactivity. This is primarily what I want to avoid. Rather than a Dioxus element or a trait implementation that takes some context, I just want a normal Bevy function. Something like:

fn counter(mut commands: Commands) -> impl Bundle {
    let (count, set_count) = commands.signal(0);
    let text = commands.derive(move || {
        Text::new(format!("Count: {}", count.get()))
    });

    (
        Button,
        OnClick::new(move || set_count.update(|c| *c += 1)),
        children![text]
    )
}

Should this be feasible, which I think it is, my hope is that its applicability and generality are obvious. If we allow derived signals to be arbitrary systems, then the constraints begin to melt away (at the cost of potentially worse performance).

#

I know it's not always instructive to conjure examples that happen to be elegantly expressed by a particular paradigm, so take this with a grain of salt.

#

Really, my main point is that I'd love if we could find a solution that doesn't put reactivity in a little box, separate from the rest of your Bevy code. (And a solution that isn't coupled tightly to UI, since reactivity is broadly useful.)

candid halo
feral tinsel
#

I'll check it out after work

mental meadow
candid halo
jade hearth
#

I think the reason that signals were invented was to solve a bigger problem, which is writing large, complex apps at scale.
This may be true, although I've actually found signals to be easiest to use and reason about for local state. The more complex an app gets, the less its data tends to "flow," and signals are not very nice when data doesn't flow.

I can't make much more of an argument for this though. It would take me a while to formalize my thoughts on it. My perception might also be too coupled to Leptos, which has some issues when it comes to complex state management.

mental meadow
#

Hmm now that I am looking at the code, couldn't this be merged into Query or a ReactQuery.
ReactQuery - Initialize (Entity, Tick) from Archetypes, then update via Query Events, adding or removing on Enter/Leave, and updating the tick OnMutate(via QueryParam's producing commands).

ReactQuery::should_react(t: Tick) -> bool returns true when there are entities in the buffer that are higher then the passed in system tick.

Then if not already, check should_react() either separately or as part of validation on Systems and Observers, and you have reactive Systems 🤡

stiff basin
#

throwing it out here, and i don't know maybe i'll have time to write my own addendum, but i think that signals as futures is actually really nice and ergonomic, you get a lot of useful things like selecting one of many, using futures select, etc

fossil zenith
#

all the state is held in a single World inside the context

native mantle
quaint dome
#

@raven aurora lol

image.png
raven aurora
#

I had a feeling after I submitted that and saw you were active 3 minutes before.

candid halo
#

@feral tinsel @jade hearth @mental meadow So this morning I had another thought about reactive queries: what if there was an .iter_changed() method, that would only returns rows that were different from the previous run? (Maybe flecs already does this). This would address the "over-reacting" issue - although it doesn't do anything for reactions with heterogenous inputs.

Furthermore, we could mark such queries as fallible if the number of changes was zero, so that the system execution would be skipped entirely.

jade hearth
#

I wrote a tiny Query wrapper with the intention of tracking gets, actually. I believe it's tenable, although it might be tricky to manage more than one component (and things like Entity which doesn't make sense to track).

jade hearth
candid halo
mental meadow
candid halo
#

BTW a question was asked earlier today in (#ui message) that is a great example of how our current approach falls short.

jade hearth
#

I'm going down the road now of "best-effort" incrementalization. With some systems, we can trivially determine whether their inputs have changed even now. For example, if it only takes resources or tracked queries. Others, like systems that take normal queries, have to be re-run every time. I've written a little trait to help distinguish the two, and it could be made even a bit better with deeper integration in bevy_ecs.

candid halo
candid halo
mental meadow
#

I believe the backing state for both queries would be different, unless regular Queries were to be built around my suggestion, but I think that would leave some perf on the table for non-reactive uses.

candid halo
jade hearth
#

But I imagine there's a more clever solution.

#

Oh well I could just write some convenience signals that are only updated by lifecycle events blobthink something like:

let pressed = commands.has::<Pressed>(entity);
let hovered = commands.signal_query::<Option<&Hovered>>(entity);
let variant = commands.signal_query::<&ButtonVariant>(entity);

let color = commands.derive(move || {
    // bail if the variant doesn't exist
    let variant = variant.get()?;
    Some(calculate_color(hovered.get(), pressed.get(), variant))
});

Maybe that's too un-Bevy-like. But it would only re-run following changes from lifecycle events (or change detection in the signal_query case I suppose).

candid halo
#

I assume that commands holds the hidden tracking context?

jade hearth
#

No, I've decided to try reference counting / garbage collection. In a sense it holds the context -- it spawns an entity that I check after every reactive pass, despawning it if no one holds it anymore.

#

It's potentially another efficiency hit, but I think it better matches the often non-trivial lifetime characteristics of entities. And it places no constraints on how signals are created, except that they need Commands.

oak cipher
native mantle
oak cipher
#

Currently doing a detailed docs pass

candid halo
# mental meadow Sounds similiar to what I suggested here https://discord.com/channels/6910524315...

I think there's a subtle difference between your proposal and mine, although I may be misunderstanding yours. We both like the idea of a query that reports changes, but there's a difference in what we mean by a "change". I think what you are proposing is that a change happens when an entity starts or stops matching the query constraint, what you describe as Enter/Leave. What I'm proposing is that a change happens when the data for a row (specifically, the destructured values returned by .get() or .iter()) are different than what was returned the previous time.

#

Although I don't mean different in the sense of having to do equality comparison with component values, just whether any of the returned components were added, removed, or mutated.

oak cipher
#

@native mantle, I've done a review pass on https://github.com/bevyengine/bevy/pull/20731, and I'm really happy with it. The terms are clear, the docs are good, and the core architecture is both less elaborate and more extensible. Before we merge:

  1. I'd like you to go over my suggested docs changes: nothing huge, but I think there's some helpful cleanup in there
  2. I'm going to improve the migration story by adding more deprecated methods and type, per @quaint dome's feedback
  3. I would like at least one more community approval
native mantle
oak cipher
#

@quaint dome, the lifecycle events already have a deprected type alias 🙂

quaint dome
oak cipher
# 
    /// A deprecated way to retrieve the entity that this [`EntityEvent`] targeted at.
    ///
    /// Access the event via [`On::event`], then read the entity that the event was targeting.
    /// Prefer using the field name directly for clarity,
    /// but if you are working in a generic context, you can use [`EntityEvent::event_target`].
    #[deprecated(
        since = "0.17.0",
        note = "Call On::event() to access the event, then read the target entity from the event directly."
    )]
    pub fn target(&self) -> Entity {
        self.event.event_target()
    }
#

Just pushed this up

quaint dome
candid halo
oak cipher
#

And another one:

    /// A deprecated alias for [`trigger`](Self::trigger) to ease migration.
    ///
    /// Instead of specifying the trigger target separately,
    /// information about the target of the event is embedded in the data held by
    /// the event type itself.
    #[deprecated(since = "0.17.0", note = "Use `World::trigger` instead.")]
    pub fn trigger_targets<'a>(&mut self, event: impl Event<Trigger<'a>: Default>) {
        self.trigger(event)
    }
#

Anything else you'd like to ease migration @quaint dome?

#

I've tackled all of the items listed in your comment

#

Except the macro

#

I don't have the macro-fu to improve that :p

quaint dome
quaint dome
#

the macro-fu is already there

image.png
#

just need to improve the message

#

Maybe something like

Expected a struct in the form of
#[derive(EntityEvent)]
struct FooEvent {
entity: Entity,
// optional additional fields
}

oak cipher
quaint dome
oak cipher
#

@quaint dome

            "EntityEvent derive does not work on unit structs. You must have a field to store the Entity target.",
#

How's that?

quaint dome
#

(or be a newtype for Entity)

#

hmm

#

I don't think someone new-ish to Rust and Bevy will have any idea what that means

#

Hence why my suggestion has a specific example

oak cipher
#

But the docs have detailed instructions?

#

Surely "oh, I should read the docs" is the natural response?

quaint dome
#

But you probably have more experience with this than me 😄

#

So I'll trust your judgement

oak cipher
#

I don't think new users will fail in this particular way very much

#

"EntityEvent derive does not work on unit structs. Your type must have a field to store the Entity target, such as Message(Entity) or Message { entity: Entity }.",

quaint dome
#

So this would be semantically wrong hmm

quaint dome
oak cipher
#

Kill is good but a bit morbid

quaint dome
crude bough
#

Attack?

oak cipher
#

Went with Attack and pushed changes

quaint dome
#

approving then check_accept

oak cipher
#

Excellent. I think that just leaves the safety comment 🙂

candid halo
#

Or Bulletin

oak cipher
#

Notify would be a good option, but I'm happy with Attack

mental meadow
oak cipher
native mantle
#

I'm reasonably convinced the safety work is fine / im leaning to merge if we don't hear back soon

oak cipher
feral tinsel
deft thorn
oak cipher
oak cipher
#

Okay, I'm largely in over my head here, but I've done my best to understand, summarize and do some of the more mechanical changes involved with the safety requirements for #20731.

https://github.com/cart/bevy/pull/44

This is still draft: I'm not confident that I understand the problem well enough to actually write up the safety comments, and there's another more complex change that still needs to be made

GitHub
Mark `Trigger::trigger` as `unsafe` by alice-i-cecile · Pull Reque...

I am attempting to implement the suggestions discussed in bevyengine#20731 (comment), courtesy of @SkiFire13 and @chescock.
TODO

write all of the safety comments arising from marking Trigger::tri...

#

@native mantle / @deft thorn / @ruby roost, feel free to push to this branch as needed (ask me for permissions if you'd like), or use the code suggestion tool to improve / correct me

#

The problem and machinery is too complex for me to get a full fix up on my own, but hopefully this helps save y'all some time and provides a nice thread for discussion. I also really wanted to include some clear meta-commentary on "why is this even needed" with backlinks, because the problem is extremely subtle and likely to be lost to the sands of time

native mantle
#

(and @oak cipher's)

native mantle
blazing ice
#

It looks like EntityCommands has lost its trigger method in 0.17. You can't do commands.entity(entity).trigger(...? I can't see that mentioned in the migration guides? They discuss the switch from commands.trigger_targets to just commands.trigger, but would be good to also mention EntityCommands.trigger also being sunset. Deprecation message would be nice too, they're helpful elsewhere in the Observers overhaul.

autumn abyss
#

I've never seen commands.entity(entity).trigger()

native mantle
#

But I think thats worth it

#

I'll throw that in

sleek pivot
#

is there an "intended" approach for implementing Events with occupied type parameters? I'm trying to faff about with bevy_http_client and now the dreaded trio of 'static + Sync + Send compiler suggestions has bubbled up when they weren't previously there.

#

i.e. MyObserverEvent<T> {data: T}

native mantle
native mantle
native mantle
sleek pivot
#

Wonderful :)

blazing ice
#

Love the observers overhaul. Really nice to migrate to, and a much clearer API.

sleek pivot
#

is there a generic struct Observed<T> for some Event or Message T which just tacks on the entity: Entity field to make it an EntityEvent?

lavish snow
#

Since we've moved to EntityEvent having target Entity as it's a value somewhere in event's struct, would it be useful for #[derive(EntityEvent)] to also resolve target to the only named field of type Entity?

Presently it resolves entity: Entity to target automatically. But this doesn't seem to lead as far from the previous event.target() that's only present on EntityEvent, just adds a bit of boiler-plate (need to type that entity: Entity on all entity events.

If you have semantic_name: Entity, that gives some semantic gain. But it won't be resolved as target automatically when the struct has only one Entity field, requiring it to be marked with #[event_entity] explicitly, which feels boiler-platy.

I found myself leaning to use entity: Event because of that. I know, just need to type that #[event_entity] but it feels egghhh

plucky storm
native mantle
frank harness
#

I'm migrating to 0.17 and the overhaul seems great, thank you for your work.

A small nit is that since we don't use the On* prefix anymore and neither use the past tense we end up with very generic names for events in games:

  • Before: struct OnDamage/OnDeath/OnArrival/OnReward
  • After: struct Damage/Death/Arrival/Reward

This makes it difficult to do things like OnReward(Reward)/Rewarded(Reward).

blazing ice
#

What if the events were a verb. Optionally you could have some indication of tense (depending on whether the event is triggered before or after the thing happening) On<Arrive> or On<WillArrive> or On<DidArrive>. Then the wrapped data object could be the noun Arrive(Arrival). Admittedly trickier for words where the verb and noun are the same like reward, and damage.

#

Will / Did is useful sometimes to indicate whether the event is the actual trigger for the thing happening, or a reaction after the thing has happened

frank harness
#

Native events seem to use verbs more than I do (e.g. remove vs removal). I'll give it a try, thanks.

But I think I maintain my feedback that a unique name pattern for events had some advantages that I'll miss.

Screenshot_2025-09-24_at_16.09.23.png
quaint dome
#

After working with the event rearchitecture for a while now, I must say: this is really really really good

#

great work Cart 😄

#

I must admit I was a skeptic of the entity field in entity events having to be defined as part of the struct, but I've come around to it after trying it out in practice

#

It's quite elegant, actually

#

Made me think if there's places in my own crates where I could redesign an API that way

native mantle
sleek pivot
blazing ice
#

I think I might have hit a bit of a limitation in the new system. In my project in 0.16 I used to have a set of events that I would add to animation clips, that were set to auto-propagate. I would then observe these events from the SceneRoot. The AnimationPlayer is not at the SceneRoot, it's a couple of nodes deep. I'm not sure how to port this to 0.17. My event has to conform to AnimationEvent to be added to a clip. The event struct can't contain an Entity, because the clip is a resource that will be applied to many entities. AnimationEvent gives you target().animation_player, but that's not at the root, and I don't think the event can be made to propagate further. Not sure how to proceed. I could:

  1. Try to spawn the scene at the deeper point in the hierarchy where the rig is (and hence the AnimationPlayer gets added). Even then though, I think the AnimationPlayer would still be one node deeper than the SceneRoot (as SceneRoot spawns the scene as a child node).
  2. Or, split each of these events into two, the AnimationEvent on the clip and then a secondary EntityEvent, triggered by the AnimationEvent, that targets the root, or propagates up to the root
native mantle
#

Can you open an issue?

native mantle
#

Although if the goal is to build the ideal consumable interface, its probably better to duplicate the event (and give the "better" name to whatever is intended to be public)

blazing ice
#

There is something about the way propagation works now which really confused me and I don't think is explicitly documented. So now with EntityEvent we have to supply the event target ourselves, and we are encouraged to be specific about the naming of the entity. So for instance, with my Propagate<E: AnimationEvent> I originally labelled the root entity animation_player, because the player is what the event gets triggered on:

#[derive(EntityEvent)]
#[entity_event(propagate, auto_propagate)]
pub struct Propagate<E: AnimationEvent> {
    #[event_target] 
    pub animation_player: Entity, 
    event: PhantomData<E>
}

impl<E: AnimationEvent> Propagate<E> {
    pub fn new(animation_player: Entity) -> Self {
        Self { animation_player, event: PhantomData }
    }
}

pub fn on_propagatable<E: AnimationEvent>(
    propagatable: On<E>,
    mut commands: Commands,
) {
    commands.trigger(Propagate::<E>::new(propagatable.trigger().animation_player));
    
}

My assumption was that when it propagates, this entity would remain unchanged. This is not in fact the case. As it propagates, this entity gets updated. In other words, it is no longer equal to the original_event_target, it becomes the current event_target at each layer of the propagation. This means that the specific naming, in my case animation_player is incorrect:

    assert_eq!(trigger.event_target(), trigger.animation_player);
    assert_ne!(trigger.original_event_target(), trigger.animation_player);

So if we're creating a propagating event, we shouldn't give the event_target a specific name. It probably should just be entity, because it will be referring to different entities at the callsite and observation site. I think this needs to be made clearer in the docs.

native mantle
#

I agree that the name of the propagated entity should match the context of the event in every step of the propagation.

oak cipher
#

I would really appreciate ideas and opinions on the best route here

pastel hound
oak cipher
native mantle
#

Also left some thoughts

oak cipher
#

Thank you all; really appreciate it!

feral tinsel
#

@candid halo

I also think that we (the Bevy community) often roadblock ourselves by requiring the perfect infrastructure to be in place
As a counterpoint & as someone who's been around since pretty much the start, the community has IME also had a tendency to delay complex/controversial work. You have to bite the bullet at some point, and it's better to do that when you have a good use case for it

oak cipher
pastel hound
#

I am generally a proponent of incrementalism and "imperfect-now over perfect-later". in this specific case, i think it's probably good to focus effort onto a workable version of many-to-many, since we already have somewhat imperfect implementations of both observers and relations.

#

the remaining work is refactoring, polish and design, and probably isn't blocking anything.

feral tinsel
#

I am generally a proponent of incrementalism and "imperfect-now over perfect-later".
That's not at odds with doing complex/controversial work. You can incrementally implement something that's complex

#

My point is more about delaying a complex solution and go with* an entirely different simpler solution to fix a short term need

#

Which is totatlly fine, but you need to balance that

pastel hound
crude bough
#

(heck sorry wrong thread)

upper ravine
#

A little summary on porting an observer based control flow library to bevy 0.17.

TLDR probs to early to say but the implementation got me wondering if there could be room for a more flexible superset of EntityEvent in some places, something like:

trait IntoEntityTargetEvent<M>{
  type Event: Event;
  type Trigger: Trigger;

  fn into_entity_target_event(self, entity: Entity) -> (Self::Event, Self::Trigger);
}
GitHub
Porting `beet_flow` to 0.17 Observer Overhaul · bevyengine bevy ·...

The Awesome I've come round to the EntityEvent api in most parts of beet, thanks @Diddykonga i think you're right its best not to think of the Event type literally as a 'payload', r...

plucky marlin
#

Is there a way to order two observers on an object? As in, I have two observers on an object that both watch the click action, and I want one of them to resolve before the other

#

I have a hacky workaround available (using bevy_defer to make the other observer wait a couple frames before taking effect), but I was wondering if there's a way to do that without an arbitrary time delay

candid halo
#

The only thing you can know for sure is that all observers on an entity will complete before propagating to the next entity (if you are using propagation)

plucky marlin
#

Makes sense

#

I may just want to find a different way to go about this; I gave my in-world menu buttons the ability to act as both a radio button of sorts and a regular button, so when I click one of them, the radio group observer will fire as well as the regular button click observer

#

What I intended to do with that is have the radio group fire first and set the selected option, then let the button observer handle responding to that changed selection

candid halo
plucky marlin
#

Alright, that would work

#

I think

#

I'll note that down in case I end up rearchitecting my system; currently I just have a number of other things that need to happen first

candid halo
#

In general, rather than assign multiple meanings to a single type of event, I prefer to cancel the event and trigger a new event that has a more specific meaning.

plucky marlin
#

That actually makes a lot of sense

#

Thank you! 😄

oak cipher
candid halo
#

@oak cipher @native mantle I know this is going to be controversial, but the more I think about it, the more I would like to have a require_observe or require_on feature:

#[derive(Component, Clone, Debug, Default)]
#[require(AccessibilityNode(accesskit::Node::new(Role::Button)))]
#[require_observers(
    button_on_key_event,
    button_on_pointer_down,
    button_on_pointer_up,
    button_on_pointer_click,
    button_on_pointer_drag_end,
    button_on_pointer_cancel,
)]
pub struct Button;

This would call entity.observe() with the given observer functions when the component was inserted. This would have a number of advantages:

  • I could get rid of about 10 separate plugins from ui_widgets and feathers.
  • The user would no longer have to install those plugin groups, simplifying their code and removing a potential footgun.
  • Users only pay the cost for the widgets they use.
  • The individual observer functions could be simplified, since I no longer have to filter out global events that aren't meant for my entity.
  • Performance wins: we wouldn't have to be calling 8 callbacks each time a picking event bubbles.
  • It means that observer registration has feature parity and is more consistent with component hooks.
    The downside is a small increase in memory consumption, because we're registering the observers individually on the entity instead of using universal observers. But I've been increasingly uncomfortable with the way universal observers are used for the widgets anyway, and my discomfort grows as the number of widgets increases.
#

In effect, what this does is to make components more like self-contained modules of functionality: the act of inserting a component automatically installs the logic that goes along with that component.

visual jackal
#

Something similar can be achieved by using a global observer for On<Add, Button> and whatever observer is needed, right? The user still would have to add the plugins, but the cost of having one global observer per component isn't high.

The current implementation of Button, in the bevy_ui_widgets uses global observers, but I don't see why this wouldn't work with entity observers also.

#

I like the idea of require_observers, because there is one additional benefit: It's easier to know what observers affect that Component, just look at the definition of the component.

stiff basin
spice dew
pastel hound
#

I wonder if we could de-duplicate observers created in this way, so that each handler function is only spawned as an observer entity once.

oak cipher
#

I think that spawning extra entities with a single component is too much implicitness. Why can't we return a template/fragment instead, and have users spawn those?

#

(other than BSN isn't merged yet)

oak cipher
#

The downside is a small increase in memory consumption, because we're registering the observers individually on the entity instead of using universal observers. But I've been increasingly uncomfortable with the way universal observers are used for the widgets anyway, and my discomfort grows as the number of widgets increases.
Is this still true if we had proper query filtering in observers?

feral tinsel
#

Isn't using universal observers much more efficient? 🤔 Trying to understand the concern here. If I have a list with a few hundred elements and an observer per element, I'd be much happier with a universal observer than each widget having its own

mental meadow
#

Also, how do you replace/provide these observers, so that the require doesn't get triggered, the signature would need to exactly match and even then idk

stiff basin
#

What are the tradeoffs then?

oak cipher
#

I really don't see what's so bad about a single app.add_plugins(FeathersPlugins) call once per app

#

Which is how every other bit of logic in Bevy works

mental meadow
stiff basin
#

At a higher conceptual level

#

🤔

#

That's where the disconnect comes from

visual jackal
stiff basin
#

In general you want components to be avaliable without behaviors (systems or observers), but not always, and in this specific case we wanna tie them together

mental meadow
#

Yeah, it's bit weird, cause you dont add systems this way, you add them regardless of the components being used, although observers are slightly different since they can be targeted.

stiff basin
mental meadow
stiff basin
#

Yeah, and plugins allow you to switch out behavior without switching out data

#

Which is very nice

visual jackal
#

In a way, observers are like components, but ones which impl IntoSystem with fixed first param.

If we think that way, it is almost like:

#[derive(Component, Clone, Debug, Default)]
#[require(AccessibilityNode(accesskit::Node::new(Role::Button)))]
#[require(
    ButtonOnKeyEvent,
    ButtonOnPointerDown,
    ButtonOnPointerUp,
    ButtonOnPointerClick,
    ButtonOnPointerDragEnd,
    ButtonOnPointerCancel,
)]
pub struct Button;

but at this point, observers are components which carries behavior, not data. 🤔

fluid basin
#

Couldn’t the solution here be global observers with query filters? ex: On<PointerUp, Button>

native mantle
# candid halo <@159873981174906880> <@153249376947535872> I know this is going to be controver...

The user would no longer have to install those plugin groups, simplifying their code and removing a potential footgun.
I think "no plugins needed" is the biggest appeal here. But imo that isn't a problem specific to observers, and this approach operates under the assumption that we will forever limit Button (and widget functionality in general) to required components and observers (which may or may not be true). And the local vs universal observer question ideally should be made based on how it behaves and performs, not which one is more convenient (ideally both are equally convenient).

I think we might want to consider a more generalized solution to the "components are standalone / drive their own behavior" problem (ex: they register whatever functionality they need when they are inserted). @cerulean turtle might be interested in this, as theres a chance a solution would cut down on binary sizes / improve our granularity there (although Reflect might get in the way of that, as it would put the init code in the registry).

I also think this isn't currently the biggest problem that needs solving atm and I'd rather not get caught in the design weeds here prior to getting bsn out.

candid halo
#

Wow, that's a lot of responses! 🙂

#

With respect to universal observers, the concern I have (performance wise) is that they trigger for every click/up/down/drag/keypress event, even for entities which aren't widgets. Consider what happens if you click on an entity that is not a widget:

  • It runs the on_click event for each registered widget: button, slider, checkbox, and so on. Each one says, "nope, this is not for me" and lets the event propagate.
  • The event propagates upward, and we again run every registered observer for button, slider, checkbox, and so on.
  • This continues until we get all the way to the UI root.
    So the number of handlers fired is (number of widget types * depth of hierarchy). This means that the cost increases as we add more widget types. If the hierarchy depth is only like 1 or 2 levels this isn't so bad, but what if it's like 10? That's 50 or 60 observers being called for a click event that isn't even doing anything.
candid halo
oak cipher
#

With respect to universal observers, the concern I have (performance wise) is that they trigger for every click/up/down/drag/keypress event, even for entities which aren't widgets
With a first-class observer filtering mechanisms we could bake in caching for what observers are relevant for each entity

#

Which I believe is something that flecs does

candid halo
#

With respect to the issue of plugins, part of the complexity for users is that feathers widgets have a web of dependencies:

  • The plugins for the feathers widgets themselves (button, slider, checkbox, etc.)
  • The underlying plugins for the headless widgets (also button, slider, checkbox, etc.)
  • The feathers infrastructure plugin, which installs:
    • cursor shape plugin
    • text propagation
    • embedded assets (fonts, shaders, icons)
    • theme stuff
  • the input focus / tab navigation plugin
  • (soon to be added) the plugin for popover positioning
    ...all of which is relatively easy if you want to just install everything; but if you decide that you want just buttons and sliders and don't want to pay the costs for checkboxes and menus it starts to get complicated.
oak cipher
#

Can we just have a single high-level plugin with fields for config?

# 
struct FeathersPlugin {
   buttons: bool,
   sliders: bool,
}
candid halo
quaint dome
# candid halo <@159873981174906880> <@153249376947535872> I know this is going to be controver...

For my downstream work, I also use patterns like this. I sometimes need to encapsulate big observers in their own little subplugins (internal pub(super) fn plugin(...), not externally visible ones) in own files, but adding those observers across files by exporting the functions is sometimes a bit spaghetty-i, so I like to have a component (in this case Button) that has an On<Add> observer that then wires up the entity observers. In that case, that component kinda is the observer, API wise.

#

So can confirm that this would be something useful for me downstream 🙂

upper ravine
#

My behavior library is built on this pattern, I'm currently getting by with a proc macro that hijacks the on_add hook but a derive-only solution would be nice.

#[action(do_something)]
#[derive(Component)]
struct DoSomething;

fn do_something(run: On<Run>) {
  ...
}

I call this state + behavior type an Action, it could be worth formalizing something like that if its gonna be super common

upper ravine
#

Before opening up an issue/pr can i get some initial feedback on upstreaming IntoEntityTargetEvent ?

The goal is to improve ergonomics for third party entity event types.

As discussed bevy should only officially support a single flavor of entity events with the target stored on the Event not the Trigger, however the event_fn: impl FnOnce(Entity) -> E pattern only allows for events with a default Trigger. This results in third parties introducing awkward extension methods for EntityCommands, EntityMut etc.

I'd like to open a PR to upstream IntoEntityTargetEvent which is a superset of FnOnce(Entity) -> E that allows for a non-default Trigger:

pub trait IntoEntityTargetEvent<M> {
  type Event: for<'a> Event<Trigger<'a> = Self::Trigger>;
  type Trigger: Trigger<Self::Event>;

  fn into_entity_target_event(
    self,
    entity: Entity,
  ) -> (Self::Event, Self::Trigger);
}

This is a non-breaking change 100% compatible with the current api and allows third party implementers to use the entity.trigger() method, as well as opening up more possibilities for official patterns.

GitHub
beet/crates/beet_core/src/bevy_utils/entity_target_event.rs at 0f11...

A very bevy metaframework 🦄. Contribute to mrchantey/beet development by creating an account on GitHub.

oak cipher
#

I'm sympathetic to those concerns too, although I would really like the broad adoption of kinded entities, both in Bevy and in the ecosystem more broadly

#

Aha, there we go

#

I like that implementation better than your proposal at first blush at least 🙂

upper ravine
# oak cipher I like that implementation better than your proposal at first blush at least 🙂

haha we've been here before, these issues look similar on the surface but I think they are different.
This proposal does not touch EntityEvent, it replaces the FnOnce(Entity) -> E where E::Trigger: Default with a trait to allow for non-default triggers.
To be clear I'm not proposing the EntityTargetEvent struct in the link I provided, just the trait.

GitHub
Porting `beet_flow` to 0.17 Observer Overhaul · bevyengine bevy ·...

The Awesome I've come round to the EntityEvent api in most parts of beet, thanks @Diddykonga i think you're right its best not to think of the Event type literally as a 'payload', r...

upper ravine
#

@steel zephyr are you able to confirm this?
Reading though #21384 I gather your issue is about EntityEvent support for the kinded entities, and is not related to custom Trigger types.

That said, you did mention the current limitations of EntityCommand::trigger.
My proposal is to replace the FnOnce with a trait similar to IntoScheduleConfigs<M> which would allow you to implement your own patterns in moonshine_kind, would that help?

GitHub
New Event/Observer API is unfriendly to Entity-like types · Issue ...

What problem does this solve or what need does it fill? Hello! I maintain the moonshine-kind crate, which adds the Instance<T> type. The new changes for entity events in Bevy 0.17 make hard a...

steel zephyr
# upper ravine <@179622566456066048> are you able to confirm this? Reading though [#21384](htt...

I'm not 100% sure. It's been a while and I've been distracted from Bevy dev.
But my main issues with the Observer/Relationship API is that they're now too intrusive and limiting in usage because they require mutable access to the underlying entity. As far as I understand, for events this is required because of propagation (which imo is an edge case feature and not worth this limitation), and for relations it is to support spawning entities with pre-existing relations (which again, to me is not common usage).

Both of these issues imply that library authors cannot guarantee any invariants on the target of event/relationship types, and it's kinda killed my motivation to continue my main project. I still haven't migrated my main project to Bevy 0.17 because of all this... 🙁

#

My "instance" events now look like this. but this is just flat out wrong

image.png image.png
upper ravine
steel zephyr
#

Yah, and that's totally fair. 😄 I figured I can wait it out a bit.
But imo, an event, on the base level, should be immutable after dispatch. I understand mutating the event opens room for optimization because you can use the same event to invoke multiple handlers.
But to me, that should be an opt-in feature, not something that's required

#

If we can prevent mutation after dispatch, and if we have the API to create events with entity-like targets (which is what I believe you're proposing), then I'd be happy

upper ravine
#

hmm thats interesting i did find myself copy-pasting the propagation logic when writing my own triggers, maybe thats another sign theres further work to do in propagation

upper ravine
steel zephyr
#

Maybe we could have an ImmutableTrigger 🤔

#

Could work, but it feels like a workaround. Now all my events would have to use this special trigger

#

Especially since I'm trying to delete a feature Bevy really wants to give me

upper ravine
#

I agree that custom impls currently feel like 'special triggers' requiring their own extension methods, macros, propagation etc
I also feel like we're getting a better understanding of whats required to make them feel like first class citizens, just like bevy's other beautifully modular patterns.

upper ravine
candid halo
# upper ravine pr is up, have at it 😜 https://github.com/bevyengine/bevy/pull/21799

I looked at the PR and the other PRs linked. I haven't been involved in the discussion, so take this with a grain of salt. I'm not sure I understand the motivation for this change: what exactly are you trying to do that the current design prevents you from doing? I've been coding a lot of event types as part of both the feathers and bevy_reactor work, and so far I haven't felt the need to go outside the bounds of the current design.

upper ravine
ivory raft
#

Is this in 0.17.3? how do you emit this event after a On<Pointer<Click>> event? I haven't been able to find any examples