diff --git a/crates/bevy_asset/src/asset_changed.rs b/crates/bevy_asset/src/asset_changed.rs index 272dca46f3ae2..6e50b7776f1cd 100644 --- a/crates/bevy_asset/src/asset_changed.rs +++ b/crates/bevy_asset/src/asset_changed.rs @@ -278,7 +278,7 @@ unsafe impl QueryFilter for AssetChanged { // SAFETY: We delegate to the inner `fetch` for `A` unsafe { let handle = <&A>::fetch(&state.asset_id, inner, entity, table_row); - fetch.check.has_changed(handle) + handle.is_some_and(|handle| fetch.check.has_changed(handle)) } }) } diff --git a/crates/bevy_ecs/macros/src/query_data.rs b/crates/bevy_ecs/macros/src/query_data.rs index e558e2e634985..3459a171234ea 100644 --- a/crates/bevy_ecs/macros/src/query_data.rs +++ b/crates/bevy_ecs/macros/src/query_data.rs @@ -266,6 +266,7 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { unsafe impl #user_impl_generics #path::query::QueryData for #read_only_struct_name #user_ty_generics #user_where_clauses { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true #(&& <#read_only_field_types as #path::query::QueryData>::IS_ARCHETYPAL)*; type ReadOnly = #read_only_struct_name #user_ty_generics; type Item<'__w, '__s> = #read_only_item_struct_name #user_ty_generics_with_world_and_state; @@ -294,10 +295,10 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { _fetch: &mut ::Fetch<'__w>, _entity: #path::entity::Entity, _table_row: #path::storage::TableRow, - ) -> Self::Item<'__w, '__s> { - Self::Item { - #(#field_idents: <#read_only_field_types>::fetch(&_state.#named_field_idents, &mut _fetch.#named_field_idents, _entity, _table_row),)* - } + ) -> Option> { + Some(Self::Item { + #(#field_idents: <#read_only_field_types>::fetch(&_state.#named_field_idents, &mut _fetch.#named_field_idents, _entity, _table_row)?,)* + }) } } @@ -312,6 +313,12 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { } } } + + impl #user_impl_generics #path::query::ArchetypeQueryData + for #read_only_struct_name #user_ty_generics #user_where_clauses + // Make these HRTBs with an unused lifetime parameter to allow trivial constraints + // See https://github.com/rust-lang/rust/issues/48214 + where #(for<'__a> #field_types: #path::query::ArchetypeQueryData,)* {} } } else { quote! {} @@ -324,6 +331,7 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { unsafe impl #user_impl_generics #path::query::QueryData for #struct_name #user_ty_generics #user_where_clauses { const IS_READ_ONLY: bool = #is_read_only; + const IS_ARCHETYPAL: bool = true #(&& <#field_types as #path::query::QueryData>::IS_ARCHETYPAL)*; type ReadOnly = #read_only_struct_name #user_ty_generics; type Item<'__w, '__s> = #item_struct_name #user_ty_generics_with_world_and_state; @@ -352,10 +360,10 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { _fetch: &mut ::Fetch<'__w>, _entity: #path::entity::Entity, _table_row: #path::storage::TableRow, - ) -> Self::Item<'__w, '__s> { - Self::Item { - #(#field_idents: <#field_types>::fetch(&_state.#named_field_idents, &mut _fetch.#named_field_idents, _entity, _table_row),)* - } + ) -> Option> { + Some(Self::Item { + #(#field_idents: <#field_types>::fetch(&_state.#named_field_idents, &mut _fetch.#named_field_idents, _entity, _table_row)?,)* + }) } } @@ -371,6 +379,12 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { } } + impl #user_impl_generics #path::query::ArchetypeQueryData + for #struct_name #user_ty_generics #user_where_clauses + // Make these HRTBs with an unused lifetime parameter to allow trivial constraints + // See https://github.com/rust-lang/rust/issues/48214 + where #(for<'__a> #field_types: #path::query::ArchetypeQueryData,)* {} + #read_only_data_impl } }; diff --git a/crates/bevy_ecs/macros/src/query_filter.rs b/crates/bevy_ecs/macros/src/query_filter.rs index 5ae2d2325fdab..712befb71007d 100644 --- a/crates/bevy_ecs/macros/src/query_filter.rs +++ b/crates/bevy_ecs/macros/src/query_filter.rs @@ -97,7 +97,7 @@ pub fn derive_query_filter_impl(input: TokenStream) -> TokenStream { // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access. unsafe impl #user_impl_generics #path::query::QueryFilter for #struct_name #user_ty_generics #user_where_clauses { - const IS_ARCHETYPAL: bool = true #(&& <#field_types>::IS_ARCHETYPAL)*; + const IS_ARCHETYPAL: bool = true #(&& <#field_types as #path::query::QueryFilter>::IS_ARCHETYPAL)*; #[allow(unused_variables)] #[inline(always)] diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 7b1ff2387f1d4..a549336465de2 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -284,6 +284,15 @@ pub unsafe trait QueryData: WorldQuery { /// True if this query is read-only and may not perform mutable access. const IS_READ_ONLY: bool; + /// Returns true if (and only if) this query data relies strictly on archetypes to limit which + /// entities are accessed by the Query. + /// + /// This enables optimizations for [`QueryIter`](`crate::query::QueryIter`) that rely on knowing exactly how + /// many elements are being iterated (such as `Iterator::collect()`). + /// + /// If this is `true`, then [`QueryData::fetch`] must always return `Some`. + const IS_ARCHETYPAL: bool; + /// The read-only variant of this [`QueryData`], which satisfies the [`ReadOnlyQueryData`] trait. type ReadOnly: ReadOnlyQueryData::State>; @@ -319,6 +328,9 @@ pub unsafe trait QueryData: WorldQuery { /// [`WorldQuery::set_archetype`] with an `entity` in the current archetype. /// Accesses components registered in [`WorldQuery::update_component_access`]. /// + /// This method returns `None` if the entity does not match the query. + /// If `Self` implements [`ArchetypeQueryData`], this must always return `Some`. + /// /// # Safety /// /// - Must always be called _after_ [`WorldQuery::set_table`] or [`WorldQuery::set_archetype`]. `entity` and @@ -329,7 +341,7 @@ pub unsafe trait QueryData: WorldQuery { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Self::Item<'w, 's>; + ) -> Option>; } /// A [`QueryData`] that is read only. @@ -355,6 +367,14 @@ pub trait ReleaseStateQueryData: QueryData { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static>; } +/// A marker trait to indicate that the query data filters at an archetype level. +/// +/// This is needed to implement [`ExactSizeIterator`] for +/// [`QueryIter`](crate::query::QueryIter) that contains archetype-level filters. +/// +/// The trait must only be implemented for query data where its corresponding [`QueryData::IS_ARCHETYPAL`] is [`prim@true`]. +pub trait ArchetypeQueryData: QueryData {} + /// SAFETY: /// `update_component_access` does nothing. /// This is sound because `fetch` does not access components. @@ -410,6 +430,7 @@ unsafe impl WorldQuery for Entity { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for Entity { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = Entity; @@ -426,8 +447,8 @@ unsafe impl QueryData for Entity { _fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { - entity + ) -> Option> { + Some(entity) } } @@ -440,6 +461,8 @@ impl ReleaseStateQueryData for Entity { } } +impl ArchetypeQueryData for Entity {} + /// SAFETY: /// `update_component_access` does nothing. /// This is sound because `fetch` does not access components. @@ -500,6 +523,7 @@ unsafe impl WorldQuery for EntityLocation { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for EntityLocation { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = EntityLocation; @@ -515,9 +539,9 @@ unsafe impl QueryData for EntityLocation { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { // SAFETY: `fetch` must be called with an entity that exists in the world - unsafe { fetch.get(entity).debug_checked_unwrap() } + Some(unsafe { fetch.get(entity).debug_checked_unwrap() }) } } @@ -530,6 +554,8 @@ impl ReleaseStateQueryData for EntityLocation { } } +impl ArchetypeQueryData for EntityLocation {} + /// The `SpawnDetails` query parameter fetches the [`Tick`] the entity was spawned at. /// /// To evaluate whether the spawn happened since the last time the system ran, the system @@ -663,6 +689,7 @@ unsafe impl WorldQuery for SpawnDetails { // Is its own ReadOnlyQueryData. unsafe impl QueryData for SpawnDetails { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = Self; @@ -678,19 +705,19 @@ unsafe impl QueryData for SpawnDetails { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { // SAFETY: only living entities are queried let (spawned_by, spawn_tick) = unsafe { fetch .entities .entity_get_spawned_or_despawned_unchecked(entity) }; - Self { + Some(Self { spawned_by, spawn_tick, last_run: fetch.last_run, this_run: fetch.this_run, - } + }) } } @@ -703,6 +730,8 @@ impl ReleaseStateQueryData for SpawnDetails { } } +impl ArchetypeQueryData for SpawnDetails {} + /// The [`WorldQuery::Fetch`] type for WorldQueries that can fetch multiple components from an entity /// ([`EntityRef`], [`EntityMut`], etc.) #[derive(Copy, Clone)] @@ -782,6 +811,7 @@ unsafe impl<'a> WorldQuery for EntityRef<'a> { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl<'a> QueryData for EntityRef<'a> { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = EntityRef<'w>; @@ -797,7 +827,7 @@ unsafe impl<'a> QueryData for EntityRef<'a> { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { // SAFETY: `fetch` must be called with an entity that exists in the world let cell = unsafe { fetch @@ -806,7 +836,7 @@ unsafe impl<'a> QueryData for EntityRef<'a> { .debug_checked_unwrap() }; // SAFETY: Read-only access to every component has been registered. - unsafe { EntityRef::new(cell) } + Some(unsafe { EntityRef::new(cell) }) } } @@ -819,6 +849,8 @@ impl ReleaseStateQueryData for EntityRef<'_> { } } +impl ArchetypeQueryData for EntityRef<'_> {} + /// SAFETY: The accesses of `Self::ReadOnly` are a subset of the accesses of `Self` unsafe impl<'a> WorldQuery for EntityMut<'a> { type Fetch<'w> = EntityFetch<'w>; @@ -885,6 +917,7 @@ unsafe impl<'a> WorldQuery for EntityMut<'a> { /// SAFETY: access of `EntityRef` is a subset of `EntityMut` unsafe impl<'a> QueryData for EntityMut<'a> { const IS_READ_ONLY: bool = false; + const IS_ARCHETYPAL: bool = true; type ReadOnly = EntityRef<'a>; type Item<'w, 's> = EntityMut<'w>; @@ -900,7 +933,7 @@ unsafe impl<'a> QueryData for EntityMut<'a> { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { // SAFETY: `fetch` must be called with an entity that exists in the world let cell = unsafe { fetch @@ -909,7 +942,7 @@ unsafe impl<'a> QueryData for EntityMut<'a> { .debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - unsafe { EntityMut::new(cell) } + Some(unsafe { EntityMut::new(cell) }) } } @@ -919,6 +952,8 @@ impl ReleaseStateQueryData for EntityMut<'_> { } } +impl ArchetypeQueryData for EntityMut<'_> {} + /// SAFETY: The accesses of `Self::ReadOnly` are a subset of the accesses of `Self` unsafe impl WorldQuery for FilteredEntityRef<'_, '_> { type Fetch<'w> = EntityFetch<'w>; @@ -987,6 +1022,7 @@ unsafe impl WorldQuery for FilteredEntityRef<'_, '_> { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for FilteredEntityRef<'_, '_> { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = FilteredEntityRef<'w, 's>; @@ -1021,7 +1057,7 @@ unsafe impl QueryData for FilteredEntityRef<'_, '_> { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { // SAFETY: `fetch` must be called with an entity that exists in the world let cell = unsafe { fetch @@ -1030,13 +1066,15 @@ unsafe impl QueryData for FilteredEntityRef<'_, '_> { .debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - unsafe { FilteredEntityRef::new(cell, access) } + Some(unsafe { FilteredEntityRef::new(cell, access) }) } } /// SAFETY: Access is read-only. unsafe impl ReadOnlyQueryData for FilteredEntityRef<'_, '_> {} +impl ArchetypeQueryData for FilteredEntityRef<'_, '_> {} + /// SAFETY: The accesses of `Self::ReadOnly` are a subset of the accesses of `Self` unsafe impl WorldQuery for FilteredEntityMut<'_, '_> { type Fetch<'w> = EntityFetch<'w>; @@ -1105,6 +1143,7 @@ unsafe impl WorldQuery for FilteredEntityMut<'_, '_> { /// SAFETY: access of `FilteredEntityRef` is a subset of `FilteredEntityMut` unsafe impl<'a, 'b> QueryData for FilteredEntityMut<'a, 'b> { const IS_READ_ONLY: bool = false; + const IS_ARCHETYPAL: bool = true; type ReadOnly = FilteredEntityRef<'a, 'b>; type Item<'w, 's> = FilteredEntityMut<'w, 's>; @@ -1137,7 +1176,7 @@ unsafe impl<'a, 'b> QueryData for FilteredEntityMut<'a, 'b> { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { // SAFETY: `fetch` must be called with an entity that exists in the world let cell = unsafe { fetch @@ -1146,10 +1185,12 @@ unsafe impl<'a, 'b> QueryData for FilteredEntityMut<'a, 'b> { .debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - unsafe { FilteredEntityMut::new(cell, access) } + Some(unsafe { FilteredEntityMut::new(cell, access) }) } } +impl ArchetypeQueryData for FilteredEntityMut<'_, '_> {} + /// SAFETY: `EntityRefExcept` guards access to all components in the bundle `B` /// and populates `Access` values so that queries that conflict with this access /// are rejected. @@ -1236,6 +1277,7 @@ where B: Bundle, { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = EntityRefExcept<'w, 's, B>; @@ -1250,12 +1292,12 @@ where fetch: &mut Self::Fetch<'w>, entity: Entity, _: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { let cell = fetch .world .get_entity_with_ticks(entity, fetch.last_run, fetch.this_run) .unwrap(); - EntityRefExcept::new(cell, access) + Some(EntityRefExcept::new(cell, access)) } } @@ -1263,6 +1305,8 @@ where /// components. unsafe impl ReadOnlyQueryData for EntityRefExcept<'_, '_, B> where B: Bundle {} +impl ArchetypeQueryData for EntityRefExcept<'_, '_, B> {} + /// SAFETY: `EntityMutExcept` guards access to all components in the bundle `B` /// and populates `Access` values so that queries that conflict with this access /// are rejected. @@ -1350,6 +1394,7 @@ where B: Bundle, { const IS_READ_ONLY: bool = false; + const IS_ARCHETYPAL: bool = true; type ReadOnly = EntityRefExcept<'a, 'b, B>; type Item<'w, 's> = EntityMutExcept<'w, 's, B>; @@ -1364,15 +1409,17 @@ where fetch: &mut Self::Fetch<'w>, entity: Entity, _: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { let cell = fetch .world .get_entity_with_ticks(entity, fetch.last_run, fetch.this_run) .unwrap(); - EntityMutExcept::new(cell, access) + Some(EntityMutExcept::new(cell, access)) } } +impl ArchetypeQueryData for EntityMutExcept<'_, '_, B> {} + /// SAFETY: /// `update_component_access` does nothing. /// This is sound because `fetch` does not access components. @@ -1433,6 +1480,7 @@ unsafe impl WorldQuery for &Archetype { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for &Archetype { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = &'w Archetype; @@ -1448,12 +1496,12 @@ unsafe impl QueryData for &Archetype { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { let (entities, archetypes) = *fetch; // SAFETY: `fetch` must be called with an entity that exists in the world let location = unsafe { entities.get(entity).debug_checked_unwrap() }; // SAFETY: The assigned archetype for a living entity must always be valid. - unsafe { archetypes.get(location.archetype_id).debug_checked_unwrap() } + Some(unsafe { archetypes.get(location.archetype_id).debug_checked_unwrap() }) } } @@ -1466,6 +1514,8 @@ impl ReleaseStateQueryData for &Archetype { } } +impl ArchetypeQueryData for &Archetype {} + /// The [`WorldQuery::Fetch`] type for `& T`. pub struct ReadFetch<'w, T: Component> { components: StorageSwitch< @@ -1585,6 +1635,7 @@ unsafe impl WorldQuery for &T { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for &T { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = &'w T; @@ -1600,8 +1651,8 @@ unsafe impl QueryData for &T { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Self::Item<'w, 's> { - fetch.components.extract( + ) -> Option> { + Some(fetch.components.extract( |table| { // SAFETY: set_table was previously called let table = unsafe { table.debug_checked_unwrap() }; @@ -1619,7 +1670,7 @@ unsafe impl QueryData for &T { }; item.deref() }, - ) + )) } } @@ -1632,6 +1683,8 @@ impl ReleaseStateQueryData for &T { } } +impl ArchetypeQueryData for &T {} + #[doc(hidden)] pub struct RefFetch<'w, T: Component> { components: StorageSwitch< @@ -1768,6 +1821,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl<'__w, T: Component> QueryData for Ref<'__w, T> { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = Ref<'w, T>; @@ -1783,8 +1837,8 @@ unsafe impl<'__w, T: Component> QueryData for Ref<'__w, T> { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Self::Item<'w, 's> { - fetch.components.extract( + ) -> Option> { + Some(fetch.components.extract( |table| { // SAFETY: set_table was previously called let (table_components, added_ticks, changed_ticks, callers) = @@ -1825,7 +1879,7 @@ unsafe impl<'__w, T: Component> QueryData for Ref<'__w, T> { changed_by: caller.map(|caller| caller.deref()), } }, - ) + )) } } @@ -1838,6 +1892,8 @@ impl ReleaseStateQueryData for Ref<'_, T> { } } +impl ArchetypeQueryData for Ref<'_, T> {} + /// The [`WorldQuery::Fetch`] type for `&mut T`. pub struct WriteFetch<'w, T: Component> { components: StorageSwitch< @@ -1974,6 +2030,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { /// SAFETY: access of `&T` is a subset of `&mut T` unsafe impl<'__w, T: Component> QueryData for &'__w mut T { const IS_READ_ONLY: bool = false; + const IS_ARCHETYPAL: bool = true; type ReadOnly = &'__w T; type Item<'w, 's> = Mut<'w, T>; @@ -1989,8 +2046,8 @@ unsafe impl<'__w, T: Component> QueryData for &'__w mut T fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Self::Item<'w, 's> { - fetch.components.extract( + ) -> Option> { + Some(fetch.components.extract( |table| { // SAFETY: set_table was previously called let (table_components, added_ticks, changed_ticks, callers) = @@ -2031,7 +2088,7 @@ unsafe impl<'__w, T: Component> QueryData for &'__w mut T changed_by: caller.map(|caller| caller.deref_mut()), } }, - ) + )) } } @@ -2041,6 +2098,8 @@ impl> ReleaseStateQueryData for &mut T { } } +impl> ArchetypeQueryData for &mut T {} + /// When `Mut` is used in a query, it will be converted to `Ref` when transformed into its read-only form, providing access to change detection methods. /// /// By contrast `&mut T` will result in a `Mut` item in mutable form to record mutations, but result in a bare `&T` in read-only form. @@ -2123,6 +2182,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Mut<'__w, T> { // SAFETY: access of `Ref` is a subset of `Mut` unsafe impl<'__w, T: Component> QueryData for Mut<'__w, T> { const IS_READ_ONLY: bool = false; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Ref<'__w, T>; type Item<'w, 's> = Mut<'w, T>; @@ -2142,7 +2202,7 @@ unsafe impl<'__w, T: Component> QueryData for Mut<'__w, T> fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { <&mut T as QueryData>::fetch(state, fetch, entity, table_row) } } @@ -2153,6 +2213,8 @@ impl> ReleaseStateQueryData for Mut<'_, T> { } } +impl> ArchetypeQueryData for Mut<'_, T> {} + #[doc(hidden)] pub struct OptionFetch<'w, T: WorldQuery> { fetch: T::Fetch<'w>, @@ -2264,6 +2326,9 @@ unsafe impl WorldQuery for Option { // SAFETY: defers to soundness of `T: WorldQuery` impl unsafe impl QueryData for Option { const IS_READ_ONLY: bool = T::IS_READ_ONLY; + // `Option` matches all entities, even if `T` does not, + // so it's always an `ArchetypeQueryData`, even for non-archetypal `T`. + const IS_ARCHETYPAL: bool = true; type ReadOnly = Option; type Item<'w, 's> = Option>; @@ -2279,11 +2344,14 @@ unsafe impl QueryData for Option { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Self::Item<'w, 's> { - fetch - .matches - // SAFETY: The invariants are upheld by the caller. - .then(|| unsafe { T::fetch(state, &mut fetch.fetch, entity, table_row) }) + ) -> Option> { + Some( + fetch + .matches + // SAFETY: The invariants are upheld by the caller. + .then(|| unsafe { T::fetch(state, &mut fetch.fetch, entity, table_row) }) + .flatten(), + ) } } @@ -2296,6 +2364,10 @@ impl ReleaseStateQueryData for Option { } } +// `Option` matches all entities, even if `T` does not, +// so it's always an `ArchetypeQueryData`, even for non-archetypal `T`. +impl ArchetypeQueryData for Option {} + /// Returns a bool that describes if an entity has the component `T`. /// /// This can be used in a [`Query`](crate::system::Query) if you want to know whether or not entities @@ -2438,6 +2510,7 @@ unsafe impl WorldQuery for Has { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for Has { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = bool; @@ -2453,8 +2526,8 @@ unsafe impl QueryData for Has { fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { - *fetch + ) -> Option> { + Some(*fetch) } } @@ -2467,6 +2540,8 @@ impl ReleaseStateQueryData for Has { } } +impl ArchetypeQueryData for Has {} + /// The `AnyOf` query parameter fetches entities with any of the component types included in T. /// /// `Query>` is equivalent to `Query<(Option<&A>, Option<&B>, Option<&mut C>), Or<(With, With, With)>>`. @@ -2496,6 +2571,7 @@ macro_rules! impl_tuple_query_data { // SAFETY: defers to soundness `$name: WorldQuery` impl unsafe impl<$($name: QueryData),*> QueryData for ($($name,)*) { const IS_READ_ONLY: bool = true $(&& $name::IS_READ_ONLY)*; + const IS_ARCHETYPAL: bool = true $(&& $name::IS_ARCHETYPAL)*; type ReadOnly = ($($name::ReadOnly,)*); type Item<'w, 's> = ($($name::Item<'w, 's>,)*); @@ -2522,11 +2598,11 @@ macro_rules! impl_tuple_query_data { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow - ) -> Self::Item<'w, 's> { + ) -> Option> { let ($($state,)*) = state; let ($($name,)*) = fetch; // SAFETY: The invariants are upheld by the caller. - ($(unsafe { $name::fetch($state, $name, entity, table_row) },)*) + Some(($(unsafe { $name::fetch($state, $name, entity, table_row) }?,)*)) } } @@ -2548,6 +2624,9 @@ macro_rules! impl_tuple_query_data { ($($name::release_state($item),)*) } } + + $(#[$meta])* + impl<$($name: ArchetypeQueryData),*> ArchetypeQueryData for ($($name,)*) {} }; } @@ -2685,6 +2764,7 @@ macro_rules! impl_anytuple_fetch { // SAFETY: defers to soundness of `$name: WorldQuery` impl unsafe impl<$($name: QueryData),*> QueryData for AnyOf<($($name,)*)> { const IS_READ_ONLY: bool = true $(&& $name::IS_READ_ONLY)*; + const IS_ARCHETYPAL: bool = true $(&& $name::IS_ARCHETYPAL)*; type ReadOnly = AnyOf<($($name::ReadOnly,)*)>; type Item<'w, 's> = ($(Option<$name::Item<'w, 's>>,)*); @@ -2701,13 +2781,25 @@ macro_rules! impl_anytuple_fetch { _fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow - ) -> Self::Item<'w, 's> { + ) -> Option> { let ($($name,)*) = _fetch; let ($($state,)*) = _state; - ($( + let result = ($( // SAFETY: The invariants are required to be upheld by the caller. - $name.1.then(|| unsafe { $name::fetch($state, &mut $name.0, _entity, _table_row) }), - )*) + $name.1.then(|| unsafe { $name::fetch($state, &mut $name.0, _entity, _table_row) }).flatten(), + )*); + // If this is an archetypal query, then it is guaranteed to return `Some`, + // and we can help the compiler remove branches by checking the const `IS_ARCHETYPAL` first. + (Self::IS_ARCHETYPAL + // We want to return `Some` if the query matches this entity, + // which happens if at least one subquery returns `Some`. + // So, fetch everything as usual, but if all the subqueries return `None` then return `None` instead. + || !matches!(result, ($(Option::>::None,)*)) + // If *none* of the subqueries matched the archetype, then this archetype was added in a transmute. + // We must treat those as matching in order to be consistent with `size_hint` for archetypal queries, + // so we treat them as matching for non-archetypal queries, as well. + || !(false $(|| $name.1)*)) + .then_some(result) } } @@ -2728,6 +2820,9 @@ macro_rules! impl_anytuple_fetch { ($($item.map(|$item| $name::release_state($item)),)*) } } + + $(#[$meta])* + impl<$($name: ArchetypeQueryData),*> ArchetypeQueryData for AnyOf<($($name,)*)> {} }; } @@ -2809,6 +2904,7 @@ unsafe impl WorldQuery for NopWorldQuery { /// SAFETY: `Self::ReadOnly` is `Self` unsafe impl QueryData for NopWorldQuery { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = (); @@ -2823,7 +2919,8 @@ unsafe impl QueryData for NopWorldQuery { _fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { + Some(()) } } @@ -2834,6 +2931,8 @@ impl ReleaseStateQueryData for NopWorldQuery { fn release_state<'w>(_item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> {} } +impl ArchetypeQueryData for NopWorldQuery {} + /// SAFETY: /// `update_component_access` does nothing. /// This is sound because `fetch` does not access components. @@ -2891,6 +2990,7 @@ unsafe impl WorldQuery for PhantomData { /// SAFETY: `Self::ReadOnly` is `Self` unsafe impl QueryData for PhantomData { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = (); @@ -2904,7 +3004,8 @@ unsafe impl QueryData for PhantomData { _fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { + Some(()) } } @@ -2915,6 +3016,8 @@ impl ReleaseStateQueryData for PhantomData { fn release_state<'w>(_item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> {} } +impl ArchetypeQueryData for PhantomData {} + /// A compile-time checked union of two different types that differs based on the /// [`StorageType`] of a given component. pub(super) union StorageSwitch { @@ -3094,6 +3197,7 @@ mod tests { unsafe impl QueryData for NonReleaseQueryData { type ReadOnly = Self; const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type Item<'w, 's> = (); @@ -3108,13 +3212,16 @@ mod tests { _fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { + Some(()) } } /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for NonReleaseQueryData {} + impl ArchetypeQueryData for NonReleaseQueryData {} + #[derive(QueryData)] pub struct DerivedNonReleaseRead { non_release: NonReleaseQueryData, diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 4ebc38098503a..5ffc4c01f63c9 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -415,6 +415,11 @@ macro_rules! impl_or_query_filter { #[inline] unsafe fn set_table<'w, 's>(fetch: &mut Self::Fetch<'w>, state: &'s Self::State, table: &'w Table) { + // If this is an archetypal query, then it is guaranteed to match all entities, + // so `filter_fetch` will ignore `$filter.matches` and we don't need to initialize it. + if Self::IS_ARCHETYPAL { + return; + } let ($($filter,)*) = fetch; let ($($state,)*) = state; $( @@ -433,6 +438,11 @@ macro_rules! impl_or_query_filter { archetype: &'w Archetype, table: &'w Table ) { + // If this is an archetypal query, then it is guaranteed to match all entities, + // so `filter_fetch` will ignore `$filter.matches` and we don't need to initialize it. + if Self::IS_ARCHETYPAL { + return; + } let ($($filter,)*) = fetch; let ($($state,)*) = &state; $( @@ -506,8 +516,15 @@ macro_rules! impl_or_query_filter { ) -> bool { let ($($state,)*) = state; let ($($filter,)*) = fetch; - // SAFETY: The invariants are upheld by the caller. - false $(|| ($filter.matches && unsafe { $filter::filter_fetch($state, &mut $filter.fetch, entity, table_row) }))* + // If this is an archetypal query, then it is guaranteed to return true, + // and we can help the compiler remove branches by checking the const `IS_ARCHETYPAL` first. + (Self::IS_ARCHETYPAL + // SAFETY: The invariants are upheld by the caller. + $(|| ($filter.matches && unsafe { $filter::filter_fetch($state, &mut $filter.fetch, entity, table_row) }))* + // If *none* of the subqueries matched the archetype, then this archetype was added in a transmute. + // We must treat those as matching in order to be consistent with `size_hint` for archetypal queries, + // so we treat them as matching for non-archetypal queries, as well. + || !(false $(|| $filter.matches)*)) } } }; diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index cab2ee9c9391d..69c3730cd4cd6 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -4,7 +4,7 @@ use crate::{ bundle::Bundle, component::Tick, entity::{ContainsEntity, Entities, Entity, EntityEquivalent, EntitySet, EntitySetIterator}, - query::{ArchetypeFilter, DebugCheckedUnwrap, QueryState, StorageId}, + query::{ArchetypeFilter, ArchetypeQueryData, DebugCheckedUnwrap, QueryState, StorageId}, storage::{Table, TableRow, Tables}, world::{ unsafe_world_cell::UnsafeWorldCell, EntityMut, EntityMutExcept, EntityRef, EntityRefExcept, @@ -239,14 +239,14 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: set_table was called prior. // Caller assures `row` in range of the current archetype. - let item = D::fetch( + if let Some(item) = D::fetch( &self.query_state.fetch_state, &mut self.cursor.fetch, *entity, row, - ); - - accum = func(accum, item); + ) { + accum = func(accum, item); + } } accum } @@ -307,16 +307,16 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: set_archetype was called prior, `index` is an archetype index in range of the current archetype // Caller assures `index` in range of the current archetype. - let item = unsafe { + if let Some(item) = unsafe { D::fetch( &self.query_state.fetch_state, &mut self.cursor.fetch, archetype_entity.id(), archetype_entity.table_row(), ) - }; - - accum = func(accum, item); + } { + accum = func(accum, item); + } } accum } @@ -384,14 +384,14 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: set_table was called prior. // Caller assures `row` in range of the current archetype. - let item = D::fetch( + if let Some(item) = D::fetch( &self.query_state.fetch_state, &mut self.cursor.fetch, entity, row, - ); - - accum = func(accum, item); + ) { + accum = func(accum, item); + } } accum } @@ -918,7 +918,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Iterator for QueryIter<'w, 's, D, F> fn size_hint(&self) -> (usize, Option) { let max_size = self.cursor.max_remaining(self.tables, self.archetypes); - let archetype_query = F::IS_ARCHETYPAL; + let archetype_query = D::IS_ARCHETYPAL && F::IS_ARCHETYPAL; let min_size = if archetype_query { max_size } else { 0 }; (min_size as usize, Some(max_size as usize)) } @@ -1040,7 +1040,7 @@ where /// # Safety /// `entity` must stem from `self.entity_iter`, and not have been passed before. #[inline(always)] - unsafe fn fetch_next(&mut self, entity: Entity) -> D::Item<'w, 's> { + unsafe fn fetch_next(&mut self, entity: Entity) -> Option> { let (location, archetype, table); // SAFETY: // `tables` and `archetypes` belong to the same world that the [`QueryIter`] @@ -1089,13 +1089,20 @@ where #[inline(always)] fn next(&mut self) -> Option { - let entity = self.entity_iter.next()?; - // SAFETY: `entity` is passed from `entity_iter` the first time. - unsafe { self.fetch_next(entity).into() } + while let Some(entity) = self.entity_iter.next() { + // SAFETY: `entity` is passed from `entity_iter` the first time. + if let Some(item) = unsafe { self.fetch_next(entity) } { + return Some(item); + } + } + None } fn size_hint(&self) -> (usize, Option) { - self.entity_iter.size_hint() + let (min_size, max_size) = self.entity_iter.size_hint(); + let archetype_query = D::IS_ARCHETYPAL; + let min_size = if archetype_query { min_size } else { 0 }; + (min_size, max_size) } } @@ -1106,16 +1113,18 @@ where { #[inline(always)] fn next_back(&mut self) -> Option { - let entity = self.entity_iter.next_back()?; - // SAFETY: `entity` is passed from `entity_iter` the first time. - unsafe { self.fetch_next(entity).into() } + while let Some(entity) = self.entity_iter.next_back() { + // SAFETY: `entity` is passed from `entity_iter` the first time. + if let Some(item) = unsafe { self.fetch_next(entity) } { + return Some(item); + } + } + None } } -impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> ExactSizeIterator - for QuerySortedIter<'w, 's, D, F, I> -where - I: ExactSizeIterator, +impl> ExactSizeIterator + for QuerySortedIter<'_, '_, D, F, I> { } @@ -1248,9 +1257,12 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> // SAFETY: // - set_archetype was called prior, `location.archetype_row` is an archetype index in range of the current archetype // - fetch is only called once for each entity. - return Some(unsafe { + let item = unsafe { D::fetch(&query_state.fetch_state, fetch, entity, location.table_row) - }); + }; + if let Some(item) = item { + return Some(item); + } } } None @@ -2000,7 +2012,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// It is always safe for shared access. /// `entity` must stem from `self.entity_iter`, and not have been passed before. #[inline(always)] - unsafe fn fetch_next_aliased_unchecked(&mut self, entity: Entity) -> D::Item<'w, 's> { + unsafe fn fetch_next_aliased_unchecked(&mut self, entity: Entity) -> Option> { let (location, archetype, table); // SAFETY: // `tables` and `archetypes` belong to the same world that the [`QueryIter`] @@ -2042,16 +2054,19 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// Get next result from the query #[inline(always)] pub fn fetch_next(&mut self) -> Option> { - let entity = self.entity_iter.next()?; - - // SAFETY: - // We have collected the entity_iter once to drop all internal lens query item - // references. - // We are limiting the returned reference to self, - // making sure this method cannot be called multiple times without getting rid - // of any previously returned unique references first, thus preventing aliasing. - // `entity` is passed from `entity_iter` the first time. - unsafe { D::shrink(self.fetch_next_aliased_unchecked(entity)).into() } + while let Some(entity) = self.entity_iter.next() { + // SAFETY: + // We have collected the entity_iter once to drop all internal lens query item + // references. + // We are limiting the returned reference to self, + // making sure this method cannot be called multiple times without getting rid + // of any previously returned unique references first, thus preventing aliasing. + // `entity` is passed from `entity_iter` the first time. + if let Some(item) = unsafe { self.fetch_next_aliased_unchecked(entity) } { + return Some(D::shrink(item)); + } + } + None } } @@ -2061,16 +2076,19 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: DoubleEndedIterator /// Get next result from the query #[inline(always)] pub fn fetch_next_back(&mut self) -> Option> { - let entity = self.entity_iter.next_back()?; - - // SAFETY: - // We have collected the entity_iter once to drop all internal lens query item - // references. - // We are limiting the returned reference to self, - // making sure this method cannot be called multiple times without getting rid - // of any previously returned unique references first, thus preventing aliasing. - // `entity` is passed from `entity_iter` the first time. - unsafe { D::shrink(self.fetch_next_aliased_unchecked(entity)).into() } + while let Some(entity) = self.entity_iter.next_back() { + // SAFETY: + // We have collected the entity_iter once to drop all internal lens query item + // references. + // We are limiting the returned reference to self, + // making sure this method cannot be called multiple times without getting rid + // of any previously returned unique references first, thus preventing aliasing. + // `entity` is passed from `entity_iter` the first time. + if let Some(item) = unsafe { self.fetch_next_aliased_unchecked(entity) } { + return Some(D::shrink(item)); + } + } + None } } @@ -2081,15 +2099,22 @@ impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter, I: Iterator> I #[inline(always)] fn next(&mut self) -> Option { - let entity = self.entity_iter.next()?; - // SAFETY: - // It is safe to alias for ReadOnlyWorldQuery. - // `entity` is passed from `entity_iter` the first time. - unsafe { self.fetch_next_aliased_unchecked(entity).into() } + while let Some(entity) = self.entity_iter.next() { + // SAFETY: + // It is safe to alias for ReadOnlyWorldQuery. + // `entity` is passed from `entity_iter` the first time. + if let Some(item) = unsafe { self.fetch_next_aliased_unchecked(entity) } { + return Some(D::shrink(item)); + } + } + None } fn size_hint(&self) -> (usize, Option) { - self.entity_iter.size_hint() + let (min_size, max_size) = self.entity_iter.size_hint(); + let archetype_query = D::IS_ARCHETYPAL; + let min_size = if archetype_query { min_size } else { 0 }; + (min_size, max_size) } } @@ -2098,16 +2123,23 @@ impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter, I: DoubleEndedIterator Option { - let entity = self.entity_iter.next_back()?; - // SAFETY: - // It is safe to alias for ReadOnlyWorldQuery. - // `entity` is passed from `entity_iter` the first time. - unsafe { self.fetch_next_aliased_unchecked(entity).into() } + while let Some(entity) = self.entity_iter.next_back() { + // SAFETY: + // It is safe to alias for ReadOnlyWorldQuery. + // `entity` is passed from `entity_iter` the first time. + if let Some(item) = unsafe { self.fetch_next_aliased_unchecked(entity) } { + return Some(D::shrink(item)); + } + } + None } } -impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter, I: ExactSizeIterator> - ExactSizeIterator for QuerySortedManyIter<'w, 's, D, F, I> +impl< + D: ReadOnlyQueryData + ArchetypeQueryData, + F: QueryFilter, + I: ExactSizeIterator, + > ExactSizeIterator for QuerySortedManyIter<'_, '_, D, F, I> { } @@ -2327,16 +2359,15 @@ impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter, const K: usize> Iterator Some(acc + choose(n as usize, K - i)?) }); - let archetype_query = F::IS_ARCHETYPAL; + let archetype_query = D::IS_ARCHETYPAL && F::IS_ARCHETYPAL; let known_max = max_combinations.unwrap_or(usize::MAX); let min_combinations = if archetype_query { known_max } else { 0 }; (min_combinations, max_combinations) } } -impl<'w, 's, D: QueryData, F: QueryFilter> ExactSizeIterator for QueryIter<'w, 's, D, F> -where - F: ArchetypeFilter, +impl<'w, 's, D: ArchetypeQueryData, F: ArchetypeFilter> ExactSizeIterator + for QueryIter<'w, 's, D, F> { fn len(&self) -> usize { self.size_hint().0 @@ -2454,13 +2485,13 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // - `set_table` must have been called previously either in `next` or before it. // - `*entity` and `index` are in the current table. unsafe { - Some(D::fetch( + D::fetch( &query_state.fetch_state, &mut self.fetch, *entity, // SAFETY: This is from an exclusive range, so it can't be max. TableRow::new(NonMaxU32::new_unchecked(index)), - )) + ) } } else { // SAFETY: This must have been called previously in `next` as `current_row > 0` @@ -2470,12 +2501,12 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // - `set_archetype` must have been called previously either in `next` or before it. // - `archetype_entity.id()` and `archetype_entity.table_row()` are in the current archetype. unsafe { - Some(D::fetch( + D::fetch( &query_state.fetch_state, &mut self.fetch, archetype_entity.id(), archetype_entity.table_row(), - )) + ) } } } else { @@ -2485,7 +2516,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { /// How many values will this cursor return at most? /// - /// Note that if `F::IS_ARCHETYPAL`, the return value + /// Note that if `D::IS_ARCHETYPAL && F::IS_ARCHETYPAL`, the return value /// will be **the exact count of remaining values**. fn max_remaining(&self, tables: &'w Tables, archetypes: &'w Archetypes) -> u32 { let ids = self.storage_id_iter.clone(); @@ -2540,8 +2571,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { unsafe { self.table_entities.get_unchecked(self.current_row as usize) }; // SAFETY: The row is less than the u32 len, so it must not be max. let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(self.current_row)) }; + self.current_row += 1; + if !F::filter_fetch(&query_state.filter_state, &mut self.filter, *entity, row) { - self.current_row += 1; continue; } @@ -2552,9 +2584,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // - fetch is only called once for each `entity`. let item = unsafe { D::fetch(&query_state.fetch_state, &mut self.fetch, *entity, row) }; - - self.current_row += 1; - return Some(item); + if let Some(item) = item { + return Some(item); + } } } else { loop { @@ -2592,13 +2624,14 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { self.archetype_entities .get_unchecked(self.current_row as usize) }; + self.current_row += 1; + if !F::filter_fetch( &query_state.filter_state, &mut self.filter, archetype_entity.id(), archetype_entity.table_row(), ) { - self.current_row += 1; continue; } @@ -2615,8 +2648,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { archetype_entity.table_row(), ) }; - self.current_row += 1; - return Some(item); + if let Some(item) = item { + return Some(item); + } } } } diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 81f1c4ba49308..4db472908ae12 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -110,8 +110,8 @@ mod tests { component::{Component, ComponentId, Components, Tick}, prelude::{AnyOf, Changed, Entity, Or, QueryState, Resource, With, Without}, query::{ - ArchetypeFilter, FilteredAccess, Has, QueryCombinationIter, QueryData, - ReadOnlyQueryData, WorldQuery, + ArchetypeFilter, ArchetypeQueryData, FilteredAccess, Has, QueryCombinationIter, + QueryData, QueryFilter, ReadOnlyQueryData, WorldQuery, }, schedule::{IntoScheduleConfigs, Schedule}, storage::{Table, TableRow}, @@ -119,7 +119,6 @@ mod tests { world::{unsafe_world_cell::UnsafeWorldCell, World}, }; use alloc::{vec, vec::Vec}; - use bevy_ecs_macros::QueryFilter; use core::{any::type_name, fmt::Debug, hash::Hash}; use std::{collections::HashSet, println}; @@ -164,7 +163,7 @@ mod tests { } fn assert_combination(world: &mut World, expected_size: usize) where - D: ReadOnlyQueryData, + D: ReadOnlyQueryData + ArchetypeQueryData, F: ArchetypeFilter, { let mut query = world.query_filtered::(); @@ -178,7 +177,7 @@ mod tests { } fn assert_all_sizes_equal(world: &mut World, expected_size: usize) where - D: ReadOnlyQueryData, + D: ReadOnlyQueryData + ArchetypeQueryData, F: ArchetypeFilter, { let mut query = world.query_filtered::(); @@ -879,6 +878,7 @@ mod tests { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for ReadsRData { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = (); @@ -893,13 +893,16 @@ mod tests { _fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { + Some(()) } } /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for ReadsRData {} + impl ArchetypeQueryData for ReadsRData {} + #[test] fn read_res_read_res_no_conflict() { fn system(_q1: Query>, _q2: Query>) {} diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 09821a718c668..869a1831b319a 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -2085,6 +2085,32 @@ mod tests { assert_eq!(matched, 2); } + #[test] + fn transmute_to_or_filter() { + let mut world = World::new(); + world.spawn(()); + world.spawn(A(0)); + + let mut query = world + .query::>() + .transmute_filtered::,)>>(&world); + let iter = query.iter(&world); + let len = iter.len(); + let count = iter.count(); + // `transmute_filtered` keeps the same matched tables, so it should match both entities + // More importantly, `count()` and `len()` should return the same result! + assert_eq!(len, 2); + assert_eq!(count, len); + + let mut query = world + .query::>() + .transmute_filtered::,)>>(&world); + let iter = query.iter(&world); + let count = iter.count(); + // The behavior of a non-archetypal filter like `Changed` should be the same as an archetypal one like `With`. + assert_eq!(count, 2); + } + #[test] fn join() { let mut world = World::new(); diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index fb89216075afa..d238e3209d5cb 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -549,6 +549,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } /// Returns another `Query` from this does not return any data, which can be faster. + /// + /// The resulting query will ignore any non-archetypal filters in `D`, + /// so this is only equivalent if `D::IS_ARCHETYPAL` is `true`. fn as_nop(&self) -> Query<'_, 's, NopWorldQuery, F> { let new_state = self.state.as_nop(); // SAFETY: @@ -1587,13 +1590,13 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { &mut filter, entity, location.table_row, + ) && let Some(item) = D::fetch( + &self.state.fetch_state, + &mut fetch, + entity, + location.table_row, ) { - Ok(D::fetch( - &self.state.fetch_state, - &mut fetch, - entity, - location.table_row, - )) + Ok(item) } else { Err(QueryEntityError::QueryDoesNotMatch( entity, @@ -1998,7 +2001,13 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// [`Spawned`]: crate::query::Spawned #[inline] pub fn is_empty(&self) -> bool { - self.as_nop().iter().next().is_none() + // If the query data matches every entity, then `as_nop()` can safely + // skip the cost of initializing the fetch for data that won't be used. + if D::IS_ARCHETYPAL { + self.as_nop().iter().next().is_none() + } else { + self.iter().next().is_none() + } } /// Returns `true` if the given [`Entity`] matches the query. @@ -2027,14 +2036,20 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// ``` #[inline] pub fn contains(&self, entity: Entity) -> bool { - self.as_nop().get(entity).is_ok() + // If the query data matches every entity, then `as_nop()` can safely + // skip the cost of initializing the fetch for data that won't be used. + if D::IS_ARCHETYPAL { + self.as_nop().get(entity).is_ok() + } else { + self.get(entity).is_ok() + } } /// Counts the number of entities that match the query. /// /// This is equivalent to `self.iter().count()` but may be more efficient in some cases. /// - /// If [`F::IS_ARCHETYPAL`](QueryFilter::IS_ARCHETYPAL) is `true`, + /// If [`D::IS_ARCHETYPAL`](QueryData::IS_ARCHETYPAL) && [`F::IS_ARCHETYPAL`](QueryFilter::IS_ARCHETYPAL) is `true`, /// this will do work proportional to the number of matched archetypes or tables, but will not iterate each entity. /// If it is `false`, it will have to do work for each entity. /// @@ -2053,14 +2068,17 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// # bevy_ecs::system::assert_is_system(targeting_system); /// ``` pub fn count(&self) -> usize { - let iter = self.as_nop().into_iter(); - if F::IS_ARCHETYPAL { + // If the query data matches every entity, then `as_nop()` can safely + // skip the cost of initializing the fetch for data that won't be used. + if !D::IS_ARCHETYPAL { + self.into_iter().count() + } else if !F::IS_ARCHETYPAL { + // If we have non-archetypal filters, we have to check each entity. + self.as_nop().into_iter().count() + } else { // For archetypal queries, the `size_hint()` is exact, // and we can get the count from the archetype and table counts. - iter.size_hint().0 - } else { - // If we have non-archetypal filters, we have to check each entity. - iter.count() + self.as_nop().into_iter().size_hint().0 } } diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 16b7a863b4442..7bfe42fb390b4 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -1032,7 +1032,7 @@ impl<'w> UnsafeEntityCell<'w> { unsafe { Q::set_archetype(&mut fetch, &state, archetype, table) } // SAFETY: Called after set_archetype above. Entity and location are guaranteed to exist. let item = unsafe { Q::fetch(&state, &mut fetch, self.id(), location.table_row) }; - Some(Q::release_state(item)) + item.map(Q::release_state) } else { None } diff --git a/crates/bevy_render/src/sync_world.rs b/crates/bevy_render/src/sync_world.rs index ba3d6d860f60d..83637cb3da7f9 100644 --- a/crates/bevy_render/src/sync_world.rs +++ b/crates/bevy_render/src/sync_world.rs @@ -280,7 +280,10 @@ mod render_entities_world_query_impls { archetype::Archetype, component::{ComponentId, Components, Tick}, entity::Entity, - query::{FilteredAccess, QueryData, ReadOnlyQueryData, ReleaseStateQueryData, WorldQuery}, + query::{ + ArchetypeQueryData, FilteredAccess, QueryData, ReadOnlyQueryData, + ReleaseStateQueryData, WorldQuery, + }, storage::{Table, TableRow}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; @@ -359,6 +362,7 @@ mod render_entities_world_query_impls { // Self::ReadOnly matches exactly the same archetypes/tables as Self. unsafe impl QueryData for RenderEntity { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = <&MainEntity as QueryData>::IS_ARCHETYPAL; type ReadOnly = RenderEntity; type Item<'w, 's> = Entity; @@ -374,17 +378,19 @@ mod render_entities_world_query_impls { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { // SAFETY: defers to the `&T` implementation, with T set to `RenderEntity`. let component = unsafe { <&RenderEntity as QueryData>::fetch(state, fetch, entity, table_row) }; - component.id() + component.map(RenderEntity::id) } } // SAFETY: the underlying `Entity` is copied, and no mutable access is provided. unsafe impl ReadOnlyQueryData for RenderEntity {} + impl ArchetypeQueryData for RenderEntity {} + impl ReleaseStateQueryData for RenderEntity { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -465,6 +471,7 @@ mod render_entities_world_query_impls { // Self::ReadOnly matches exactly the same archetypes/tables as Self. unsafe impl QueryData for MainEntity { const IS_READ_ONLY: bool = true; + const IS_ARCHETYPAL: bool = <&MainEntity as QueryData>::IS_ARCHETYPAL; type ReadOnly = MainEntity; type Item<'w, 's> = Entity; @@ -480,17 +487,19 @@ mod render_entities_world_query_impls { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Self::Item<'w, 's> { + ) -> Option> { // SAFETY: defers to the `&T` implementation, with T set to `MainEntity`. let component = unsafe { <&MainEntity as QueryData>::fetch(state, fetch, entity, table_row) }; - component.id() + component.map(MainEntity::id) } } // SAFETY: the underlying `Entity` is copied, and no mutable access is provided. unsafe impl ReadOnlyQueryData for MainEntity {} + impl ArchetypeQueryData for MainEntity {} + impl ReleaseStateQueryData for MainEntity { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item diff --git a/release-content/migration-guides/archetype_query_data.md b/release-content/migration-guides/archetype_query_data.md new file mode 100644 index 0000000000000..34967124901bb --- /dev/null +++ b/release-content/migration-guides/archetype_query_data.md @@ -0,0 +1,27 @@ +--- +title: "`ArchetypeQueryData` trait" +pull_requests: [21581] +--- + +To support richer querying across relations, +Bevy now supports query data that are not archetypal: the query can return entities based on conditions that do not exclusively involve the entity's archetype. + +An example of non-archetypal filter is `Changed`: the entity is filtered based on the archetype (having the component C) but also based on the change ticks of the component. + +Code that requires queries to `impl ExactSizeIterator` may need to replace `QueryData` bounds with `ArchetypeQueryData`. + +```rust +// 0.17 +fn requires_exact_size(q: Query) -> usize { + q.into_iter().len() +} +// 0.18 +fn requires_exact_size(q: Query) -> usize { + q.into_iter().len() +} +``` + +Manual implementations of `QueryData` will now need to provide the `IS_ARCHETYPAL` associated constant. +This will be `true` for most existing queries, +although queries that wrap other queries should delegate as appropriate. +In addition, queries with `IS_ARCHETYPAL = true` should implement `ArchetypeQueryData`.