From 4a210c650258edf91916e45b1bbd2e24b9568ddf Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Sun, 29 Sep 2024 10:48:48 -0400 Subject: [PATCH 1/5] Introduce methods on QueryState to obtain a Query, and methods on Query to consume Self and return the original lifetime. --- .../tests/ui/query_to_readonly.rs | 20 +- crates/bevy_ecs/src/query/iter.rs | 56 +- crates/bevy_ecs/src/query/state.rs | 122 +++- crates/bevy_ecs/src/system/mod.rs | 36 +- crates/bevy_ecs/src/system/query.rs | 537 ++++++++++++------ crates/bevy_ecs/src/system/system_param.rs | 30 +- crates/bevy_ecs/src/world/deferred_world.rs | 14 +- crates/bevy_render/src/camera/camera.rs | 2 +- 8 files changed, 540 insertions(+), 277 deletions(-) diff --git a/crates/bevy_ecs/compile_fail/tests/ui/query_to_readonly.rs b/crates/bevy_ecs/compile_fail/tests/ui/query_to_readonly.rs index 7e5a23cb2e361..e8c66cff300b9 100644 --- a/crates/bevy_ecs/compile_fail/tests/ui/query_to_readonly.rs +++ b/crates/bevy_ecs/compile_fail/tests/ui/query_to_readonly.rs @@ -6,29 +6,29 @@ struct Foo; fn for_loops(mut query: Query<&mut Foo>) { // this should fail to compile for _ in query.iter_mut() { - for _ in query.to_readonly().iter() {} + for _ in query.as_readonly().iter() {} //~^ E0502 } // this should fail to compile - for _ in query.to_readonly().iter() { + for _ in query.as_readonly().iter() { for _ in query.iter_mut() {} //~^ E0502 } // this should *not* fail to compile - for _ in query.to_readonly().iter() { - for _ in query.to_readonly().iter() {} + for _ in query.as_readonly().iter() { + for _ in query.as_readonly().iter() {} } // this should *not* fail to compile - for _ in query.to_readonly().iter() { + for _ in query.as_readonly().iter() { for _ in query.iter() {} } // this should *not* fail to compile for _ in query.iter() { - for _ in query.to_readonly().iter() {} + for _ in query.as_readonly().iter() {} } } @@ -38,11 +38,11 @@ fn single_mut_query(mut query: Query<&mut Foo>) { let mut mut_foo = query.single_mut(); // This solves "temporary value dropped while borrowed" - let readonly_query = query.to_readonly(); + let readonly_query = query.as_readonly(); //~^ E0502 let ref_foo = readonly_query.single(); - + *mut_foo = Foo; println!("{ref_foo:?}"); @@ -51,7 +51,7 @@ fn single_mut_query(mut query: Query<&mut Foo>) { // this should fail to compile { // This solves "temporary value dropped while borrowed" - let readonly_query = query.to_readonly(); + let readonly_query = query.as_readonly(); let ref_foo = readonly_query.single(); @@ -66,7 +66,7 @@ fn single_mut_query(mut query: Query<&mut Foo>) { // this should *not* fail to compile { // This solves "temporary value dropped while borrowed" - let readonly_query = query.to_readonly(); + let readonly_query = query.as_readonly(); let readonly_foo = readonly_query.single(); diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index bf4c5432546f9..d2cd23c71df0d 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -508,13 +508,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: // `self.world` has permission to access the required components. // The original query iter has not been iterated on, so no items are aliased from it. - let query_lens = unsafe { - query_lens_state.iter_unchecked_manual( - world, - world.last_change_tick(), - world.change_tick(), - ) - }; + let query_lens = unsafe { query_lens_state.query_unchecked_manual(world) }.into_iter(); let mut keyed_query: Vec<_> = query_lens .map(|(key, entity)| (key, NeutralOrd(entity))) .collect(); @@ -600,13 +594,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: // `self.world` has permission to access the required components. // The original query iter has not been iterated on, so no items are aliased from it. - let query_lens = unsafe { - query_lens_state.iter_unchecked_manual( - world, - world.last_change_tick(), - world.change_tick(), - ) - }; + let query_lens = unsafe { query_lens_state.query_unchecked_manual(world) }.into_iter(); let mut keyed_query: Vec<_> = query_lens .map(|(key, entity)| (key, NeutralOrd(entity))) .collect(); @@ -700,13 +688,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: // `self.world` has permission to access the required components. // The original query iter has not been iterated on, so no items are aliased from it. - let query_lens = unsafe { - query_lens_state.iter_unchecked_manual( - world, - world.last_change_tick(), - world.change_tick(), - ) - }; + let query_lens = unsafe { query_lens_state.query_unchecked_manual(world) }.into_iter(); let mut keyed_query: Vec<_> = query_lens.collect(); keyed_query.sort_by(|(key_1, _), (key_2, _)| compare(key_1, key_2)); let entity_iter = keyed_query.into_iter().map(|(.., entity)| entity); @@ -766,13 +748,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: // `self.world` has permission to access the required components. // The original query iter has not been iterated on, so no items are aliased from it. - let query_lens = unsafe { - query_lens_state.iter_unchecked_manual( - world, - world.last_change_tick(), - world.change_tick(), - ) - }; + let query_lens = unsafe { query_lens_state.query_unchecked_manual(world) }.into_iter(); let mut keyed_query: Vec<_> = query_lens.collect(); keyed_query.sort_by(|(key_1, _), (key_2, _)| compare(key_1, key_2)); let entity_iter = keyed_query.into_iter().map(|(.., entity)| entity); @@ -895,13 +871,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: // `self.world` has permission to access the required components. // The original query iter has not been iterated on, so no items are aliased from it. - let query_lens = unsafe { - query_lens_state.iter_unchecked_manual( - world, - world.last_change_tick(), - world.change_tick(), - ) - }; + let query_lens = unsafe { query_lens_state.query_unchecked_manual(world) }.into_iter(); let mut keyed_query: Vec<_> = query_lens.collect(); keyed_query.sort_by_key(|(lens, _)| f(lens)); let entity_iter = keyed_query.into_iter().map(|(.., entity)| entity); @@ -964,13 +934,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: // `self.world` has permission to access the required components. // The original query iter has not been iterated on, so no items are aliased from it. - let query_lens = unsafe { - query_lens_state.iter_unchecked_manual( - world, - world.last_change_tick(), - world.change_tick(), - ) - }; + let query_lens = unsafe { query_lens_state.query_unchecked_manual(world) }.into_iter(); let mut keyed_query: Vec<_> = query_lens.collect(); keyed_query.sort_unstable_by_key(|(lens, _)| f(lens)); let entity_iter = keyed_query.into_iter().map(|(.., entity)| entity); @@ -1033,13 +997,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: // `self.world` has permission to access the required components. // The original query iter has not been iterated on, so no items are aliased from it. - let query_lens = unsafe { - query_lens_state.iter_unchecked_manual( - world, - world.last_change_tick(), - world.change_tick(), - ) - }; + let query_lens = unsafe { query_lens_state.query_unchecked_manual(world) }.into_iter(); let mut keyed_query: Vec<_> = query_lens.collect(); keyed_query.sort_by_cached_key(|(lens, _)| f(lens)); let entity_iter = keyed_query.into_iter().map(|(.., entity)| entity); diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 69b67368fe4f3..0a1be0ac24a96 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -8,6 +8,7 @@ use crate::{ Access, DebugCheckedUnwrap, FilteredAccess, QueryCombinationIter, QueryIter, QueryParIter, }, storage::{SparseSetIndex, TableId}, + system::Query, world::{unsafe_world_cell::UnsafeWorldCell, World, WorldId}, }; use bevy_utils::tracing::warn; @@ -150,9 +151,7 @@ impl QueryState { pub fn matched_archetypes(&self) -> impl Iterator + '_ { self.matched_archetypes.ones().map(ArchetypeId::new) } -} -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); @@ -251,6 +250,123 @@ impl QueryState { state } + /// Creates a [`Query`] from the given [`QueryState`] and [`World`]. + /// + /// This can only be called for read-only queries, see [`Self::query_mut`] for write-queries. + pub fn query<'w, 's>(&'s mut self, world: &'w World) -> Query<'w, 's, D::ReadOnly, F> { + self.update_archetypes(world); + self.query_manual(world) + } + + /// Creates a [`Query`] from the given [`QueryState`] and [`World`]. + /// + /// This method is slightly more efficient than [`QueryState::query`] in some situations, since + /// it does not update this instance's internal cache. The resulting query may skip an entity that + /// belongs to an archetype that has not been cached. + /// + /// To ensure that the cache is up to date, call [`QueryState::update_archetypes`] before this method. + /// The cache is also updated in [`QueryState::new`], `QueryState::get`, or any method with mutable + /// access to `self`. + /// + /// This can only be called for read-only queries, see [`Self::query_mut`] for mutable queries. + pub fn query_manual<'w, 's>(&'s self, world: &'w World) -> Query<'w, 's, D::ReadOnly, F> { + // SAFETY: We have read access to the entire world, and we call `as_readonly()` so the query only performs read access. + unsafe { + self.as_readonly() + .query_unchecked_manual(world.as_unsafe_world_cell_readonly()) + } + } + + /// Creates a [`Query`] from the given [`QueryState`] and [`World`]. + pub fn query_mut<'w, 's>(&'s mut self, world: &'w mut World) -> Query<'w, 's, D, F> { + let last_run = world.last_change_tick(); + let this_run = world.change_tick(); + // SAFETY: We have exclusive access to the entire world. + unsafe { self.query_unchecked_with_ticks(world.as_unsafe_world_cell(), last_run, this_run) } + } + + /// Creates a [`Query`] from the given [`QueryState`] and [`World`]. + /// + /// # Safety + /// + /// This does not check for mutable query correctness. To be safe, make sure mutable queries + /// have unique access to the components they query. + pub unsafe fn query_unchecked<'w, 's>( + &'s mut self, + world: UnsafeWorldCell<'w>, + ) -> Query<'w, 's, D, F> { + self.update_archetypes_unsafe_world_cell(world); + self.query_unchecked_manual(world) + } + + /// Creates a [`Query`] from the given [`QueryState`] and [`World`]. + /// + /// This method is slightly more efficient than [`QueryState::query_unchecked`] in some situations, since + /// it does not update this instance's internal cache. The resulting query may skip an entity that + /// belongs to an archetype that has not been cached. + /// + /// To ensure that the cache is up to date, call [`QueryState::update_archetypes`] before this method. + /// The cache is also updated in [`QueryState::new`], `QueryState::get`, or any method with mutable + /// access to `self`. + /// + /// # Safety + /// + /// This does not check for mutable query correctness. To be safe, make sure mutable queries + /// have unique access to the components they query. + pub unsafe fn query_unchecked_manual<'w, 's>( + &'s self, + world: UnsafeWorldCell<'w>, + ) -> Query<'w, 's, D, F> { + let last_run = world.last_change_tick(); + let this_run = world.change_tick(); + // SAFETY: The caller ensured we have the correct access to the world. + unsafe { self.query_unchecked_manual_with_ticks(world, last_run, this_run) } + } + + /// Creates a [`Query`] from the given [`QueryState`] and [`World`]. + /// + /// # Safety + /// + /// This does not check for mutable query correctness. To be safe, make sure mutable queries + /// have unique access to the components they query. + pub unsafe fn query_unchecked_with_ticks<'w, 's>( + &'s mut self, + world: UnsafeWorldCell<'w>, + last_run: Tick, + this_run: Tick, + ) -> Query<'w, 's, D, F> { + self.update_archetypes_unsafe_world_cell(world); + // SAFETY: The caller ensured we have the correct access to the world. + unsafe { self.query_unchecked_manual_with_ticks(world, last_run, this_run) } + } + + /// Creates a [`Query`] from the given [`QueryState`] and [`World`]. + /// + /// This method is slightly more efficient than [`QueryState::query_unchecked_with_ticks`] in some situations, since + /// it does not update this instance's internal cache. The resulting query may skip an entity that + /// belongs to an archetype that has not been cached. + /// + /// To ensure that the cache is up to date, call [`QueryState::update_archetypes`] before this method. + /// The cache is also updated in [`QueryState::new`], `QueryState::get`, or any method with mutable + /// access to `self`. + /// + /// # Safety + /// + /// This does not check for mutable query correctness. To be safe, make sure mutable queries + /// have unique access to the components they query. + pub unsafe fn query_unchecked_manual_with_ticks<'w, 's>( + &'s self, + world: UnsafeWorldCell<'w>, + last_run: Tick, + this_run: Tick, + ) -> Query<'w, 's, D, F> { + self.validate_world(world.id()); + // SAFETY: + // - The caller ensured we have the correct access to the world. + // - `validate_world` did not panic, so the world matches. + unsafe { Query::new(world, self, last_run, this_run) } + } + /// Checks if the query is empty for the given [`World`], where the last change and current tick are given. /// /// This is equivalent to `self.iter().next().is_none()`, and thus the worst case runtime will be `O(n)` @@ -557,7 +673,7 @@ impl QueryState { /// You should not call [`update_archetypes`](Self::update_archetypes) on the returned [`QueryState`] as the result will be unpredictable. /// 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`](crate::system::Query). + /// best used through a [`Query`] pub fn transmute<'a, NewD: QueryData>( &self, world: impl Into>, diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index dd6a950f72a8e..de1aa88ea85c9 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -317,7 +317,7 @@ mod tests { archetype::{ArchetypeComponentId, Archetypes}, bundle::Bundles, change_detection::DetectChanges, - component::{Component, Components, Tick}, + component::{Component, Components}, entity::{Entities, Entity}, prelude::{AnyOf, EntityRef}, query::{Added, Changed, Or, With, Without}, @@ -1331,7 +1331,7 @@ mod tests { fn mutable_query(mut query: Query<&mut A>) { for _ in &mut query {} - immutable_query(query.to_readonly()); + immutable_query(query.as_readonly()); } fn immutable_query(_: Query<&A>) {} @@ -1346,7 +1346,7 @@ mod tests { fn mutable_query(mut query: Query>) { for _ in &mut query {} - immutable_query(query.to_readonly()); + immutable_query(query.as_readonly()); } fn immutable_query(_: Query>) {} @@ -1361,7 +1361,7 @@ mod tests { fn mutable_query(mut query: Query<(&mut A, &B)>) { for _ in &mut query {} - immutable_query(query.to_readonly()); + immutable_query(query.as_readonly()); } fn immutable_query(_: Query<(&A, &B)>) {} @@ -1376,7 +1376,7 @@ mod tests { fn mutable_query(mut query: Query<(&mut A, &mut B)>) { for _ in &mut query {} - immutable_query(query.to_readonly()); + immutable_query(query.as_readonly()); } fn immutable_query(_: Query<(&A, &B)>) {} @@ -1391,7 +1391,7 @@ mod tests { fn mutable_query(mut query: Query<(&mut A, &mut B), With>) { for _ in &mut query {} - immutable_query(query.to_readonly()); + immutable_query(query.as_readonly()); } fn immutable_query(_: Query<(&A, &B), With>) {} @@ -1406,7 +1406,7 @@ mod tests { fn mutable_query(mut query: Query<(&mut A, &mut B), Without>) { for _ in &mut query {} - immutable_query(query.to_readonly()); + immutable_query(query.as_readonly()); } fn immutable_query(_: Query<(&A, &B), Without>) {} @@ -1421,7 +1421,7 @@ mod tests { fn mutable_query(mut query: Query<(&mut A, &mut B), Added>) { for _ in &mut query {} - immutable_query(query.to_readonly()); + immutable_query(query.as_readonly()); } fn immutable_query(_: Query<(&A, &B), Added>) {} @@ -1436,7 +1436,7 @@ mod tests { fn mutable_query(mut query: Query<(&mut A, &mut B), Changed>) { for _ in &mut query {} - immutable_query(query.to_readonly()); + immutable_query(query.as_readonly()); } fn immutable_query(_: Query<(&A, &B), Changed>) {} @@ -1542,24 +1542,6 @@ mod tests { }); } - #[test] - #[should_panic = "Encountered a mismatched World."] - fn query_validates_world_id() { - let mut world1 = World::new(); - let world2 = World::new(); - let qstate = world1.query::<()>(); - // SAFETY: doesnt access anything - let query = unsafe { - Query::new( - world2.as_unsafe_world_cell_readonly(), - &qstate, - Tick::new(0), - Tick::new(0), - ) - }; - query.iter(); - } - #[test] #[should_panic] fn assert_system_does_not_conflict() { diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index f0116e60da50c..7dbccd42dca45 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -367,6 +367,14 @@ pub struct Query<'world, 'state, D: QueryData, F: QueryFilter = ()> { this_run: Tick, } +impl Clone for Query<'_, '_, D, F> { + fn clone(&self) -> Self { + *self + } +} + +impl Copy for Query<'_, '_, D, F> {} + impl core::fmt::Debug for Query<'_, '_, D, F> { fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { f.debug_struct("Query") @@ -382,14 +390,11 @@ impl core::fmt::Debug for Query<'_, '_, D, F> { impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// Creates a new query. /// - /// # Panics - /// - /// This will panic if the world used to create `state` is not `world`. - /// /// # Safety /// - /// This will create a query that could violate memory safety rules. Make sure that this is only - /// called in ways that ensure the queries have unique mutable access. + /// * This will create a query that could violate memory safety rules. Make sure that this is only + /// called in ways that ensure the queries have unique mutable access. + /// * `world` must be the world used to create `state`. #[inline] pub(crate) unsafe fn new( world: UnsafeWorldCell<'w>, @@ -397,8 +402,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { last_run: Tick, this_run: Tick, ) -> Self { - state.validate_world(world.id()); - Self { world, state, @@ -412,9 +415,30 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// For example, `Query<(&mut D1, &D2, &mut D3), With>` will become `Query<(&D1, &D2, &D3), With>`. /// This can be useful when working around the borrow checker, /// or reusing functionality between systems via functions that accept query types. - pub fn to_readonly(&self) -> Query<'_, 's, D::ReadOnly, F> { + /// + /// # See also + /// + /// [`into_readonly`](Self::into_readonly) for a version that consumes the `Query` to return one with the full `'world` lifetime. + pub fn as_readonly(&self) -> Query<'_, 's, D::ReadOnly, F> { + // SAFETY: The reborrowed query is converted to read-only, so it cannot perform mutable access, + // and the original query is held with a shared borrow, so it cannot perform mutable access either. + unsafe { self.reborrow_unsafe() }.into_readonly() + } + + /// Returns another `Query` from this that fetches the read-only version of the query items. + /// + /// For example, `Query<(&mut D1, &D2, &mut D3), With>` will become `Query<(&D1, &D2, &D3), With>`. + /// This can be useful when working around the borrow checker, + /// or reusing functionality between systems via functions that accept query types. + /// + /// # See also + /// + /// [`as_readonly`](Self::as_readonly) for a version that borrows the `Query` instead of consuming it. + pub fn into_readonly(self) -> Query<'w, 's, D::ReadOnly, F> { let new_state = self.state.as_readonly(); - // SAFETY: This is memory safe because it turns the query immutable. + // SAFETY: + // - This is memory safe because it turns the query immutable. + // - The world matches because it was the same one used to construct self. unsafe { Query::new(self.world, new_state, self.last_run, self.this_run) } } @@ -445,6 +469,24 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn reborrow(&mut self) -> Query<'_, 's, D, F> { // SAFETY: this query is exclusively borrowed while the new one exists, so // no overlapping access can occur. + unsafe { self.reborrow_unsafe() } + } + + /// Returns a new `Query` reborrowing the access from this one. + /// The current query will still be usable while the new one exists, but must not be used in a way that violates aliasing. + /// + /// # Safety + /// + /// This function makes it possible to violate Rust's aliasing guarantees. + /// You must make sure this call does not result in a mutable or shared reference to a component with a mutable reference. + /// + /// # See also + /// + /// - [`reborrow`](Self::reborrow) for the safe versions. + pub unsafe fn reborrow_unsafe(&self) -> Query<'_, 's, D, F> { + // SAFETY: + // - This is memory safe because the caller ensures that there are no conflicting references. + // - The world matches because it was the same one used to construct self. unsafe { Query::new(self.world, self.state, self.last_run, self.this_run) } } @@ -476,14 +518,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// [`iter_mut`](Self::iter_mut) for mutable query items. #[inline] pub fn iter(&self) -> QueryIter<'_, 's, D::ReadOnly, F> { - // SAFETY: - // - `self.world` has permission to access the required components. - // - The query is read-only, so it can be aliased even if it was originally mutable. - unsafe { - self.state - .as_readonly() - .iter_unchecked_manual(self.world, self.last_run, self.this_run) - } + self.as_readonly().into_iter() } /// Returns an [`Iterator`] over the query items. @@ -514,11 +549,7 @@ 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> { - // SAFETY: `self.world` has permission to access the required components. - unsafe { - self.state - .iter_unchecked_manual(self.world, self.last_run, self.this_run) - } + self.reborrow().into_iter() } /// Returns a [`QueryCombinationIter`] over all combinations of `K` read-only query items without repetition. @@ -543,20 +574,12 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// # See also /// /// - [`iter_combinations_mut`](Self::iter_combinations_mut) for mutable query item combinations. + /// - [`iter_combinations_inner`](Self::iter_combinations_inner) for mutable query item combinations with the full `'world` lifetime. #[inline] pub fn iter_combinations( &self, ) -> QueryCombinationIter<'_, 's, D::ReadOnly, F, K> { - // SAFETY: - // - `self.world` has permission to access the required components. - // - The query is read-only, so it can be aliased even if it was originally mutable. - unsafe { - self.state.as_readonly().iter_combinations_unchecked_manual( - self.world, - self.last_run, - self.this_run, - ) - } + self.as_readonly().iter_combinations_inner() } /// Returns a [`QueryCombinationIter`] over all combinations of `K` query items without repetition. @@ -581,10 +604,39 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// # See also /// /// - [`iter_combinations`](Self::iter_combinations) for read-only query item combinations. + /// - [`iter_combinations_inner`](Self::iter_combinations_inner) for mutable query item combinations with the full `'world` lifetime. #[inline] pub fn iter_combinations_mut( &mut self, ) -> QueryCombinationIter<'_, 's, D, F, K> { + self.reborrow().iter_combinations_inner() + } + + /// Returns a [`QueryCombinationIter`] over all combinations of `K` query items without repetition. + /// + /// This iterator is always guaranteed to return results from each unique pair of matching entities. + /// Iteration order is not guaranteed. + /// + /// # Example + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # #[derive(Component)] + /// # struct ComponentA; + /// fn some_system(query: Query<&mut ComponentA>) { + /// let mut combinations = query.iter_combinations_inner(); + /// while let Some([mut a1, mut a2]) = combinations.fetch_next() { + /// // mutably access components data + /// } + /// } + /// ``` + /// + /// # See also + /// + /// - [`iter_combinations`](Self::iter_combinations) for read-only query item combinations. + /// - [`iter_combinations_mut`](Self::iter_combinations_mut) for mutable query item combinations. + #[inline] + pub fn iter_combinations_inner(self) -> QueryCombinationIter<'w, 's, D, F, K> { // SAFETY: `self.world` has permission to access the required components. unsafe { self.state @@ -628,22 +680,13 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// # See also /// /// - [`iter_many_mut`](Self::iter_many_mut) to get mutable query items. + /// - [`iter_many_inner`](Self::iter_many_inner) to get mutable query items with the full `'world` lifetime. #[inline] pub fn iter_many>>( &self, entities: EntityList, ) -> QueryManyIter<'_, 's, D::ReadOnly, F, EntityList::IntoIter> { - // SAFETY: - // - `self.world` has permission to access the required components. - // - The query is read-only, so it can be aliased even if it was originally mutable. - unsafe { - self.state.as_readonly().iter_many_unchecked_manual( - entities, - self.world, - self.last_run, - self.this_run, - ) - } + self.as_readonly().iter_many_inner(entities) } /// Returns an iterator over the query items generated from an [`Entity`] list. @@ -679,11 +722,32 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// } /// # bevy_ecs::system::assert_is_system(system); /// ``` + /// # See also + /// + /// - [`iter_many`](Self::iter_many) to get read-only query items. + /// - [`iter_many_inner`](Self::iter_many_inner) to get mutable query items with the full `'world` lifetime. #[inline] pub fn iter_many_mut>>( &mut self, entities: EntityList, ) -> QueryManyIter<'_, 's, D, F, EntityList::IntoIter> { + self.reborrow().iter_many_inner(entities) + } + + /// Returns an iterator over the query items generated from an [`Entity`] list. + /// + /// Items are returned in the order of the list of entities, and may not be unique if the input + /// doesn't guarantee uniqueness. Entities that don't match the query are skipped. + /// + /// # See also + /// + /// - [`iter_many`](Self::iter_many) to get read-only query items. + /// - [`iter_many_mut`](Self::iter_many_mut) to get mutable query items. + #[inline] + pub fn iter_many_inner>>( + self, + entities: EntityList, + ) -> QueryManyIter<'w, 's, D, F, EntityList::IntoIter> { // SAFETY: `self.world` has permission to access the required components. unsafe { self.state.iter_many_unchecked_manual( @@ -710,13 +774,8 @@ 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> { - // SAFETY: - // - `self.world` has permission to access the required components. - // - The caller ensures that this operation will not result in any aliased mutable accesses. - unsafe { - self.state - .iter_unchecked_manual(self.world, self.last_run, self.this_run) - } + // SAFETY: The caller promises that this will not result in multiple mutable references. + unsafe { self.reborrow_unsafe() }.into_iter() } /// Iterates over all possible combinations of `K` query items without repetition. @@ -736,13 +795,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub unsafe fn iter_combinations_unsafe( &self, ) -> QueryCombinationIter<'_, 's, D, F, K> { - // SAFETY: - // - `self.world` has permission to access the required components. - // - The caller ensures that this operation will not result in any aliased mutable accesses. - unsafe { - self.state - .iter_combinations_unchecked_manual(self.world, self.last_run, self.this_run) - } + // SAFETY: The caller promises that this will not result in multiple mutable references. + unsafe { self.reborrow_unsafe() }.iter_combinations_inner() } /// Returns an [`Iterator`] over the query items generated from an [`Entity`] list. @@ -763,17 +817,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { &self, entities: EntityList, ) -> QueryManyIter<'_, 's, D, F, EntityList::IntoIter> { - // SAFETY: - // - `self.world` has permission to access the required components. - // - The caller ensures that this operation will not result in any aliased mutable accesses. - unsafe { - self.state.iter_many_unchecked_manual( - entities, - self.world, - self.last_run, - self.this_run, - ) - } + // SAFETY: The caller promises that this will not result in multiple mutable references. + unsafe { self.reborrow_unsafe() }.iter_many_inner(entities) } /// Returns a parallel iterator over the query results for the given [`World`]. @@ -793,13 +838,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// [`World`]: crate::world::World #[inline] pub fn par_iter(&self) -> QueryParIter<'_, '_, D::ReadOnly, F> { - QueryParIter { - world: self.world, - state: self.state.as_readonly(), - last_run: self.last_run, - this_run: self.this_run, - batching_strategy: BatchingStrategy::new(), - } + self.as_readonly().par_iter_inner() } /// Returns a parallel iterator over the query results for the given [`World`]. @@ -834,6 +873,36 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// [`World`]: crate::world::World #[inline] pub fn par_iter_mut(&mut self) -> QueryParIter<'_, '_, D, F> { + self.reborrow().par_iter_inner() + } + + /// Returns a parallel iterator over the query results for the given [`World`](crate::world::World). + /// + /// This parallel iterator is always guaranteed to return results from each matching entity once and + /// only once. Iteration order and thread assignment is not guaranteed. + /// + /// If the `multithreaded` feature is disabled, iterating with this operates identically to [`Iterator::for_each`] + /// on [`QueryIter`]. + /// + /// # Example + /// + /// Here, the `gravity_system` updates the `Velocity` component of every entity that contains it: + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # + /// # #[derive(Component)] + /// # struct Velocity { x: f32, y: f32, z: f32 } + /// fn gravity_system(query: Query<&mut Velocity>) { + /// const DELTA: f32 = 1.0 / 60.0; + /// query.par_iter_inner().for_each(|mut velocity| { + /// velocity.y -= 9.8 * DELTA; + /// }); + /// } + /// # bevy_ecs::system::assert_is_system(gravity_system); + /// ``` + #[inline] + pub fn par_iter_inner(self) -> QueryParIter<'w, 's, D, F> { QueryParIter { world: self.world, state: self.state, @@ -878,16 +947,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// - [`get_mut`](Self::get_mut) to get a mutable query item. #[inline] pub fn get(&self, entity: Entity) -> Result, QueryEntityError> { - // SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict - unsafe { - self.state.as_readonly().get_unchecked_manual( - self.world, - entity, - self.last_run, - self.this_run, - ) - } + self.as_readonly().get_inner(entity) } /// Returns the read-only query items for the given array of [`Entity`]. @@ -905,13 +965,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { &self, entities: [Entity; N], ) -> Result<[ROQueryItem<'_, D>; N], QueryEntityError> { - // SAFETY: - // - `&self` ensures there is no mutable access to any components accessible to this query. - // - `self.world` matches `self.state`. - unsafe { - self.state - .get_many_read_only_manual(self.world, entities, self.last_run, self.this_run) - } + self.as_readonly().get_many_inner(entities) } /// Returns the read-only query items for the given array of [`Entity`]. @@ -995,6 +1049,20 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// - [`get`](Self::get) to get a read-only query item. #[inline] pub fn get_mut(&mut self, entity: Entity) -> Result, QueryEntityError> { + self.reborrow().get_inner(entity) + } + + /// Returns the query item for the given [`Entity`], with the actual "inner" world lifetime, by consuming the [`Query`]. + /// + /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is returned instead. + /// + /// This is always guaranteed to run in `O(1)` time. + /// + /// # See also + /// + /// - [`get_mut`](Self::get_mut) to get the item using a mutable borrow of the [`Query`]. + #[inline] + pub fn get_inner(self, entity: Entity) -> Result, QueryEntityError<'w>> { // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { @@ -1017,6 +1085,23 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { &mut self, entities: [Entity; N], ) -> Result<[D::Item<'_>; N], QueryEntityError> { + self.reborrow().get_many_inner(entities) + } + + /// Returns the query items for the given array of [`Entity`]. + /// + /// The returned query items are in the same order as the input. + /// In case of a nonexisting entity, duplicate entities or mismatched component, a [`QueryEntityError`] is returned instead. + /// + /// # See also + /// + /// - [`get_many`](Self::get_many) to get read-only query items. + /// - [`get_many_mut`](Self::get_many_mut) to get items using a mutable reference. + #[inline] + pub fn get_many_inner( + self, + entities: [Entity; N], + ) -> Result<[D::Item<'w>; N], QueryEntityError<'w>> { // SAFETY: scheduler ensures safe Query world access unsafe { self.state @@ -1097,12 +1182,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// - [`get_mut`](Self::get_mut) for the safe version. #[inline] pub unsafe fn get_unchecked(&self, entity: Entity) -> Result, QueryEntityError> { - // SEMI-SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict - unsafe { - self.state - .get_unchecked_manual(self.world, entity, self.last_run, self.this_run) - } + // SAFETY: The caller promises that this will not result in multiple mutable references. + unsafe { self.reborrow_unsafe() }.get_inner(entity) } /// Returns a single read-only query item when there is exactly one entity matching the query. @@ -1168,16 +1249,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// - [`single`](Self::single) for the panicking version. #[inline] pub fn get_single(&self) -> Result, QuerySingleError> { - // SAFETY: - // the query ensures that the components it accesses are not mutably accessible somewhere else - // and the query is read only. - unsafe { - self.state.as_readonly().get_single_unchecked_manual( - self.world, - self.last_run, - self.this_run, - ) - } + self.as_readonly().get_single_inner() } /// Returns a single query item when there is exactly one entity matching the query. @@ -1239,6 +1311,36 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// - [`single_mut`](Self::single_mut) for the panicking version. #[inline] pub fn get_single_mut(&mut self) -> Result, QuerySingleError> { + self.reborrow().get_single_inner() + } + + /// Returns a single query item when there is exactly one entity matching the query. + /// + /// If the number of query items is not exactly one, a [`QuerySingleError`] is returned instead. + /// + /// # Example + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # + /// # #[derive(Component)] + /// # struct Player; + /// # #[derive(Component)] + /// # struct Health(u32); + /// # + /// fn regenerate_player_health_system(query: Query<&mut Health, With>) { + /// let mut health = query.get_single_inner().expect("Error: Could not find a single player."); + /// health.0 += 1; + /// } + /// # bevy_ecs::system::assert_is_system(regenerate_player_health_system); + /// ``` + /// + /// # See also + /// + /// - [`get_single`](Self::get_single) to get the read-only query item. + /// - [`get_single_mut`](Self::get_single_mut) to get items using a mutable reference. + #[inline] + pub fn get_single_inner(self) -> Result, QuerySingleError> { // SAFETY: // the query ensures mutable access to the components it accesses, and the query // is uniquely borrowed @@ -1392,6 +1494,79 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { self.transmute_lens_filtered::() } + /// Returns a [`QueryLens`] that can be used to get a query with a more general fetch. + /// + /// For example, this can transform a `Query<(&A, &mut B)>` to a `Query<&B>`. + /// This can be useful for passing the query to another function. Note that since + /// filter terms are dropped, non-archetypal filters like [`Added`](crate::query::Added) and + /// [`Changed`](crate::query::Changed) will not be respected. To maintain or change filter + /// terms see [`Self::transmute_lens_filtered`] + /// + /// ## Panics + /// + /// This will panic if `NewD` is not a subset of the original fetch `Q` + /// + /// ## Example + /// + /// ```rust + /// # use bevy_ecs::prelude::*; + /// # use bevy_ecs::system::QueryLens; + /// # + /// # #[derive(Component)] + /// # struct A(usize); + /// # + /// # #[derive(Component)] + /// # struct B(usize); + /// # + /// # let mut world = World::new(); + /// # + /// # world.spawn((A(10), B(5))); + /// # + /// fn reusable_function(mut lens: QueryLens<&A>) { + /// assert_eq!(lens.query().single().0, 10); + /// } + /// + /// // We can use the function in a system that takes the exact query. + /// fn system_1(query: Query<&A>) { + /// reusable_function(query.into_query_lens()); + /// } + /// + /// // We can also use it with a query that does not match exactly + /// // by transmuting it. + /// fn system_2(query: Query<(&mut A, &B)>) { + /// let mut lens = query.transmute_lens_inner::<&A>(); + /// reusable_function(lens); + /// } + /// + /// # let mut schedule = Schedule::default(); + /// # schedule.add_systems((system_1, system_2)); + /// # schedule.run(&mut world); + /// ``` + /// + /// ## Allowed Transmutes + /// + /// Besides removing parameters from the query, you can also + /// make limited changes to the types of parameters. + /// + /// * Can always add/remove [`Entity`] + /// * Can always add/remove [`EntityLocation`] + /// * Can always add/remove [`&Archetype`] + /// * `Ref` <-> `&T` + /// * `&mut T` -> `&T` + /// * `&mut T` -> `Ref` + /// * [`EntityMut`](crate::world::EntityMut) -> [`EntityRef`](crate::world::EntityRef) + /// + /// [`EntityLocation`]: crate::entity::EntityLocation + /// [`&Archetype`]: crate::archetype::Archetype + /// + /// # See also + /// + /// - [`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> { + self.transmute_lens_filtered_inner::() + } + /// Equivalent to [`Self::transmute_lens`] but also includes a [`QueryFilter`] type. /// /// Note that the lens will iterate the same tables and archetypes as the original query. This means that @@ -1402,6 +1577,23 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn transmute_lens_filtered( &mut self, ) -> QueryLens<'_, NewD, NewF> { + self.reborrow().transmute_lens_filtered_inner() + } + + /// Equivalent to [`Self::transmute_lens_inner`] but also includes a [`QueryFilter`] type. + /// + /// Note that the lens will iterate the same tables and archetypes as the original query. This means that + /// additional archetypal query terms like [`With`](crate::query::With) and [`Without`](crate::query::Without) + /// will not necessarily be respected and non-archetypal terms like [`Added`](crate::query::Added) and + /// [`Changed`](crate::query::Changed) will only be respected if they are in the type signature. + /// + /// # See also + /// + /// - [`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( + self, + ) -> QueryLens<'w, NewD, NewF> { let state = self.state.transmute_filtered::(self.world); QueryLens { world: self.world, @@ -1416,6 +1608,15 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { self.transmute_lens() } + /// Gets a [`QueryLens`] with the same accesses as the existing query + /// + /// # 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> { + self.transmute_lens_inner() + } + /// Returns a [`QueryLens`] that can be used to get a query with the combined fetch. /// /// For example, this can take a `Query<&A>` and a `Query<&B>` and return a `Query<(&A, &B)>`. @@ -1477,6 +1678,32 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { self.join_filtered(other) } + /// Returns a [`QueryLens`] that can be used to get a query with the combined fetch. + /// + /// For example, this can take a `Query<&A>` and a `Query<&B>` and return a `Query<(&A, &B)>`. + /// The returned query will only return items with both `A` and `B`. Note that since filters + /// are dropped, non-archetypal filters like `Added` and `Changed` will not be respected. + /// To maintain or change filter terms see `Self::join_filtered`. + /// + /// ## Panics + /// + /// This will panic if `NewD` is not a subset of the union of the original fetch `Q` and `OtherD`. + /// + /// ## Allowed Transmutes + /// + /// Like `transmute_lens` the query terms can be changed with some restrictions. + /// See [`Self::transmute_lens`] for more details. + /// + /// # See also + /// + /// - [`join`](Self::join) to join using a mutable borrow of the [`Query`]. + pub fn join_inner( + self, + other: &mut Query, + ) -> QueryLens<'w, NewD> { + self.join_filtered_inner(other) + } + /// Equivalent to [`Self::join`] but also includes a [`QueryFilter`] type. /// /// Note that the lens with iterate a subset of the original queries' tables @@ -1493,6 +1720,29 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { &mut self, other: &mut Query, ) -> QueryLens<'_, NewD, NewF> { + self.reborrow().join_filtered_inner(other) + } + + /// Equivalent to [`Self::join_inner`] but also includes a [`QueryFilter`] type. + /// + /// Note that the lens with iterate a subset of the original queries' tables + /// and archetypes. This means that additional archetypal query terms like + /// `With` and `Without` will not necessarily be respected and non-archetypal + /// terms like `Added` and `Changed` will only be respected if they are in + /// the type signature. + /// + /// # See also + /// + /// - [`join_filtered`](Self::join_filtered) to join using a mutable borrow of the [`Query`]. + pub fn join_filtered_inner< + OtherD: QueryData, + OtherF: QueryFilter, + NewD: QueryData, + NewF: QueryFilter, + >( + self, + other: &mut Query, + ) -> QueryLens<'w, NewD, NewF> { let state = self .state .join_filtered::(self.world, other.state); @@ -1505,6 +1755,21 @@ 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> { + type Item = D::Item<'w>; + type IntoIter = QueryIter<'w, 's, D, F>; + + fn into_iter(self) -> Self::IntoIter { + // SAFETY: + // - `self.world` has permission to access the required components. + // - The query is read-only, so it can be aliased even if it was originally mutable. + unsafe { + self.state + .iter_unchecked_manual(self.world, self.last_run, self.this_run) + } + } +} + impl<'w, 's, D: QueryData, F: QueryFilter> IntoIterator for &'w Query<'_, 's, D, F> { type Item = ROQueryItem<'w, D>; type IntoIter = QueryIter<'w, 's, D::ReadOnly, F>; @@ -1524,52 +1789,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> IntoIterator for &'w mut Query<'_, 's } impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter> Query<'w, 's, D, F> { - /// Returns the query item for the given [`Entity`], with the actual "inner" world lifetime. - /// - /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is - /// returned instead. - /// - /// This can only return immutable data (mutable data will be cast to an immutable form). - /// See [`get_mut`](Self::get_mut) for queries that contain at least one mutable component. - /// - /// # Example - /// - /// Here, `get` is used to retrieve the exact query item of the entity specified by the - /// `SelectedCharacter` resource. - /// - /// ``` - /// # use bevy_ecs::prelude::*; - /// # - /// # #[derive(Resource)] - /// # struct SelectedCharacter { entity: Entity } - /// # #[derive(Component)] - /// # struct Character { name: String } - /// # - /// fn print_selected_character_name_system( - /// query: Query<&Character>, - /// selection: Res - /// ) - /// { - /// if let Ok(selected_character) = query.get(selection.entity) { - /// println!("{}", selected_character.name); - /// } - /// } - /// # bevy_ecs::system::assert_is_system(print_selected_character_name_system); - /// ``` - #[inline] - pub fn get_inner(&self, entity: Entity) -> Result, QueryEntityError> { - // SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict - unsafe { - self.state.as_readonly().get_unchecked_manual( - self.world, - entity, - self.last_run, - self.this_run, - ) - } - } - /// Returns an [`Iterator`] over the query items, with the actual "inner" world lifetime. /// /// This can only return immutable data (mutable data will be cast to an immutable form). @@ -1595,13 +1814,7 @@ impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter> Query<'w, 's, D, F> { /// ``` #[inline] pub fn iter_inner(&self) -> QueryIter<'w, 's, D::ReadOnly, F> { - // SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict - unsafe { - self.state - .as_readonly() - .iter_unchecked_manual(self.world, self.last_run, self.this_run) - } + (*self).into_iter() } } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 9d06aec04d9ef..ef721ca4c33c6 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -318,7 +318,7 @@ unsafe impl SystemParam for Qu // SAFETY: We have registered all of the query's world accesses, // so the caller ensures that `world` has permission to access any // world data that the query needs. - unsafe { Query::new(world, state, system_meta.last_run, change_tick) } + unsafe { state.query_unchecked_manual_with_ticks(world, system_meta.last_run, change_tick) } } } @@ -384,10 +384,12 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam fo ) -> Self::Item<'w, 's> { state.validate_world(world.id()); // SAFETY: State ensures that the components it accesses are not accessible somewhere elsewhere. - let result = - unsafe { state.get_single_unchecked_manual(world, system_meta.last_run, change_tick) }; - let single = - result.expect("The query was expected to contain exactly one matching entity."); + let query = unsafe { + state.query_unchecked_manual_with_ticks(world, system_meta.last_run, change_tick) + }; + let single = query + .get_single_inner() + .expect("The query was expected to contain exactly one matching entity."); Single { item: single, _filter: PhantomData, @@ -403,14 +405,14 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam fo state.validate_world(world.id()); // SAFETY: State ensures that the components it accesses are not mutably accessible elsewhere // and the query is read only. - let result = unsafe { - state.as_readonly().get_single_unchecked_manual( + let query = unsafe { + state.query_unchecked_manual_with_ticks( world, system_meta.last_run, world.change_tick(), ) }; - let is_valid = result.is_ok(); + let is_valid = query.get_single_inner().is_ok(); if !is_valid { system_meta.try_warn_param::(); } @@ -448,9 +450,10 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam ) -> Self::Item<'w, 's> { state.validate_world(world.id()); // SAFETY: State ensures that the components it accesses are not accessible elsewhere. - let result = - unsafe { state.get_single_unchecked_manual(world, system_meta.last_run, change_tick) }; - match result { + let query = unsafe { + state.query_unchecked_manual_with_ticks(world, system_meta.last_run, change_tick) + }; + match query.get_single_inner() { Ok(single) => Some(Single { item: single, _filter: PhantomData, @@ -469,13 +472,14 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam state.validate_world(world.id()); // SAFETY: State ensures that the components it accesses are not mutably accessible elsewhere // and the query is read only. - let result = unsafe { - state.as_readonly().get_single_unchecked_manual( + let query = unsafe { + state.query_unchecked_manual_with_ticks( world, system_meta.last_run, world.change_tick(), ) }; + let result = query.get_single_inner(); let is_valid = !matches!(result, Err(QuerySingleError::MultipleEntities(_))); if !is_valid { system_meta.try_warn_param::(); diff --git a/crates/bevy_ecs/src/world/deferred_world.rs b/crates/bevy_ecs/src/world/deferred_world.rs index 45e7a7cf873ee..62560d3bb5be1 100644 --- a/crates/bevy_ecs/src/world/deferred_world.rs +++ b/crates/bevy_ecs/src/world/deferred_world.rs @@ -253,18 +253,8 @@ impl<'w> DeferredWorld<'w> { &mut self, state: &'s mut QueryState, ) -> Query<'_, 's, D, F> { - state.validate_world(self.world.id()); - state.update_archetypes(self); - // SAFETY: We ran validate_world to ensure our state matches - unsafe { - let world_cell = self.world; - Query::new( - world_cell, - state, - world_cell.last_change_tick(), - world_cell.change_tick(), - ) - } + // SAFETY: We have mutable access to the entire world + unsafe { state.query_unchecked(self.world) } } /// Gets a mutable reference to the resource of the given type diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index 4c22cf93ff9b3..cc7b7b5964f48 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -917,7 +917,7 @@ pub fn camera_system( || camera.computed.old_sub_camera_view != camera.sub_camera_view { let new_computed_target_info = normalized_target.get_render_target_info( - &windows, + windows, &images, &manual_texture_views, ); From cfe03a1df301dcd5049707fb51455217dd694edf Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Wed, 1 Jan 2025 22:05:35 -0500 Subject: [PATCH 2/5] Replace `Borrow` with `EntityBorrow` in `iter_many_inner()`. --- crates/bevy_ecs/src/system/query.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index bea9f8d2372fe..215b2a4f3d67b 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -746,7 +746,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// - [`iter_many`](Self::iter_many) to get read-only query items. /// - [`iter_many_mut`](Self::iter_many_mut) to get mutable query items. #[inline] - pub fn iter_many_inner>>( + pub fn iter_many_inner>( self, entities: EntityList, ) -> QueryManyIter<'w, 's, D, F, EntityList::IntoIter> { From cfebde7a15d7d4ad745727516046f17315526b72 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Wed, 5 Feb 2025 09:57:45 -0500 Subject: [PATCH 3/5] Fix bad merge on QueryManyIter::sort_impl. --- crates/bevy_ecs/src/query/iter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 4b462deaf8d3c..cb00cd12e0991 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -1683,7 +1683,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> // SAFETY: // `self.world` has permission to access the required components. // The original query iter has not been iterated on, so no items are aliased from it. - let query_lens = unsafe { query_lens_state.query_unchecked_manual(world) }.into_iter(); + let query_lens = unsafe { query_lens_state.iter_many_unchecked_manual(world, self.entity_iter) }.into_iter(); let mut keyed_query: Vec<_> = query_lens .map(|(key, entity)| (key, NeutralOrd(entity))) .collect(); From b3251d1143771bcfb458da1461c850a72f2692fc Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Wed, 5 Feb 2025 10:21:41 -0500 Subject: [PATCH 4/5] Fix bad fix on QueryManyIter::sort_impl. --- crates/bevy_ecs/src/query/iter.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index cb00cd12e0991..0769fb74699b1 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -1683,7 +1683,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> // SAFETY: // `self.world` has permission to access the required components. // The original query iter has not been iterated on, so no items are aliased from it. - let query_lens = unsafe { query_lens_state.iter_many_unchecked_manual(world, self.entity_iter) }.into_iter(); + let query_lens = unsafe { query_lens_state.query_unchecked_manual(world) } + .iter_many_inner(self.entity_iter); let mut keyed_query: Vec<_> = query_lens .map(|(key, entity)| (key, NeutralOrd(entity))) .collect(); From 8a3b6be08da21bc5f63d987fe4064fe520f3d7b3 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Wed, 5 Feb 2025 12:55:47 -0500 Subject: [PATCH 5/5] Fix issues found in review. --- crates/bevy_ecs/src/query/state.rs | 13 +++++++------ crates/bevy_ecs/src/system/query.rs | 17 ++++++++++++++--- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index c6f8dc161fe3d..01fe7845656aa 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -320,7 +320,7 @@ impl QueryState { /// Creates a [`Query`] from the given [`QueryState`] and [`World`]. /// - /// This can only be called for read-only queries, see [`Self::query_mut`] for write-queries. + /// This will create read-only queries, see [`Self::query_mut`] for mutable queries. pub fn query<'w, 's>(&'s mut self, world: &'w World) -> Query<'w, 's, D::ReadOnly, F> { self.update_archetypes(world); self.query_manual(world) @@ -333,10 +333,10 @@ impl QueryState { /// belongs to an archetype that has not been cached. /// /// To ensure that the cache is up to date, call [`QueryState::update_archetypes`] before this method. - /// The cache is also updated in [`QueryState::new`], `QueryState::get`, or any method with mutable + /// The cache is also updated in [`QueryState::new`], [`QueryState::get`], or any method with mutable /// access to `self`. /// - /// This can only be called for read-only queries, see [`Self::query_mut`] for mutable queries. + /// This will create read-only queries, see [`Self::query_mut`] for mutable queries. pub fn query_manual<'w, 's>(&'s self, world: &'w World) -> Query<'w, 's, D::ReadOnly, F> { // SAFETY: We have read access to the entire world, and we call `as_readonly()` so the query only performs read access. unsafe { @@ -364,7 +364,8 @@ impl QueryState { world: UnsafeWorldCell<'w>, ) -> Query<'w, 's, D, F> { self.update_archetypes_unsafe_world_cell(world); - self.query_unchecked_manual(world) + // SAFETY: Caller ensures we have the required access + unsafe { self.query_unchecked_manual(world) } } /// Creates a [`Query`] from the given [`QueryState`] and [`World`]. @@ -374,7 +375,7 @@ impl QueryState { /// belongs to an archetype that has not been cached. /// /// To ensure that the cache is up to date, call [`QueryState::update_archetypes`] before this method. - /// The cache is also updated in [`QueryState::new`], `QueryState::get`, or any method with mutable + /// The cache is also updated in [`QueryState::new`], [`QueryState::get`], or any method with mutable /// access to `self`. /// /// # Safety @@ -415,7 +416,7 @@ impl QueryState { /// belongs to an archetype that has not been cached. /// /// To ensure that the cache is up to date, call [`QueryState::update_archetypes`] before this method. - /// The cache is also updated in [`QueryState::new`], `QueryState::get`, or any method with mutable + /// The cache is also updated in [`QueryState::new`], [`QueryState::get`], or any method with mutable /// access to `self`. /// /// # Safety diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 0ef7f400973d7..ecca479d3bf9c 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -627,6 +627,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } /// Returns a [`QueryCombinationIter`] over all combinations of `K` query items without repetition. + /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. /// /// This iterator is always guaranteed to return results from each unique pair of matching entities. /// Iteration order is not guaranteed. @@ -749,6 +750,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } /// Returns an iterator over the query items generated from an [`Entity`] list. + /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. /// /// Items are returned in the order of the list of entities, and may not be unique if the input /// doesn't guarantee uniqueness. Entities that don't match the query are skipped. @@ -1044,6 +1046,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } /// Returns a parallel iterator over the query results for the given [`World`](crate::world::World). + /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. /// /// This parallel iterator is always guaranteed to return results from each matching entity once and /// only once. Iteration order and thread assignment is not guaranteed. @@ -1219,7 +1222,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { self.reborrow().get_inner(entity) } - /// Returns the query item for the given [`Entity`], with the actual "inner" world lifetime, by consuming the [`Query`]. + /// Returns the query item for the given [`Entity`]. + /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. /// /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is returned instead. /// @@ -1256,6 +1260,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } /// Returns the query items for the given array of [`Entity`]. + /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. /// /// The returned query items are in the same order as the input. /// In case of a nonexisting entity, duplicate entities or mismatched component, a [`QueryEntityError`] is returned instead. @@ -1482,6 +1487,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } /// Returns a single query item when there is exactly one entity matching the query. + /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. /// /// If the number of query items is not exactly one, a [`QuerySingleError`] is returned instead. /// @@ -1505,7 +1511,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// # See also /// /// - [`get_single`](Self::get_single) to get the read-only query item. - /// - [`get_single_mut`](Self::get_single_mut) to get items using a mutable reference. + /// - [`get_single_mut`](Self::get_single_mut) to get the mutable query item. #[inline] pub fn get_single_inner(self) -> Result, QuerySingleError> { // SAFETY: @@ -1751,6 +1757,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } /// Returns a [`QueryLens`] that can be used to get a query with a more general fetch. + /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. /// /// For example, this can transform a `Query<(&A, &mut B)>` to a `Query<&B>`. /// This can be useful for passing the query to another function. Note that since @@ -1837,6 +1844,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } /// Equivalent to [`Self::transmute_lens_inner`] but also includes a [`QueryFilter`] type. + /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. /// /// Note that the lens will iterate the same tables and archetypes as the original query. This means that /// additional archetypal query terms like [`With`](crate::query::With) and [`Without`](crate::query::Without) @@ -1935,6 +1943,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } /// Returns a [`QueryLens`] that can be used to get a query with the combined fetch. + /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. /// /// For example, this can take a `Query<&A>` and a `Query<&B>` and return a `Query<(&A, &B)>`. /// The returned query will only return items with both `A` and `B`. Note that since filters @@ -1980,6 +1989,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { } /// Equivalent to [`Self::join_inner`] but also includes a [`QueryFilter`] type. + /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. /// /// Note that the lens with iterate a subset of the original queries' tables /// and archetypes. This means that additional archetypal query terms like @@ -2018,7 +2028,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter> IntoIterator for Query<'w, 's, D, F> fn into_iter(self) -> Self::IntoIter { // SAFETY: // - `self.world` has permission to access the required components. - // - The query is read-only, so it can be aliased even if it was originally mutable. + // - We consume the query, so mutable queries cannot alias. + // Read-only queries are `Copy`, but may alias themselves. unsafe { self.state .iter_unchecked_manual(self.world, self.last_run, self.this_run)