diff --git a/crates/bevy_ecs/macros/src/query_data.rs b/crates/bevy_ecs/macros/src/query_data.rs index 1e9dea94bbca3..935e627cbe82c 100644 --- a/crates/bevy_ecs/macros/src/query_data.rs +++ b/crates/bevy_ecs/macros/src/query_data.rs @@ -269,6 +269,17 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { } } + // SAFETY: Access is read-only + unsafe impl #user_impl_generics #path::query::IterQueryData + for #read_only_struct_name #user_ty_generics #user_where_clauses {} + + // SAFETY: All fields only access the current entity + unsafe impl #user_impl_generics #path::query::SingleEntityQueryData + 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::QueryData,)* {} + impl #user_impl_generics #path::query::ReleaseStateQueryData for #read_only_struct_name #user_ty_generics #user_where_clauses // Make these HRTBs with an unused lifetime parameter to allow trivial constraints @@ -334,6 +345,20 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { } } + // SAFETY: All fields are iterable + unsafe impl #user_impl_generics #path::query::IterQueryData + 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::IterQueryData,)* {} + + // SAFETY: All fields only access the current entity + unsafe impl #user_impl_generics #path::query::SingleEntityQueryData + 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::SingleEntityQueryData,)* {} + impl #user_impl_generics #path::query::ReleaseStateQueryData for #struct_name #user_ty_generics #user_where_clauses // Make these HRTBs with an unused lifetime parameter to allow trivial constraints diff --git a/crates/bevy_ecs/macros/src/world_query.rs b/crates/bevy_ecs/macros/src/world_query.rs index 39feecf3798b8..048a3333aec83 100644 --- a/crates/bevy_ecs/macros/src/world_query.rs +++ b/crates/bevy_ecs/macros/src/world_query.rs @@ -156,6 +156,15 @@ pub(crate) fn world_query_impl( #( <#field_types>::update_component_access(&state.#field_aliases, _access); )* } + fn init_nested_access( + state: &Self::State, + _system_name: Option<&str>, + _component_access_set: &mut #path::query::FilteredAccessSet, + _world: #path::world::unsafe_world_cell::UnsafeWorldCell, + ) { + #( <#field_types>::init_nested_access(&state.#field_aliases, _system_name, _component_access_set, _world); )* + } + fn init_state(world: &mut #path::world::World) -> #state_struct_name #user_ty_generics { #state_struct_name { #(#field_aliases: <#field_types>::init_state(world),)* @@ -171,6 +180,10 @@ pub(crate) fn world_query_impl( fn matches_component_set(state: &Self::State, _set_contains_id: &impl Fn(#path::component::ComponentId) -> bool) -> bool { true #(&& <#field_types>::matches_component_set(&state.#field_aliases, _set_contains_id))* } + + fn update_archetypes(_state: &mut Self::State, _world: #path::world::unsafe_world_cell::UnsafeWorldCell) { + #(<#field_types>::update_archetypes(&mut _state.#field_aliases, _world);)* + } } } } diff --git a/crates/bevy_ecs/src/query/access.rs b/crates/bevy_ecs/src/query/access.rs index 6a0e77a632c03..6fe97c0cf8543 100644 --- a/crates/bevy_ecs/src/query/access.rs +++ b/crates/bevy_ecs/src/query/access.rs @@ -1,5 +1,5 @@ use crate::component::ComponentId; -use crate::world::World; +use crate::world::unsafe_world_cell::UnsafeWorldCell; use alloc::{format, string::String, vec, vec::Vec}; use core::{fmt, fmt::Debug}; use derive_more::From; @@ -991,7 +991,7 @@ impl AccessConflicts { } } - pub(crate) fn format_conflict_list(&self, world: &World) -> String { + pub(crate) fn format_conflict_list(&self, world: UnsafeWorldCell) -> String { match self { AccessConflicts::All => String::new(), AccessConflicts::Individual(indices) => indices @@ -1000,7 +1000,7 @@ impl AccessConflicts { format!( "{}", world - .components + .components() .get_name(ComponentId::new(index)) .unwrap() .shortname() diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 06fc17727e6fe..3cf8a17b41507 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -4,8 +4,12 @@ use crate::{ change_detection::{ComponentTicksMut, ComponentTicksRef, MaybeLocation, Tick}, component::{Component, ComponentId, Components, Mutable, StorageType}, entity::{Entities, Entity, EntityLocation}, - query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery}, + query::{ + Access, DebugCheckedUnwrap, FilteredAccess, FilteredAccessSet, QueryFilter, QueryState, + WorldQuery, + }, storage::{ComponentSparseSet, Table, TableRow}, + system::Query, world::{ unsafe_world_cell::UnsafeWorldCell, EntityMut, EntityMutExcept, EntityRef, EntityRefExcept, FilteredEntityMut, FilteredEntityRef, Mut, Ref, World, @@ -273,7 +277,6 @@ use variadics_please::all_tuples; /// and `Self::ReadOnly` must match exactly the same archetypes/tables as `Self` /// - `IS_READ_ONLY` must be `true` if and only if `Self: ReadOnlyQueryData` /// -/// [`Query`]: crate::system::Query /// [`ReadOnly`]: Self::ReadOnly #[diagnostic::on_unimplemented( message = "`{Self}` is not valid to request as data in a `Query`", @@ -336,6 +339,8 @@ pub unsafe trait QueryData: WorldQuery { /// - Must always be called _after_ [`WorldQuery::set_table`] or [`WorldQuery::set_archetype`]. `entity` and /// `table_row` must be in the range of the current table and archetype. /// - There must not be simultaneous conflicting component access registered in `update_component_access`. + /// - If `Self` does not impl `ReadOnlyQueryData`, then there must not be any other `Item`s alive for the current entity + /// - If `Self` does not impl `IterQueryData`, then there must not be any other `Item`s alive for *any* entity unsafe fn fetch<'w, 's>( state: &'s Self::State, fetch: &mut Self::Fetch<'w>, @@ -344,19 +349,47 @@ pub unsafe trait QueryData: WorldQuery { ) -> Option>; } +/// A [`QueryData`] for which instances may be alive for different entities concurrently. +/// +/// Rust [`Iterator`]s don't connect the lifetime in [`Iterator::next`] to anything in [`Iterator::Item`], +/// so later calls don't invalidate earlier items. +/// This is how methods like [`Iterator::collect`] work. +/// It is therefore unsound to offer an [`Iterator`] for a [`QueryData`] for which only one instance may be alive concurrently. +/// +/// All [`SingleEntityQueryData`] types are [`IterQueryData`], +/// because queries on different entities would not alias. +/// +/// All [`ReadOnlyQueryData`] types are [`IterQueryData`], +/// because it's sound for read-only queries to alias. +/// +/// Queries with a nested query that performs mutable access should generally *not* be [`IterQueryData`], +/// although they can be if they have a way to prove that all accesses through the nested query are disjoint. +/// +/// # Safety +/// +/// This [`QueryData`] must not perform conflicting access when fetched for different entities. +pub unsafe trait IterQueryData: QueryData {} + /// A [`QueryData`] that is read only. /// /// # Safety /// /// This must only be implemented for read-only [`QueryData`]'s. -pub unsafe trait ReadOnlyQueryData: QueryData {} +pub unsafe trait ReadOnlyQueryData: IterQueryData {} + +/// A [`QueryData`] that only accesses data from the current entity. +/// +/// # Safety +/// +/// This [`QueryData`] must only access data from the current entity, and not any other entities. +pub unsafe trait SingleEntityQueryData: IterQueryData {} /// The item type returned when a [`WorldQuery`] is iterated over pub type QueryItem<'w, 's, Q> = ::Item<'w, 's>; /// The read-only variant of the item type returned when a [`QueryData`] is iterated over immutably pub type ROQueryItem<'w, 's, D> = QueryItem<'w, 's, ::ReadOnly>; -/// A [`QueryData`] that does not borrow from its [`QueryState`](crate::query::QueryState). +/// A [`QueryData`] that does not borrow from its [`QueryState`]. /// /// This is implemented by most `QueryData` types. /// The main exceptions are [`FilteredEntityRef`], [`FilteredEntityMut`], [`EntityRefExcept`], and [`EntityMutExcept`], @@ -452,9 +485,15 @@ unsafe impl QueryData for Entity { } } +/// SAFETY: access is read only and only on the current entity +unsafe impl IterQueryData for Entity {} + /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for Entity {} +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for Entity {} + impl ReleaseStateQueryData for Entity { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -545,9 +584,15 @@ unsafe impl QueryData for EntityLocation { } } +/// SAFETY: access is read only and only on the current entity +unsafe impl IterQueryData for EntityLocation {} + /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for EntityLocation {} +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for EntityLocation {} + impl ReleaseStateQueryData for EntityLocation { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -721,9 +766,15 @@ unsafe impl QueryData for SpawnDetails { } } +/// SAFETY: access is read only and only on the current entity +unsafe impl IterQueryData for SpawnDetails {} + /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for SpawnDetails {} +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for SpawnDetails {} + impl ReleaseStateQueryData for SpawnDetails { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -840,9 +891,15 @@ unsafe impl<'a> QueryData for EntityRef<'a> { } } +/// SAFETY: access is read only and only on the current entity +unsafe impl IterQueryData for EntityRef<'_> {} + /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for EntityRef<'_> {} +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for EntityRef<'_> {} + impl ReleaseStateQueryData for EntityRef<'_> { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -946,6 +1003,12 @@ unsafe impl<'a> QueryData for EntityMut<'a> { } } +/// SAFETY: access is only on the current entity +unsafe impl IterQueryData for EntityMut<'_> {} + +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for EntityMut<'_> {} + impl ReleaseStateQueryData for EntityMut<'_> { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -1070,9 +1133,15 @@ unsafe impl QueryData for FilteredEntityRef<'_, '_> { } } -/// SAFETY: Access is read-only. +/// SAFETY: access is read only and only on the current entity +unsafe impl IterQueryData for FilteredEntityRef<'_, '_> {} + +/// SAFETY: access is read only unsafe impl ReadOnlyQueryData for FilteredEntityRef<'_, '_> {} +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for FilteredEntityRef<'_, '_> {} + impl ArchetypeQueryData for FilteredEntityRef<'_, '_> {} /// SAFETY: The accesses of `Self::ReadOnly` are a subset of the accesses of `Self` @@ -1189,6 +1258,12 @@ unsafe impl<'a, 'b> QueryData for FilteredEntityMut<'a, 'b> { } } +/// SAFETY: access is only on the current entity +unsafe impl IterQueryData for FilteredEntityMut<'_, '_> {} + +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for FilteredEntityMut<'_, '_> {} + impl ArchetypeQueryData for FilteredEntityMut<'_, '_> {} /// SAFETY: `EntityRefExcept` guards access to all components in the bundle `B` @@ -1301,10 +1376,15 @@ where } } -/// SAFETY: `EntityRefExcept` enforces read-only access to its contained -/// components. +/// SAFETY: access is read only and only on the current entity +unsafe impl IterQueryData for EntityRefExcept<'_, '_, B> where B: Bundle {} + +/// SAFETY: access is read only unsafe impl ReadOnlyQueryData for EntityRefExcept<'_, '_, B> where B: Bundle {} +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for EntityRefExcept<'_, '_, B> where B: Bundle {} + impl ArchetypeQueryData for EntityRefExcept<'_, '_, B> {} /// SAFETY: `EntityMutExcept` guards access to all components in the bundle `B` @@ -1418,6 +1498,12 @@ where } } +/// SAFETY: access is only on the current entity +unsafe impl IterQueryData for EntityMutExcept<'_, '_, B> where B: Bundle {} + +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for EntityMutExcept<'_, '_, B> where B: Bundle {} + impl ArchetypeQueryData for EntityMutExcept<'_, '_, B> {} /// SAFETY: @@ -1505,9 +1591,15 @@ unsafe impl QueryData for &Archetype { } } +/// SAFETY: access is read only and only on the current entity +unsafe impl IterQueryData for &Archetype {} + /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for &Archetype {} +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for &Archetype {} + impl ReleaseStateQueryData for &Archetype { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -1674,9 +1766,15 @@ unsafe impl QueryData for &T { } } +/// SAFETY: access is read only and only on the current entity +unsafe impl IterQueryData for &T {} + /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for &T {} +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for &T {} + impl ReleaseStateQueryData for &T { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -1886,9 +1984,15 @@ unsafe impl<'__w, T: Component> QueryData for Ref<'__w, T> { } } +/// SAFETY: access is read only and only on the current entity +unsafe impl<'__w, T: Component> IterQueryData for Ref<'__w, T> {} + /// SAFETY: access is read only unsafe impl<'__w, T: Component> ReadOnlyQueryData for Ref<'__w, T> {} +/// SAFETY: access is only on the current entity +unsafe impl<'__w, T: Component> SingleEntityQueryData for Ref<'__w, T> {} + impl ReleaseStateQueryData for Ref<'_, T> { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -2098,6 +2202,12 @@ unsafe impl<'__w, T: Component> QueryData for &'__w mut T } } +/// SAFETY: access is only on the current entity +unsafe impl> IterQueryData for &mut T {} + +/// SAFETY: access is only on the current entity +unsafe impl> SingleEntityQueryData for &mut T {} + impl> ReleaseStateQueryData for &mut T { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -2213,6 +2323,12 @@ unsafe impl<'__w, T: Component> QueryData for Mut<'__w, T> } } +/// SAFETY: access is only on the current entity +unsafe impl> IterQueryData for Mut<'_, T> {} + +/// SAFETY: access is only on the current entity +unsafe impl> SingleEntityQueryData for Mut<'_, T> {} + impl> ReleaseStateQueryData for Mut<'_, T> { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -2221,6 +2337,234 @@ impl> ReleaseStateQueryData for Mut<'_, T> { impl> ArchetypeQueryData for Mut<'_, T> {} +/// A helper type for accessing a [`Query`] within a [`QueryData`]. +/// +/// This is intended to be used inside other implementations of [`QueryData`], +/// either for manual implementations or `#[derive(QueryData)]`. +/// It is not normally useful to query directly, +/// since it's equivalent to adding another [`Query`] parameter to a system. +/// +/// Note that this is only an [`IterQueryData`] if the underlying query data is [`ReadOnlyQueryData`], +/// to prevent mutable aliasing. +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// # use bevy_ecs::query::NestedQuery; +/// # +/// # #[derive(Component)] +/// # struct A; +/// fn system(mut query: Query>, entity: Entity) { +/// // This works, because it performs read-only iteration +/// for a in &query { +/// let a: Query<&A> = a; +/// } +/// // And this works, because it can only be called for one entity at a time +/// let a: Query<&mut A> = query.get_mut(entity).unwrap(); +/// } +/// ``` +/// +/// ```compile_fail +/// # use bevy_ecs::prelude::*; +/// # use bevy_ecs::query::NestedQuery; +/// # +/// # #[derive(Component)] +/// # struct A; +/// fn system(mut query: Query>) { +/// // This fails, because it would allow mutable aliasing of `&mut A` +/// for a in &mut query { +/// // ^^^^^^^^^^ `&mut bevy_ecs::system::Query<'_, '_, NestedQuery<&mut A>>` is not an iterator +/// let a: Query<&mut A> = a; +/// } +/// } +/// ``` +/// +/// # Example +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// # use bevy_ecs::query::{QueryData, NestedQuery}; +/// # +/// #[derive(Component)] +/// struct A(Entity); +/// +/// #[derive(Component)] +/// struct Name(String); +/// +/// #[derive(QueryData)] +/// struct NameFromA { +/// a: &'static A, +/// query: NestedQuery<&'static Name>, +/// } +/// +/// impl<'w, 's> NameFromAItem<'w, 's> { +/// fn name(&self) -> Option<&str> { +/// self.query.get(self.a.0).ok().map(|name| &*name.0) +/// } +/// } +/// +/// fn system(query: Query) { +/// for item in query { +/// let name: Option<&str> = item.name(); +/// } +/// } +/// ``` +pub struct NestedQuery( + PhantomData>, +); + +#[doc(hidden)] +#[derive(Clone)] +pub struct NestedQueryFetch<'w> { + world: UnsafeWorldCell<'w>, + last_run: Tick, + this_run: Tick, +} + +// SAFETY: +// Does not access any components on the current entity +// Accesses through the nested query are registered in `init_nested_access` +unsafe impl WorldQuery for NestedQuery { + type Fetch<'w> = NestedQueryFetch<'w>; + // Ideally this would be `QueryState`, + // but `QueryData` requires `Self::State == Self::ReadOnly::State`, + // and `QueryState != QueryState`. + // `QueryState` does support *transmuting* between those types, though, + // so we convert the state to `QueryState` and transmute it back to use it. + type State = QueryState; + + fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { + fetch + } + + #[inline] + unsafe fn init_fetch<'w, 's>( + world: UnsafeWorldCell<'w>, + _state: &'s Self::State, + last_run: Tick, + this_run: Tick, + ) -> Self::Fetch<'w> { + NestedQueryFetch { + world, + last_run, + this_run, + } + } + + const IS_DENSE: bool = true; + + #[inline] + unsafe fn set_archetype<'w>( + _fetch: &mut Self::Fetch<'w>, + _state: &Self::State, + _archetype: &'w Archetype, + _table: &'w Table, + ) { + } + + #[inline] + unsafe fn set_table<'w>(_fetch: &mut Self::Fetch<'w>, _state: &Self::State, _table: &'w Table) { + } + + fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) {} + + fn init_nested_access( + state: &Self::State, + system_name: Option<&str>, + component_access_set: &mut FilteredAccessSet, + world: UnsafeWorldCell, + ) { + state.init_access(system_name, component_access_set, world); + } + + fn init_state(world: &mut World) -> Self::State { + // SAFETY: `WorldQuery::init_nested_access` calls `QueryState::init_access`, + // `WorldQuery::init_nested_access` must be called before `WorldQuery::init_fetch, + // which must be called before `QueryData::fetch`, + // and we only call methods on the `QueryState` in `fetch`. + unsafe { QueryState::::new_unchecked(world).into_readonly() } + } + + fn get_state(_components: &Components) -> Option { + // This is not currently possible. + // `QueryState::new` requires read access to the `DefaultQueryFilters` resource, + // but this method may be called during `transmute` or `join` + // when we have no such access. + None + } + + fn matches_component_set( + _state: &Self::State, + _set_contains_id: &impl Fn(ComponentId) -> bool, + ) -> bool { + true + } + + fn update_archetypes(state: &mut Self::State, world: UnsafeWorldCell) { + state.update_archetypes_unsafe_world_cell(world); + } +} + +// SAFETY: +// `Self::ReadOnly` accesses `D::ReadOnly`, which is a subset of the data accessed by `D` +// `IS_READ_ONLY` iff `D::IS_READ_ONLY` iff `D: ReadOnlyQueryData` iff `Self: ReadOnlyQueryData` +unsafe impl QueryData for NestedQuery { + const IS_READ_ONLY: bool = D::IS_READ_ONLY; + // Nested queries are always archetypal because `fetch` always returns `Some`. + // If `D::IS_ARCHETYPAL == false` or `F::IS_ARCHETYPAL == false`, + // then the nested query may filter out some entities that *it* matches, + // but it will not filter the outer query. + const IS_ARCHETYPAL: bool = true; + type ReadOnly = NestedQuery; + type Item<'w, 's> = Query<'w, 's, D, F>; + + fn shrink<'wlong: 'wshort, 'wshort, 's>( + item: Self::Item<'wlong, 's>, + ) -> Self::Item<'wshort, 's> { + item + } + + #[inline(always)] + unsafe fn fetch<'w, 's>( + state: &'s Self::State, + fetch: &mut Self::Fetch<'w>, + _entity: Entity, + _table_row: TableRow, + ) -> Option> { + // SAFETY: The state was either created from `D`, or it was created from + // another `NestedQuery` and converted to `NestedQuery`, + // in which case `D::ReadOnly == D`. + let state = unsafe { QueryState::from_readonly(state) }; + // SAFETY: + // - We registered the required access in `init_nested_access`, so it's available. + // - If we are fetching multiple entities concurrently, + // then `Self: IterQueryData`, so `D: ReadOnlyQueryData`, + // so it's safe to alias the queries. + unsafe { + Some(state.query_unchecked_manual_with_ticks( + fetch.world, + fetch.last_run, + fetch.this_run, + )) + } + } +} + +// SAFETY: All access is through `D`, which is read-only +// The nested query does not access the entity from the outer query, +// but the nested access also needs to be read-only because we can copy it. +unsafe impl ReadOnlyQueryData for NestedQuery {} + +// SAFETY: All access to other entities is through `D`, which is read-only and does not conflict. +// Note that we must not impl IterQueryData for queries with mutable access, +// since the nested query must only be live for one entity at a time. +unsafe impl IterQueryData for NestedQuery {} + +// Nested queries are always archetypal because `fetch` always returns `Some`. +// If `D::IS_ARCHETYPAL == false` or `F::IS_ARCHETYPAL == false`, +// then the nested query may filter out some entities that *it* matches, +// but it will never filter the outer query. +impl ArchetypeQueryData for NestedQuery {} + #[doc(hidden)] pub struct OptionFetch<'w, T: WorldQuery> { fetch: T::Fetch<'w>, @@ -2313,6 +2657,15 @@ unsafe impl WorldQuery for Option { access.extend_access(&intermediate); } + fn init_nested_access( + state: &Self::State, + system_name: Option<&str>, + component_access_set: &mut FilteredAccessSet, + world: UnsafeWorldCell, + ) { + T::init_nested_access(state, system_name, component_access_set, world); + } + fn init_state(world: &mut World) -> T::State { T::init_state(world) } @@ -2327,6 +2680,10 @@ unsafe impl WorldQuery for Option { ) -> bool { true } + + fn update_archetypes(state: &mut Self::State, world: UnsafeWorldCell) { + T::update_archetypes(state, world); + } } // SAFETY: defers to soundness of `T: WorldQuery` impl @@ -2361,9 +2718,15 @@ unsafe impl QueryData for Option { } } -/// SAFETY: [`OptionFetch`] is read only because `T` is read only +/// SAFETY: `Option` is iterable because `T` is iterable +unsafe impl IterQueryData for Option {} + +/// SAFETY: `Option` is read only because `T` is read only unsafe impl ReadOnlyQueryData for Option {} +/// SAFETY: `Option` only accesses the current entity because `T` only accesses the current entity +unsafe impl SingleEntityQueryData for Option {} + impl ReleaseStateQueryData for Option { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item.map(T::release_state) @@ -2376,7 +2739,7 @@ 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 +/// This can be used in a [`Query`] if you want to know whether or not entities /// have the component `T` but don't actually care about the component's value. /// /// # Footguns @@ -2537,9 +2900,15 @@ unsafe impl QueryData for Has { } } -/// SAFETY: [`Has`] is read only +/// SAFETY: access is read only and only on the current entity +unsafe impl IterQueryData for Has {} + +/// SAFETY: access is read only unsafe impl ReadOnlyQueryData for Has {} +/// SAFETY: access is only on the current entity +unsafe impl SingleEntityQueryData for Has {} + impl ReleaseStateQueryData for Has { fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> { item @@ -2612,10 +2981,18 @@ macro_rules! impl_tuple_query_data { } } + $(#[$meta])* + /// SAFETY: each item in the tuple is iterable + unsafe impl<$($name: IterQueryData),*> IterQueryData for ($($name,)*) {} + $(#[$meta])* /// SAFETY: each item in the tuple is read only unsafe impl<$($name: ReadOnlyQueryData),*> ReadOnlyQueryData for ($($name,)*) {} + $(#[$meta])* + /// SAFETY: each item in the tuple only accesses the current entity + unsafe impl<$($name: SingleEntityQueryData),*> SingleEntityQueryData for ($($name,)*) {} + #[expect( clippy::allow_attributes, reason = "This is a tuple-related macro; as such the lints below may not always apply." @@ -2737,6 +3114,16 @@ macro_rules! impl_anytuple_fetch { <($(Option<$name>,)*)>::update_component_access(state, access); } + + fn init_nested_access( + state: &Self::State, + system_name: Option<&str>, + component_access_set: &mut FilteredAccessSet, + world: UnsafeWorldCell, + ) { + <($(Option<$name>,)*)>::init_nested_access(state, system_name, component_access_set, world); + } + fn init_state(world: &mut World) -> Self::State { ($($name::init_state(world),)*) } @@ -2748,6 +3135,10 @@ macro_rules! impl_anytuple_fetch { let ($($name,)*) = _state; false $(|| $name::matches_component_set($name, _set_contains_id))* } + + fn update_archetypes(state: &mut Self::State, world: UnsafeWorldCell) { + <($(Option<$name>,)*)>::update_archetypes(state, world); + } } #[expect( @@ -2809,10 +3200,18 @@ macro_rules! impl_anytuple_fetch { } } + $(#[$meta])* + /// SAFETY: each item in the tuple is iterable + unsafe impl<$($name: IterQueryData),*> IterQueryData for AnyOf<($($name,)*)> {} + $(#[$meta])* /// SAFETY: each item in the tuple is read only unsafe impl<$($name: ReadOnlyQueryData),*> ReadOnlyQueryData for AnyOf<($($name,)*)> {} + $(#[$meta])* + /// SAFETY: each item in the tuple only accesses the current entity + unsafe impl<$($name: SingleEntityQueryData),*> SingleEntityQueryData for AnyOf<($($name,)*)> {} + #[expect( clippy::allow_attributes, reason = "This is a tuple-related macro; as such the lints below may not always apply." @@ -2905,6 +3304,10 @@ unsafe impl WorldQuery for NopWorldQuery { ) -> bool { D::matches_component_set(state, set_contains_id) } + + fn update_archetypes(state: &mut Self::State, world: UnsafeWorldCell) { + D::update_archetypes(state, world); + } } /// SAFETY: `Self::ReadOnly` is `Self` @@ -2930,9 +3333,15 @@ unsafe impl QueryData for NopWorldQuery { } } +/// SAFETY: `NopFetch` never accesses any data +unsafe impl IterQueryData for NopWorldQuery {} + /// SAFETY: `NopFetch` never accesses any data unsafe impl ReadOnlyQueryData for NopWorldQuery {} +/// SAFETY: `NopFetch` never accesses any data +unsafe impl SingleEntityQueryData for NopWorldQuery {} + impl ReleaseStateQueryData for NopWorldQuery { fn release_state<'w>(_item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> {} } @@ -3015,9 +3424,15 @@ unsafe impl QueryData for PhantomData { } } -/// SAFETY: `PhantomData` never accesses any world data. +/// SAFETY: `PhantomData` never accesses any data +unsafe impl IterQueryData for PhantomData {} + +/// SAFETY: `PhantomData` never accesses any data unsafe impl ReadOnlyQueryData for PhantomData {} +/// SAFETY: `PhantomData` never accesses any data +unsafe impl SingleEntityQueryData for PhantomData {} + impl ReleaseStateQueryData for PhantomData { fn release_state<'w>(_item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> {} } @@ -3226,6 +3641,9 @@ mod tests { /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for NonReleaseQueryData {} + /// SAFETY: access is read only + unsafe impl IterQueryData for NonReleaseQueryData {} + impl ArchetypeQueryData for NonReleaseQueryData {} #[derive(QueryData)] diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index d0261fae976a4..55080f6d97462 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -3,7 +3,7 @@ use crate::{ change_detection::Tick, component::{Component, ComponentId, Components, StorageType}, entity::{Entities, Entity}, - query::{DebugCheckedUnwrap, FilteredAccess, StorageSwitch, WorldQuery}, + query::{DebugCheckedUnwrap, FilteredAccess, FilteredAccessSet, StorageSwitch, WorldQuery}, storage::{ComponentSparseSet, Table, TableRow}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; @@ -477,6 +477,16 @@ macro_rules! impl_or_query_filter { *access = new_access; } + fn init_nested_access( + state: &Self::State, + _system_name: Option<&str>, + _component_access_set: &mut FilteredAccessSet, + _world: UnsafeWorldCell, + ) { + let ($($state,)*) = state; + $($filter::init_nested_access($state, _system_name, _component_access_set, _world);)* + } + fn init_state(world: &mut World) -> Self::State { ($($filter::init_state(world),)*) } @@ -489,6 +499,11 @@ macro_rules! impl_or_query_filter { let ($($filter,)*) = state; false $(|| $filter::matches_component_set($filter, set_contains_id))* } + + fn update_archetypes(state: &mut Self::State, _world: UnsafeWorldCell) { + let ($($filter,)*) = state; + $($filter::update_archetypes($filter, _world);)* + } } #[expect( diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 40b33a220d017..8587e3e0737a2 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -4,7 +4,10 @@ use crate::{ bundle::Bundle, change_detection::Tick, entity::{ContainsEntity, Entities, Entity, EntityEquivalent, EntitySet, EntitySetIterator}, - query::{ArchetypeFilter, ArchetypeQueryData, DebugCheckedUnwrap, QueryState, StorageId}, + query::{ + ArchetypeFilter, ArchetypeQueryData, DebugCheckedUnwrap, IterQueryData, QueryState, + SingleEntityQueryData, StorageId, + }, storage::{Table, TableRow, Tables}, world::{ unsafe_world_cell::UnsafeWorldCell, EntityMut, EntityMutExcept, EntityRef, EntityRefExcept, @@ -125,7 +128,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { cursor: self.cursor.reborrow(), } } +} +impl<'w, 's, D: IterQueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// Executes the equivalent of [`Iterator::fold`] over a contiguous segment /// from a storage. /// @@ -237,8 +242,11 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { continue; } - // SAFETY: set_table was called prior. - // Caller assures `row` in range of the current archetype. + // SAFETY: + // - set_table was called prior. + // - Caller assures `row` in range of the current archetype. + // - Each row is unique, so each entity is only alive once + // - `D: IterQueryData` if let Some(item) = D::fetch( &self.query_state.fetch_state, &mut self.cursor.fetch, @@ -305,8 +313,11 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { continue; } - // 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. + // 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. + // - Each row is unique, so each entity is only alive once + // - `D: IterQueryData` if let Some(item) = unsafe { D::fetch( &self.query_state.fetch_state, @@ -382,8 +393,11 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { continue; } - // SAFETY: set_table was called prior. - // Caller assures `row` in range of the current archetype. + // SAFETY: + // - set_table was called prior. + // - Caller assures `row` in range of the current archetype. + // - Each row is unique, so each entity is only alive once + // - `D: IterQueryData` if let Some(item) = D::fetch( &self.query_state.fetch_state, &mut self.cursor.fetch, @@ -395,7 +409,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { } accum } +} +impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// Sorts all query items into a new iterator, using the query lens as a key. /// /// This sort is stable (i.e., does not reorder equal elements). @@ -406,6 +422,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -508,7 +528,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// # schedule.add_systems((system_1, system_2, system_3)); /// # schedule.run(&mut world); /// ``` - pub fn sort( + pub fn sort( self, ) -> QuerySortedIter< 'w, @@ -533,6 +553,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// This includes the allowed parameter type changes listed under [allowed transmutes].. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -565,7 +589,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// # schedule.add_systems((system_1)); /// # schedule.run(&mut world); /// ``` - pub fn sort_unstable( + pub fn sort_unstable( self, ) -> QuerySortedIter< 'w, @@ -590,6 +614,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -629,7 +657,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// # schedule.add_systems((system_1)); /// # schedule.run(&mut world); /// ``` - pub fn sort_by( + pub fn sort_by( self, mut compare: impl FnMut(&L::Item<'_, '_>, &L::Item<'_, '_>) -> Ordering, ) -> QuerySortedIter< @@ -654,6 +682,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -661,7 +693,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// # Panics /// /// This will panic if `next` has been called on `QueryIter` before, unless the underlying `Query` is empty. - pub fn sort_unstable_by( + pub fn sort_unstable_by( self, mut compare: impl FnMut(&L::Item<'_, '_>, &L::Item<'_, '_>) -> Ordering, ) -> QuerySortedIter< @@ -686,6 +718,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -753,7 +789,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// # schedule.add_systems((system_1, system_2)); /// # schedule.run(&mut world); /// ``` - pub fn sort_by_key( + pub fn sort_by_key( self, mut f: impl FnMut(&L::Item<'_, '_>) -> K, ) -> QuerySortedIter< @@ -779,6 +815,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -786,7 +826,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// # Panics /// /// This will panic if `next` has been called on `QueryIter` before, unless the underlying `Query` is empty. - pub fn sort_unstable_by_key( + pub fn sort_unstable_by_key( self, mut f: impl FnMut(&L::Item<'_, '_>) -> K, ) -> QuerySortedIter< @@ -814,6 +854,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -821,7 +865,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// # Panics /// /// This will panic if `next` has been called on `QueryIter` before, unless the underlying `Query` is empty. - pub fn sort_by_cached_key( + pub fn sort_by_cached_key( self, mut f: impl FnMut(&L::Item<'_, '_>) -> K, ) -> QuerySortedIter< @@ -844,6 +888,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -851,7 +899,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { /// # Panics /// /// This will panic if `next` has been called on `QueryIter` before, unless the underlying `Query` is empty. - fn sort_impl( + fn sort_impl( self, f: impl FnOnce(&mut Vec<(L::Item<'_, '_>, NeutralOrd)>), ) -> QuerySortedIter< @@ -902,7 +950,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { } } -impl<'w, 's, D: QueryData, F: QueryFilter> Iterator for QueryIter<'w, 's, D, F> { +impl<'w, 's, D: IterQueryData, F: QueryFilter> Iterator for QueryIter<'w, 's, D, F> { type Item = D::Item<'w, 's>; #[inline(always)] @@ -945,7 +993,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Iterator for QueryIter<'w, 's, D, F> } // This is correct as [`QueryIter`] always returns `None` once exhausted. -impl<'w, 's, D: QueryData, F: QueryFilter> FusedIterator for QueryIter<'w, 's, D, F> {} +impl<'w, 's, D: IterQueryData, F: QueryFilter> FusedIterator for QueryIter<'w, 's, D, F> {} // SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once. unsafe impl<'w, 's, F: QueryFilter> EntitySetIterator for QueryIter<'w, 's, Entity, F> {} @@ -1038,7 +1086,10 @@ where } /// # Safety - /// `entity` must stem from `self.entity_iter`, and not have been passed before. + /// + /// - `entity` must stem from `self.entity_iter` + /// - If `Self` does not impl `ReadOnlyQueryData`, then there must not be any other `Item`s alive for the current entity + /// - If `Self` does not impl `IterQueryData`, then there must not be any other `Item`s alive for *any* entity #[inline(always)] unsafe fn fetch_next(&mut self, entity: Entity) -> Option> { let (location, archetype, table); @@ -1068,7 +1119,7 @@ where // The entity list has already been filtered by the query lens, so we forego filtering here. // 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. + // - Caller ensures there are no conflicting items alive unsafe { D::fetch( &self.query_state.fetch_state, @@ -1080,7 +1131,7 @@ where } } -impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> Iterator +impl<'w, 's, D: IterQueryData, F: QueryFilter, I: Iterator> Iterator for QuerySortedIter<'w, 's, D, F, I> where I: Iterator, @@ -1090,7 +1141,9 @@ where #[inline(always)] fn next(&mut self) -> Option { while let Some(entity) = self.entity_iter.next() { - // SAFETY: `entity` is passed from `entity_iter` the first time. + // SAFETY: + // - `entity` is passed from `entity_iter` the first time. + // - `D: IterQueryData` if let Some(item) = unsafe { self.fetch_next(entity) } { return Some(item); } @@ -1106,7 +1159,7 @@ where } } -impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> DoubleEndedIterator +impl<'w, 's, D: IterQueryData, F: QueryFilter, I: Iterator> DoubleEndedIterator for QuerySortedIter<'w, 's, D, F, I> where I: DoubleEndedIterator, @@ -1114,7 +1167,9 @@ where #[inline(always)] fn next_back(&mut self) -> Option { while let Some(entity) = self.entity_iter.next_back() { - // SAFETY: `entity` is passed from `entity_iter` the first time. + // SAFETY: + // - `entity` is passed from `entity_iter` the first time. + // - `D: IterQueryData` if let Some(item) = unsafe { self.fetch_next(entity) } { return Some(item); } @@ -1123,13 +1178,16 @@ where } } -impl> ExactSizeIterator - for QuerySortedIter<'_, '_, D, F, I> +impl< + D: ArchetypeQueryData + IterQueryData, + F: QueryFilter, + I: ExactSizeIterator, + > ExactSizeIterator for QuerySortedIter<'_, '_, D, F, I> { } // This is correct as [`QuerySortedIter`] returns `None` once exhausted if `entity_iter` does. -impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> FusedIterator +impl<'w, 's, D: IterQueryData, F: QueryFilter, I: Iterator> FusedIterator for QuerySortedIter<'w, 's, D, F, I> where I: FusedIterator, @@ -1200,13 +1258,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> } /// # Safety - /// All arguments must stem from the same valid `QueryManyIter`. /// - /// The lifetime here is not restrictive enough for Fetch with &mut access, - /// as calling `fetch_next_aliased_unchecked` multiple times can produce multiple - /// references to the same component, leading to unique reference aliasing. - /// - /// It is always safe for shared access. + /// - All arguments must stem from the same valid `QueryManyIter`. + /// - If `Self` does not impl `ReadOnlyQueryData`, then there must not be any other `Item`s alive for the current entity + /// - If `Self` does not impl `IterQueryData`, then there must not be any other `Item`s alive for *any* entity #[inline(always)] unsafe fn fetch_next_aliased_unchecked( entity_iter: impl Iterator, @@ -1256,7 +1311,7 @@ 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. + // - Caller ensures there are no conflicting items alive let item = unsafe { D::fetch(&query_state.fetch_state, fetch, entity, location.table_row) }; @@ -1300,6 +1355,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -1384,7 +1443,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// # schedule.add_systems((system_1, system_2, system_3)); /// # schedule.run(&mut world); /// ``` - pub fn sort( + pub fn sort( self, ) -> QuerySortedManyIter< 'w, @@ -1409,6 +1468,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// This includes the allowed parameter type changes listed under [allowed transmutes].. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -1442,7 +1505,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// # schedule.add_systems((system_1)); /// # schedule.run(&mut world); /// ``` - pub fn sort_unstable( + pub fn sort_unstable( self, ) -> QuerySortedManyIter< 'w, @@ -1467,6 +1530,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -1507,7 +1574,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// # schedule.add_systems((system_1)); /// # schedule.run(&mut world); /// ``` - pub fn sort_by( + pub fn sort_by( self, mut compare: impl FnMut(&L::Item<'_, '_>, &L::Item<'_, '_>) -> Ordering, ) -> QuerySortedManyIter< @@ -1532,13 +1599,17 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes /// /// Unlike the sort methods on [`QueryIter`], this does NOT panic if `next`/`fetch_next` has been /// called on [`QueryManyIter`] before. - pub fn sort_unstable_by( + pub fn sort_unstable_by( self, mut compare: impl FnMut(&L::Item<'_, '_>, &L::Item<'_, '_>) -> Ordering, ) -> QuerySortedManyIter< @@ -1563,6 +1634,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes @@ -1632,7 +1707,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// # schedule.add_systems((system_1, system_2)); /// # schedule.run(&mut world); /// ``` - pub fn sort_by_key( + pub fn sort_by_key( self, mut f: impl FnMut(&L::Item<'_, '_>) -> K, ) -> QuerySortedManyIter< @@ -1658,13 +1733,17 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes /// /// Unlike the sort methods on [`QueryIter`], this does NOT panic if `next`/`fetch_next` has been /// called on [`QueryManyIter`] before. - pub fn sort_unstable_by_key( + pub fn sort_unstable_by_key( self, mut f: impl FnMut(&L::Item<'_, '_>) -> K, ) -> QuerySortedManyIter< @@ -1692,13 +1771,17 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes /// /// Unlike the sort methods on [`QueryIter`], this does NOT panic if `next`/`fetch_next` has been /// called on [`QueryManyIter`] before. - pub fn sort_by_cached_key( + pub fn sort_by_cached_key( self, mut f: impl FnMut(&L::Item<'_, '_>) -> K, ) -> QuerySortedManyIter< @@ -1721,13 +1804,17 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// This includes the allowed parameter type changes listed under [allowed transmutes]. /// However, the lens uses the filter of the original query when present. /// + /// The lens needs to be a [`SingleEntityQueryData`] because the current implementation + /// of query transmutes does not support nested queries. + /// This restriction may be lifted in the future. + /// /// The sort is not cached across system runs. /// /// [allowed transmutes]: crate::system::Query#allowed-transmutes /// /// Unlike the sort methods on [`QueryIter`], this does NOT panic if `next`/`fetch_next` has been /// called on [`QueryManyIter`] before. - fn sort_impl( + fn sort_impl( self, f: impl FnOnce(&mut Vec<(L::Item<'_, '_>, NeutralOrd)>), ) -> QuerySortedManyIter< @@ -1889,11 +1976,11 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> /// [`iter_many_unique`]: crate::system::Query::iter_many /// [`iter_many_unique_mut`]: crate::system::Query::iter_many_mut /// [`Query`]: crate::system::Query -pub struct QueryManyUniqueIter<'w, 's, D: QueryData, F: QueryFilter, I: EntitySetIterator>( +pub struct QueryManyUniqueIter<'w, 's, D: IterQueryData, F: QueryFilter, I: EntitySetIterator>( QueryManyIter<'w, 's, D, F, I>, ); -impl<'w, 's, D: QueryData, F: QueryFilter, I: EntitySetIterator> +impl<'w, 's, D: IterQueryData, F: QueryFilter, I: EntitySetIterator> QueryManyUniqueIter<'w, 's, D, F, I> { /// # Safety @@ -1916,14 +2003,16 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: EntitySetIterator> } } -impl<'w, 's, D: QueryData, F: QueryFilter, I: EntitySetIterator> Iterator +impl<'w, 's, D: IterQueryData, F: QueryFilter, I: EntitySetIterator> Iterator for QueryManyUniqueIter<'w, 's, D, F, I> { type Item = D::Item<'w, 's>; #[inline(always)] fn next(&mut self) -> Option { - // SAFETY: Entities are guaranteed to be unique, thus do not alias. + // SAFETY: + // - Entities are guaranteed to be unique, thus do not alias. + // - `D: IterQueryData` unsafe { QueryManyIter::<'w, 's, D, F, I>::fetch_next_aliased_unchecked( &mut self.0.entity_iter, @@ -1944,7 +2033,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: EntitySetIterator> Iterator } // This is correct as [`QueryManyIter`] always returns `None` once exhausted. -impl<'w, 's, D: QueryData, F: QueryFilter, I: EntitySetIterator> FusedIterator +impl<'w, 's, D: IterQueryData, F: QueryFilter, I: EntitySetIterator> FusedIterator for QueryManyUniqueIter<'w, 's, D, F, I> { } @@ -1955,7 +2044,7 @@ unsafe impl<'w, 's, F: QueryFilter, I: EntitySetIterator> EntitySetIterator { } -impl<'w, 's, D: QueryData, F: QueryFilter, I: EntitySetIterator> Debug +impl<'w, 's, D: IterQueryData, F: QueryFilter, I: EntitySetIterator> Debug for QueryManyUniqueIter<'w, 's, D, F, I> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { @@ -2005,12 +2094,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> } /// # Safety - /// The lifetime here is not restrictive enough for Fetch with &mut access, - /// as calling `fetch_next_aliased_unchecked` multiple times can produce multiple - /// references to the same component, leading to unique reference aliasing. /// - /// It is always safe for shared access. - /// `entity` must stem from `self.entity_iter`, and not have been passed before. + /// - `entity` must stem from `self.entity_iter` + /// - If `Self` does not impl `ReadOnlyQueryData`, then there must not be any other `Item`s alive for the current entity + /// - If `Self` does not impl `IterQueryData`, then there must not be any other `Item`s alive for *any* entity #[inline(always)] unsafe fn fetch_next_aliased_unchecked(&mut self, entity: Entity) -> Option> { let (location, archetype, table); @@ -2040,7 +2127,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> // The entity list has already been filtered by the query lens, so we forego filtering here. // 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. + // - Caller ensures there are no conflicting items alive unsafe { D::fetch( &self.query_state.fetch_state, @@ -2366,7 +2453,7 @@ impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter, const K: usize> Iterator } } -impl<'w, 's, D: ArchetypeQueryData, F: ArchetypeFilter> ExactSizeIterator +impl<'w, 's, D: ArchetypeQueryData + IterQueryData, F: ArchetypeFilter> ExactSizeIterator for QueryIter<'w, 's, D, F> { fn len(&self) -> usize { diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index eb3dad12bd367..c2cbfc11d8e31 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -111,8 +111,8 @@ mod tests { component::{Component, ComponentId, Components}, prelude::{AnyOf, Changed, Entity, Or, QueryState, Resource, With, Without}, query::{ - ArchetypeFilter, ArchetypeQueryData, FilteredAccess, Has, QueryCombinationIter, - QueryData, QueryFilter, ReadOnlyQueryData, WorldQuery, + ArchetypeFilter, ArchetypeQueryData, FilteredAccess, Has, IterQueryData, + QueryCombinationIter, QueryData, QueryFilter, ReadOnlyQueryData, WorldQuery, }, schedule::{IntoScheduleConfigs, Schedule}, storage::{Table, TableRow}, @@ -902,6 +902,9 @@ mod tests { /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for ReadsRData {} + /// SAFETY: access is read only + unsafe impl IterQueryData for ReadsRData {} + impl ArchetypeQueryData for ReadsRData {} #[test] diff --git a/crates/bevy_ecs/src/query/par_iter.rs b/crates/bevy_ecs/src/query/par_iter.rs index 6242f8e39b8be..2391a06fae0fc 100644 --- a/crates/bevy_ecs/src/query/par_iter.rs +++ b/crates/bevy_ecs/src/query/par_iter.rs @@ -5,7 +5,7 @@ use crate::{ world::unsafe_world_cell::UnsafeWorldCell, }; -use super::{QueryData, QueryFilter, QueryItem, QueryState, ReadOnlyQueryData}; +use super::{IterQueryData, QueryFilter, QueryItem, QueryState, ReadOnlyQueryData}; use alloc::vec::Vec; @@ -13,7 +13,7 @@ use alloc::vec::Vec; /// /// This struct is created by the [`Query::par_iter`](crate::system::Query::par_iter) and /// [`Query::par_iter_mut`](crate::system::Query::par_iter_mut) methods. -pub struct QueryParIter<'w, 's, D: QueryData, F: QueryFilter> { +pub struct QueryParIter<'w, 's, D: IterQueryData, F: QueryFilter> { pub(crate) world: UnsafeWorldCell<'w>, pub(crate) state: &'s QueryState, pub(crate) last_run: Tick, @@ -21,7 +21,7 @@ pub struct QueryParIter<'w, 's, D: QueryData, F: QueryFilter> { pub(crate) batching_strategy: BatchingStrategy, } -impl<'w, 's, D: QueryData, F: QueryFilter> QueryParIter<'w, 's, D, F> { +impl<'w, 's, D: IterQueryData, F: QueryFilter> QueryParIter<'w, 's, D, F> { /// Changes the batching strategy used when iterating. /// /// For more information on how this affects the resultant iteration, see @@ -161,7 +161,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryParIter<'w, 's, D, F> { /// /// [`Entity`]: crate::entity::Entity /// [`Query::par_iter_many`]: crate::system::Query::par_iter_many -pub struct QueryParManyIter<'w, 's, D: QueryData, F: QueryFilter, E: EntityEquivalent> { +pub struct QueryParManyIter<'w, 's, D: IterQueryData, F: QueryFilter, E: EntityEquivalent> { pub(crate) world: UnsafeWorldCell<'w>, pub(crate) state: &'s QueryState, pub(crate) entity_list: Vec, @@ -315,8 +315,13 @@ impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter, E: EntityEquivalent + Sync> /// [`EntitySet`]: crate::entity::EntitySet /// [`Query::par_iter_many_unique`]: crate::system::Query::par_iter_many_unique /// [`Query::par_iter_many_unique_mut`]: crate::system::Query::par_iter_many_unique_mut -pub struct QueryParManyUniqueIter<'w, 's, D: QueryData, F: QueryFilter, E: EntityEquivalent + Sync> -{ +pub struct QueryParManyUniqueIter< + 'w, + 's, + D: IterQueryData, + F: QueryFilter, + E: EntityEquivalent + Sync, +> { pub(crate) world: UnsafeWorldCell<'w>, pub(crate) state: &'s QueryState, pub(crate) entity_list: UniqueEntityEquivalentVec, @@ -325,7 +330,7 @@ pub struct QueryParManyUniqueIter<'w, 's, D: QueryData, F: QueryFilter, E: Entit pub(crate) batching_strategy: BatchingStrategy, } -impl<'w, 's, D: QueryData, F: QueryFilter, E: EntityEquivalent + Sync> +impl<'w, 's, D: IterQueryData, F: QueryFilter, E: EntityEquivalent + Sync> QueryParManyUniqueIter<'w, 's, D, F, E> { /// Changes the batching strategy used when iterating. diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 0f37fd17cea7e..4084332de73ec 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -5,7 +5,10 @@ use crate::{ entity::{Entity, EntityEquivalent, EntitySet, UniqueEntityArray}, entity_disabling::DefaultQueryFilters, prelude::FromWorld, - query::{FilteredAccess, QueryCombinationIter, QueryIter, QueryParIter, WorldQuery}, + query::{ + FilteredAccess, FilteredAccessSet, IterQueryData, QueryCombinationIter, QueryIter, + QueryParIter, SingleEntityQueryData, WorldQuery, + }, storage::{SparseSetIndex, TableId}, system::Query, world::{unsafe_world_cell::UnsafeWorldCell, World, WorldId}, @@ -14,7 +17,7 @@ use crate::{ #[cfg(all(not(target_arch = "wasm32"), feature = "multi_threaded"))] use crate::entity::UniqueEntityEquivalentSlice; -use alloc::vec::Vec; +use alloc::{format, vec::Vec}; use bevy_utils::prelude::DebugName; use core::{fmt, ptr}; use fixedbitset::FixedBitSet; @@ -61,6 +64,13 @@ pub(super) union StorageId { /// [`State`]: crate::query::world_query::WorldQuery::State /// [`Fetch`]: crate::query::world_query::WorldQuery::Fetch /// [`Table`]: crate::storage::Table +/// +/// # Safety +/// +/// If the query is not read-only, +/// then before calling any other methods on a new `QueryState` +/// other than [`QueryState::update_archetypes`], [`QueryState::update_archetypes_unsafe_world_cell`], +/// or `QueryState::into_readonly`, [`Self::init_access`] must be called. #[repr(C)] // SAFETY NOTE: // Do not add any new fields that use the `D` or `F` generic parameters as this may @@ -109,6 +119,32 @@ impl FromWorld for QueryState { } impl QueryState { + /// Converts this `QueryState` into a `QueryState` that does not access anything mutably, + /// but that can be converted back to the original type using [`Self::from_readonly`]. + /// + /// # Safety + /// + /// If the query is not read-only, + /// then before calling any other methods on the returned `QueryState` + /// other than [`QueryState::update_archetypes`], [`QueryState::update_archetypes_unsafe_world_cell`], + /// or [`QueryState::into_readonly`], [`Self::init_access`] must be called. + pub(crate) unsafe fn into_readonly(self) -> QueryState { + // SAFETY: Caller ensures `init_access` is called + QueryState { + world_id: self.world_id, + archetype_generation: self.archetype_generation, + matched_storage_ids: self.matched_storage_ids, + is_dense: self.is_dense, + fetch_state: self.fetch_state, + filter_state: self.filter_state, + component_access: self.component_access, + matched_tables: self.matched_tables, + matched_archetypes: self.matched_archetypes, + #[cfg(feature = "trace")] + par_iter_span: self.par_iter_span, + } + } + /// Converts this `QueryState` reference to a `QueryState` that does not access anything mutably. pub fn as_readonly(&self) -> &QueryState { // SAFETY: invariant on `WorldQuery` trait upholds that `D::ReadOnly` and `F::ReadOnly` @@ -127,6 +163,21 @@ impl QueryState { unsafe { self.as_transmuted_state::, F>() } } + /// Converts a reference to a read-only `QueryState` back to a reference to the original. + /// + /// This is used to implement `NestedQuery`, since it stores a `QueryState` in `WorldQuery::State` + /// and needs to ensure that `Self::ReadOnly::State == Self::State`. + /// + /// # Safety + /// + /// Either `D == D::ReadOnly`, or the referenced `QueryState` must have been + /// created from [`QueryState::into_readonly`] called on a `QueryState`. + pub(crate) unsafe fn from_readonly(state: &QueryState) -> &Self { + // SAFETY: Caller ensures this is either a no-op, + // or that the contents were created as a valid `QueryState` + unsafe { &*ptr::from_ref(state).cast::>() } + } + /// Converts this `QueryState` reference to any other `QueryState` with /// the same `WorldQuery::State` associated types. /// @@ -161,18 +212,73 @@ impl QueryState { } /// Creates a new [`QueryState`] from a given [`World`] and inherits the result of `world.id()`. - pub fn new(world: &mut World) -> Self { - let mut state = Self::new_uninitialized(world); + /// This does not check access of nested queries, so [`Self::init_access`] + /// must be called before querying using this state or returning it to safe code. + /// + /// # Safety + /// + /// If the query is not read-only, + /// then before calling any other methods on the returned `QueryState` + /// other than [`QueryState::update_archetypes`], [`QueryState::update_archetypes_unsafe_world_cell`], + /// or `QueryState::into_readonly`, [`Self::init_access`] must be called. + pub unsafe fn new_unchecked(world: &mut World) -> Self { + let fetch_state = D::init_state(world); + let filter_state = F::init_state(world); + // SAFETY: Caller ensures `init_access` is called + let mut state = + unsafe { Self::from_states_uninitialized(world, fetch_state, filter_state) }; state.update_archetypes(world); state } + /// Adds all access from this query and any nested queries to the `component_access_set`. + /// Panics if the access from this query and any nested queries conflict with each other + /// or with any previous access. + pub fn init_access( + &self, + system_name: Option<&str>, + component_access_set: &mut FilteredAccessSet, + world: UnsafeWorldCell, + ) { + let conflicts = component_access_set.get_conflicts_single(&self.component_access); + if !conflicts.is_empty() { + let mut accesses = conflicts.format_conflict_list(world); + // Access list may be empty (if access to all components requested) + if !accesses.is_empty() { + accesses.push(' '); + } + let type_name = DebugName::type_name::>(); + let type_name = type_name.shortname(); + let system = system_name + .map(|name| format!(" in system {name}")) + .unwrap_or_default(); + panic!("error[B0001]: {type_name}{system} accesses component(s) {accesses}in a way that conflicts with a previous system parameter. Consider using `Without` to create disjoint Queries or merging conflicting Queries into a `ParamSet`. See: https://bevy.org/learn/errors/b0001",); + } + + component_access_set.add(self.component_access.clone()); + D::init_nested_access(&self.fetch_state, system_name, component_access_set, world); + F::init_nested_access(&self.filter_state, system_name, component_access_set, world); + } + + /// Creates a new [`QueryState`] from a given [`World`] and inherits the result of `world.id()`. + pub fn new(world: &mut World) -> Self { + // SAFETY: We immediately call `init_access` + let state = unsafe { Self::new_unchecked(world) }; + state.init_access(None, &mut FilteredAccessSet::new(), world.into()); + state + } + /// Creates a new [`QueryState`] from an immutable [`World`] reference and inherits the result of `world.id()`. /// /// This function may fail if, for example, /// the components that make up this query have not been registered into the world. pub fn try_new(world: &World) -> Option { - let mut state = Self::try_new_uninitialized(world)?; + let fetch_state = D::get_state(world.components())?; + let filter_state = F::get_state(world.components())?; + // SAFETY: We immediately call `init_access` + let mut state = + unsafe { Self::from_states_uninitialized(world, fetch_state, filter_state) }; + state.init_access(None, &mut FilteredAccessSet::new(), world.into()); state.update_archetypes(world); Some(state) } @@ -181,31 +287,14 @@ impl QueryState { /// /// `new_archetype` and its variants must be called on all of the World's archetypes before the /// state can return valid query results. - fn new_uninitialized(world: &mut World) -> Self { - let fetch_state = D::init_state(world); - let filter_state = F::init_state(world); - Self::from_states_uninitialized(world, fetch_state, filter_state) - } - - /// Creates a new [`QueryState`] but does not populate it with the matched results from the World yet /// - /// `new_archetype` and its variants must be called on all of the World's archetypes before the - /// state can return valid query results. - fn try_new_uninitialized(world: &World) -> Option { - let fetch_state = D::get_state(world.components())?; - let filter_state = F::get_state(world.components())?; - Some(Self::from_states_uninitialized( - world, - fetch_state, - filter_state, - )) - } - - /// Creates a new [`QueryState`] but does not populate it with the matched results from the World yet + /// # Safety /// - /// `new_archetype` and its variants must be called on all of the World's archetypes before the - /// state can return valid query results. - fn from_states_uninitialized( + /// If the query is not read-only, + /// then before calling any other methods on the returned `QueryState` + /// other than [`QueryState::update_archetypes`], [`QueryState::update_archetypes_unsafe_world_cell`], + /// or [`QueryState::into_readonly`], [`Self::init_access`] must be called. + unsafe fn from_states_uninitialized( world: &World, fetch_state: ::State, filter_state: ::State, @@ -232,6 +321,7 @@ impl QueryState { is_dense &= default_filters.is_dense(world.components()); } + // SAFETY: Caller ensures `init_access` is called Self { world_id: world.id(), archetype_generation: ArchetypeGeneration::initial(), @@ -274,6 +364,7 @@ impl QueryState { is_dense &= default_filters.is_dense(builder.world().components()); } + // SAFETY: We immediately call `init_access` let mut state = Self { world_id: builder.world().id(), archetype_generation: ArchetypeGeneration::initial(), @@ -291,6 +382,7 @@ impl QueryState { filter = core::any::type_name::(), ), }; + state.init_access(None, &mut FilteredAccessSet::new(), builder.world().into()); state.update_archetypes(builder.world()); state } @@ -505,6 +597,8 @@ impl QueryState { /// If `world` does not match the one used to call `QueryState::new` for this instance. pub fn update_archetypes_unsafe_world_cell(&mut self, world: UnsafeWorldCell) { self.validate_world(world.id()); + D::update_archetypes(&mut self.fetch_state, world); + F::update_archetypes(&mut self.filter_state, world); if self.component_access.required.is_empty() { let archetypes = world.archetypes(); let old_generation = @@ -630,7 +724,7 @@ impl QueryState { /// You might end up with a mix of archetypes that only matched the original query + archetypes that only match /// the new [`QueryState`]. Most of the safe methods on [`QueryState`] call [`QueryState::update_archetypes`] internally, so this /// best used through a [`Query`] - pub fn transmute<'a, NewD: QueryData>( + pub fn transmute<'a, NewD: SingleEntityQueryData>( &self, world: impl Into>, ) -> QueryState { @@ -641,7 +735,7 @@ impl QueryState { /// as self but with a new type signature. /// /// Panics if `NewD` or `NewF` require accesses that this query does not have. - pub fn transmute_filtered<'a, NewD: QueryData, NewF: QueryFilter>( + pub fn transmute_filtered<'a, NewD: SingleEntityQueryData, NewF: QueryFilter>( &self, world: impl Into>, ) -> QueryState { @@ -696,6 +790,7 @@ impl QueryState { // then there was a sparse set component in the `required` set, and the query has `is_dense = false`. let is_dense = self.is_dense; + // SAFETY: `D: SingleEntityQueryData`, so we do not need to call `init_access` QueryState { world_id: self.world_id, archetype_generation: self.archetype_generation, @@ -734,7 +829,7 @@ impl QueryState { /// ## Panics /// /// Will panic if `NewD` contains accesses not in `Q` or `OtherQ`. - pub fn join<'a, OtherD: QueryData, NewD: QueryData>( + pub fn join<'a, OtherD: QueryData, NewD: SingleEntityQueryData>( &self, world: impl Into>, other: &QueryState, @@ -752,7 +847,7 @@ impl QueryState { 'a, OtherD: QueryData, OtherF: QueryFilter, - NewD: QueryData, + NewD: SingleEntityQueryData, NewF: QueryFilter, >( &self, @@ -843,6 +938,7 @@ impl QueryState { .collect() }; + // SAFETY: `D: SingleEntityQueryData`, so we do not need to call `init_access` QueryState { world_id: self.world_id, archetype_generation: self.archetype_generation, @@ -1014,7 +1110,10 @@ impl QueryState { &mut self, world: &'w mut World, entities: [Entity; N], - ) -> Result<[D::Item<'w, '_>; N], QueryEntityError> { + ) -> Result<[D::Item<'w, '_>; N], QueryEntityError> + where + D: IterQueryData, + { self.query_mut(world).get_many_mut_inner(entities) } @@ -1059,7 +1158,10 @@ impl QueryState { &mut self, world: &'w mut World, entities: UniqueEntityArray, - ) -> Result<[D::Item<'w, '_>; N], QueryEntityError> { + ) -> Result<[D::Item<'w, '_>; N], QueryEntityError> + where + D: IterQueryData, + { self.query_mut(world).get_many_unique_inner(entities) } @@ -1118,7 +1220,10 @@ impl QueryState { /// This iterator is always guaranteed to return results from each matching entity once and only once. /// Iteration order is not guaranteed. #[inline] - pub fn iter_mut<'w, 's>(&'s mut self, world: &'w mut World) -> QueryIter<'w, 's, D, F> { + pub fn iter_mut<'w, 's>(&'s mut self, world: &'w mut World) -> QueryIter<'w, 's, D, F> + where + D: IterQueryData, + { self.query_mut(world).into_iter() } @@ -1297,7 +1402,10 @@ impl QueryState { &'s mut self, world: &'w mut World, entities: EntityList, - ) -> QueryManyUniqueIter<'w, 's, D, F, EntityList::IntoIter> { + ) -> QueryManyUniqueIter<'w, 's, D, F, EntityList::IntoIter> + where + D: IterQueryData, + { self.query_mut(world).iter_many_unique_inner(entities) } /// Returns an [`Iterator`] over the query results for the given [`World`]. @@ -1313,7 +1421,10 @@ impl QueryState { pub unsafe fn iter_unchecked<'w, 's>( &'s mut self, world: UnsafeWorldCell<'w>, - ) -> QueryIter<'w, 's, D, F> { + ) -> QueryIter<'w, 's, D, F> + where + D: IterQueryData, + { self.query_unchecked(world).into_iter() } @@ -1397,7 +1508,10 @@ impl QueryState { /// [`par_iter`]: Self::par_iter /// [`ComputeTaskPool`]: bevy_tasks::ComputeTaskPool #[inline] - pub fn par_iter_mut<'w, 's>(&'s mut self, world: &'w mut World) -> QueryParIter<'w, 's, D, F> { + pub fn par_iter_mut<'w, 's>(&'s mut self, world: &'w mut World) -> QueryParIter<'w, 's, D, F> + where + D: IterQueryData, + { self.query_mut(world).par_iter_inner() } @@ -1429,6 +1543,7 @@ impl QueryState { ) where FN: Fn(T, D::Item<'w, 's>) -> T + Send + Sync + Clone, INIT: Fn() -> T + Sync + Send + Clone, + D: IterQueryData, { // NOTE: If you are changing query iteration code, remember to update the following places, where relevant: // QueryIter, QueryIterationCursor, QueryManyIter, QueryCombinationIter,QueryState::par_fold_init_unchecked_manual, @@ -1545,6 +1660,7 @@ impl QueryState { FN: Fn(T, D::Item<'w, 's>) -> T + Send + Sync + Clone, INIT: Fn() -> T + Sync + Send + Clone, E: EntityEquivalent + Sync, + D: IterQueryData, { // NOTE: If you are changing query iteration code, remember to update the following places, where relevant: // QueryIter, QueryIterationCursor, QueryManyIter, QueryCombinationIter,QueryState::par_fold_init_unchecked_manual @@ -1737,7 +1853,10 @@ impl QueryState { pub fn single_mut<'w>( &mut self, world: &'w mut World, - ) -> Result, QuerySingleError> { + ) -> Result, QuerySingleError> + where + D: IterQueryData, + { self.query_mut(world).single_inner() } @@ -1754,7 +1873,10 @@ impl QueryState { pub unsafe fn single_unchecked<'w>( &mut self, world: UnsafeWorldCell<'w>, - ) -> Result, QuerySingleError> { + ) -> Result, QuerySingleError> + where + D: IterQueryData, + { self.query_unchecked(world).single_inner() } @@ -1776,7 +1898,10 @@ impl QueryState { world: UnsafeWorldCell<'w>, last_run: Tick, this_run: Tick, - ) -> Result, QuerySingleError> { + ) -> Result, QuerySingleError> + where + D: IterQueryData, + { // SAFETY: // - The caller ensured we have the correct access to the world. // - The caller ensured that the world matches. diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index fef5b0257f167..c629b4e5144b1 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -2,7 +2,7 @@ use crate::{ archetype::Archetype, change_detection::Tick, component::{ComponentId, Components}, - query::FilteredAccess, + query::{FilteredAccess, FilteredAccessSet}, storage::Table, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; @@ -61,8 +61,15 @@ pub unsafe trait WorldQuery { /// /// - `state` must have been initialized (via [`WorldQuery::init_state`]) using the same `world` passed /// in to this function. - /// - `world` must have the **right** to access any access registered in `update_component_access`. - /// - There must not be simultaneous resource access conflicting with readonly resource access registered in [`WorldQuery::update_component_access`]. + /// - `world` must have the **right** to access any access registered in [`WorldQuery::update_component_access`] + /// or [`WorldQuery::init_nested_access`]. + /// - [`WorldQuery::update_component_access`] must not request conflicting access. + /// If `Self` is `ReadOnlyQueryData` or `QueryFilter`, the access is read-only and can never conflict. + /// Otherwise, [`WorldQuery::update_component_access`] must be called to ensure it does not panic. + /// - [`WorldQuery::init_nested_access`] must not request conflicting access. + /// If `Self` is [`ReadOnlyQueryData`](crate::query::ReadOnlyQueryData) or [`QueryFilter`](crate::query::QueryFilter), the access is read-only and can never conflict. + /// If `Self` is [`SingleEntityQueryData`](crate::query::SingleEntityQueryData), there is no external access and it cannot conflict. + /// Otherwise, [`WorldQuery::init_nested_access`] must be called to ensure it does not panic. unsafe fn init_fetch<'w, 's>( world: UnsafeWorldCell<'w>, state: &'s Self::State, @@ -108,13 +115,27 @@ pub unsafe trait WorldQuery { table: &'w Table, ); - /// Adds any component accesses used by this [`WorldQuery`] to `access`. + /// Adds any component accesses to the current entity used by this [`WorldQuery`] to `access`. /// /// Used to check which queries are disjoint and can run in parallel // This does not have a default body of `{}` because 99% of cases need to add accesses // and forgetting to do so would be unsound. fn update_component_access(state: &Self::State, access: &mut FilteredAccess); + /// Adds any component accesses to other entities used by this [`WorldQuery`]. + /// + /// This method must panic if the access would conflict with any existing access in the [`FilteredAccessSet`]. + /// + /// This is used for queries to request access to entities other than the current one, + /// such as to follow relations. + fn init_nested_access( + _state: &Self::State, + _system_name: Option<&str>, + _component_access_set: &mut FilteredAccessSet, + _world: UnsafeWorldCell, + ) { + } + /// Creates and initializes a [`State`](WorldQuery::State) for this [`WorldQuery`] type. fn init_state(world: &mut World) -> Self::State; @@ -131,6 +152,10 @@ pub unsafe trait WorldQuery { state: &Self::State, set_contains_id: &impl Fn(ComponentId) -> bool, ) -> bool; + + /// Called when the query state is updating its archetype cache. + /// This can be used by nested queries to update their internal archetype caches. + fn update_archetypes(_state: &mut Self::State, _world: UnsafeWorldCell) {} } macro_rules! impl_tuple_world_query { @@ -205,6 +230,17 @@ macro_rules! impl_tuple_world_query { let ($($name,)*) = state; $($name::update_component_access($name, access);)* } + + fn init_nested_access( + state: &Self::State, + _system_name: Option<&str>, + _component_access_set: &mut FilteredAccessSet, + _world: UnsafeWorldCell, + ) { + let ($($state,)*) = state; + $($name::init_nested_access($state, _system_name, _component_access_set, _world);)* + } + fn init_state(world: &mut World) -> Self::State { ($($name::init_state(world),)*) } @@ -216,6 +252,11 @@ macro_rules! impl_tuple_world_query { let ($($name,)*) = state; true $(&& $name::matches_component_set($name, set_contains_id))* } + + fn update_archetypes(state: &mut Self::State, _world: UnsafeWorldCell) { + let ($($name,)*) = state; + $($name::update_archetypes($name, _world);)* + } } }; } diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index a0551a4892224..7b88b47fe4e04 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -411,7 +411,7 @@ mod tests { lifecycle::RemovedComponents, name::Name, prelude::{Add, AnyOf, EntityRef, On}, - query::{Added, Changed, Or, SpawnDetails, Spawned, With, Without}, + query::{Added, Changed, NestedQuery, Or, SpawnDetails, Spawned, With, Without}, resource::Resource, schedule::{ common_conditions::resource_exists, ApplyDeferred, IntoScheduleConfigs, Schedule, @@ -893,6 +893,42 @@ mod tests { run_system(&mut world, sys); } + #[test] + #[should_panic = "error[B0001]"] + fn nested_query_conflicts_with_main_query() { + fn sys(_: Query<(&mut A, NestedQuery<&A>)>) {} + + let mut world = World::default(); + run_system(&mut world, sys); + } + + #[test] + #[should_panic = "error[B0001]"] + fn nested_query_conflicts_with_other_nested_query() { + fn sys(_: Query<(NestedQuery<&mut A>, NestedQuery<&A>)>) {} + + let mut world = World::default(); + run_system(&mut world, sys); + } + + #[test] + #[should_panic = "error[B0001]"] + fn nested_query_conflicts_with_earlier_query() { + fn sys(_: Query<&mut A>, _: Query>) {} + + let mut world = World::default(); + run_system(&mut world, sys); + } + + #[test] + #[should_panic = "error[B0001]"] + fn nested_query_conflicts_with_later_query() { + fn sys(_: Query>, _: Query<&mut A>) {} + + let mut world = World::default(); + run_system(&mut world, sys); + } + #[test] fn query_set_system() { fn sys(mut _set: ParamSet<(Query<&mut A>, Query<&A>)>) {} diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index dff7ea618348c..214947eb1d23b 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -5,9 +5,10 @@ use crate::{ change_detection::Tick, entity::{Entity, EntityDoesNotExistError, EntityEquivalent, EntitySet, UniqueEntityArray}, query::{ - DebugCheckedUnwrap, NopWorldQuery, QueryCombinationIter, QueryData, QueryEntityError, - QueryFilter, QueryIter, QueryManyIter, QueryManyUniqueIter, QueryParIter, QueryParManyIter, - QueryParManyUniqueIter, QuerySingleError, QueryState, ROQueryItem, ReadOnlyQueryData, + DebugCheckedUnwrap, IterQueryData, NopWorldQuery, QueryCombinationIter, QueryData, + QueryEntityError, QueryFilter, QueryIter, QueryManyIter, QueryManyUniqueIter, QueryParIter, + QueryParManyIter, QueryParManyUniqueIter, QuerySingleError, QueryState, ROQueryItem, + ReadOnlyQueryData, SingleEntityQueryData, }, world::unsafe_world_cell::UnsafeWorldCell, }; @@ -704,7 +705,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// [`iter`](Self::iter) for read-only query items. #[inline] - pub fn iter_mut(&mut self) -> QueryIter<'_, 's, D, F> { + pub fn iter_mut(&mut self) -> QueryIter<'_, 's, D, F> + where + D: IterQueryData, + { self.reborrow().into_iter() } @@ -1022,7 +1026,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn iter_many_unique_mut( &mut self, entities: EntityList, - ) -> QueryManyUniqueIter<'_, 's, D, F, EntityList::IntoIter> { + ) -> QueryManyUniqueIter<'_, 's, D, F, EntityList::IntoIter> + where + D: IterQueryData, + { self.reborrow().iter_many_unique_inner(entities) } @@ -1077,7 +1084,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn iter_many_unique_inner( self, entities: EntityList, - ) -> QueryManyUniqueIter<'w, 's, D, F, EntityList::IntoIter> { + ) -> QueryManyUniqueIter<'w, 's, D, F, EntityList::IntoIter> + where + D: IterQueryData, + { // SAFETY: `self.world` has permission to access the required components. unsafe { QueryManyUniqueIter::new( @@ -1104,7 +1114,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// - [`iter`](Self::iter) and [`iter_mut`](Self::iter_mut) for the safe versions. #[inline] - pub unsafe fn iter_unsafe(&self) -> QueryIter<'_, 's, D, F> { + pub unsafe fn iter_unsafe(&self) -> QueryIter<'_, 's, D, F> + where + D: IterQueryData, + { // SAFETY: The caller promises that this will not result in multiple mutable references. unsafe { self.reborrow_unsafe() }.into_iter() } @@ -1169,7 +1182,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub unsafe fn iter_many_unique_unsafe( &self, entities: EntityList, - ) -> QueryManyUniqueIter<'_, 's, D, F, EntityList::IntoIter> { + ) -> QueryManyUniqueIter<'_, 's, D, F, EntityList::IntoIter> + where + D: IterQueryData, + { // SAFETY: The caller promises that this will not result in multiple mutable references. unsafe { self.reborrow_unsafe() }.iter_many_unique_inner(entities) } @@ -1225,7 +1241,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// [`par_iter`]: Self::par_iter /// [`World`]: crate::world::World #[inline] - pub fn par_iter_mut(&mut self) -> QueryParIter<'_, 's, D, F> { + pub fn par_iter_mut(&mut self) -> QueryParIter<'_, 's, D, F> + where + D: IterQueryData, + { self.reborrow().par_iter_inner() } @@ -1256,7 +1275,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// # bevy_ecs::system::assert_is_system(gravity_system); /// ``` #[inline] - pub fn par_iter_inner(self) -> QueryParIter<'w, 's, D, F> { + pub fn par_iter_inner(self) -> QueryParIter<'w, 's, D, F> + where + D: IterQueryData, + { QueryParIter { world: self.world, state: self.state, @@ -1343,7 +1365,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn par_iter_many_unique_mut>( &mut self, entities: EntityList, - ) -> QueryParManyUniqueIter<'_, 's, D, F, EntityList::Item> { + ) -> QueryParManyUniqueIter<'_, 's, D, F, EntityList::Item> + where + D: IterQueryData, + { QueryParManyUniqueIter { world: self.world, state: self.state, @@ -1677,7 +1702,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn get_many_mut( &mut self, entities: [Entity; N], - ) -> Result<[D::Item<'_, 's>; N], QueryEntityError> { + ) -> Result<[D::Item<'_, 's>; N], QueryEntityError> + where + D: IterQueryData, + { self.reborrow().get_many_mut_inner(entities) } @@ -1745,7 +1773,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn get_many_unique_mut( &mut self, entities: UniqueEntityArray, - ) -> Result<[D::Item<'_, 's>; N], QueryEntityError> { + ) -> Result<[D::Item<'_, 's>; N], QueryEntityError> + where + D: IterQueryData, + { self.reborrow().get_many_unique_inner(entities) } @@ -1764,7 +1795,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn get_many_mut_inner( self, entities: [Entity; N], - ) -> Result<[D::Item<'w, 's>; N], QueryEntityError> { + ) -> Result<[D::Item<'w, 's>; N], QueryEntityError> + where + D: IterQueryData, + { // Verify that all entities are unique for i in 0..N { for j in 0..i { @@ -1814,7 +1848,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn get_many_unique_inner( self, entities: UniqueEntityArray, - ) -> Result<[D::Item<'w, 's>; N], QueryEntityError> { + ) -> Result<[D::Item<'w, 's>; N], QueryEntityError> + where + D: IterQueryData, + { // SAFETY: All entities are unique, so the results don't alias. unsafe { self.get_many_impl(entities.into_inner()) } } @@ -1829,11 +1866,15 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { unsafe fn get_many_impl( self, entities: [Entity; N], - ) -> Result<[D::Item<'w, 's>; N], QueryEntityError> { + ) -> Result<[D::Item<'w, 's>; N], QueryEntityError> + where + D: IterQueryData, + { let mut values = [(); N].map(|_| MaybeUninit::uninit()); for (value, entity) in core::iter::zip(&mut values, entities) { - // SAFETY: The caller asserts that the results don't alias + // SAFETY: The caller asserts that the results don't alias, + // and `D: IterQueryData` so its valid to have items alive for different entitiess let item = unsafe { self.copy_unsafe() }.get_inner(entity)?; *value = MaybeUninit::new(item); } @@ -1925,7 +1966,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// - [`single`](Self::single) to get the read-only query item. #[inline] - pub fn single_mut(&mut self) -> Result, QuerySingleError> { + pub fn single_mut(&mut self) -> Result, QuerySingleError> + where + D: IterQueryData, + { self.reborrow().single_inner() } @@ -1956,7 +2000,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// - [`single`](Self::single) to get the read-only query item. /// - [`single_mut`](Self::single_mut) to get the mutable query item. #[inline] - pub fn single_inner(self) -> Result, QuerySingleError> { + pub fn single_inner(self) -> Result, QuerySingleError> + where + D: IterQueryData, + { let mut query = self.into_iter(); let first = query.next(); let extra = query.next().is_some(); @@ -2188,16 +2235,16 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// # prelude::*, /// # archetype::Archetype, /// # entity::EntityLocation, - /// # query::{QueryData, QueryFilter}, + /// # query::{QueryData, QueryFilter, SingleEntityQueryData}, /// # world::{FilteredEntityMut, FilteredEntityRef}, /// # }; /// # use std::marker::PhantomData; /// # - /// # fn assert_valid_transmute() { + /// # fn assert_valid_transmute() { /// # assert_valid_transmute_filtered::(); /// # } /// # - /// # fn assert_valid_transmute_filtered() { + /// # fn assert_valid_transmute_filtered() { /// # let mut world = World::new(); /// # // Make sure all components in the new query are initialized /// # let state = world.query_filtered::(); @@ -2258,7 +2305,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// assert_valid_transmute_filtered::, (), Entity, Or<(Changed, With)>>(); /// ``` #[track_caller] - pub fn transmute_lens(&mut self) -> QueryLens<'_, NewD> { + pub fn transmute_lens(&mut self) -> QueryLens<'_, NewD> { self.transmute_lens_filtered::() } @@ -2314,7 +2361,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// - [`transmute_lens`](Self::transmute_lens) to convert to a lens using a mutable borrow of the [`Query`]. #[track_caller] - pub fn transmute_lens_inner(self) -> QueryLens<'w, NewD> { + pub fn transmute_lens_inner(self) -> QueryLens<'w, NewD> { self.transmute_lens_filtered_inner::() } @@ -2328,7 +2375,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// [`Changed`](crate::query::Changed) and [`Spawned`](crate::query::Spawned) will only be respected if they /// are in the type signature. #[track_caller] - pub fn transmute_lens_filtered( + pub fn transmute_lens_filtered( &mut self, ) -> QueryLens<'_, NewD, NewF> { self.reborrow().transmute_lens_filtered_inner() @@ -2349,7 +2396,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// - [`transmute_lens_filtered`](Self::transmute_lens_filtered) to convert to a lens using a mutable borrow of the [`Query`]. #[track_caller] - pub fn transmute_lens_filtered_inner( + pub fn transmute_lens_filtered_inner( self, ) -> QueryLens<'w, NewD, NewF> { let state = self.state.transmute_filtered::(self.world); @@ -2362,7 +2409,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } /// Gets a [`QueryLens`] with the same accesses as the existing query - pub fn as_query_lens(&mut self) -> QueryLens<'_, D> { + pub fn as_query_lens(&mut self) -> QueryLens<'_, D> + where + D: SingleEntityQueryData, + { self.transmute_lens() } @@ -2371,7 +2421,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// # See also /// /// - [`as_query_lens`](Self::as_query_lens) to convert to a lens using a mutable borrow of the [`Query`]. - pub fn into_query_lens(self) -> QueryLens<'w, D> { + pub fn into_query_lens(self) -> QueryLens<'w, D> + where + D: SingleEntityQueryData, + { self.transmute_lens_inner() } @@ -2429,7 +2482,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// Like `transmute_lens` the query terms can be changed with some restrictions. /// See [`Self::transmute_lens`] for more details. - pub fn join<'a, OtherD: QueryData, NewD: QueryData>( + pub fn join<'a, OtherD: QueryData, NewD: SingleEntityQueryData>( &'a mut self, other: &'a mut Query, ) -> QueryLens<'a, NewD> { @@ -2456,7 +2509,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// # See also /// /// - [`join`](Self::join) to join using a mutable borrow of the [`Query`]. - pub fn join_inner( + pub fn join_inner( self, other: Query<'w, '_, OtherD>, ) -> QueryLens<'w, NewD> { @@ -2474,7 +2527,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { 'a, OtherD: QueryData, OtherF: QueryFilter, - NewD: QueryData, + NewD: SingleEntityQueryData, NewF: QueryFilter, >( &'a mut self, @@ -2498,7 +2551,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn join_filtered_inner< OtherD: QueryData, OtherF: QueryFilter, - NewD: QueryData, + NewD: SingleEntityQueryData, NewF: QueryFilter, >( self, @@ -2516,7 +2569,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } } -impl<'w, 's, D: QueryData, F: QueryFilter> IntoIterator for Query<'w, 's, D, F> { +impl<'w, 's, D: IterQueryData, F: QueryFilter> IntoIterator for Query<'w, 's, D, F> { type Item = D::Item<'w, 's>; type IntoIter = QueryIter<'w, 's, D, F>; @@ -2538,7 +2591,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> IntoIterator for &'w Query<'_, 's, D, } } -impl<'w, 's, D: QueryData, F: QueryFilter> IntoIterator for &'w mut Query<'_, 's, D, F> { +impl<'w, 's, D: IterQueryData, F: QueryFilter> IntoIterator for &'w mut Query<'_, 's, D, F> { type Item = D::Item<'w, 's>; type IntoIter = QueryIter<'w, 's, D, F>; @@ -2621,7 +2674,7 @@ impl<'w, 's, Q: QueryData, F: QueryFilter> From<&'s mut QueryLens<'w, Q, F>> } } -impl<'w, 'q, Q: QueryData, F: QueryFilter> From<&'q mut Query<'w, '_, Q, F>> +impl<'w, 'q, Q: SingleEntityQueryData, F: QueryFilter> From<&'q mut Query<'w, '_, Q, F>> for QueryLens<'q, Q, F> { fn from(value: &'q mut Query<'w, '_, Q, F>) -> QueryLens<'q, Q, F> { @@ -2654,12 +2707,12 @@ impl<'w, 'q, Q: QueryData, F: QueryFilter> From<&'q mut Query<'w, '_, Q, F>> /// ``` /// Note that because [`Single`] implements [`Deref`] and [`DerefMut`], methods and fields like `health` can be accessed directly. /// You can also access the underlying data manually, by calling `.deref`/`.deref_mut`, or by using the `*` operator. -pub struct Single<'w, 's, D: QueryData, F: QueryFilter = ()> { +pub struct Single<'w, 's, D: IterQueryData, F: QueryFilter = ()> { pub(crate) item: D::Item<'w, 's>, pub(crate) _filter: PhantomData, } -impl<'w, 's, D: QueryData, F: QueryFilter> Deref for Single<'w, 's, D, F> { +impl<'w, 's, D: IterQueryData, F: QueryFilter> Deref for Single<'w, 's, D, F> { type Target = D::Item<'w, 's>; fn deref(&self) -> &Self::Target { @@ -2667,13 +2720,13 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Deref for Single<'w, 's, D, F> { } } -impl<'w, 's, D: QueryData, F: QueryFilter> DerefMut for Single<'w, 's, D, F> { +impl<'w, 's, D: IterQueryData, F: QueryFilter> DerefMut for Single<'w, 's, D, F> { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.item } } -impl<'w, 's, D: QueryData, F: QueryFilter> Single<'w, 's, D, F> { +impl<'w, 's, D: IterQueryData, F: QueryFilter> Single<'w, 's, D, F> { /// Returns the inner item with ownership. pub fn into_inner(self) -> D::Item<'w, 's> { self.item @@ -2716,7 +2769,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Populated<'w, 's, D, F> { } } -impl<'w, 's, D: QueryData, F: QueryFilter> IntoIterator for Populated<'w, 's, D, F> { +impl<'w, 's, D: IterQueryData, F: QueryFilter> IntoIterator for Populated<'w, 's, D, F> { type Item = as IntoIterator>::Item; type IntoIter = as IntoIterator>::IntoIter; @@ -2736,7 +2789,9 @@ impl<'a, 'w, 's, D: QueryData, F: QueryFilter> IntoIterator for &'a Populated<'w } } -impl<'a, 'w, 's, D: QueryData, F: QueryFilter> IntoIterator for &'a mut Populated<'w, 's, D, F> { +impl<'a, 'w, 's, D: IterQueryData, F: QueryFilter> IntoIterator + for &'a mut Populated<'w, 's, D, F> +{ type Item = <&'a mut Query<'w, 's, D, F> as IntoIterator>::Item; type IntoIter = <&'a mut Query<'w, 's, D, F> as IntoIterator>::IntoIter; diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 909e434c19242..36d4b40f2268d 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -6,8 +6,8 @@ use crate::{ component::{ComponentId, Components}, entity::Entities, query::{ - Access, FilteredAccess, FilteredAccessSet, QueryData, QueryFilter, QuerySingleError, - QueryState, ReadOnlyQueryData, + Access, FilteredAccess, FilteredAccessSet, IterQueryData, QueryData, QueryFilter, + QuerySingleError, QueryState, ReadOnlyQueryData, }, resource::Resource, storage::ResourceData, @@ -223,7 +223,9 @@ pub unsafe trait SystemParam: Sized { /// Creates a new instance of this param's [`State`](SystemParam::State). fn init_state(world: &mut World) -> Self::State; - /// Registers any [`World`] access used by this [`SystemParam`] + /// Registers any [`World`] access used by this [`SystemParam`]. + /// + /// This method must panic if the access would conflict with any existing access in the [`FilteredAccessSet`]. fn init_access( state: &Self::State, system_meta: &mut SystemMeta, @@ -301,6 +303,9 @@ pub unsafe trait SystemParam: Sized { /// /// - The passed [`UnsafeWorldCell`] must have access to any world data registered /// in [`init_access`](SystemParam::init_access). + /// - [`SystemParam::init_access`] must not request conflicting access. + /// If `Self` is `ReadOnlySystemParam`, the access is read-only and can never conflict. + /// Otherwise, [`SystemParam::init_access`] must be called to ensure it does not panic. /// - `world` must be the same [`World`] that was used to initialize [`state`](SystemParam::init_state). unsafe fn get_param<'world, 'state>( state: &'state mut Self::State, @@ -332,7 +337,10 @@ unsafe impl SystemParam for Qu type Item<'w, 's> = Query<'w, 's, D, F>; fn init_state(world: &mut World) -> Self::State { - QueryState::new(world) + // SAFETY: `SystemParam::init_access` calls `QueryState::init_access`, + // `SystemParam::init_access` must be called before `SystemParam::get_param`, + // and we only call methods on the `QueryState` in `get_param`. + unsafe { QueryState::new_unchecked(world) } } fn init_access( @@ -341,15 +349,7 @@ unsafe impl SystemParam for Qu component_access_set: &mut FilteredAccessSet, world: &mut World, ) { - assert_component_access_compatibility( - &system_meta.name, - DebugName::type_name::(), - DebugName::type_name::(), - component_access_set, - &state.component_access, - world, - ); - component_access_set.add(state.component_access.clone()); + state.init_access(Some(system_meta.name()), component_access_set, world.into()); } #[inline] @@ -367,29 +367,9 @@ unsafe impl SystemParam for Qu } } -fn assert_component_access_compatibility( - system_name: &DebugName, - query_type: DebugName, - filter_type: DebugName, - system_access: &FilteredAccessSet, - current: &FilteredAccess, - world: &World, -) { - let conflicts = system_access.get_conflicts_single(current); - if conflicts.is_empty() { - return; - } - let mut accesses = conflicts.format_conflict_list(world); - // Access list may be empty (if access to all components requested) - if !accesses.is_empty() { - accesses.push(' '); - } - panic!("error[B0001]: Query<{}, {}> in system {system_name} accesses component(s) {accesses}in a way that conflicts with a previous system parameter. Consider using `Without` to create disjoint Queries or merging conflicting Queries into a `ParamSet`. See: https://bevy.org/learn/errors/b0001", query_type.shortname(), filter_type.shortname()); -} - // SAFETY: Relevant query ComponentId access is applied to SystemMeta. If // this Query conflicts with any prior access, a panic will occur. -unsafe impl<'a, 'b, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam +unsafe impl<'a, 'b, D: IterQueryData + 'static, F: QueryFilter + 'static> SystemParam for Single<'a, 'b, D, F> { type State = QueryState; @@ -2603,7 +2583,7 @@ unsafe impl SystemParam for FilteredResources<'_, '_> { let combined_access = component_access_set.combined_access(); let conflicts = combined_access.get_conflicts(access); if !conflicts.is_empty() { - let accesses = conflicts.format_conflict_list(world); + let accesses = conflicts.format_conflict_list(world.into()); let system_name = &system_meta.name; panic!("error[B0002]: FilteredResources in system {system_name} accesses resources(s){accesses} in a way that conflicts with a previous system parameter. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002"); } @@ -2652,7 +2632,7 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> { let combined_access = component_access_set.combined_access(); let conflicts = combined_access.get_conflicts(access); if !conflicts.is_empty() { - let accesses = conflicts.format_conflict_list(world); + let accesses = conflicts.format_conflict_list(world.into()); let system_name = &system_meta.name; panic!("error[B0002]: FilteredResourcesMut in system {system_name} accesses resources(s){accesses} in a way that conflicts with a previous system parameter. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002"); } diff --git a/crates/bevy_ecs/src/traversal.rs b/crates/bevy_ecs/src/traversal.rs index 33e54613cb655..4ecfb2b50c0ff 100644 --- a/crates/bevy_ecs/src/traversal.rs +++ b/crates/bevy_ecs/src/traversal.rs @@ -2,7 +2,7 @@ use crate::{ entity::Entity, - query::{ReadOnlyQueryData, ReleaseStateQueryData}, + query::{ReadOnlyQueryData, ReleaseStateQueryData, SingleEntityQueryData}, relationship::Relationship, }; @@ -25,7 +25,9 @@ use crate::{ /// [event propagation]: crate::observer::On::propagate /// [observers]: crate::observer::Observer /// [`EntityEvent`]: crate::event::EntityEvent -pub trait Traversal: ReadOnlyQueryData + ReleaseStateQueryData { +pub trait Traversal: + ReadOnlyQueryData + ReleaseStateQueryData + SingleEntityQueryData +{ /// Returns the next entity to visit. fn traverse(item: Self::Item<'_, '_>, data: &D) -> Option; } diff --git a/crates/bevy_ecs/src/world/entity_access/entity_mut.rs b/crates/bevy_ecs/src/world/entity_access/entity_mut.rs index c7ecdb4adb62f..eb60c9f530bbf 100644 --- a/crates/bevy_ecs/src/world/entity_access/entity_mut.rs +++ b/crates/bevy_ecs/src/world/entity_access/entity_mut.rs @@ -3,7 +3,7 @@ use crate::{ change_detection::{ComponentTicks, MaybeLocation, Tick}, component::{Component, ComponentId, Mutable}, entity::{ContainsEntity, Entity, EntityEquivalent, EntityLocation}, - query::{Access, ReadOnlyQueryData, ReleaseStateQueryData}, + query::{Access, ReadOnlyQueryData, ReleaseStateQueryData, SingleEntityQueryData}, world::{ error::EntityComponentError, unsafe_world_cell::UnsafeEntityCell, DynamicComponentFetch, EntityRef, FilteredEntityMut, FilteredEntityRef, Mut, Ref, @@ -144,13 +144,15 @@ impl<'w> EntityMut<'w> { /// # Panics /// /// If the entity does not have the components required by the query `Q`. - pub fn components(&self) -> Q::Item<'_, 'static> { + pub fn components( + &self, + ) -> Q::Item<'_, 'static> { self.as_readonly().components::() } /// Returns read-only components for the current entity that match the query `Q`, /// or `None` if the entity does not have the components required by the query `Q`. - pub fn get_components( + pub fn get_components( &self, ) -> Option> { self.as_readonly().get_components::() @@ -184,7 +186,7 @@ impl<'w> EntityMut<'w> { /// # Safety /// It is the caller's responsibility to ensure that /// the `QueryData` does not provide aliasing mutable references to the same component. - pub unsafe fn get_components_mut_unchecked( + pub unsafe fn get_components_mut_unchecked( &mut self, ) -> Option> { // SAFETY: Caller the `QueryData` does not provide aliasing mutable references to the same component @@ -219,7 +221,9 @@ impl<'w> EntityMut<'w> { /// # Safety /// It is the caller's responsibility to ensure that /// the `QueryData` does not provide aliasing mutable references to the same component. - pub unsafe fn into_components_mut_unchecked( + pub unsafe fn into_components_mut_unchecked< + Q: ReleaseStateQueryData + SingleEntityQueryData, + >( self, ) -> Option> { // SAFETY: diff --git a/crates/bevy_ecs/src/world/entity_access/entity_ref.rs b/crates/bevy_ecs/src/world/entity_access/entity_ref.rs index 9978c775bc4b7..c6b4ed050b0f0 100644 --- a/crates/bevy_ecs/src/world/entity_access/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_access/entity_ref.rs @@ -3,7 +3,7 @@ use crate::{ change_detection::{ComponentTicks, MaybeLocation, Tick}, component::{Component, ComponentId}, entity::{ContainsEntity, Entity, EntityEquivalent, EntityLocation}, - query::{Access, ReadOnlyQueryData, ReleaseStateQueryData}, + query::{Access, ReadOnlyQueryData, ReleaseStateQueryData, SingleEntityQueryData}, world::{ error::EntityComponentError, unsafe_world_cell::UnsafeEntityCell, DynamicComponentFetch, FilteredEntityRef, Ref, @@ -261,14 +261,16 @@ impl<'w> EntityRef<'w> { /// # Panics /// /// If the entity does not have the components required by the query `Q`. - pub fn components(&self) -> Q::Item<'w, 'static> { + pub fn components( + &self, + ) -> Q::Item<'w, 'static> { self.get_components::() .expect("Query does not match the current entity") } /// Returns read-only components for the current entity that match the query `Q`, /// or `None` if the entity does not have the components required by the query `Q`. - pub fn get_components( + pub fn get_components( &self, ) -> Option> { // SAFETY: diff --git a/crates/bevy_ecs/src/world/entity_access/world_mut.rs b/crates/bevy_ecs/src/world/entity_access/world_mut.rs index f11ff26e16d23..ce2638a666b68 100644 --- a/crates/bevy_ecs/src/world/entity_access/world_mut.rs +++ b/crates/bevy_ecs/src/world/entity_access/world_mut.rs @@ -11,7 +11,9 @@ use crate::{ event::{EntityComponentsTrigger, EntityEvent}, lifecycle::{Despawn, Remove, Replace, DESPAWN, REMOVE, REPLACE}, observer::Observer, - query::{Access, DebugCheckedUnwrap, ReadOnlyQueryData, ReleaseStateQueryData}, + query::{ + Access, DebugCheckedUnwrap, ReadOnlyQueryData, ReleaseStateQueryData, SingleEntityQueryData, + }, relationship::RelationshipHookMode, resource::Resource, storage::{SparseSets, Table}, @@ -254,7 +256,9 @@ impl<'w> EntityWorldMut<'w> { /// If the entity does not have the components required by the query `Q` or if the entity /// has been despawned while this `EntityWorldMut` is still alive. #[inline] - pub fn components(&self) -> Q::Item<'_, 'static> { + pub fn components( + &self, + ) -> Q::Item<'_, 'static> { self.as_readonly().components::() } @@ -265,7 +269,7 @@ impl<'w> EntityWorldMut<'w> { /// /// If the entity has been despawned while this `EntityWorldMut` is still alive. #[inline] - pub fn get_components( + pub fn get_components( &self, ) -> Option> { self.as_readonly().get_components::() @@ -299,7 +303,7 @@ impl<'w> EntityWorldMut<'w> { /// # Safety /// It is the caller's responsibility to ensure that /// the `QueryData` does not provide aliasing mutable references to the same component. - pub unsafe fn get_components_mut_unchecked( + pub unsafe fn get_components_mut_unchecked( &mut self, ) -> Option> { // SAFETY: Caller the `QueryData` does not provide aliasing mutable references to the same component @@ -334,7 +338,9 @@ impl<'w> EntityWorldMut<'w> { /// # Safety /// It is the caller's responsibility to ensure that /// the `QueryData` does not provide aliasing mutable references to the same component. - pub unsafe fn into_components_mut_unchecked( + pub unsafe fn into_components_mut_unchecked< + Q: ReleaseStateQueryData + SingleEntityQueryData, + >( self, ) -> Option> { // SAFETY: Caller the `QueryData` does not provide aliasing mutable references to the same component diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 7adbc00962414..2075eaa117012 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -14,7 +14,7 @@ use crate::{ lifecycle::RemovedComponentMessages, observer::Observers, prelude::Component, - query::{DebugCheckedUnwrap, ReleaseStateQueryData}, + query::{DebugCheckedUnwrap, ReleaseStateQueryData, SingleEntityQueryData}, resource::Resource, storage::{ComponentSparseSet, Storages, Table}, world::RawCommandQueue, @@ -980,7 +980,7 @@ impl<'w> UnsafeEntityCell<'w> { /// - the [`UnsafeEntityCell`] has permission to access the queried data immutably /// - no mutable references to the queried data exist at the same time /// - The `QueryData` does not provide aliasing mutable references to the same component. - pub(crate) unsafe fn get_components( + pub(crate) unsafe fn get_components( &self, ) -> Option> { // SAFETY: World is only used to access query data and initialize query state diff --git a/crates/bevy_render/src/sync_world.rs b/crates/bevy_render/src/sync_world.rs index bac8c4a3c6202..32e68acb4b1dc 100644 --- a/crates/bevy_render/src/sync_world.rs +++ b/crates/bevy_render/src/sync_world.rs @@ -282,8 +282,8 @@ mod render_entities_world_query_impls { component::{ComponentId, Components}, entity::Entity, query::{ - ArchetypeQueryData, FilteredAccess, QueryData, ReadOnlyQueryData, - ReleaseStateQueryData, WorldQuery, + ArchetypeQueryData, FilteredAccess, IterQueryData, QueryData, ReadOnlyQueryData, + ReleaseStateQueryData, SingleEntityQueryData, WorldQuery, }, storage::{Table, TableRow}, world::{unsafe_world_cell::UnsafeWorldCell, World}, @@ -387,9 +387,15 @@ mod render_entities_world_query_impls { } } - // SAFETY: the underlying `Entity` is copied, and no mutable access is provided. + /// SAFETY: access is read only and only on the current entity + unsafe impl IterQueryData for RenderEntity {} + + /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for RenderEntity {} + /// SAFETY: access is only on the current entity + unsafe impl SingleEntityQueryData for RenderEntity {} + impl ArchetypeQueryData for RenderEntity {} impl ReleaseStateQueryData for RenderEntity { @@ -496,9 +502,15 @@ mod render_entities_world_query_impls { } } - // SAFETY: the underlying `Entity` is copied, and no mutable access is provided. + /// SAFETY: access is read only and only on the current entity + unsafe impl IterQueryData for MainEntity {} + + /// SAFETY: access is read only unsafe impl ReadOnlyQueryData for MainEntity {} + /// SAFETY: access is only on the current entity + unsafe impl SingleEntityQueryData for MainEntity {} + impl ArchetypeQueryData for MainEntity {} impl ReleaseStateQueryData for MainEntity { diff --git a/release-content/migration-guides/nested_queries.md b/release-content/migration-guides/nested_queries.md new file mode 100644 index 0000000000000..ff4842904be6c --- /dev/null +++ b/release-content/migration-guides/nested_queries.md @@ -0,0 +1,52 @@ +--- +title: Nested query access +pull_requests: [21557] +--- + +Some queries may now access data from multiple entities. +This will enable richer querying across relations, +and supports accessing resources in queries now that resources are entities. + +However, some query operations are not sound for queries that access multiple entities, +and need additional trait bounds to ensure they are only used soundly. + +An `IterQueryData` bound has been added to iteration methods on `Query`: + +* `iter_mut`/ `iter_unsafe` / `into_iter` +* `iter_many_unique_mut` / `iter_many_unique_unsafe` / `iter_many_unique_inner` +* `get_many_mut` / `get_many_inner` / `get_many_unique_mut` / `get_many_unique_inner` +* `par_iter_mut` / `par_iter_inner` / `par_iter_many_unique_mut` +* `single_mut` / `single_inner` + +`iter`, `iter_many`, `par_iter`, and `single` have no extra bounds, +since read-only queries are always sound to iterate. +`iter_many_mut` and `iter_many_inner` methods have no extra bounds, either, +since they already prohibit concurrent access to multiple entities. + +In addition, a `SingleEntityQueryData` bound has been added to + +* The `EntityRef::get_components` family of methods +* The `Traversal` trait +* The `Query::transmute` and `Query::join` families of methods +* The `QueryIter::sort` family of methods + +All existing query types will satisfy those bounds, but generic code may need to add bounds. + +```rust +// 0.17 +fn generic_func(query: Query) { + for item in &mut query { ... } +} +// 0.18 +fn generic_func(query: Query) { + for item in &mut query { ... } +} +``` + +Conversely, manual implementations of `QueryData` may want to implement `IterQueryData` and `SingleEntityQueryData` if appropriate. + +Finally, two new methods have been added to `WorldQuery`: `init_nested_access` and `update_archetypes`. +Manual implementations of `WorldQuery` should implement those methods as appropriate. +Queries that only access the current entity may leave them empty, +but queries that delegate to other implementations, especially generic ones, +should delegate the new methods as well.