:rainbow: Rainbow Team Relations :rainbow:

[unborn quail]

Thank you

[velvet wharf]

Has anyone explored how lattices might fit into relations? Like being able to query for the shortest path between two entities or the degree of separation, etc. The addition of those semantics in datalog is nice in that it helps prevent blowup while still remaining unopinionated about what the lattice relation is computing.

https://paulbutcher.com/blog/2022/datalog3/

[rose pendant]

That's cool. I've been wanting to have a mechanism in flecs queries where you could "group" results on a variable, and use a function to combine grouped values, like group_by($spaceship), sum(Attack.value). Interesting to see how datalog goes about it

[velvet wharf]

Ascent in rust has support for lattices so might be a nice reference https://s-arash.github.io/ascent/

[velvet wharf]

That sounds very cool too!

[velvet wharf]

The problem with a lattice is the naming isn't very intuitive but the idea is quite powerful in that it is a generalisation of aggregation that also supports enriching the query with edge data

lol. At some point someone is going to build a datalog layer over bevy

[rose pendant]

You'll have to convince @cloud plover of that, but flecs is already there :p

[velvet wharf]

Beautiful

[velvet wharf]

Someone will also invariably propose a CSS selector syntax instead (if they haven't already). And media queries would be reborn in bevy

[rose pendant]

@cosmic elk probably has some ideas on that (that's something I'd also like to see get mapped to ECS, not sure how yet)

[cosmic elk]

I did build a CSS-like system for Bevy - with dynamic selectors and all that - however the reactions I got on discord weren't encouraging so I abandoned that approach. Actually, what I built was a carefully curated subset of CSS functionality. Even in the HTML world, a lot of CSS frameworks deliberately restrict what you can do with CSS as a way of making it easier to reason about, so I used a similar approach.

[velvet wharf]

Carts bsn patch proposal could work with selectors which could be an interesting API. Like imagine you have a selector for a shiny button that's a child of a menu, then you could write a patch for the style component that adds the shiny style

[cosmic elk]

@runic shadow Are we planning on implementing observers/hooks for resources?

[tight salmon]

#ecs-dev message

[runic shadow]

It's not a particularly complex thing to do conceptually, it would just be easier if resources were actual entities so we could just re-use everything

If we resolve the render world issues (and merge the component index) components as entities would be a pretty easy change

Depends how many places we still have sparse set issues actually

But yeah at any point we can do resource observers and hooks, it's just annoying since it would (currently) need a bunch of new APIs

[cosmic elk]

@crude oracle @cloud plover @runic shadow @rose pendant Now that we have observers, I want to raise the issue of derived/memos. Solid and React both have this concept, it's the next level up from reactive signals/state. Let me give a concrete example: Say I have a component or resource that is being mutated at high frequency - this means that any reactive view that depends on it is also going to update at high frequency. However, it's often the case that what the view is showing is a summary or distillation of the data - the length of an array rather than the actual elements, the region the player is in rather than the exact coordinates. It would be ideal if the view would only update when the computed value changes, not when the underlying data changes.

This is normally done by memoizing the computation. You can do this today by explicitly copying the resource or component to another resource or component, but that involves a fair amount of boilerplate. React/Solid have primitives that take care of the memoization for you, implicitly storing the memo value and managing its lifetime.

This is an area where Quill arguably does worse than Haalka or bevy_reactor - Quill has memos, but because it's coarse-grained reactivity those memos can only be recomputed when the template goes through an update cycle. In other words, if your resource is updating at high frequency, the template has to update at high frequency in order to call the closure that produces the memoized value.

So I can imagine some kind of observer-like construct which produces an output value, which has change detection that only triggers when the output value is different from the previous evaluation (using either PartialEq or a custom comparator).

[ancient spear]

I’d be interested to see where discussion goes with this (if anywhere) since I’m working on something similar, though specialized for the new render graph

most of what I’ve been working around is reducing the effective size of the comparison/hashing key as much as possible, and automatically

[tight salmon]

So like?:
State Components -> Observer A
Observer A -> Computed Components
Computed Components -> Observer B
Observer B -> View Components
Since Observer can have Queries of there own, you could do this with OnMutate as suggested above I think 🤔

[rose pendant]

Been thinking about how to do computed components for a while, one possible direction is pluggable storages

With accessor functions that can abstract away the difference between something that's actually stored in memory vs. something that's computed

That way most of the complexity (ideally all) is hidden from the user facing API

[tight salmon]

Is that equivelant to how Bevy has storage type for, dense vs sparse, on the Component Impl now, and the fetcher just knows what do

[rose pendant]

It'd be up to the storage to implement memoization, and with the right interface would also allow for things like storing/retrieving data over time

[rose pendant]

If it allows for what I just described then yes 😉

I don't think it's a solution for high frequency updates, whether you'd use computed values or not has little to do with how often the data changes

But having computed values as a feature would be really nice. Another solution, which is a bit less taxing on the ECS, is to add support for computed values to the template language

e.g. something like

const length = my_prop.length();

ent {
  Position: {y: $length}
}

You could make the incrementalization logic clever enough so that it knows only to update Position if the value of length changes

[rose pendant]

Not really in favor of observer chains like this, that adds a lot of unnecessary overhead

[velvet wharf]

Been reading the data driven design book and in the perf section it talks about how recomputing values can often times be more efficient than caching it in memory due to cache coherency. Is this the type of overhead you mean?

[rose pendant]

Yes and no. By the time you're 2 observers away from a change in the data cache coherency is shot anyway. Anything could've happened by that point

It's more the inherent overhead that observers have, finding the right observers for an event + component, invoking all of them, deferring commands, running command queues etc

That's a lot of work for just figuring out that something's changed, especially if that something is local to a template

[crude oracle]

Very on board for upstream inclusion of something in this category. Although I think we should focus our energy on landing air-tight / as-cheap-as-possible OnMutate observers first, as that feels like a more fundamental building block

[small summit]

how does ordering actually work with relations?
Given that, as I understand, ui is reliant on the order of children and the hierarchy system is intended to be moved to relations

[viscid python]

order of items in a table

with archetype level relations all of a parents children are in the same table

flecs lets you sort queries which bevy currently doesn't have

See: https://github.com/bevyengine/bevy/issues/13464

[alpine patrol]

Topologically sorted relations are extremely important to me, it unlocks some massive traversal improvements.

[small summit]

ah, right, that makes a lot of sense. So you'd have some sort of hidden "child index" that you can sort by in queries and presumably some way of setting it to e.g. change the order of items in a list. Lots of re-sorting then but probably fine.

it is probably worth noting this because afaict it does push the timeline for replacing the current hierarchy system out quite a bit further than I got the impression it was just based on how many things said they were blocked on it

so it's like
render graph rework -> other relations prereqs -> min relations -> non-zst relations -> sorted relation queries -> new hierarchy system

which is not too bad but still seems far enough out that it might be worth improving the current hierarchy system first after all

[viscid python]

non-zst relations isn't a thing that will be in min relations

I don't think you'd have to specialize sorted queries to work with relations either

there's depth indexing which is a different optimization

from min relations it's really just adding some new query capabilities & optimizations to do the traversals & you're pretty much there

[small summit]

ah, RFC should perhaps be updated then, still says

Pairs can be inserted and removed like any other component. For simplicity the initial implementation will have all relationships take the form of (ZST Component, entity) pairs.

[viscid python]

maybe I'm misremembering 🤔

either way it wouldn't matter because both Parent & Children are ZST relations

(with relations we would just have Parent)

so it wouldn't be a blocker for hierarchy work

[small summit]

but that means you have no control over order as you do now, which would be a pain for UI. That's why I'm asking.

[viscid python]

I think you're not understanding something

If e0, Pos, Vel is the parent
then all it's children look something like:
e1, .., (Parent, e0)
e2, .., (Parent, e0)
e3, .., (Parent, e0)

You do not need non-ZST relations for this

you just reorder the table

[small summit]

oh I get it now. You can have a separate ChildIndex component, and then world.query::<(ChildIndex, Targets<Parent>)>().sort(by_child_index)

[viscid python]

something like that yeah

[small summit]

I was assuming the index would have to be part of the relation to sort the table by it

[ancient spear]

Render graph rework isn’t a prereq thank god 😅

Just the retained render world pr

[timid mountain]

The ComponentIndex PR has been merged!
Other in-flight PRs:

• @wind willow 's PR on retaining entities in the render world
• my PR on removing FixedBitSet for Access and replicating them with sorted Vecs (which might need a tiny bit more work)

The big next step after that (or can be done in parallel) is probably the Queries-as-entities PR, which I think @unborn quail and @runic shadow are working on

[unborn quail]

I am working on it! I am just slow

[unborn quail]

Hey @runic shadow, I basically caught up to your old queries-as-entities branch and I'm wondering what the next step might be. My first instinct is to remove new_archetype from SystemParam, as that will be done by observers. But I'm not entirely sure how to reference a query entity in the non-entified machinery of systems

[worthy wigeon]

An OnMutate observer would be brilliant for all kinds of use cases, is anyone currently working on it?

[timid mountain]

I think @cloud plover

[cloud plover]

Yep 🙂

[runic shadow]

I don't think removing new_archetype on SystemParam is strictly necessary. I imagine there are very few third party implementations of SystemParam that use it and I think it's only queries that use it internally but no need to add another breaking API change immediately.

The more salient part is no longer using that path for queries and transitioning to observers instead. In order to do that the query entity needs to also be an observer that listens to archetype/table creation events for the components they require and when triggered evaluate the archetype to see if it matches the query and if so add it to the cache.

Systems/the SystemParam implementation then won't have to deal with mutating the query cache at all and are just responsible for grabbing it to use for iteration in systems.

Eventually we obviously want archetype/table deletion events as well but that's a lot of additional complexity that should be reserved for a follow up.

With that, the component index and a retained render world components as entities should be unblocked which would then put us in a position to actually implement MVP relations.

[unborn quail]

right, that makes sense for keeping the pr small

I currently have implemented Component for QueryState, with an OnAdded hook that adds an observer that listens for a ArchetypeCreated event

now I just need to actually store queries in archetype storage and also make sure that the ArchetypeCreated event actually fires every time that happens

I'm just curious as to what systems and such will store if queries become entities

[runic shadow]

One thing to be careful about is ensuring there's no time period in which the query cache has already been initialized but the observer does not yet exist as that leaves an opportunity to have an archetype be created and not seen by the query

[unborn quail]

just an Entity or maybe an EntityMut

[runic shadow]

I would test just storing an entity in there for now and see if it has a big perf impact

You would hope fetching a component on an entity shouldn't be a bottleneck for an ECS

[unborn quail]

fair enough

I'll get back to you, I'm going on a bike ride

(but do keep typing, I'll read it)

[runic shadow]

In order to ensure this I would try having the query and observer be the same entity. We have the internal APIs to allow this

[tight salmon]

Can observers 'observe' multiple events? if not, then the query would need to be split from the observer at some point.

[alpine patrol]

They cannot, would need to implement runtime-dynamic sum types.

[runic shadow]

Observers can absolutely observe multiple events

(unsafely)

[alpine patrol]

What does the trigger return for the event?

[runic shadow]

You have to use unsafe APIs

So you can make the trigger return any type and you just have to promise it's safe

The observer for a query should set the ObserverRunner manually to not pay for system overhead

[tight salmon]

Right since you cant carry that information via generics, unsafe is the only way (variadics when?)

[runic shadow]

And the ObserverRunner API just gets a pointer and deferred world

(similar to a hook)

[wary fjord]

I wonder if you could wrap that safely in a Trigger<Either<A, B>> type api

[alpine patrol]

You could, but only for pairs. There’s probably a very cursed api with Tuples and type id on the event getter.

[wary fjord]

bevy developers aren't averse to the cursed

actually probably something like a Trigger<Or<(A, B, C, ...)>>

[alpine patrol]

I considered something like this for mod picking but didn’t want to push the complexity of observers further

It would be extremely useful

[wary fjord]

I'll make an issue 🙂

[wary fjord]

https://github.com/bevyengine/bevy/issues/14649

bevy: "rust can we please have anonymous enums"
rust: "no we have named enums"
bevy: "fine. I'll do it myself."

[unborn quail]

I made another draft PR. It's bad like all of my other ones, but I'll keep working on it: https://github.com/bevyengine/bevy/pull/14668

[wary fjord]

I figured out a pretty clever way to do multiple event types safely, ergonomically, AND in a backwards compatible way: https://github.com/bevyengine/bevy/pull/14674

It's in a working state, but there's a few things left to do still (as noted in the PR).

Some help on figuring out how to shrink the lifetimes would be great 🙂

[wary fjord]

Got the right shrinking down, Trigger::event, Trigger::event_mut, and Trigger::event_ptr are now re-implemented (and the test makes use of them)

[steep veldt]

that's pretty nice, how are you matching it? generating an underlying enum that wraps those events?

[wary fjord]

Just like with the rest of bevy we'll be limited by the 12-16 tuple rule, but generally yes: https://github.com/ItsDoot/bevy/blob/ebc3207336ef8f5aa4df9b1002bf0711f6d34935/crates/bevy_ecs/src/observer/set.rs#L112-L176
I haven't converted the implementations over to a macro yet, so it's just 2 and 3 variant enums currently

and usage example here: https://github.com/ItsDoot/bevy/blob/ebc3207336ef8f5aa4df9b1002bf0711f6d34935/crates/bevy_ecs/src/observer/mod.rs#L605-L624

[steep veldt]

nice, pretty cool, maybe I can make use of it in Flecs Rust as well.

Currently it's only "Type safe" for 1-typed Observers for the .param() field and indentifying events is through if else currently

if event == flecs::OnAdd 
...
else if event == MyEvent
etc

unless you use an enum ofc

[wary fjord]

Yea that should be doable just fine in flecs from my limited experience with it

[steep veldt]

Some day I'll have a crack at it, not on the radar for the near future 😛 got enough things to do

good job 🙌

[wind willow]

with query/system as entities, it seems that each query/system entity will be completely in a different archtypes ?

[runic shadow]

It depends how type erased the storage is

[rose pendant]

It'd have to be type erased right? Otherwise you can't query for say all systems

[runic shadow]

You could support that with a marker component if you wanted

[rose pendant]

Ah, sure

[runic shadow]

If each system entity just has a System component that stores the system then no worries, if that component is generic over the system parameters then we'll have a lot of fragmentation

[wind willow]

but it is difficult for Bevy's existing query implementation to achieve type erasure without performance loss.🤔

[rose pendant]

Just checked, the tower defense demo has ~2.3 systems per table. Probably due to module/phase fragmentation (higher* than I expected)

[runic shadow]

Not really, it just requires more unsafe

Yeah I would expect fragmentation isn't a huge deal here anyway

[wind willow]

Highly fragmentation system/query entities may block some future optimizations, it would be much better if they could be in same query/system CompoentSparseSet

[runic shadow]

I'm sure you can get them in the same table if that's really desirable, just maybe not the same archetype

[wind willow]

with type erasure querystate, the current for-style iteration model of Rust will inevitably introduce branches in the hot path(currently our for-style iteration is already slower than foreach )

[runic shadow]

There's no overhead in just casting type erased storage, I'm not saying to expose the query to the user in a type erased way

The struggle is with the arbitrary layouts of the tuple types, but without moving/addressing those such that the types are heterogeneous you are never going to get dense iteration anyway

[polar vortex]

queries don't need to store tuples

the API receives a tuple as part of the query declaration but the only tuple it ever has to return is a tuple of references for the fetch

the storage could work like this if you needed consistent layouts
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9629274646d3d9d8b74cb5098faae91c
(edit: you can go further and store the data on the heap to achieve complete type erasure, for example: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=f42dd31b836f2d06de5978d4301cbbad)

(same goes for systems)

[wary fjord]

This PR is now ready for review.

All that's missing on my end is more docs and some doc examples, but the PR description should cover the explanation well enough for now. Also added 7 tests and 3 benchmark groups. I've ran the benchmarks on both a Ryzen 7 7800X3D and an Intel i7-14700HX, and my results seem to conclude that this feature introduces no extra overhead, even with high-arity multi-event observers. I implore others to try the benchmarks out themselves to see if I'm missing something.

[tight salmon]

Very nice feature, thank you for this
The OrX types do hurt a little, and that is probably just a lack of variadic's, so not much can be done there
I wonder if instead of supporting the auto-registration feature, if it would make more sense to use the type parameter version of DynamicEvent to fulfill the SemiDynamicEvent's purpose, I am not aware if there is another feature/part of the engine that auto-registers dynamic things (if there is, then nevermind), so it feels a little wierd for this feature to exist only for observer use.

Just nitpicks, overall though looks like a great feature.

[wary fjord]

I wonder if instead of supporting the auto-registration feature, if it would make more sense to use the type parameter version of DynamicEvent to fulfill the SemiDynamicEvent's purpose, I am not aware if there is another feature/part of the engine that auto-registers dynamic things (if there is, then nevermind), so it feels a little wierd for this feature to exist only for observer use.
Yea that's fair, I'm not bullish on having the auto-registration feature. 👍

I'm also quite open to renaming DynamicEvent and SemiDynamicEvent, or merging them

[wary fjord]

I've now removed the auto-registration feature, but would like suggestions on a new name if DynamicEvent and SemiDynamicEvent are to be merged (alternatively new names for both of them if they aren't merged).

[unborn quail]

Question @runic shadow, I'm looking at the as_readonly on QueryState where you can change the type of a QueryState and I'm wondering if that can stay when QueryState is a component

shouldn't all that be type erased?

[runic shadow]

The readonly variants should share the same state type so you can do that cast for free, it shouldn't allow you to do anything that actually mutates the state

[unborn quail]

I suppose that makes sense

yeah that makes sense

I was briefly worried that by changing the type (which doesn't really happen) that the QueryState would fall into a different table column or something like that

types are annoying in that way in which they are not real in the final binary, but when it comes to bevy they are kind of very real with regards to component ids

[tight salmon]

This will be especially true, if we get disabled components that use a bit.

[unborn quail]

I really need to learn how type erasure works

like I think I know, but I'm not sure that I know

[cosmic elk]

Type erasure in Rust isn't just one thing (unlike, say, Java), it's a grab-bag of different techniques. The most common of these techniques involves dyn trait objects.

[runic shadow]

Type erasue in ECS internals though is mostly just casting to and from untyped pointers for storage purposes

[alpine patrol]

if you value your sanity do not look too closely at how commands are queued.

[runic shadow]

At least with component columns everything has the same layout, commands having different sizes leads to a couple more complications

[polar vortex]

type erasure in bevy is mostly about monomorphization

each component type T generates a set of functions with specialized content (casting to and from untyped pointers like James said) but common signatures and the world caches them in vtables for "dynamic dispatch" later

the world builds a struct similar to this for each component type

pub struct ComponentVTable {
    name: &'static str,
    type_name: Option<fn() -> &'static str>,
    type_id: Option<fn() -> TypeId>,
    layout: fn() -> Layout,
    dangling: fn() -> NonNull<u8>,
    clone_ctor: Option<unsafe fn(NonNull<u8>, NonNull<u8>)>
    default_ctor: Option<unsafe fn(NonNull<u8>)>,
    drop: Option<unsafe fn(NonNull<u8>)>,
}

by monomorphizing functions like these

fn dangling<T>() -> NonNull<u8> {
    NonNull::<T>::dangling().cast()
}

unsafe fn default_ctor_dyn<T: Default>(ptr: NonNull<u8>) {
    ptr.cast::<T>().write(T::default());
}

unsafe fn drop_dyn<T>(ptr: NonNull<u8>) {
    ptr.cast::<T>().drop_in_place();
}

[polar vortex]

That part is pretty straightforward. Type erasure is a little more complicated where you need to deal with tuples. Setting up the plumbing between Bevy's strongly-typed "dependency injection" APIs (for query states and system states) and type-erased storage can get a little nutty.

[polar vortex]

I made this playground to show what it could look like.

[unborn quail]

Thank you @polar vortex, sorry I got to this so late

I can't say that I understand it fully sadly

and it's one of those things where I don't even know what questions I should ask

[unborn quail]

where to even begin

[tight salmon]

So to prevent dynamic dispatch, which is slow, using a dyn Component and trait methods.
You instead monomoprhize individual functions a single time when registering the Component, and store those functions for later use under a user-constructed VTable, so that you essentially get the speed of using a impl Component<T> , without providing the T.
(Probably not the best explanation )

[unborn quail]

I'm going to google a bunch of those terms and get back to you on that

[timid mountain]

The explanation is basically that for each type T, we have a function fn my_function<T>() that you want to type-erase. The code for that function is stored somewhere in the stack. You can pass around the pointer to that function code, represented by fn(). It's type-erased because it's just a pointer to a memory location in the stack.
Then:

• you can transmute the pointer fn() back to something like Fn<T> if you know at runtime what type the pointer corresponds to.
• or you can directly call the type-erased fn(). Rust will basically go to that memory location and execute the function code there

(that's by basic understanding)

[polar vortex]

🤔 Maybe it would help to start with the fact that World has no generic type parameters. The ECS doesn't have prior knowledge about any of the components you'll be registering at runtime.

And internally, all low-level operations—moving entities between archetypes/tables, finding them using queries, etc—are performed with just the IDs of components.

When you register a component, the World creates a representation of that type that it can use even though it's parameter-less and can't understand types itself.

That representation could have been some strongly-typed struct whose type is erased by storing it as a dyn Vtable trait object (like how we do systems).

That would have been OK, but Bevy opted to use non-generic structs here instead. Having these type registrations be well-defined structs instead of opaque trait objects means the World doesn't have to incur dynamic dispatch costs to use them.

[unborn quail]

what are non-generic structs?

[polar vortex]

I just mean a struct without a generic type parameter.

like MyStruct instead of MyStruct<T>

[unborn quail]

right, but why don't you have to dynamic dispatch then?

I thought that generic structs could use static dispatch, and non-generic structs can't

[polar vortex]

Right, but World has no type parameters, so its fields (and their fields) can't have them either.

[unborn quail]

that makes sense

[polar vortex]

So if you register component T, the only ways the World is even capable of remembering it is either a struct that doesn't have a parameter or with a trait object.

[unborn quail]

and by creating a struct without a parameter, you effectively put the type information in the runtime

(I don't know how trait objects work, I'll read up on it later)

[polar vortex]

Rust's traits are like interfaces, and a "trait object" like dyn Trait is "an instance of some (unknown) type that implements Trait".

Pointers and references to trait objects are "fat", i.e. &dyn Trait actually has two fields, a pointer to the data and a pointer to the erased type's implementation of Trait that was generated by the compiler.

If x is a &dyn Trait reference, then calling one of its trait methods will first have to deref that second pointer in order to lookup the correct fn pointer. That's dynamic dispatch.

[unborn quail]

in what situation is the type unknown?

because usually if x: SomeType and SomeType implements Trait, then this extra step is not necessary

the compiler can figure it out at compile time (static dispatch, if I believe)

[polar vortex]

if all you have is a reference to some dyn Trait, you don't know what the original type was

[unborn quail]

I guess I'm just wondering in what circumstances those references are created

but I suppose I can google that

[polar vortex]

well most of the time you don't have to deal with them, most of the time you can just write functions like this

fn foo<T: MyTrait>(value: &T) { /* ... */ }

where T will be known and it'll use static dispatch internally

[unborn quail]

yes

[polar vortex]

what's different with us is the World does things on its own

when you despawn an entity, you don't expect to have to specify every component it has, right?

[unborn quail]

right, the components are hidden behind an opaque Entity

[polar vortex]

Yeah, so that's at least one context where the compiler doesn't have any type information, the user is telling a parameter-less World to destroy a parameter-less Entity.

Some of those components may have Drop logic, so somehow the World still needs a way to perform that even though the call to despawn doesn't carry any type info.

[unborn quail]

huh, that's interesting

I thought that at this point everything was type erased, but I suppose you still need that info for things like Drop

[polar vortex]

Yeah, everything is type erased, but there are a few operations (like dropping component values) where we need to "un-erase" the type.

[unborn quail]

dude where's my type?

[polar vortex]

It's for those cases that the World creates this "vtable" when the component type is registered (world.init_component::<T>()).

[unborn quail]

I'm looking through the code, is the ComponentDescriptor the vtable that is created?

[polar vortex]

yes

[unborn quail]

cool, I was grepping for vtable earlier, but couldn't find it

[polar vortex]

my bad

[unborn quail]

nah it's good

[polar vortex]

What I was saying above is that this "vtable" could have been one generated by the compiler (i.e. some dyn Trait object) but Bevy opted to use a regular struct because it's still pretty clean and it avoids dynamic dispatch.

[unborn quail]

aren't you doing dynamic dispatch manually now though?

[polar vortex]

yes

lol

[unborn quail]

okay

I was confused about that

[unborn quail]

because this sounded a little like: we avoid dynamic dispatch by doing dynamic dispatch

by hand, but still

[ancient spear]

I guess it avoids having a trait object for every component instance? So you only have the one vtable to deref per component id

[unborn quail]

I assume it has performance benefits

[ancient spear]

And lets you do some other funky stuff like skipping Drop where possible

[unborn quail]

the way it's done now

yeah

[polar vortex]

doing it manually can sometimes be faster because if we grab the ComponentDescriptor once we have all the fn pointers on the stack, we don't have to go through the cache or memory

also, since it's like this we can easily support components that aren't rust types without needing an entirely different code path

[polar vortex]

It is possible to have "one type, one vtable" with an approach based on trait-objects.

i.e. world.init_component::<T>() could generate a ComponentDescriptor<T> struct that implements the trait DynComponentDescriptor

and then wrap it in a Box<dyn DynComponentDescriptor> and store that

we do use that kind of type erasure for storing systems

where internally this happens

let system_struct = IntoSystem::into_system(user_provided_system_function);
let system_trait_object: Box<dyn System> = Box::new(system_struct);

[unborn quail]

and all the type information in the function, like FnOnce(i32, i32, &str) -> bool is removed (or well, hidden) here

[polar vortex]

yeah, from the outside

[unborn quail]

So you can do handy things, like put them in a vec: Vec<Box<dyn System>> or more general scheduling shenanigans

[polar vortex]

exactly

[unborn quail]

got it

I think we should also do this for queries

or, I think we have to do this in order to get queries as entities

[polar vortex]

Yeah, if you wanted to store queries as standalone entities and easily find those entities, we probably want to erase the types so there's just one component and we don't have to deal with Query<(A, B)> being a different type than Query<(B, A)>.

🤔 Now that I write that, I realize I completely misunderstood the reason y'all were talking about type erasure in the first place.

[unborn quail]

I realized the same, but it was educational so I didn't try to stop you

[polar vortex]

I appreciate it.

All that other Rust playground stuff is still kinda relevant since IMO we'll eventually need type erasure inside queries so that their guts end up the same whether or not you have type info when you create one.

[unborn quail]

this was legitimately very helpfull

[somber girder]

I've wondered, with procedural macros I feel you could convert every tuple variation of a f.e. QueryData type into one "canonical representation" and back. Could this not address the tuple situation?

[unborn quail]

currently I'm trying to do the very bruteforce thing where I make QueryState a component and have Query contain an Entity pointing to the QueryState

and I'm running into a bunch of type and lifetime bullshit

which I don't fully understand

[polar vortex]

you probably could but since tuples can be pretty long I think you might run into "combinatorial explosion" and longer compile times

[unborn quail]

wait, can components be nested in one-another?

no right, no almost definitely not

[polar vortex]

no, I don't think so

for the most part, I think the only nesting we deal with is custom system params

and maybe bundles?

[somber girder]

hmm, I don't think you need the combinatorial explosion actually. I haven't fully thought it through, but rather than deal with combinations, the macro would simplify a tuple variation into a single tuple, then sort its contents.
I won't deny the compile time increase though

[polar vortex]

ah right, you don't have to generate a trait for every permutation only the ones that are used

yeah ig if the typing is erased, it's unclear how to recover it when you actually want to use the query, so if query tuples could be canonicalized, you wouldn't need to erase for searching

[somber girder]

yeah

[polar vortex]

Soooooo, coming back to this, there probably are some minor performance benefits, but I think why Bevy does it this way actually stems from the fact that Component originally wasn't a trait you implemented/derived.

[unborn quail]

So you could just struct MyComponent and be off to the races

hmm, that's interesting

[polar vortex]

@cloud plover I thought I was tripping for sec, but there was this. https://github.com/bevyengine/bevy/issues/1843

I'm basically trying to rationalize why Bevy did the dance with unsafe here instead of using trait objects because it's kinda "uncharacteristic" of the usual attitude.

[cloud plover]

I think that the initial blanket impl is a good hypothesis TBH

[cloud plover]

It was such a footgun T_T

[polar vortex]

I think it kinda makes sense in light of Component initially being blanked implemented and how these descriptors are also used for non-send resources. The is_send_and_sync field isn't something we can infer from the type (negative trait bounds and specialization never stabilized).

[cloud plover]

Little typos of like &Option<T> vs Option<&T> would just silently do nothing

[whole wigeon]

I am really curious, is anyone actively working on relations? or, if there are still some things blocking it, what are those?

[noble panther]

Retaining the render world is still the main blocker afaik

[viscid python]

yeah it's just slow

[timid mountain]

• @wind willow has this PR ready for retained render world entities. I don't know if there any actual blockers left, but there's a lot of comments: https://github.com/bevyengine/bevy/pull/14449
• @unborn quail has this draft of queries-as-entities: https://github.com/bevyengine/bevy/pull/14668 which is a major short-term step. Do you need help @unborn quail? Is there some way to divide/plan the work?
• I have this PR: https://github.com/bevyengine/bevy/pull/14385 to not use FixedBitSets in Access which will use too much memory when relations are out. Not super urgent

[cloud plover]

In terms of observers, the most useful thing would be ordering observers, but I'm reluctant to do that before systems-as-entities

https://github.com/bevyengine/bevy/issues/14467 seems quite doable, but isn't very important

https://github.com/bevyengine/bevy/issues/14598 / https://github.com/bevyengine/bevy/issues/14084 are docs issues, which would be good to tackle

[whole wigeon]

I am curious, is it technically possible to get started on relations today? like, even if it cannot be merged yet due to blockers, would it be possible to start a draft PR? If so, I think I am capable of doing this, as I have experimented a lot with flecs, and even writing my own very basic ecs with relations

Im just really excited for relations, and even if we just have a draft PR, we can actually start experimenting with the API for it.

[alpine patrol]

Yes, I believe this is the case.

[cloud plover]

With hooks in place I don't think there are any blockers to prototyping

And it looks like the outstanding work probably won't merge conflict badly either

[tight salmon]

I think(?), most of the pre-work is just massive optimizations that would make relations worth it, but not require it to be impl'ed

[whole wigeon]

I'll go ahead and get started, I don't see a reason why not to! I will probably need help here and there, but I am really excited to get started!

[timid mountain]

Which part do you want to work on? Adding 'relations' identifiers?

[unborn quail]

Yes, I do actually need help

Quite stuck atm

I'm going to work on it this evening and see if I can put some of my problems into answerable questions

[unborn quail]

From my understanding: yes absolutely

Most of this stuff is pre-emptive optimization to deal with the exploding number of archetypes

[timid mountain]

It would be great to have an early implementation of relations + associated benchmarks, so that all the other work (archetype cleanup, removing fixedbitset, etc.) could show clear improvements on the benchmark. I think it's a great idea

[whole wigeon]

I think it makes sense to start there, I have one question immediately: what do we name the 32 bits where the relation target will be stored, right now its named generation of course.

[whole wigeon]

yes for sure

[timid mountain]

Maybe "relationship target"

[tight salmon]

Yeah target should be the name (source: self.entity, (kind: entity1, target: entity2))

[whole wigeon]

Is there any writeup anywhere of how we want the IDs? before I implement something that wasnt intended

Think this is it?

https://github.com/james-j-obrien/rfcs/blob/minimal-fragmenting-relationships/rfcs/79-minimal-fragmenting-relationships.md

[tight salmon]

Yeah, the second half under Minimal Implementation
Identifier PR:
https://github.com/bevyengine/bevy/pull/9797

3rd Party Crate, for reference:
https://crates.io/crates/aery

[viscid python]

aery's pretty irrelevant to what a fragmenting relations impl would look like

Same for the query API. Should be more like the API in flecs.

[whole wigeon]

Alright, after reading a bit, here are my initial thoughts: any notes, corrections, etc would be appreciated.

We already have a more generic Identifier, but that stores an identifier type in the high bits, wont that cause issues by masking of some of the target bits?

We need to make everything that currently requires a ComponentId take an Identifier instead, and for now just convert the ComponentId to an identifier, until we have components as entities.

The Relationship type will be very similar to Entity, just renamed fields.

Then, we should be able to just add some methods to add relations

(no cleanup yet)

[whole wigeon]

We also most likely need to change the SparseSetIndex trait, because currently it works with usize, and it probably should be u64 (this is because we need to convert Identifier to a SparseSetIndex).

[cloud plover]

I'm down for that; seems like a simple PR

[whole wigeon]

yeah probably should do that seperately from the big relations PR. I'll do that first.

[whole wigeon]

actually, might not be that simple. FixedBitSet needs a usize

[runic shadow]

The problem with just trying to implement it now is there are still sparse sets and fixed bit sets in places where entity ids would end up which would blow out memory consumption

The PR would have to be huge to fix all of that and also implement relations on top, although I suppose you could try and do it in backwards to show perf improvements

You also need archetype deletion for when you delete the target of a pair which is another huge complex change, you can't just leave the archetypes around as there is no way to differentiate the pair from a semantically different pair after the id is recycled. It's not just a perf issue

(which effectively requries queries as entities)

If you try and just implement it you will end up running into all the pitfalls the RFC lays out

[whole wigeon]

Hmm, where would you recommend I start? Right now it feels like everything just depends on eachother, and there is no real way to just do it in smaller steps.

[runic shadow]

A pr to just replace all the sparse sets and fixed bitsets can be done alone, we need to benchmark that anyway so good to be it's own PR

We also have a component index now but I believe the columns are still stored redundantly on the table so a PR to remove those and use the index instead would be good

At that point it does become difficult to not be blocked by the render world as queries and components as entities are the next two big steps

(component as entities not strictly required for relations but would be a good step to prove out archetype deletion, otherwise you only actually use it once relations are fully implemented)

(and also not particularly complex other than the parts that are also needed for relations)

[whole wigeon]

Not entirely sure what you mean by this. what do I replace it with?

[runic shadow]

Depends on the usage I suppose. We can't have any places where the memory consumption is O(value of id) since those ids could include components that are id 1,000,000

(actually much more than that including high bits for pairs)

[whole wigeon]

yeah

[runic shadow]

Paged sparse sets are one approach that flecs uses alot, arrays of ids also are sometimes good enough like in the access PR

[whole wigeon]

I am struggling to find something to work on right now, it seems like everything is blocked by at least one PR. or I am missing something. Either way, if someone can give me a concrete "assignment", that would be greatly appreciated.

[runic shadow]

Removing the sparse set on the tables specifically is pretty well scoped, the columns are already stored in the component index so all that needs to happen is remove the sparse set in the table and look up the column in the index instead

Should be quite a small PR

[runic shadow]

Personally I think ordering observers is not particularly desirable. I understand how conceptually it is but implementation wise when do you resolve the ordering? How do you do the lookup when invoking them without paying out the nose?

Its a 1 to many mapping so do you collect all the observers you want to involve and resolve ordering? That seems too expensive.

Maybe there's some really fancy way to architect the index such that you can do all the work during registration and do lookups efficiently but that seems like a really difficult engineering problem. Maybe I'm just not seeing some easy way to do it.

[rose pendant]

One thing that doesn't have too many dependencies (correct me if I'm wrong) is to introduce relationships that only consist of types (so no dynamic entities)

That doesn't require cleanup, archetype deletion, refactors of the query cache etc

[runic shadow]

Yeah that would be doable, to prove out the identifier usage over straight component ids. Still has issues with bit sets and high IDs in places though

[rose pendant]

You'd have to solve the bitset/component map issue, but iirc there are solutions for that

[runic shadow]

This is still required for everything and about as approachable as you can get for ECS internals

[rose pendant]

You could introduce wildcard queries as a follow up PR which also would be pretty self contained. And all useful steps towards real relationships

[runic shadow]

I guess if you don't have storage for type pairs you don't need it, but there will still be places you do

[rose pendant]

That can also be done in a follow up PR

(these are 4 of the 5 first steps in the roadmap to relationships blog)

[runic shadow]

If the type pairs aren't backed by any storage you could probably do it now, but then you run into the issue that no IDs right now don't have storage, even ZSTs do, so you would need to implement a mechanism to allow that

[rose pendant]

Maybe you could start with a mechanism that always uses the first element as type

ZST or not

[runic shadow]

Yeah I'd be on board for that, but you will still run into high id issues in that case I imagine

[rose pendant]

Though I agree that ZST optimizations would be nice to have before implementing this

[rose pendant]

For the bitset I think the idea was also to just use the first element of a pair for that and ignore the target

The sparse set on the archetype can be replaced with a map, or with the component index

[runic shadow]

Yeah I'm more talking the columns in the tables where you can't necessarily do that as it could have the first type as well

Using the component index for that is the first change I'm suggesting

After that you can do type pairs unblocked I believe

[rose pendant]

Yep

get would take a perf hit, at least initially

But maybe nothing too extreme. I'm guessing the implementation of a rust hashmap is better than what I have in flecs

[cloud plover]

Yeah Rust hashmaps are really fast

At least the one we use :p

The default one is DOS resistant, and not suitable for games

[rose pendant]

Do you mean something like a table with Position, (Position, Start)?

[runic shadow]

Yeah, you couldn't support that with the current sparse sets in tables, not a hard thing to overcome though

The component index is already merged so there's only a little more that needs to be done there

[alpine patrol]

You run into the problem using observers for UI. If you have multiple observers for a Pointer<Drag> event, it's possible they will conflict with eachother.

[runic shadow]

Conceptually I'm on board, implementation wise how does a fix actually work though?
Implementing it the same way system ordering works doesn't really seem feasible

[rose pendant]

I'm guessing observer ordering wouldn't apply to OnAdd/OnRemove observers? That seems impossible to do

[runic shadow]

A general solution for all observers definitely seems like it would destroy OnAdd/OnRemove performance

[rose pendant]

Right, but what would that order even look like

There are so many ways observers can be sliced

[tight salmon]

It would just be building a Schedule graph but observers and rebuilding when new observers for an Event type are inserted/removed, no?

[rose pendant]

An absolute ordering over all observers is maybe the only way, but that's not feasible from a performance pov

And the storage will have changed by the time you're evaluating your ordered list of observers, so it wouldn't work anyway

[rose pendant]

Not really, there is no "observer schedule". Observers are called when events are emitted

[tight salmon]

Right, you could build a schedule for them though, but this wouldn't work with recursion though so probably just not feasible

[rose pendant]

If I first add X to an entity and then remove Y and that triggers two observers. There's no way the observer for Y could happen before X

Things just happen in the order that events are emitted

you could build a schedule for them though
I don't think you can, but happily be proven wrong :p

[tight salmon]

The idea for above is intra-event ordering, not inter

[rose pendant]

I'm still not sure what that means. You want to bundle/order all OnAdd events? That's not going to work either

[alpine patrol]

we only really need ordering within observers of a given event types. So if there are two OnAdd observers on component X, we want to control which one runs first.

[rose pendant]

Across entities or for the same entity?

[alpine patrol]

on the same entity

but the more i think about it, the more i am actually pretty happy with observers as they exist today, without being able to watch multiple event types or order different callbacks

[rose pendant]

What if you have a multi component observer that subscribes for both X and Y

[runic shadow]

The most control I can see allowing is to have specific entity observers run in order of registration, that's probably feasible, beyond that it seems very complex

[alpine patrol]

under the hood, we keep them all in mapped lists. we could back the ordering into the list.

but the front end api is really tricky

[tight salmon]

let obs = ent.observe(|_| print!("Second")).id();
ent.observe(|_| print!("First")).try_before(obs)?;
ent.insert(A);
``` ![thonk](https://cdn.discordapp.com/emojis/1153851465765490742.webp?size=128 "thonk")

[alpine patrol]

this wouldn't be very useful but it would be consistent at least

i think if you want to order A and B which both observe X, write observer C for X and have it call A and B manually.

[rose pendant]

One thing to keep in mind is that observers will eventually (or already? not sure) match queries, not single components. How would you define the order between two observers that match a number of components?

[alpine patrol]

our observers are very different from your observers.

[rose pendant]

For now

[runic shadow]

They aren't that different

[rose pendant]

If observers are going to be used in bevy to keep query caches in sync, they'll be very similar

[alpine patrol]

i like the current functionality, i don't want to see it go away if we move to queries.

[runic shadow]

We match a set of components, which internally is similar to flecs, the main thing we don't do is derive that set from the query directly

If/when we move to queries the API would barely change

[alpine patrol]

i'm not really talking about component observers

[runic shadow]

Non-component observers won't change at all

This Trigger<OnAdd, Component> would become Trigger<OnAdd, &Component> and you could access the component through the trigger

[alpine patrol]

yeah, i don't care about ordering for component observers, only non-component observers.

as a side note, i dislike how we have combined these two very different things.

[rose pendant]

Okay, so not this then 🙂

[alpine patrol]

yeah, probably not that.

[rose pendant]

That's feasible 👍

[alpine patrol]

i keep forgetting how OnAdd and friends are special

can we call that "observers" and start calling the other thing "event callbacks" or something so we don't have this semantic confusion?

[runic shadow]

They share all the same internals and the same API so seems hard to seperate them

[rose pendant]

Fwiw in flecs they're also the same thing with the exact same code running for both. The only thing that's different is when/how events are emitted

[alpine patrol]

users don't care about the internals, and the use is very different

[runic shadow]

If you want a different feature with different semantics then they can be split out

[alpine patrol]

like the fact that ordering is well-defined for one of these cases but not the other, that feels like we are confusing two concepts that happen to share code.

[runic shadow]

OnAdd and friends aren't special in any way, all events can target components

[alpine patrol]

but they are going to run on queries, are triggered events going to run on queries?

[rose pendant]

This is why I've argued repeatedly that observers aren't a good fit for UI

Allowing for custom observer events just makes the observer mechanism more flexible

[runic shadow]

Events with no components would just be equivalent to an empty query

[rose pendant]

But you shouldn't want to impose restrictions on that implementation, since it's very perf critical

[tight salmon]

observers are simply exlusive systems/one-shots that are called based on an event that is triggered being filtered by event type and target entities/component-id

[unborn quail]

Don't feel discouraged. This all is quite complex but by the virtue of doing something you'll get experience and we can always help you if something is not quite clear

[timid mountain]

@whole wigeon feel free to take over my PR: https://github.com/bevyengine/bevy/pull/14385. It doesn't have any outright blockers.
It just needs:

• to be rebased on bevy main
• to chase people to review/approve, which is usually the hardest part. It would probably entail convincing people that this is the right approach. I included a benchmark to show that there is no performance hit, but it would be easier to show that there would be a big performance issue if we had very high component identifiers

Or should we use paged sparse sets instead? cc @runic shadow

[runic shadow]

For this paged sparse sets wouldn't work since the main operation that needs to be fast is comparison

Paged sparse sets are good when you want lookup to be fast

[timid mountain]

Another thing is what James mentioned:

Removing the sparse set on the tables specifically is pretty well scoped, the columns are already stored in the component index so all that needs to happen is remove the sparse set in the table and look up the column in the index instead

1. remove the sparse-set here that tracks how to access a given column for a component. (again, the problem with those sparse-sets is that the memory usage is linear in the size of the ComponentId stored, and with relations we will have ComponentIds with big 'high bits')
2. We have this ComponentIndex which maps from a ComponentId to the archetype that contains the component + the associated Column in the table. So we could use this index to fetch the Column of an Archetype based on the ComponentId, instead of using the sparse-set
   Hopefully that's enough context to get started, let us know if it's not clear

But yea @whole wigeon i agree that i myself am confused as to what can be worked on now and what are parts are currently blocked. I feel like we should try to start work on an implementation without archetype cleanups and other "great to haves". That implementation won't necessarily work in all situations, but it will allow us to benchmark stuff

[rose pendant]

If you restrain yourself to pairs that can only consist of types you have something that doesn't have too many dependencies & is also useful

Without too many footguns

[timid mountain]

So relations like <Likes, Dogs> where both parts are components?

[rose pendant]

Yep exactly

You don't have to worry about cleanup because components don't get deleted (yet)

Which means you don't have to worry about (recursive) cleanup, deleting archetypes, refactoring the query cache

You can implement the first 5 steps of https://ajmmertens.medium.com/a-roadmap-to-entity-relationships-5b1d11ebb4eb (maybe minus the first one if that's complicated with the renderer world etc)

And minus observers, since they're already done, so the three remaining steps

[unborn quail]

alright, instead of working on queries as entities this evening, I instead read the trans dysphoria bible, which should still make me a better rust developer in the long run

||I'm sorry that the joke is so revealing, it was too good not to make||

Anyway, I'll get back to my explicit lifetime annotations

[timid mountain]

I updated my PR https://github.com/bevyengine/bevy/pull/14385 to replace FixedBitSet in Access with sorted vecs, which is one task in the overarching goal of removing sparse sets of component ids in preparation for relations.

[unborn quail]

I'll review it when I'm home

Maybe I'll get distracted again, but maybe not

That's always a fun coin toss

[timid mountain]

Second PR https://github.com/bevyengine/bevy/pull/14928/files to remove the SparseSet used in Table to map from ComponentId to Column. (instead we use the ComponentIndex).
(looks like there's some miri error, it's not clear to me why)

After this we'll have only a few SparseSet<ComponentId> remaining:

• in SparseSets: maybe this can be turned into a HashMap?
• in RemovedComponentEvents (which I think is planned to be deprecated anyway)
• in Archetype: maybe this can be turned into a HashMap?
• in Resources: this one should be ok to be untouched, no?

[unborn quail]

Hey would you look at that, I did get to the review

as usual I can't be asked to care about performance like some nerds (cart), but someone else seems to have it covered (also cart)

||that was a joke, please take my reviews serious still||

[polar vortex]

Yeah, ordering seems like it would add a noticeable overhead, and I think such an impact would be exacerbated because observers are kinda too popular.

[rose pendant]

are kinda too popular
not just in bevy

[polar vortex]

More fundamentally, scheduling isn't "core". At its base level, bevy_ecs provides a unicast calling convention (one-shot systems) and a multicast calling convention (observer systems).

A schedule is just one instance of a one-shot exclusive system. The fact that it calls other systems is just an implementation detail.

Between the command queue, manually calling systems in an exclusive system, and schedules, I don't really see a need for other built-in ways to order things.

UI plugins could mix reactivity and ordering by, for example, registering one observer (reactive) targeting a specific entity that issues a command to run a schedule (ordered). The API to associate a system with a schedule is basically the same as the one to associate it with an event, so style-wise, they aren't that different to an end user.

[cosmic elk]

Reactive systems often want to debounce the updates anyway. If a reaction has 5 dependencies, and all 5 change, you don't necessarily want to run the reaction 5 times.

[polar vortex]

I'm hoping that and stuff like stopping events from propagating can handled with clever use of schedules (or other custom indexes) and observers without really changing how observers work in general

I think it'll be painful if the observer infrastructure tries to be more than a simple index. If we want to use relationships to implement stuff like shared components and prefabs, it'll be a challenge just to get them to propagate OnAdd/OnRemove events properly (and things will get especially constrained when we implement command batching as an optimization so that we can skip intermediate table copies and move entities directly to their final spots).

[cosmic elk]

OK, I have to say that observers have spoiled me. That is, the fact that observers support dependency injection means that I am frequently tempted to make something an observer when it really should be a command, just because it's so much easier than having to extract all the parameters I need from the world in a way that satisfies the borrow checker.

[alpine patrol]

I actually have been wondering if we should support command with normal system arguments

Still run them exclusively

[runic shadow]

That would just be API over one-shot systems effectively no?

[alpine patrol]

Sure. It could also just be a medication to commands.add

[cloud plover]

I think you want it as API over one shot systems ( @elder eagle) so you can cache the state nicely

[runic shadow]

You just wouldn't want people accidentally initialising cached queries for each command instance

[cloud plover]

Which should actually make them faster than most custom commands

[alpine patrol]

Anonymous one-shots then? I don’t want to have to pipe the id around. Not sure how you would do the cache without.

[runic shadow]

You could probably have a shared cache for all of the anonymous commands of that signature

Locals would be a little weird but eh

[cosmic elk]

Many of my commands are for things that happen infrequently, caching isn't desirable.

[alpine patrol]

Same

[runic shadow]

Doesn't matter whether you want it or not system parameters are cached

We don't support uncached queries, so you would be paying unnecessary init costs every time if you throw away the cache

[cosmic elk]

Basically, I don't worry about performance for anything that is triggered by a keystroke or mouse click.

[alpine patrol]

He is saying that would be acceptable for the ergonomics

[runic shadow]

Where are you going to put a warning that's loud enough for all users to hear it?

[alpine patrol]

If we can make a nice cached api the. All the better

[runic shadow]

It's a massive footgun otherwise

[cosmic elk]

would that be a footcannon, then? 🙂

[alpine patrol]

I feel like that’s really not trusting users to know what they want

[runic shadow]

This isn't just "Oh a little extra indirection", this is a bunch of allocations, safety checks etc.

Sharing anonymous caches seems super doable

Like if someone implements it and it gets approved I can't stop it from being shipped, I just would want a more performant solution before merging

[alpine patrol]

My position, to be clear, Is that an api like this is worth a large perf compromise (because the alternative is to overuse observers and they are slow). I’m not anti-cache.

[rose pendant]

How hard would it be to implement uncached queries now that the component index is merged?

[runic shadow]

Not difficult but a bunch of code churn

The problem with that PR will be getting it reviewed and merged while chasing the tail of conflicts

Or we duplicate everything for the uncached impl and leave the cached impl as is

[rose pendant]

Fwiw this function implements 80% of what's necessary for uncached queries w/the component index

The only thing it's missing is operators

Which for the most part aren't super difficult to support (iirc bevy has Not, AnyOf, Option?)

[runic shadow]

Our iteration code is pretty straightforward anyway, there will just be a lot of changes to how the state is handled and passed around currently to de-tangle things

[rose pendant]

Ah that's fair

[runic shadow]

There are also some questions as to what the uncached query API is supposed to look like in bevy

[elder eagle]

i have a wip pr that uses the system's unique function type as the key to a CachedSystemId resource

(https://github.com/bevyengine/bevy/pull/14920)

[alpine patrol]

Could we not just add a new level somewhere between run_system and run_system_once where the state is cached by system type id?

For lambdas and anonymous functions, this is worst-case just as (in)efficent as run_system_once

With function pointers, it's just as efficient as run_system

[alpine patrol]

Organizing systems by inputs/outputs is interesting but doesn't immediately seem to be what I want. Perhaps it's because it's unfinished, but I struggle to imagine how i would use this.

[alpine patrol]

Basically treat the function name as a label like normal systems, so that the following is equivalent basically

commands.run_system(my_oneshot_system)

let id = lambda_registry.entry(TypeId::of::<my_oneshot_system>()).or_insert_with(|| commands.register_system(my_oneshot_system))
commands.run_system(id)

[alpine patrol]

We could perhaps make this work verbatim with the existing run_system api

trait AsSystemId<I, O> {
    fn as_system_id(&self, world: &mut World) -> SystemId<I, O>;
}

impl<I, O> AsSystemId<I, O> for SystemId<I, O>
where
    I: 'static,
    O: 'static,
{
    fn as_system_id(&self, _world: &mut World) -> SystemId<I, O> {
        self
    }
}

impl<I, O, M, S> AsSystemId<I, O> for S
where
    I: 'static,
    O: 'static,
    M: 'static,
    S: IntoSystem<I, O, M> + 'static,
{
    fn as_system_id(&self, world: &mut World) -> SystemId<I, O> {
        world.res_mut::<LambdaSystems>().entry(self.system_type_id).or_insert_with(|| world.register_system(self))
    }
}

[elder eagle]

it's not by inputs/outputs

unless i'm misunderstanding

if you have a fn foo() {} and a fn bar() {} they would each have their own CachedSystemId because S would be different

[alpine patrol]

oh yeah sorry, it was like 5 am when i looked at this

neat, that's what i want. why use separate resources instead of a TypeMap?

[elder eagle]

i haven't considered a TypeMap. what are the tradeoffs?

is it so you can see all the cached system ids in one place? or for perf

[alpine patrol]

🫵 thats what i was asking you, idk

i'll think about it more when i have time

[azure turret]

Function pointers with the same signature would be treated like the same system despite maybe being not. But to be fair that is already the case with bevy and it seems nobody cares for that as it is unlikely to happen. 😅

[elder eagle]

true..

[polar vortex]

Once the render world stuff is resolved, could we make scheduled systems entities even before relationships are implemented?

Admittedly, I've always been saying "need to wait for 🌈", but we could keep using the existing Schedule code until then and just change the storage from "Box<dyn System> owned by schedule" to "Arc<RwLock<dyn System>> referenced by its entity and the schedule".

some context
#editor-dev message
#editor-dev message

[viscid python]

What would the benefit be without relations?

We don't even have entity disabling or default filters so can't even do anything there

[polar vortex]

well, for editor purposes, just discovery and disabling would be enough for now

we would need to add disabling though, IMO we should at least add the tag component—even if there isn't a consensus on how queries should handle it—so that observers could implement whatever "being disabled" means in context

the petgraph stuff inside Schedule is already working as a substitute for stuff we would do with relationships if we had them, but since we have the substitute, we don't need to wait for the replacement

[runic shadow]

The render world should be the only thing blocking systems as entities as far as I'm aware

[polar vortex]

like... the benefit of doing it sooner is that people won't introduce a bunch of workarounds like macros to achieve stuff like disabling systems (editor is interested in disabling systems to toggle between "edit mode" and "play mode")

[wind willow]

I did some simple experiments today , could we pre-split entity space to different world just like subnet division?

TBH, I don't think #14449 is a good PR to resolve render world stuff , especially for rendering people

[wind willow]

https://github.com/bevyengine/bevy/pull/14988 A very rough idea

The biggest problem is that we need to change the allocation method of entities exposed to the users.

The advantage is that we don't need to use EntityHashmap or other ways to handle the inconsistency Enitity ID across different worlds , and also it may open another door to asset as entities and networks

[timid mountain]

The ability to reserve a range of entities would be useful for networking too

[fossil basin]

Perhaps I'm out of my element but what if you had multiple "worlds", each with their own set of archetypes, but they all share the same entity space.

Basically each entity can have multiple sets of components that are independent of each other. Deleting an entity in one world would not delete it from the rest.

I dunno if that solves the render world problem but I haven't seen anyone talk about multiple worlds with shared entity ids

[polar vortex]

I've talked about it before, I just called it "partitioning".

[polar vortex]

Partitioning wouldn't help. Pipelined rendering requires the ability to simulate frame N+1 while rendering frame N.

That means "app logic" and "rendering logic" need to run concurrently. A single world would interrupt that every time it applies commands. That's why we need two dedicated threads and why each thread has its own World. (Even if they didn't share entities, the renderer would probably still use a World because ECS is just a nice API.)

One world with multiple partitions could not achieve the same concurrency as two worlds without a lot of pain and suffering. You'd have to do weird stuff like try to interleave systems of multiple schedules.

[indigo meteor]

it doesn’t seem like anyone is blocking on the implementation of #14449 at this point? they only wanted more docs, I thought

[fossil basin]

Ah, I figured I probably wasn’t the first one to think of it.

I’m guessing commands would interrupt all partitions because they still need to synchronize entity id allocation?

And yeah I can see how scheduling would be challenging since presumably you’d have systems that can query multiple partitions at once.

[unborn quail]

yes

from an implementation stand point it's done

it's just that this change is decently large and all the users that are currently doing their own stuff to get things into the render world could probably use a little help

as that's quite finnicky already, having more than zero docs is a goo idea

[polar vortex]

Yeah, since the partitions would all share the same entity administration, they wouldn't be exempt from the way commands force serial execution.

Because of that, it doesn't win any parallelism, so IMO there'd be no reason to ban systems from querying data from multiple partitions.

In terms of implementation though, it would be better to still limit queries to one partition each. Filtering entities across multiple partitions would require "joins" and IMO doing them internally is icky, so if I was implementing them, queries would be limited to one each and I'd tell users to do, like, q1.join(q2).iter(). (but I'm getting off-topic)

[unborn quail]

I also kind of wonder if we can do the current sync / extract machinery within the same world

everything is lifted in the sync/extract phase to a seperate 'disjoint' entity, so it should be possible for the render systems to be parallalized next to the main world systems

file it under 'future work' I suppose

(I want to eliminate the render world so badly)

not badly enough to actually touch it, but then again...

[azure turret]

can't the components the renderer reads contain N and at the end update them to the N+1 state that is within the same component?

hm this probably does not work with entities that were removed at N+1

[polar vortex]

fundamentally, partitioning inside a world doesn't provide the level of concurrency we want (where renderer and app run totally async aside from the one sync point)

[wind willow]

They just share the entity space, unless a world is at a "world sync point" (not command/schedule sync,more like our current ExtractSchedule) ,it should be no way to spawn/despawn the entity that not in pre-allocated entity space.

[polar vortex]

but yeah, if it's a shared set of entities, then if the app logic wants to delete an entity, the renderer wouldn't be able to use that same entity

[wind willow]

For example, if all entities in main world are in even space, 0v1, 2v1, 4v1, and all entities in the render world are in odd space, 1v1, 3v1, 5v1 , When sharing messages across worlds (ExtractSchedule), we can ensure that the world Entity ID will never conflict

[fossil basin]

The way I was imagining it, deleting an entity in one partition would delete the entity data in that partition and "hide" it from the partition. An entity would only truly be deleted if all partitions delete it

[polar vortex]

the whole point of having partitions inside the world would be to share a single entity administration

[unborn quail]

that has been proposed before, with tombstoning I believe

where deleting an entity doesn't truly delete an entity

[fossil basin]

I don't see how that conflicts with what I said?

[fossil basin]

Sort of a generalization of tombstoning

But yeah didn't mean to keep an idea that doesn't work alive. Just wanted to clarify

[unborn quail]

I'm not even suggesting that your idea (or any idea for that matter) cannot work. It's just that someone has to do them (and by their nature they are pretty substantive refactors) and than look if they at least maintain the same performance as the current 2 world approach. I'm assuming here that the first thing you'd use this on is to simplify the render world.

[fossil basin]

The reason I bring it up is because it seems like a few features (namely rendering and networking) use some sort of snapshot system. I imagine it would be convenient if you could keep the same entities across snapshots and query them as if the world was still in that previous state. And ideally snapshot data would be retained so theoretically you would only have to process diffs to synchronize them with the main world instead of copying all the data every frame.

Since it is a big refactor I thought it was worth considering a general-purpose system that could (potentially) address all these use-cases instead of specialized systems for each use-case.

Obviously, as joy has pointed out there are issues with such a system which is too bad.

[polar vortex]

How I conceived of partitions was just an internal separation of components. It'd be fine to take an entity and remove all its components that belong to some partition, but IMO they shouldn't have different opinions about if an entity is alive or not.

If you agree with me that it would be bad for the renderer to be constantly interrupted by commands, then partitioning is just not a tool for parallelism. The only way it can be useful on its own is for snapshots, since you can isolate the components you want.

I'm not sure how to articulate why I'm so convinced it's a total dead-end for parallelism, but IMO if we'd have to add tombstones or redefine what "delete" means, that'd be silly.

[polar vortex]

I know it's not really considered an option, but idk, I still think it'd be best if the app did all the preparation and only the rendering/drawing happened concurrently. Concurrent but completely separate worlds. Trying to force concurrency earlier in the frame is why we're copying entities and I think that's the problem.

[fossil basin]

IMO they shouldn't have different opinions about if an entity is alive or not.

But that's the whole point though right? If the render partition is still rendering an entity we still want to pretend it's alive in that partition

[polar vortex]

what I've been trying to express this whole time is "No", I think parallelism is a terrible reason to be add partitioning

because it doesn't really win any, you'd have to add/change a bunch of other stuff too

I have only been trying to discourage the idea.

[polar vortex]

idk how to articulate it, but to me "partition so we can have schrodinger's entities for parallelism" is just as "um, what?" as copy-pasting one world's entities over another's

they strike me as "going against the grain" trying to circumvent basic axioms like worlds being separate, entities being unambiguously alive or dead, commands requiring serial execution, etc.

[fossil basin]

If you want pipelined rendering and networking snapshots then you either need to keep copies of an entity or create another entity that copies that entity. Calling it "schrodinger's entities" doesn't change that. No matter what, you're keeping a copy of old entity data.

Using an extension of current bevy syntax I just made up:

Query<&Enemy, ()>,
Query<&Enemy, (), RenderPartition>,

It seems simple to loop through one query and look up the same entity in the other query to synchronize.

My main point is that it'd be nice to just use the same entity id than doing some kind of awkward mapping

Like in the main world it has X entity id but in the render world it has Y entity id. That seems awkward to me which lead me down this road

Anyways it seems like an unpopular idea so I'll leave it at that

[polar vortex]

partitioning for snapshots is fine, it's the other things that would need to be done to (ab)use the partitions to try and create new parallelism that would overcomplicate things

entity allocation could be reworked to support disconnecting indices from the "common free pool", manually specifying the generation when spawning, etc.

that is a solution that would generalize, not just rendering, even worlds on two different clients could be told to create the same set of entities (partitioning wouldn't have helped with that in that scenario)

[ancient spear]

In that scenario would Extract look mostly the same as it does in the current PR, just without id mapping?

That sounds nice if so

[fossil basin]

Just to make sure I understand, you could have multiple entities with the same index but different generation? That's interesting

[ancient spear]

I thought that’s what generational indices try to prevent lol

[polar vortex]

not at all, I'm saying there could be an API to spawn an entity with a specific Entity (where you've specified both the index and generation), but if the index is already in use, it would fail

[fossil basin]

I don't see how that would solve the "I need to keep stale data around" problem

[ancient spear]

It would let you safely overwrite data in the render world with the same ids as in the main world, without nuking (if I understand correctly)

[polar vortex]

it wouldn't, being able to reserve and spawn specific entities would solve the "the main world it has X entity id but in the render world it has Y entity id" problem in a robust, general-purpose way

for networking, partitions would let users separate networked data from the non-networked data, but to create and rollback to snapshots there would need to be API for cloning and replacing a partition with the cloned data (would need those entity allocation/reservation abilities to e.g. infallibly restore entities that were predictively despawned)

rendering could use the same tools to do whatever, but for rendering, the "keep stale data around" problem is largely a self-imposed problem, bevy could choose to only render concurrently but it insists on doing the prep in the render thread too for some reason

[polar vortex]

I haven't really been giving advice on what rendering should do, I'm mostly just arguing against the specific idea of replacing the separate app and render worlds with a single world that partitions its components and allows entity liveness to be different across those partitions to try and create room for pipelining/concurrency to exist.

I just foresee issues with that last part. (Like, what if an app system tries to update a component that a render system hasn't read yet? Do we then try to infer dependencies across independent schedules? Going down this road just seems to snowball in terms of complexity.)

[ancient spear]

yeah that just sounds like trying to smash two worlds together to avoid the downsides of separate worlds

and probably wouldn’t even avoid mass data copying like Extract does now

[wind willow]

I don't think a single world is a good choice ,My idea has always been to elevate Entity Allocation to the app level, and each subapp (world) is assigned a non-overlapping entity space and synchronizes Enitity Space every frame(or time interval)

[fossil basin]

I don't think you can avoid data copying, just minimize how much you need to do

The core problem as I understand it, is that the renderer needs a stable set of data to function properly. With pipelined rendering the renderer is running in the background while the rest of the ECS is computing the next frame.

So if the renderer is reading data that the rest of the ECS is mutating then it causes problems.

[ancient spear]

would the ranges be decided by the user or adjusted automatically by the app somehow (If that's even possible/useful, I'm no expert in this area 😅)? just curious

[tight salmon]

What if you created an Command abstraction GlobalCommand these are run at the one sync point between partitions, and so any Global entity state could be managed by those while normal Commands are limited to their current partition.

[wind willow]

by user, given a entity ID during runtime, user should know it comes from which subapp(world)

[polar vortex]

I'm not saying "It's impossible to make that function," I'm saying "I don't like that".

I pretty much exclusively try to either reduce the number of concepts or reduce their complexity.

If I look at things I've worked on or advocated for, it's stuff like:

• unifying exclusive and non-exclusive systems
• storing systems (and schedules) in the world so that exclusive systems can do loops (so that we could various unhelpful abstractions like stages and looping run criteria)
  • this has since upgraded into storing everything inside the world as an entity
• giving systems and system sets the same scheduling API
• unifying the time API
• moving non-send items out the world to untangle that whole mess
• making ECS use dynamic data structures under the hood so that scripting (systems from assets) is inherently supported

So anecdotally, I'm generally opposed to the idea of splitting apart core concepts and elements into different variants.

[polar vortex]

Also for this case, I keep coming back to "if the sync moved from right before here to right before here, the renderer wouldn't need to extract entities, just finished render resources." I tried to get some of the render experts to discuss/entertain the idea a couple of times but they weren't all that interested.

[polar vortex]

I feel like that's the same thing, just derived in reverse.

Instead of putting multiple worlds in, you'd pull the entity administration out, but you still end up with "multiple 'worlds' governed by one entity administration". Partitioning the address space just sounds like a different trick for the partitions to independently handle the "same" entity's lifecycle (so that the world's state can be pipelined).

Also, bevy should sub-apps IMO. Plugins need to be entities so that bevy can handle their dependency graphs just like systems' and so that it has parent entities for their components and systems (could "clean uninstall" a plugin by despawning its entity).

The render world should just be constructed by the render plugin when the main world sorts and builds plugins (and stored as an entity until all plugins are built and we're ready to move it to the other thread).

[tight salmon]

I mean if the concept you seem to go for is unification and simplification, I'm not sure the having multiple worlds fits that idea.

[azure turret]

force the data to the gpu to be passed through a swap file on a usb2 device to get the desired async/delay

[polar vortex]

It does. If you...

• want to implement all your logic using ECS
• have two overarching tasks you want to run concurrently
• don't tolerate interruptions
  ...then your only option is to have one World for each task. Applying commands can only be done by single thread, which obv would cripple concurrency.

having many instances of the same thing/pattern is different from having many (synonymous) things/patterns

[inner peak]

notably, this is how actual databases work, i've brought this up a few times as a kind of moonshot architecture but it would be awesome to have transactional logic within an ecs 🙂

[fossil basin]

Yeah that's why it didn't seem that weird in my head. Database transactions, and git repos can have the same row (or file) in different states with potentially different data and it isn't confusing because people understand a transaction (or commit) is like a snapshot.

But I admit implementation is a big questionmark and likely complex so I'm not pushing that idea anymore. Still interesting though and there's likely an interesting abstraction there

[alpine patrol]

Databases, even in memory ones, also tend to be a lot slower than an ECS

Still fast, but maybe not fast enough to implement real-time modern graphics on top of.

Opt-in transaction overhead would be a holy grail.

[polar vortex]

I mean, you could implement transactions and multi-version concurrency control without hardcoding World to divide its address space into a fixed number of spans. The stuff described earlier just sounded pretty hacky.

If we wanted "builtin" MVCC, IMO it can be granted with fine control over entity allocation. If you could manually reserve/assign the same IDs in two worlds, then you could create however limited of a snapshot you want by backing up those entities (and whahtever components you want) to a second world. Like you say, your everyday DBMS doesn't care about being able to snapshot 20+ times a second.

Like, I like the idea of having partitions, where each one can have multiple backup copies, but idk, atm I just don't like the idea using one partition as the backup for a different partition (baking that concept into the impl of World, I mean), especially if it hinges on limiting which partitions systems can access to maintain concurrency.

[fossil basin]

The stuff described earlier just sounded pretty hacky.

Too be fair, the original idea was pretty simple... Just multiple world partitions that share the same id space and have their own set of data.

[polar vortex]

that much is fine, I think partitioning would be a great ability to have, it'd be really nice to directly clone archetypes and tables without needing to worry about components I'm not interested in and selectively restore a backed up partition without affecting the others

I think things just get weird if we also try to squeeze concurrency/parallelism out of it

since that starts pulling in other sources of complexity like "have global and partition commands / sync points", "have partitions track liveness separately, so they each need to delete an entity for it to stick", "split the ID range exactly into halves/quarters" (edit: these are what I'm calling hacky ig)

[fossil basin]

That's fair.

[inner peak]

not an ecs expert, but isn't this mostly just because ecs optimizes for table scans by using arrays? which makes sense given the access patterns of games, but if you knew your game was index friendly and were okay with extra memory use and write latency, i don't see any inherent reason you couldn't build a btree ecs with mvcc that is very fast (clustered indices can still be cache friendly). the key with mvcc (in postgres at least) is that you get to store the extra versions right in the tree, which is harder with contiguous memory because you have to copy the old rows into a different table

in other words, it's not generalizable to all games

[unborn quail]

Hey @wind willow, I wanted to help you out with the documentation for the retain rendering PR, but I'm not sure how I can make a PR to your branch, do you have an idea?

[wary fjord]

Add their fork as a git remote, then make a PR to that fork rather than to the main bevy repo

[unborn quail]

right, thank you

[unborn quail]

hey @alpine patrol, can you help me with the documentation at https://github.com/re0312/bevy/pull/1

let me know what needs to be added / cleared up

(I need someone to go back and forth with on this and alice is on a camping trip)

[alpine patrol]

Yes absolutely will do as soon as I have some free time to allocate.

[unborn quail]

^ @cloud plover whenever you have time

I hope you had a fun trip btw

[cloud plover]

Awesome, thank you!

I'll take a look today 🙂

[unborn quail]

Oops, it got merged already

Hopefully it's good

[alpine patrol]

Skimming it looks like an improvement

[cloud plover]

Agreed, I think this is an improvement too

[unborn quail]

@alpine patrol is it enough though to get a signoff for the pr?

[unborn quail]

@alpine patrol, @cloud plover, again is there more documentation needed?

because it was honestly mostly a first attempt

just improving a couple things

[cloud plover]

I'll do a proper pass on this for Friday; please remind me?

[unborn quail]

sure

also, Alice, I don't like to bother you when you're busy (rustconf and such) but it can be kind of difficult to gather when you have time and not

[cloud plover]

Yep, I don't mind at all 🙂

[unborn quail]

like, I only know rustconf is going on, because I read it in offtopic

alright

[alpine patrol]

i left a review. docs are non-blocking now imo.

[cloud plover]

I do daily planning in #engine-dev, search "yesterday" to find them 🙂

[alpine patrol]

thanks for managing this

[unborn quail]

I guess that works

[unborn quail]

I'd also recommend punting the WorldQuery impl for RenderEntity to a follow-up PR

it's a nice and easy PR to get in, but it's not necessary

[alpine patrol]

can't say i have been following to even know what the WorldQuery stuff is

minimal good

[unborn quail]

Currently when you query &RenderEntity, you have to do something.entity() to get the actual id

[alpine patrol]

all this needs now is sme signoff probably

cosmetic change then, yeah split that off

[unborn quail]

you could simplify this slightly by doing a custom WorldQuery impl for RenderEntity and then it would work the same as Entity

so no something.entity(), but just a straight entity

like, it's nice qol but it's not worth it for a 2 to 3 month PR

like, it would be a great first pr for an aspiring ecs person

so making an issue could be nice

[cloud plover]

I might PR it to the branch directly myself to reduce the noise of the diff

But agreed on it not blocking

[unborn quail]

yes, I'll prod the rendering sme's again

[wary fjord]

If we do a custom RenderEntity worldquery, we shouldn't allow &RenderEntity imo

[unborn quail]

I'm a professional botherer of people

[alpine patrol]

<@&1064694928589991936> I am given to understand https://github.com/bevyengine/bevy/pull/14449 is ready for final SME review. No pressure, I don't have time to review it myself, but it would be ideal to get his in before it either goes stale or we get too close to the release deadline to test it fully.

[unborn quail]

nice, double up those mentions

(also did it in #rendering-dev )

[alpine patrol]

oops

[unborn quail]

no it's good

this way they know that we're serious

re0 and I make a good team, in the sense that they can program while I can pull a complex PR through the review proces

[alpine patrol]

i am very appreciative of how you are keeping this group moving forward.

[cloud plover]

Seconding this; it really helps take a load off of me

[unborn quail]

I am going to get relations landed, one D-controversial at a time

(and preferably by writing as little code as possible)

[cloud plover]

A bird after my own heart

[unborn quail]

Hey @timid mountain, do any of your PRs need reviews? I forgot

[timid mountain]

Either reviews or they need to be benchmarked/improved

[unborn quail]

@wind willow I see that you haven't worked on the retained rendering pr in a while. Should I adopt it for the last couple changes?

[wary fjord]

The loss of discussion going from #14449 to #15320 is definitely unfortunate

[unborn quail]

it happens

[unborn quail]

@spring hinge I've summarized the things I didn't get to #15320. Which of these are vital? Maybe we can brainstorm a couple in here.

given the nature of the PR I'm also confused by a couple of things (even some of the stuff I wrote myself initially)

[spring hinge]

I'm very short on time, but

"Should'nt we modify the clusterizer system to store RenderEntity on Clusters directly, instead of using another query here?"
Nice to have
Mystery two lines in crates/bevy_pbr/src/lib at line 423 and 424
Important
What happened with crates/bevy_sprite/src/mesh2d/mesh.rs ?
Idr what this is
Footgun regarding adding SyncRenderWorld to spatial entities.
Not sure I actually understood the issue in the first place, but seems weird
Shrinking the size of pending in world_sync.rs
Not critical
Is recursive despawn appropriate for despawning unsynced entities?
Idr

[unborn quail]

Also hey @runic shadow, how's it going, haven't seen you around in a while

[unborn quail]

Mystery two lines in crates/bevy_pbr/src/lib at line 423 and 424
This has to do with the large changes in bevy_pbr/src/render/light.rs, I don't fully understand it also, but the observe systems in question are not too exciting.
What happened with crates/bevy_sprite/src/mesh2d/mesh.rs ?
I believe I actually was responsible for this (so so so many months ago). The old code respawned entities every frame. After retaining the entities this became an entity leak so I had to remove that code. It cleaned up quite nicely and I think that it still does what it's supposed to do.
Is recursive despawn appropriate for despawning unsynced entities?
As far as I'm aware it's not necessary to do despawn recursive given the entities that are currently being synced. This is with the caveat that I'm not sure what's going on in bevy_pbr/src/render/light.rs.

[cloud plover]

As far as I'm aware it's not necessary to do despawn recursive given the entities that are currently being synced. This is with the caveat that I'm not sure what's going on in bevy_pbr/src/render/light.rs.
IMO we should remove this for now to reduce complexity and footguns

[unborn quail]

@wind willow, just in case you know. Are there any places where a recursive respawn is actually needed for world sync?

[runic shadow]

Still alive and lurkin, work has been intense lately as the project I'm running is reaching it's final executive reviews and prod rollouts so I haven't had a lot of bandwidth to actually write open source code. Hopefully as work wraps up I'll have more time 🙂

If people have stuff they want reviewed or my opinion on happy to help where I can of course

[unborn quail]

I think that Periwink still has some PRs that could use reviews, but don't worry about that. Happy to know you're still lurkin. Now uni has started again for me, I'm just focusing on one PR at a time.

[timid mountain]

I'm busy with job hopping currently so I will have almost 0 time for quite a while

[unborn quail]

I wrote a small novel https://github.com/bevyengine/bevy/pull/15320#issuecomment-2368077467

[cloud plover]

Feedback given 🙂

[unborn quail]

IMO removing ExtractComponent from an entity should be vanishingly rare: this is enough for me here.
Thank you, I was starting to implement it and I immediately saw more edge cases

One question though: why not use required components for this? The semantics seem to be identical, and this should be faster and more standard.
Haven't used them yet, don't know how they work

[cloud plover]

Pretty much identically to your observer pattern 🙂

When you add A that requires B, B will be added if and only if it's not already on the entity

Adding B alone won't add A

And removing A or B has no special effect

[unborn quail]

right, makes sense

what's the syntax?

when quickly looking through the PR I mostly saw derive macros

[cloud plover]

#[require(B)]: https://dev-docs.bevyengine.org/bevy/ecs/component/trait.Component.html#required-components

[unborn quail]

Yeah, I don't think that works unless I'm missing something. Because ExtractComponentPlugin<C> takes a type argument I don't have access to the struct definition. (I mean I do, but for UX I don't want every developer to have to add #[require(SyncToRenderWorld)] for every extracted thing

[cloud plover]

Ah, you need @noble panther's builder API from https://github.com/bevyengine/bevy/issues/15367. Okay that's fine, just leave a TODO to swap to required components once that issue is fixed

[unborn quail]

Sure thing

[unborn quail]

Alright, I've looked at bevy_pbr/render/light.rs and done most of everything else, https://github.com/bevyengine/bevy/pull/15320 is ready for final reviews @cloud plover , @spring hinge

[cloud plover]

Awesome; that's my first priority tomorrow

[unborn quail]

after this I might adopt some of periwinks prs, because I've decided that queries as entities is probably best left to a better programmer

[spring hinge]

Very low on time lately, but I'll try to get to it tn

[unborn quail]

hugely appreciated in any case

can't wait to not to have to look at rendering internals ever again

[cloud plover]

https://github.com/bevyengine/bevy/pull/15398 could use a review

[spring hinge]

Usually it's not that bad. This is just a rather cross cutting issue PR

[unborn quail]

I've done one shot systems before this, which also took the better part of a release to get done

[wary fjord]

luckily those ended up pretty compact

[unborn quail]

I'm not expecting much better for some others of the relations pr

[cloud plover]

Yeah, thanks for finishing that one!

[unborn quail]

But there are some easier ones, admittedly

[cosmic elk]

Q: What's a good way to walk an entity's ancestor chain if I have a DeferredWorld?

[unborn quail]

Sorry, no clue

[valid perch]

Got to reading the RFC for this, and got to thinking about https://github.com/bevyengine/bevy/pull/15422. Came up with a more flexible (but probably more controversial) approach to (Reflect)MapEntities which I've isolated to its own PR here: https://github.com/bevyengine/bevy/pull/15425
Would love some feedback!
Also put up one that builds on #15425 and shows of some of the generic hierarchy traversal that it enables here: https://github.com/bevyengine/bevy/pull/15426

Side note: it makes me sad that I can't effectively stack PRs when working from a fork like I do at $dayJob where my branches all live in the main repo 😦 Would make #15426 way easier to read since it would only show the diff from #15425

[unborn quail]

@cloud plover, @spring hinge I'm still waiting for reviews for https://github.com/bevyengine/bevy/pull/15320

[spring hinge]

I'm sick unfortunately

[unborn quail]

Get wel soon

Maybe another rendering sme should take a look at it

[carmine cave]

Alright, I finally started reviewing. The fact that main world and render wold entity ids are not synced anymore worries me a bit. Is there a way to get the main world entity from the render world entity in situations where you need to go back to the main world?

[unborn quail]

see the migration guide

there is an it's called &MainWorld

[carmine cave]

I'm asking this question based on waht I read in the migration guide and I don't see this being mentioned

Maybe I'm missing something

[unborn quail]

so you have an entity in the render world and you want it's main world entity?

wouldn't that be entity.get_component::<MainWorld>.id()

[carmine cave]

Actually, I didn't think this fully. The reason I was asking for this is for shader picking, but we can just use the main world entity during extraction and everything will be good since it doesn't need to be queried in the render world

[carmine cave]

Maybe, but it's not mentioned in the migration guide. At least I'm not seeing it

[unborn quail]

it's the very last note, but maybe it's not entirely clear

my assumption was that you were mostly going to get the main world ids from queries and its written in that context

[carmine cave]

Oh, it's MainEntity, not MainWorld, that's why I was confused

[slate shoal]

FYI ci is red on the PR rn

[unborn quail]

ah I see

small mistake when merging

[slate shoal]

I think there's also some docs that didn't get updated with the SyncRenderWorld -> SyncToRenderWorld rename

[cloud plover]

FYI from an ECS perspective desyncing these IDs is a really important part of the PR. It's too brittle and limiting, and violates what we tell every other user: Entity is fully opaque and values are provided for debugging only

[carmine cave]

Yeah, that's fair, I'm just scared about migrating things at work because I know we rely on this behaviour for some things. After chatting with coworkers about it we should be fine though

[unborn quail]

Yes, there are two points to this whole PR. 1. removing the one to one mapping and 2. and no longer despawning every entity in the render world every frame

[carmine cave]

I'm still reviewing, but other than me being scared of the migration itself I'm not seeing any issues from a rendering perspective

Migration for users I mean, not bevy

And tbf, I've said to so many users that bevy is unstable and to expect changes, I'm just in a weird position of using bevy in a massive codebase and having to migrate it myself 😅

[unborn quail]

yeah...

my hope mostly is that most users are not messing so much with this sort of stuff

in any case, I'll be here to answer questions

[carmine cave]

Yeah, me and my coworkers are, but we also have the expertise to deal with it so I'm not too scared

[unborn quail]

and I think the rc period is going to highlight some gaps in the migration guide too

[cloud plover]

I'm going to try and do a pass on the Migration Guide specifically tonight: it's the most important piece of documentation here and will help me get a deeper understanding of this PR

(wow that's way better than it used to be)

[unborn quail]

let me know if you have anything to critique

I'll be awake for another hour or so

[cloud plover]

So far it's just a wording pass 🙂 Are you okay if I edit the description directly for the copyediting?

[unborn quail]

just edit it directly

my sentences are always a bit run-on-y on the first pass

that's what editting is for

[cloud plover]

However, entities are however not "unsynced" if any given ComponentToExtract is removed, as there may be multiple reasons to extract an entity.
Clarifying note

[wary fjord]

However, entities are however

[unborn quail]

a classic

[cloud plover]

ExtractComponentPlugin has been changed to only work with synced entities. Entities are automatically synced if ComponentToExtract is added to them. However, entities are not "unsynced" if any given ComponentToExtract is removed, as there may be multiple reasons to extract an entity. Be careful when only removing extracted components from entities in the render world, because it might leave an entity behind in the render world. The solution here is to avoid only removing extracted components and instead despawn the entire entity.

Full paragraph now

[unborn quail]

this has to do with that small novel I wrote a couple days ago and I didn't want to get too deep into it

[cloud plover]

Yep, I just want to give a tiny clue to users about why the heck we do this

[unborn quail]

I'll try to think of something succint, just a second

[cloud plover]

Can we mark get_or_spawn as deprecated as part of this PR?

This was the only serious use of it, and it's the only API that abuses the non-opaque Entity ids

Pretty much every use of it in 0.15 is going to break

[unborn quail]

there's not really a point to get_or_spawn in the context of the render world because all of the entities should be there

[cloud plover]

But user code might be calling it

(and I would really like to remove it)

[unborn quail]

I think so yes, this would be possible

I'm thinking if it's used in the PR currently

[cloud plover]

❤️ Sweet, that would be great to land here if possible

[unborn quail]

(might also be better as a follow-up PR)

a 'deprecate get_or_spawn'-PR

[cloud plover]

Yep, no objections to that, just make sure it doesn't get forgotten about?

[unborn quail]

there's a bunch of stuff that needs to go into an issue / several

I'm currently trying to also write my diary entry for the day

this is very distracting 😅

[cloud plover]

Alright, editing pass done 🙂

[unborn quail]

done

and an issue

https://github.com/bevyengine/bevy/issues/15459

remind me if I forgot anything

[unborn quail]

^

[cloud plover]

This looks great, thank you!

[wary fjord]

is it called spawn_or_get or get_or_spawn?

[unborn quail]

not sure, I also now a little less about why this should be deprecated

• it was get_or_spawn

[cloud plover]

Oh right, maybe I was thinking of another API...

Ah no, found it

[unborn quail]

get_or_spawn_batch?

[cloud plover]

Spawning a specific entity value is rarely the right choice. Most apps should favor Commands::spawn. This method should generally only be used for sharing entities across apps, and only when they have a scheme worked out to share an ID space (which doesn’t happen by default).

If we're not using this pattern because of footguns, users shouldn't either

[unborn quail]

I'm sort of kind of worrying about people who do networking stuff

but yes

[cloud plover]

@polar vortex's advice (and mine) is that using raw Entity ids is always a mistake, because of things like user interface entities

You need a stable networking id

[unborn quail]

alright

it's on the list (and it's also not that I'm too into the doctrine of bevy development / best practices)

[cloud plover]

Yeah, no worries 🙂

Anyways, review is done 🙂

Just doc complaints

Once that's cleaned up I'm good to merge retained rendering world <@&1064695155975803020> <@&1064694928589991936>

ECS is fully on board, migration seems fine, we have quite a bit of review from rendering folks at this point

[wary fjord]

One small step for rendering, one giant leap for ECS

[cloud plover]

Yeah 😅 I hope it's helpful for the rendering folks too!

Thanks for letting us meddle

[unborn quail]

get meddled with idiots

[unborn quail]

cleaned up

[cloud plover]

Great 🙂 Try and get CI and merge conflicts resolved for Monday please

[unborn quail]

a couple of the CI things aren't mine btw, that's why I didn't touch them

one of them errors on some example file

but yes, the other I will get ready

[polar vortex]

Things would be different if bevy supported reserving entity indices. In that case, I'd actually recommend it. You'd just have to reserve the same indices in both clients.

But it doesn't, so I don't.

[rose pendant]

Fwiw this is a common approach for networked flecs games

[cloud plover]

Yeah 🙂

If we had better APIs to do so correctly and safely, I'd be on board

But for now, it's way too error prone

[unborn quail]

one more day untill retained rendering gets merged 🥳

[tawny bobcat]

not familiar with the rules but shouldn't it pass CI first

[cloud plover]

Yes please lol

[wary fjord]

I suggested a change that hopefully passes CI

[cloud plover]

Suggestion applied, button has been pressed

[wary fjord]

merge conflicts unfortunately

[cloud plover]

💢

Sorry @unborn quail

[unborn quail]

I'll get to it I'll get to it

I'm busy seasoning my pasta sauce

I'll get back to you after dinner

[cloud plover]

Dinner >> open source

[wary fjord]

I eat PRs for breakfast

[unborn quail]

pasta update: it was good, could've used thrice the amount of garlic, slightly less pepper, or maybe I should experiment with different types of pepper not sure. More cheese always helps too

anyway, to the merge conflicts

[unborn quail]

okay @cloud plover, I think I'm done

waiting for ci...

[cloud plover]

Automerge enabled, let's flip a coin to see if it works

[unborn quail]

it still says 'update branch' I'm assuming that works automatically too right?

[wary fjord]

yep

[unborn quail]

17 minutes to merge still

this queue aint going very fast

[cloud plover]

It's an American train, okay? You have to be understanding of our cultural differences

[azure turret]

be glad it is no German train, being 17min late is something we are used to

[unborn quail]

I would start complaining about the dutch railways except that the train journey I had today took me halfway through the country and it was both on time and very boring

(which is good, boring train journeys are the good kind of train journeys)

[unborn quail]

speaking of which, how is VIA rail doing?

I'm assuming well

[wary fjord]

ok now lets get relations in as well before rc

[cloud plover]

It's been a couple years since I got stuck behind a freight train for 6 hours unplanned so my grudge against them is fading. They putter along, maybe slowly improving, but still embarassingly slow and unreliable

[unborn quail]

good plan, are you doing queries as entities, then I'm adopting some componentindex prs

[wary fjord]

I might be interested in picking it up

[unborn quail]

yes please

[unborn quail]

canada is waiting for the day when some charismatic monorail salesman wants to build a transcanadian high speed train line

[wary fjord]

Is it mostly unsoundness that needs to be fixed in the draft PR?

[unborn quail]

well uh

a bit more then that I'm afraid

give me a minute to put some thoughts on (digital) paper

[cloud plover]

It's merged!

[unborn quail]

@wary fjord
So the basic plan for queries as entities is to turn QueryState (the thing that contains all of the cache data for qeueries) into a component, so that these caches (QueryState) can live in the world. This is already implemented in the PR (although imperfectly).
The reason that this is usefull is because queries can now respond to events via observers. If for example an archetype is created, each QueryState can check if it matches on the archetype and if so, add the ArchetypeId to their cache for a quick lookup later.
This is also implemented, but the events are not being created yet "ArchetypeCreated", so currently they do nothing.

The other 90% of this PR is plumbing. The things that currently use QueryState, should instead use an Entity that points to the relevant QueryState. Whenever you need the Querystate you can simply world.get::<QueryState>(entity).

Kind of.

This is where it gets tricky, because QueryState has explicit lifetimes and is dependent on a bunch of generic types. So doing this plumbing while keeping the compiler happy is very hard. Maybe there's some PhantomType buggery you can do but I'm not good enough at rust to figure that out.

And of course there's the thing where a QueryState can change its type sort of with as_readonly

the solution is to type erase it in some way, but again, I don't know how to do that

[wary fjord]

I see

I'll try to take a look sometime this week

[scenic fox]

Silly question, because I don't understand the internals:
What happens when I do something like:

fn system(
  queries: Query<&mut QueryState>
) {
  // do something silly by mutating the QueryState of the `queries` Query
}
```?

Do you need some sort of an aliasing rule that a system cannot query its own query?

[unborn quail]

Good question, I don't know

Part of the entification of the ecs is that as things get more dymamic, more possible to change things at runtime, you can do more cursed shit like this

[unborn quail]

I think my general answer to questions like this is why? and don't.

[gleaming fulcrum]

This seems like a potential soundness issue to me, with the world and this system both having mutable references to query states. Seems like this should be some kinda compile error imo

[cloud plover]

Yep, this is a soundness issue

I think that the simplest way to resolve this is probably just to play the hokey-pokey

Like we do with schedules today

Take the query state out of the world whenever it's in use

Probably swapping it with a dummy

[gleaming fulcrum]

Cool 🙂 that's what I was too scared to suggest haha

[polar vortex]

I will once again shill using Arc<RwLock<...>> to enforce sound borrowing without having to move anything (hokey-pokey cost adds up)

[cloud plover]

Does that risk deadlocks?

[wary fjord]

^ is required to be able to implement https://github.com/bevyengine/bevy/pull/15557

[polar vortex]

yes and no

yes because technically two systems could try mutably access the same query state

no because trying to mutably access a query state is something you never need to do

only the observers that update query caches need to modify the query state and they're guaranteed to have exclusive access

[cloud plover]

kk, I'm on board then 🙂

[wary fjord]

why not just Arc<Mutex<...>>?

[polar vortex]

multiple readers is fine

potentially there's a future optimization where all instances of the same query share the same cache

[alpine patrol]

aren't locks super slow on windows?

i'd want to see perf data from windows before I approved a significant use of locks.

at least hokey-pokey is a fairly consistent overhead

if stuff is guaranteed to be exclusive, why don't we just do unsafe access?

[azure turret]

It is not guaranteed when you query for QueryState, no?

[polar vortex]

are they?

also, did std ever upstream parking_lot or nah?

[alpine patrol]

I'm having trouble finding a source but my understanding is that on windows locks (even uncontested locks) can have a surprising amount of overhead.

let me see if i can recall where i am getting that from

[polar vortex]

😠🤜 🪟

sad if true

[slate shoal]

I think they just rewrote mutex. parking_lot wouldn't work on some platforms if I remember correctly.

[tight salmon]

Two systems won't run at the same time, if they both access the same thing (no access aliasing)

[glacial yoke]

For context on locks see #93740, basically Windows, Linux and Free/Open BSD use their native locks now while other platforms (including Mac due to no suitable os functionality) use a general system

[polar vortex]

true, but you can't generally write a system function that selectively borrows one query if it's &mut QueryState<...> though

it'll block an entire column of queries

[tight salmon]

True, but I think modifying query state should probably be an exclusive action 🤔

[polar vortex]

the reason I suggest Arc<RwLock<...>> is because you get into a weird thing where the query is in the world but also kinda needs to borrow the world

IIRC to make the lifetimes work out, you either need to take the query out of the world (the component needs wrap the query state in an Option or you need to remove the component) or make it so the query is never in the world, just a handle (e.g.Arc<RwLock<...>>)

systems are the same way

I think Arc<RwLock<...>> fits the relationship that queries/systems and the world have better than the hokey-pokey pattern

[cloud plover]

I am endlessly tickled that my name for that caught on

[polar vortex]

like systems as entities, it'll be good to avoid doing the hokey-pokey every time we want to run a schedule

instead of schedules caching Entity values alone and copying tons of Box<dyn System> pointers around every frame, they can cache some Arcs and take uncontested locks

(locks being contested would be user error, systems can't recurse)

[polar vortex]

allocating queries on the heap and just storing Arc handles in the world would make it easier to think about future optimizations

like, for example, these two queries

Q1: Foo, Bar
Q2: Foo, Bar, Baz(up|ChildOf)

as well as any systems with their own instances of these queries could share the same cache (if the caching stopped before the first term that does traversal, in this case Baz)

[alpine patrol]

We are in the shake-it-all-about phase

[left iron]

thats what its all about i reckon

[echo ore]

It's not real relations, but I would love some feedback from this WG on #15635. It generalises the Parent/Child relationship into a One-to-Many system using ComponentHooks to ensure validity, and performs better than the current system while (in my opinion) being easier to reason about.

[cosmic elk]

Interesting! Something I have long wanted is an Owner/Owned relationship, such that owned entities are automatically despawned when their owner is, but are not considered "children" for purposes of layout or hierarchical traversal.

[echo ore]

That's something that is almost setup in this PR. Right now I've only designed undirected one-to-one relationships, so you'd also need two marker components for Owned and Owner. And the automatic despawn is something you'd need setup yourself as well unfortunately!

Again, this isn't real relations, but it's some of the functionality

[cosmic elk]

Yes, I assumed that you'd need two components (they wouldn't be markers I think?)

I was assuming something exactly like Parent/Child but with different names

[echo ore]

With this PR I think you'd do something like this:

#[derive(Component)]
struct Owner;

#[derive(Component)]
struct Owned;

struct OwnerRelationship;

type Ownership = OneToOne<OwnerRelationship>;

let owned = world.spawn(Owned);
let owner = world.spawn((Owner, Ownership::new(owned)));

So the Ownership will be automatically added to both owned and owner

[cosmic elk]

Particularly in my reactive experiments, nodes that are highly dynamic tend to have a lot of invisible satellite entities for holding things like dynamic styles and other reactions. Right now these are done as GhostNodes, but that's overkill for this use case.

The same principle could be applied to the text-as-entities work

That is, you can use GhostNodes for invisible leaf nodes, but you really don't need the whole ghost behavior for leaves. Being able to have non-children children would be cheaper overall, since the layout system wouldn't need to filter them out.

[echo ore]

Interesting! This is a bit out of my wheelhouse so I'll have to take your word for it sorry! I do think that the way I've setup these One-to-One and One-to-Many types makes it pretty clear how we could extend bevy_hierarchy to add directed/undirected relationships, and Many-to-Many as well.

Only reason why I didn't include them is it's already a pretty big PR and I don't have an immediate use-case for them

[cosmic elk]

That's just one of the many services we provide at Talin's use-case emporium!

[rose pendant]

For ref, in flecs that's solved by creating a new relationship (separate from ChildOf) with a "if the target of this relationship pair is deleted, delete me too" policy

[cosmic elk]

@alpine patrol @runic shadow I have an observers question: I understand that Trigger<OnRemove, MyComponent> means "I'm interested in on_remove events for this component." What would happen if I were to say Trigger<MyEvent, MyComponent>? Is that even valid?

[cloud plover]

My understanding is that this is invalid

[alpine patrol]

i think it just works like Trigger<MyEvent>

but don't quote me on it.

otherwise i would expect it to do nothing

[runic shadow]

It works the exact same way as any of the in-built events triggers

If you trigger that component on an entity the observer will fire

[alpine patrol]

the observers api unifies several things which we may eventually want to present to users as distinct imo (even if they are unified under the hood). this is one of them.

[alpine patrol]

and if you try to access the event does it panic?

[runic shadow]

No, why would it?

[cosmic elk]

The reason I'm asking: I'm trying to reduce the number of observers created by my ui framework. Right now every button has 6 observers, and I was wondering if there was a way to have a set of global observers for picking events filtered by component - so for example, say I wanted an observer that would handle Pointer<Down> events for all entities that have a ButtonState component.

[alpine patrol]

because it dosn't have event data in that case.

[alpine patrol]

how would you do this?

[runic shadow]

You still need to pass the event to trigger it, you are just targetting a component instance instead of just an entity

[alpine patrol]

o i see

and .entity() will be placeholder?

[runic shadow]

The way to do that is generally just to have a global observer for ButtonState and then check it against a query, the component in the trigger signature is about where the trigger itself is targetted so if you don't explicitly target the ButtonState component it won't fire

[runic shadow]

You generally supply an entity and a component, i.e. for <OnRemove, MyComponent> you get the entity that the component was removed from

[cosmic elk]

Do you mean a global observer for Pointer<Down>? I'm not sure what you said makes sense to me.

[runic shadow]

Yes, presuming the goal is to cut down observer count

[cosmic elk]

Hmmm, I'd need to have a global observer for each type of widget that listens for pointer events.

All that being said, this may not even be worth doing, it's just an optimization 🙂

[alpine patrol]

world.observe(|trigger: Trigger<Pointer<Down>>, buttons: Query<&mut Button>| {
    if let Some(button) = buttons.get(trigger.entity()) {
        ...
    }
})

seems like a common pattern. The ability to indicate a component on the targeted entity is neat but not super useful for most higher level apis. I feel like a way to observe all entities with a given component might be wanted maybe.

[cosmic elk]

Right, got it

I filed a ticket about something similar this morning, it's fairly common to want access to a component on the listener entity, bevy_mod_picking had a convenience API for this.

[alpine patrol]

you can make this generic too fn my_observer<T: Component + MyTrait>(trigger: ..., my_components: Query<&T>) { ... } and so on

but you have to still register for each component type

[cosmic elk]

Something like:

world.observe(|trigger: Trigger<Pointer<Down>>, button: ObserverComponentRef<&mut Button>| {

[runic shadow]

The goal was to allow: trigger: Trigger<Pointer<Down>, &mut Button> but it becomes ambiguous with the OnAdd/OnRemove component targetting so it's hard to cleanly encode in the type system

[alpine patrol]

it would be trivial to build a reactivity system around this if we also had a way to automatically insert a Button before the observer runs, if it does not exist. i've actually been experimenting with this a bit.

fixes the spawn != update issue we have currently

but that's OT

[cosmic elk]

I was thinking more along the lines of mod_picking's listener_component_mut.

But a bit more general

[alpine patrol]

yep, i have been wanting this.

idk if we can encode that in the typesystem currently

since the entity lives in the input, which we only get one of.

[cosmic elk]

Right because injected parameters are context-free

[alpine patrol]

we'd need to pull the entity out of the trigger trigger: Trigger<Event>, target: Entity, ...

which would be kind of a significant rewrite.

but i do like it better

[cosmic elk]

This is a general problem with systems, in that you often fetch more data than you need, because there's no way to constrain the injection with runtime data

[alpine patrol]

that way we could just not run systems that require an entity, instead of passing them the placeholder

interested in hearing Alice and James' thought. this sort of thing probably isn't feasible without changing how we inject stuff into systems.

but if they like it maybe we can work towards that.

[cosmic elk]

trigger: Trigger<Event>, target: Target<Entity>, parent: Target<Parent>, ...

Where Target means a query that is restricted to a single entity.

[alpine patrol]

i do like that.

Target<Option<T>> also fits naturally

[wary fjord]

N extra arguments would require a fair bit more changes

[cosmic elk]

Extra credit: trigger: Trigger<Event>, target: Target<(Entity, Parent)>, ...

[wary fjord]

but if there was a limit of 1 or 2 extra, it would be more codegen but more feasible

[runic shadow]

Yeah it's possible, just a huge pain to refactor to allow it

[alpine patrol]

this just requires 1 extra param, but it needs to be accessible as QueryData.

which is the real issue. and then there's the fact that we could add archetype optimizations to this (cache the set of observers to run for each target archetype).

boatload of work.

[runic shadow]

Currently Trigger hijacks the system input, so it's a lot of churn to make it more general, definitely would be nicer code in the end I imagine

[cosmic elk]

Anyway, this is not a blocker for me, just curious

[wary fjord]

Trigger doesn't just hijack the system input on main anymore, it is a system input type

Was changed in https://github.com/bevyengine/bevy/pull/15184

[runic shadow]

In the way I meant that it still does, on the IntoSystem trait it takes the place of the input type, and it uses the same machinery to be passed to the system when it's invoked

[cosmic elk]

So I realized that there's a problem with the "one observer to rule them all" approach to doing buttons and widgets, which is that they will bubble in the wrong order. It's important that the button widget gets pointer events before its parent does, and it's important that the widget not get events that were intercepted by children. So having a global observer for all buttons won't work.

[alpine patrol]

I’m surprised by that

Seems like a bug

Can you elaborate on the behavior you are seeing

[cosmic elk]

I haven't tried it yet

I'm just reasoning out the consequences based on what I understand

Here's an example: let's say that pointerdown on a button should set focus to the button. Let's also say that pointerdown on a window should clear focus. So obviously the button should propagate(false) so that we don't set focus and then immediately clear it.

But if we have a global handler for pointerdown - one which queries for Button like you suggested - that handler will run after everything. This means that calling propagate(false) on the event will do nothing.

[runic shadow]

I haven't checked the propagation implementation but my expectation would have been that the global observer fire multiple times with trigger.entity() being each entity in the hierarchy in the expected order