QueryBuilder is largely unusable due to keeping a reference to the World

[djeedai]

What problem does this solve or what need does it fill?

The documentation of QueryBuilder claims it can build a QueryState at runtime. However unlike QueryState, the builder retains a reference to the World. This means it can't be stored. This also means no other query can be done on the world at the same time, for example to resolve another Entity returned by the QueryBuilder result. This heavily restricts where and how QueryBuilder can be used.

What solution would you like?

Like QueryState but dynamic, without a world reference. When actually iterating, then take the target World as parameter to a method.

What alternative(s) have you considered?

There's no alternative I know of.

Additional context

Currently bevy_tweening makes heavy use of generics to be able to define systems and behaviors based on the target component being animated. This has been a source of pain since day one. The hope was to eventually abandon this once dynamic queries are available in Bevy, so that a single animation system can process an heterogeneous list of animation targets. Unfortunately, trying to implement this showed this is impossible due to the QueryBuilder limitation of taking a World reference; the animator class references a component/entity pair to animate, but once the query builder is in scope it references the world, making it impossible to query and animate (mutate) that target component.

[SkiFire13]

I would argue that QueryBuilder is the wrong tool for your usecase, as you shouldn't be creating a new QueryState every time your system is called (which is what keeping an instance of QueryBuilder would entail.

Instead, you'll likely want to query EntityMut, FilteredEntityMut or EntityMutExcept or directly use a &mut World reference.

[djeedai]

@SkiFire13 Thanks for the details. Fair enough, I may be misunderstanding what QueryBuilder is for. But I thought reading the docs that 1) this was the way to do what I want, and 2) this was the only way, since the release notes about dynamic query point to it (and nothing else IIRC; can't check right now).

So, out of curiosity, what's the use case for QueryBuilder, because now I can't think of any? I think we should maybe clarify it in the docs.

I've looked at EntityMut briefly, it seems I can get a slice of them via World::many_entities_mut(). I'm guessing that with a single system taking &mut World then I can query all my animators for their target entities, collect that (to release the world ref), then get an EntityMut for all of them and mutate one of their components that way. Does it sound like a workable solution?

[SkiFire13]

So, out of curiosity, what's the use case for QueryBuilder, because now I can't think of any? I think we should maybe clarify it in the docs.

From what I understand it is supposed to create queries based on some runtime data, but not every frame. Possible usecases include e.g. systems defined in scripting languages.

I've looked at EntityMut briefly, it seems I can get a slice of them via World::many_entities_mut(). I'm guessing that with a single system taking &mut World then I can query all my animators for their target entities, collect that (to release the world ref), then get an EntityMut for all of them and mutate one of their components that way. Does it sound like a workable solution?

Yeah that sounds better for what you're doing. The collection step sounds a bit unfortunate, though it will be hard to avoid. I think EntityMutExcept has been added for a usecase similar to yours, maybe that could be better? Though AFAIK it also has a runtime overhead, so you'll have to benchmark it against collecting and see which is faster.

[djeedai]

From what I understand it is supposed to create queries based on some runtime data, but not every frame.

Ok but if not every frame, how do you store something which references the World? That is going to hold a reference and break everything no? Or is it only meant to be used as a system parameter?

[djeedai]

EntityMutExcept doesn't seem to exist in the latest release. I assume it's on main only, so I'll ignore for now.

[djeedai]

Ok so my problem with all of this is that once you queried the EntityMut(s), you're holding a mutable reference to the world. This means from this point onward any change you make to those entities needs to be based exclusively on out-of-ECS data, because you can't access the World anymore. So you need to fetch (and potentially duplicate) all the state you need to perform all the mutations, with all the temporary storage allocations this entails, before you can actually apply those mutations. In my case this means I need to update beforehand all animations, store their transient state, as well as duplicate any data (in my case, boxed trait objects) I will need, and store that in local hash maps etc. before I retrieve the EntityMut(s) to apply the changes. This feels extremely counter-productive and non-performant. Although I guess the issue is mostly a tradeoff against querying the EntityMut(s) one by one interleaved with other queries to the World so that there's no lifetime overlap.

EDIT: Nevermind, I forgot that you can access all components of the entity with EntityMut, so you can store data into a component attached to the entity, which saves you lookups etc. at the cost of permanent ECS storage.

[SkiFire13]

Ok but if not every frame, how do you store something which references the World? That is going to hold a reference and break everything no? Or is it only meant to be used as a system parameter?

QueryBuilder is not meant to be stored, it's meant to produce a QueryState which is what then gets stored (possibly as the state for a system parameter but not necessarily).

EntityMutExcept doesn't seem to exist in the latest release. I assume it's on main only, so I'll ignore for now.

Yes, it has only been added recently

[alice-i-cecile]

Yeah my suspicion is that EntityMutExcept is likely to be the fix here: I suspect you're running into exactly the same stuff that #15282 ran into, based on your comments about animation.