Skip to content

Commit 1a7b78a

Browse files
Support non-archetypal QueryData (#21581)
# Objective For #17647, we want to create a `QueryData` that can follow a relation and query data from an entity's parent. If the parent does not have the queried data, the child entity should be skipped in the query. However, there is no way to tell from the child's archetype whether the parent will match! So, we need to support *non-archetypal* `QueryData`, just as we support non-archetypal `QueryFilter`s for `Added` and `Changed`. That is, if `Query<Parent<&T>>` yields `&T`, and we do: ```rust let parent1 = world.spawn(T).id(); let child1 = world.spawn(ChildOf(parent1)); let parent2 = world.spawn(()).id(); let child2 = world.spawn(ChildOf(parent2)); let query = world.query::<Parent<&T>>(); ``` then `query` must yield a row for `child1` but not for `child2`, even though they have the same archetype. ## Solution Change `QueryData::fetch` to return `Option` so that entities can be filtered during fetching by returning `None`. To support `ExactSizeIterator`, introduce an `ArchetypeQueryData` trait and an `QueryData::IS_ARCHETYPAL` associated constant, similar to `ArchetypeFilter` and `QueryFilter::IS_ARCHETYPAL`. Implement this trait on existing `QueryData` types. Modify `ExactSizeIterator` implementations to require `D: ArchetypeQueryData`, and the `size_hint()` methods to return a minimum size of `0` if `!D::IS_ARCHETYPAL`. ## Alternatives We could do nothing here, and have `Query<Parent<&T>>` yield `Option<&T>`. That makes the API less convenient, though. Note that if one *wants* to query for `Option`, they can use either `Query<Option<Parent<&T>>` or `Query<Parent<Option<&T>>`, depending on whether they want to include entities with no parent. Another option is to re-use the `ArchetypeFilter` trait instead of introducing a new one. There are no places where we want to abstract over both, however, and it would require writing bounds like `D: QueryData + ArchetypeFilter, F: QueryFilter + ArchetypeFilter` instead of simply `D: ArchetypeQueryData, F: ArchetypeFilter`. --------- Co-authored-by: Periwink <charlesbour@gmail.com>
1 parent 2b74e52 commit 1a7b78a

File tree

12 files changed

+420
-165
lines changed

12 files changed

+420
-165
lines changed

crates/bevy_asset/src/asset_changed.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ unsafe impl<A: AsAssetId> QueryFilter for AssetChanged<A> {
279279
// SAFETY: We delegate to the inner `fetch` for `A`
280280
unsafe {
281281
let handle = <&A>::fetch(&state.asset_id, inner, entity, table_row);
282-
fetch.check.has_changed(handle)
282+
handle.is_some_and(|handle| fetch.check.has_changed(handle))
283283
}
284284
})
285285
}

crates/bevy_ecs/macros/src/query_data.rs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,7 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream {
266266
unsafe impl #user_impl_generics #path::query::QueryData
267267
for #read_only_struct_name #user_ty_generics #user_where_clauses {
268268
const IS_READ_ONLY: bool = true;
269+
const IS_ARCHETYPAL: bool = true #(&& <#read_only_field_types as #path::query::QueryData>::IS_ARCHETYPAL)*;
269270
type ReadOnly = #read_only_struct_name #user_ty_generics;
270271
type Item<'__w, '__s> = #read_only_item_struct_name #user_ty_generics_with_world_and_state;
271272

@@ -294,10 +295,10 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream {
294295
_fetch: &mut <Self as #path::query::WorldQuery>::Fetch<'__w>,
295296
_entity: #path::entity::Entity,
296297
_table_row: #path::storage::TableRow,
297-
) -> Self::Item<'__w, '__s> {
298-
Self::Item {
299-
#(#field_idents: <#read_only_field_types>::fetch(&_state.#named_field_idents, &mut _fetch.#named_field_idents, _entity, _table_row),)*
300-
}
298+
) -> Option<Self::Item<'__w, '__s>> {
299+
Some(Self::Item {
300+
#(#field_idents: <#read_only_field_types>::fetch(&_state.#named_field_idents, &mut _fetch.#named_field_idents, _entity, _table_row)?,)*
301+
})
301302
}
302303
}
303304

@@ -312,6 +313,12 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream {
312313
}
313314
}
314315
}
316+
317+
impl #user_impl_generics #path::query::ArchetypeQueryData
318+
for #read_only_struct_name #user_ty_generics #user_where_clauses
319+
// Make these HRTBs with an unused lifetime parameter to allow trivial constraints
320+
// See https://github.com/rust-lang/rust/issues/48214
321+
where #(for<'__a> #field_types: #path::query::ArchetypeQueryData,)* {}
315322
}
316323
} else {
317324
quote! {}
@@ -324,6 +331,7 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream {
324331
unsafe impl #user_impl_generics #path::query::QueryData
325332
for #struct_name #user_ty_generics #user_where_clauses {
326333
const IS_READ_ONLY: bool = #is_read_only;
334+
const IS_ARCHETYPAL: bool = true #(&& <#field_types as #path::query::QueryData>::IS_ARCHETYPAL)*;
327335
type ReadOnly = #read_only_struct_name #user_ty_generics;
328336
type Item<'__w, '__s> = #item_struct_name #user_ty_generics_with_world_and_state;
329337

@@ -352,10 +360,10 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream {
352360
_fetch: &mut <Self as #path::query::WorldQuery>::Fetch<'__w>,
353361
_entity: #path::entity::Entity,
354362
_table_row: #path::storage::TableRow,
355-
) -> Self::Item<'__w, '__s> {
356-
Self::Item {
357-
#(#field_idents: <#field_types>::fetch(&_state.#named_field_idents, &mut _fetch.#named_field_idents, _entity, _table_row),)*
358-
}
363+
) -> Option<Self::Item<'__w, '__s>> {
364+
Some(Self::Item {
365+
#(#field_idents: <#field_types>::fetch(&_state.#named_field_idents, &mut _fetch.#named_field_idents, _entity, _table_row)?,)*
366+
})
359367
}
360368
}
361369

@@ -371,6 +379,12 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream {
371379
}
372380
}
373381

382+
impl #user_impl_generics #path::query::ArchetypeQueryData
383+
for #struct_name #user_ty_generics #user_where_clauses
384+
// Make these HRTBs with an unused lifetime parameter to allow trivial constraints
385+
// See https://github.com/rust-lang/rust/issues/48214
386+
where #(for<'__a> #field_types: #path::query::ArchetypeQueryData,)* {}
387+
374388
#read_only_data_impl
375389
}
376390
};

crates/bevy_ecs/macros/src/query_filter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ pub fn derive_query_filter_impl(input: TokenStream) -> TokenStream {
9797
// SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access.
9898
unsafe impl #user_impl_generics #path::query::QueryFilter
9999
for #struct_name #user_ty_generics #user_where_clauses {
100-
const IS_ARCHETYPAL: bool = true #(&& <#field_types>::IS_ARCHETYPAL)*;
100+
const IS_ARCHETYPAL: bool = true #(&& <#field_types as #path::query::QueryFilter>::IS_ARCHETYPAL)*;
101101

102102
#[allow(unused_variables)]
103103
#[inline(always)]

0 commit comments

Comments
 (0)