Add system parameter for relation quieries #19526
Conversation
|
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
alice-i-cecile
added
C-Feature
|
It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note. Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes. |
| world::{unsafe_world_cell::UnsafeWorldCell, World}, | ||
| }; | ||
|
|
||
| // SystemParam for combine 2 related queries |
There was a problem hiding this comment.
This is going to need much stronger doc strings.
|
|
||
| // SystemParam for combine 2 related queries | ||
| pub struct Related<'w, 's, D: QueryData, R: RelationshipTarget, F: QueryFilter> { | ||
| data_query: Query<'w, 's, D, With<R>>, |
There was a problem hiding this comment.
It feels odd that we can't specify a filter for this query. Maybe F1 and F2?
| world: UnsafeWorldCell<'world>, | ||
| _: Tick, | ||
| ) -> Self::Item<'world, 'state> { | ||
| let data_query = unsafe { state.0.query_unchecked_manual(world) }; |
| >, | ||
| > { | ||
| self.data_query | ||
| .iter_many(self.filter_query.clone().map(|r| r.get())) |
There was a problem hiding this comment.
These clone calls are suspicious; I'm not sure I understand the overall strategy here very well.
|
Interesting; I think this is worth exploring! Please let us know if you need help with anything; this is a relatively complex and high impact feature despite the simple code, so I'm going to be fairly aggressive on documentation requirements before merging. |
|
I poked at #17649 again, and I don't think that the approach I used there is viable. It's swimming upstream too much, and like you say, it will have surprising performance implications. |
|
@alice-i-cecile To be honest, I'm not sure that this implementation is the best solution, because now for each separate type of relationship query you will have to create your own system parameter. |
Objective
Improved interaction with entities in relationships. Added the ability to make combined queries.
This solution is alternative for #17647 (the goal is one, but the methods of achieving it are different)
Naive solution
Created a system parameter that combines 2 queries. One query (QueryData) is addressed to the relationship target, the other query (QueryFilter) is for related entities. After that, these queries are combined. (Each function is 2-3 lines of code, so it's easier to look at the code, it's really just 2 combined queries and nothing more.)
struct Related<D: QueryData, R: RelatedTarget, F: QueryFilter>API example (the system will select all entities that are parents with health and have children (1st order descendants) with a shield and without a helmet):
Testing
2 separate queries with brute forceandRelatedqueryComparison with alternative (WorldQuery)
General position.
It is important to understand what types of queries we want to support for relationships. The basic categories of queries that I see (I will give examples on Parent and Children, because otherwise I get confused) are queries that take into account the hierarchy (they look at any level of the hierarchy), and "linear" queries with the first or last member of the hierarchy.
Obviously, the easiest to implement are "linear" queries (these are always 2 independent combined queries). It is also important that they are predictable in performance.
Difficulties arise with queries with a hierarchy when we can get to any level of nesting (traversal of the entire relationship graph). And the main difficulty is caused by matching the identifiers of the child and parent entities (the speed depends on the depth of the hierarchy). And if you decide to compare all sublevels, then checking their correct relative position will take an irrational amount of time.
As the experiment with WorldQuery showed, the proposed API easily expresses any nesting, but these queries are initially designed to work with only one query. I believe that this behavior cannot be violated and here's why.
Another problem follows from this:
Queries over entities with relations are a separate "type" of queries, so I think that it is necessary to take it out into a separate type of system parameter. Perhaps a new API should be added for such types of queries (like combined queries).