Introduce mutation tracking query helper

[Ralith]

Seeking feedback. Some initial thoughts:

• Bikeshedding welcome on the names of things
• Modified::unset might be annoying to invoke, particularly if you have lots of them. Is some sort of helper needed? What might that look like?

CC @cart, @sdleffler

[AngelOfSol]

To be clear on the user side, if you don't use a query with Tracked<T> the change will go untracked? Does this mean you can only query either: all instances of component T with no tracking or just Tracked<T> with tracking?

[Ralith]

Correct. You might legitimately want to exempt a system from mutation tracking, even.

[Ratysz]

Bevy has query transformers that yield only the mutated items, which I think is the only reason to have a mechanism more involved than a mundane dirty: bool field on user side. (In Bevy that's built on Fetch::should_skip().)

Side note, this transformer approach to tracking is quite elegant. Might it be warranted to officially expose Fetch, so that users could implement their own convenience transformers?

Is some sort of helper needed? What might that look like?

Right now resetting tracking for all components of type T looks like this (is that correct?):

for (_, mut modified) in world.query::<&mut Modified<T>>().iter() {
    modified.unset();
}

Perhaps something as simple as World::reset_tracking_for_all<T: Component>(&self), doing just that?

Additionally, unset() method(s) on Tracking itself should allow solving the situation of "do something with component and reset it's tracking" in one query, rather than two. Alternatively, another specialized transformer just for that case.

Combining this with filtering queries from the first point creates a "drain" iterator for mutated components.

[Ralith]

query transformers that yield only the mutated items .. built on Fetch::should_skip()

I feel like this is a bit redundant to filtering the iterator. Is the interface significantly more convenient?

Might it be warranted to officially expose Fetch, so that users could implement their own convenience transformers?

I think this is a great extension point in principle. I'd be much happier if it wasn't so unsafe, though. I wonder if there's some way we can expose composition primitives without requiring people to implement all the hazardous methods.

Perhaps something as simple as World::reset_tracking_for_all<T: Component>(&self), doing just that?

I'm not sure we need a oneliner to replace a threeliner. I'm more concerned about bevy's case, where there's a large number of modification states to reset (one per component). If they're currently relying on hard-coded pseudo-components, they might be having an easier time iterating over and clearing them than this interface permits, particularly given that the set of components bevy wants to reset tracking state for isn't statically known.

do something with component and reset it's tracking

Would it be sufficient to just query the T and Modified<T> separately for that case?

[Ratysz]

Is the interface significantly more convenient?

Tentatively, yes, if only because it's more conscise. However, Bevy does expose a lot of transformers for that, to the point where I don't know which one I want without looking at docs. I suppose examples of this filtering in docs would be a better solution than exposing more types.

I'm more concerned about bevy's case, where there's a large number of modification states to reset (one per component).

Right; I haven't had an idea about that yet.

Would it be sufficient to just query the T and Modified<T> separately for that case?

Honestly, that didn't occur to me, somehow in this instance I forgot that those two are actually orthogonal components. Yes, it is sufficient (with doc examples).

[cart]

I think this approach is probably the right call for hecs because it (1) lets users opt-in to tracking without touching the default hecs code path at all (2) very straightforward.

It also might be a good call for bevy. Functionally its basically the same, but there are a few UX issues I'd probably want to solve (either in hecs upstream or in a bevy fork). First, I'm a little bit hesitant to make users filter like this:

// this pr
for mut modified in world.query::<&mut Modified<T>>().iter().filter(|m| m.is_set()) {
}

// bevy
for mut modified in world.query::<Modified<T>>().iter() {
}

If Bevy continues with the "universal" track-everything approach, it would probably mean we'd need to fork to prevent people from "breaking out" of the tracking. Ex: un-impling Query for &mut T. (which is what we currently do).

Additionally, resetting the trackers using this pr requires somehow composing a strongly-typed "reset system" for each component. Automatically resetting all component trackers on behalf of users presents a bit of a challenge.

And as mentioned in the other pr, auto-inserting the tracking components on behalf of users is also a challenge (but probably a solveable one).

I'd also like to point out that tracking "added" components doesn't fit (directly) into this model, which is something Bevy would need.

[Ralith]

First, I'm a little bit hesitant to make users filter like this:

Yeah, that does kind of suck, especially for more complex queries where you have to pick out the right field. I don't think incorporating linear filtering into queries incurs any cost or significant complexity, so ergonomics is sufficient argument to support it.

we'd need to fork to prevent people from "breaking out" of the tracking

auto-inserting the tracking components on behalf of users

I suspect this can be accomplished with some sort of wrapper, though perhaps tediously. A bevy-owned newtype of World could then apply the wrapper internally to the interfaces it chooses to expose, without duplicating the heavy lifting/unsafe stuff.

Automatically resetting all component trackers on behalf of users presents a bit of a challenge.

There's a pattern that comes up a lot which we could use here: accumulate a Vec<Box<dyn Fn(&World)>> with an entry for every unique component. It's not super graceful and it's (very fractionally) less efficient than iterating over archetypes first, though. I wonder if there's some way we could instead have one or more Box<dyn Fn>s per archetype with user-controlled per-component code inside...

tracking "added" components doesn't fit (directly) into this model

What are the semantics of that, exactly? Can a component be deemed both added and modified, or are they mutually exclusive?

One interesting trick might be to store, for each entity, the archetype it was last in. You could infer added and removed components freely by diffing the component lists against the current one.

[Ralith]

(also, as much as it's a juicy extension point, I'm glad we haven't exposed Fetch just yet because #95 and #96 just brought us a spectacular speedup by vigorously breaking it)

[cart]

Well thats really cool. I'm curious to see if those wins will apply to bevy_hecs too, or if the changes I made result in similar assembly as the PRs above. Compiler optimizations are random enough that it always just feels like magic to me.

Lets hope these wins stack :)

[cart]

Yeah, that does kind of suck, especially for more complex queries where you have to pick out the right field. I don't think incorporating linear filtering into queries incurs any cost or significant complexity, so ergonomics is sufficient argument to support it.

Yeah I think its a nice ergo win. But I do want to point out that the "linear filter" breaks things like cheap iterator length via ExactSizeIterator on QueryIter.

I suspect this can be accomplished with some sort of wrapper, though perhaps tediously. A bevy-owned newtype of World could then apply the wrapper internally to the interfaces it chooses to expose, without duplicating the heavy lifting/unsafe stuff.

Yeah that could definitely be possible. On that note, I don't think its fair to constrain hecs' design according to bevy's constraints. I do want us to consume upstream directly if we can (for the benefit of both projects), but I also don't want to force hecs in a direction thats suboptimal for non-bevy scenarios.

There's a pattern that comes up a lot which we could use here: accumulate a Vec<Box<dyn Fn(&World)>> with an entry for every unique component. It's not super graceful and it's (very fractionally) less efficient than iterating over archetypes first, though. I wonder if there's some way we could instead have one or more Boxs per archetype with user-controlled per-component code inside...

Cool yeah that would work / seems acceptable to me. User-configurable per-archetype/component code is also an interesting prospect.

What are the semantics of that, exactly? Can a component be deemed both added and modified, or are they mutually exclusive?

We store "added" and "mutated" flags for each component. "Added" is set when the component is first inserted. "Mutated" is set only when a user mutates the component. We then have Added, Mutated, and Changed queries/pointers where Added and Mutated correspond to their component states and Changed is added | mutated.

Mutated vs Changed is a little confusing, but it lets us capture every relevant scenario.

I've thought a lot about removed states (which we also support). Right now we just store a list of removed entities for each component and reset it each frame. But I really want us to be able to query (edit: the values of) removed components. I think this is possible because part of the removal process is swapping the removed component to the end of the "array". As long as that space isn't overwritten by a new component, the memory there is still valid. With a little bit of extra state management we could ensure that space isn't overwritten until a frame later.

[zakarumych]

User should be able to convert Mut<'a, T> into &'a mut T, which is now impossible as there's only DerefMut which returns mutable reference with lifetime limited by Mut value itself.

[Ralith]

Closing in favor of #366, which is more ergonomic and should perform better in most cases.