From ee6be94f03bac9f504c74feaad90548be18cb96f Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Tue, 28 Mar 2023 19:18:44 -0400 Subject: [PATCH 01/12] require `WorldQuery::fetch` to not alias --- crates/bevy_ecs/src/query/fetch.rs | 3 +++ crates/bevy_ecs/src/query/iter.rs | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index c9a18d1081b78..da86b27dd22e2 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -388,6 +388,9 @@ pub unsafe trait 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. + /// + /// If this type does not implement [`ReadOnlyWorldQuery`], then the caller must ensure that + /// no two `Self::Item`s may exist for the same entity at any given time. unsafe fn fetch<'w>( fetch: &mut Self::Fetch<'w>, entity: Entity, diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 893df47aac3a3..e77ae4c3fc893 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -164,7 +164,9 @@ where // SAFETY: set_archetype was called prior. // `location.archetype_row` is an archetype index row in range of the current archetype, because if it was not, the match above would have `continue`d if F::filter_fetch(&mut self.filter, entity, location.table_row) { - // SAFETY: set_archetype was called prior, `location.archetype_row` is an archetype index in range of the current archetype + // SAFETY: + // - set_archetype was called prior, `location.archetype_row` is an archetype index in range of the current archetype + // - fetch is only called once for each entity. return Some(Q::fetch(&mut self.fetch, entity, location.table_row)); } } @@ -592,8 +594,11 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's, continue; } - // SAFETY: set_table was called prior. - // `current_row` is a table row in range of the current table, because if it was not, then the if above would have been executed. + // SAFETY: + // - set_table was called prior. + // - `current_row` must be a table row in range of the current table, + // because if it was not, then the if above would have been executed. + // - fetch is only called once for each `entity`. let item = Q::fetch(&mut self.fetch, *entity, row); self.current_row += 1; @@ -632,8 +637,11 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's, continue; } - // SAFETY: set_archetype was called prior, `current_row` is an archetype index in range of the current archetype - // `current_row` is an archetype index row in range of the current archetype, because if it was not, then the if above would have been executed. + // SAFETY: + // - set_archetype was called prior. + // - `current_row` must be an archetype index row in range of the current archetype, + // because if it was not, then the if above would have been executed. + // - fetch is only called once for each `archetype_entity`. let item = Q::fetch( &mut self.fetch, archetype_entity.entity(), From b022837fbd9381379b35e216caf1b40fd7cd644e Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Tue, 28 Mar 2023 19:33:35 -0400 Subject: [PATCH 02/12] remove `clone_fetch` --- crates/bevy_ecs/macros/src/fetch.rs | 23 +++---- crates/bevy_ecs/src/query/fetch.rs | 100 ++++++++++++---------------- crates/bevy_ecs/src/query/filter.rs | 51 ++++++-------- crates/bevy_ecs/src/query/iter.rs | 5 +- 4 files changed, 75 insertions(+), 104 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index a72b4d10f3094..332e7e8d70453 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -208,6 +208,16 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { #(#ignored_field_idents: #ignored_field_types,)* } + impl #user_impl_generics_with_world Clone for #fetch_struct_name #user_ty_generics_with_world + #user_where_clauses_with_world { + fn clone(&self) -> Self { + Self { + #(#field_idents: self.#field_idents.clone(),)* + #(#ignored_field_idents: Default::default(),)* + } + } + } + // SAFETY: `update_component_access` and `update_archetype_component_access` are called on every field unsafe impl #user_impl_generics #path::query::WorldQuery for #struct_name #user_ty_generics #user_where_clauses { @@ -249,19 +259,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { } } - unsafe fn clone_fetch<'__w>( - _fetch: &::Fetch<'__w> - ) -> ::Fetch<'__w> { - #fetch_struct_name { - #( - #field_idents: <#field_types>::clone_fetch(& _fetch. #field_idents), - )* - #( - #ignored_field_idents: Default::default(), - )* - } - } - const IS_DENSE: bool = true #(&& <#field_types>::IS_DENSE)*; const IS_ARCHETYPAL: bool = true #(&& <#field_types>::IS_ARCHETYPAL)*; diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index da86b27dd22e2..ed56115a6883a 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -307,7 +307,7 @@ pub unsafe trait WorldQuery { type Item<'a>; /// Per archetype/table state used by this [`WorldQuery`] to fetch [`Self::Item`](crate::query::WorldQuery::Item) - type Fetch<'a>; + type Fetch<'a>: Clone; /// The read-only variant of this [`WorldQuery`], which satisfies the [`ReadOnlyWorldQuery`] trait. type ReadOnly: ReadOnlyWorldQuery; @@ -333,14 +333,6 @@ pub unsafe trait WorldQuery { this_run: Tick, ) -> Self::Fetch<'w>; - /// While this function can be called for any query, it is always safe to call if `Self: ReadOnlyWorldQuery` holds. - /// - /// # Safety - /// While calling this method on its own cannot cause UB it is marked `unsafe` as the caller must ensure - /// that the returned value is not used in any way that would cause two `QueryItem` for the same - /// `archetype_row` or `table_row` to be alive at the same time. - unsafe fn clone_fetch<'w>(fetch: &Self::Fetch<'w>) -> Self::Fetch<'w>; - /// Returns true if (and only if) every table of every archetype matched by this fetch contains /// all of the matched components. This is used to select a more efficient "table iterator" /// for "dense" queries. If this returns true, [`WorldQuery::set_table`] must be used before @@ -468,8 +460,6 @@ unsafe impl WorldQuery for Entity { ) -> Self::Fetch<'w> { } - unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} - #[inline] unsafe fn set_archetype<'w>( _fetch: &mut Self::Fetch<'w>, @@ -522,6 +512,15 @@ pub struct ReadFetch<'w, T> { sparse_set: Option<&'w ComponentSparseSet>, } +impl Clone for ReadFetch<'_, T> { + fn clone(&self) -> Self { + Self { + table_components: self.table_components, + sparse_set: self.sparse_set, + } + } +} + /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl WorldQuery for &T { type Fetch<'w> = ReadFetch<'w, T>; @@ -560,13 +559,6 @@ unsafe impl WorldQuery for &T { } } - unsafe fn clone_fetch<'w>(fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> { - ReadFetch { - table_components: fetch.table_components, - sparse_set: fetch.sparse_set, - } - } - #[inline] unsafe fn set_archetype<'w>( fetch: &mut ReadFetch<'w, T>, @@ -667,6 +659,17 @@ pub struct RefFetch<'w, T> { this_run: Tick, } +impl Clone for RefFetch<'_, T> { + fn clone(&self) -> Self { + Self { + table_data: self.table_data, + sparse_set: self.sparse_set, + last_run: self.last_run, + this_run: self.this_run, + } + } +} + /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { type Fetch<'w> = RefFetch<'w, T>; @@ -707,15 +710,6 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { } } - unsafe fn clone_fetch<'w>(fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> { - RefFetch { - table_data: fetch.table_data, - sparse_set: fetch.sparse_set, - last_run: fetch.last_run, - this_run: fetch.this_run, - } - } - #[inline] unsafe fn set_archetype<'w>( fetch: &mut RefFetch<'w, T>, @@ -828,6 +822,17 @@ pub struct WriteFetch<'w, T> { this_run: Tick, } +impl Clone for WriteFetch<'_, T> { + fn clone(&self) -> Self { + Self { + table_data: self.table_data, + sparse_set: self.sparse_set, + last_run: self.last_run, + this_run: self.this_run, + } + } +} + /// SAFETY: access of `&T` is a subset of `&mut T` unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { type Fetch<'w> = WriteFetch<'w, T>; @@ -868,15 +873,6 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { } } - unsafe fn clone_fetch<'w>(fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> { - WriteFetch { - table_data: fetch.table_data, - sparse_set: fetch.sparse_set, - last_run: fetch.last_run, - this_run: fetch.this_run, - } - } - #[inline] unsafe fn set_archetype<'w>( fetch: &mut WriteFetch<'w, T>, @@ -977,6 +973,15 @@ pub struct OptionFetch<'w, T: WorldQuery> { matches: bool, } +impl Clone for OptionFetch<'_, T> { + fn clone(&self) -> Self { + Self { + fetch: self.fetch.clone(), + matches: self.matches, + } + } +} + // SAFETY: defers to soundness of `T: WorldQuery` impl unsafe impl WorldQuery for Option { type Fetch<'w> = OptionFetch<'w, T>; @@ -1004,13 +1009,6 @@ unsafe impl WorldQuery for Option { } } - unsafe fn clone_fetch<'w>(fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> { - OptionFetch { - fetch: T::clone_fetch(&fetch.fetch), - matches: fetch.matches, - } - } - #[inline] unsafe fn set_archetype<'w>( fetch: &mut OptionFetch<'w, T>, @@ -1102,13 +1100,6 @@ macro_rules! impl_tuple_fetch { ($($name::init_fetch(_world, $name, _last_run, _this_run),)*) } - unsafe fn clone_fetch<'w>( - fetch: &Self::Fetch<'w>, - ) -> Self::Fetch<'w> { - let ($($name,)*) = &fetch; - ($($name::clone_fetch($name),)*) - } - const IS_DENSE: bool = true $(&& $name::IS_DENSE)*; const IS_ARCHETYPAL: bool = true $(&& $name::IS_ARCHETYPAL)*; @@ -1211,13 +1202,6 @@ macro_rules! impl_anytuple_fetch { ($(($name::init_fetch(_world, $name, _last_run, _this_run), false),)*) } - unsafe fn clone_fetch<'w>( - fetch: &Self::Fetch<'w>, - ) -> Self::Fetch<'w> { - let ($($name,)*) = &fetch; - ($(($name::clone_fetch(& $name.0), $name.1),)*) - } - const IS_DENSE: bool = true $(&& $name::IS_DENSE)*; const IS_ARCHETYPAL: bool = true $(&& $name::IS_ARCHETYPAL)*; @@ -1346,8 +1330,6 @@ unsafe impl WorldQuery for NopWorldQuery { #[inline(always)] unsafe fn init_fetch(_world: &World, _state: &Q::State, _last_run: Tick, _this_run: Tick) {} - unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} - #[inline(always)] unsafe fn set_archetype( _fetch: &mut (), diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 0072f166665eb..061a66dd83aa1 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -52,8 +52,6 @@ unsafe impl WorldQuery for With { unsafe fn init_fetch(_world: &World, _state: &ComponentId, _last_run: Tick, _this_run: Tick) {} - unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} - const IS_DENSE: bool = { match T::Storage::STORAGE_TYPE { StorageType::Table => true, @@ -148,8 +146,6 @@ unsafe impl WorldQuery for Without { unsafe fn init_fetch(_world: &World, _state: &ComponentId, _last_run: Tick, _this_run: Tick) {} - unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} - const IS_DENSE: bool = { match T::Storage::STORAGE_TYPE { StorageType::Table => true, @@ -245,6 +241,15 @@ pub struct OrFetch<'w, T: WorldQuery> { matches: bool, } +impl Clone for OrFetch<'_, T> { + fn clone(&self) -> Self { + Self { + fetch: self.fetch.clone(), + matches: self.matches, + } + } +} + macro_rules! impl_query_filter_tuple { ($(($filter: ident, $state: ident)),*) => { #[allow(unused_variables)] @@ -273,18 +278,6 @@ macro_rules! impl_query_filter_tuple { },)*) } - unsafe fn clone_fetch<'w>( - fetch: &Self::Fetch<'w>, - ) -> Self::Fetch<'w> { - let ($($filter,)*) = &fetch; - ($( - OrFetch { - fetch: $filter::clone_fetch(&$filter.fetch), - matches: $filter.matches, - }, - )*) - } - #[inline] unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table) { let ($($filter,)*) = fetch; @@ -403,10 +396,22 @@ macro_rules! impl_tick_filter { $(#[$fetch_meta])* pub struct $fetch_name<'w, T> { table_ticks: Option< ThinSlicePtr<'w, UnsafeCell>>, - marker: PhantomData, sparse_set: Option<&'w ComponentSparseSet>, last_run: Tick, this_run: Tick, + marker: PhantomData, + } + + impl Clone for $fetch_name<'_, T> { + fn clone(&self) -> Self { + Self { + table_ticks: self.table_ticks, + sparse_set: self.sparse_set, + last_run: self.last_run, + this_run: self.this_run, + marker: PhantomData, + } + } } // SAFETY: `Self::ReadOnly` is the same as `Self` @@ -436,18 +441,6 @@ macro_rules! impl_tick_filter { } } - unsafe fn clone_fetch<'w>( - fetch: &Self::Fetch<'w>, - ) -> Self::Fetch<'w> { - $fetch_name { - table_ticks: fetch.table_ticks, - sparse_set: fetch.sparse_set, - last_run: fetch.last_run, - this_run: fetch.this_run, - marker: PhantomData, - } - } - const IS_DENSE: bool = { match T::Storage::STORAGE_TYPE { StorageType::Table => true, diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index e77ae4c3fc893..db46c10a1a589 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -473,9 +473,8 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's, archetype_id_iter: self.archetype_id_iter.clone(), table_entities: self.table_entities, archetype_entities: self.archetype_entities, - // SAFETY: upheld by caller invariants - fetch: Q::clone_fetch(&self.fetch), - filter: F::clone_fetch(&self.filter), + fetch: self.fetch.clone(), + filter: self.filter.clone(), current_len: self.current_len, current_row: self.current_row, phantom: PhantomData, From 03a59d8990c0564598221bc6695fb303adc0b5ef Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Tue, 28 Mar 2023 19:42:27 -0400 Subject: [PATCH 03/12] derive `Clone` for tick filters --- crates/bevy_ecs/src/query/filter.rs | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 061a66dd83aa1..6eb695890c5fc 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -393,30 +393,18 @@ macro_rules! impl_tick_filter { pub struct $name(PhantomData); #[doc(hidden)] + #[derive(Clone)] $(#[$fetch_meta])* - pub struct $fetch_name<'w, T> { + pub struct $fetch_name<'w> { table_ticks: Option< ThinSlicePtr<'w, UnsafeCell>>, sparse_set: Option<&'w ComponentSparseSet>, last_run: Tick, this_run: Tick, - marker: PhantomData, - } - - impl Clone for $fetch_name<'_, T> { - fn clone(&self) -> Self { - Self { - table_ticks: self.table_ticks, - sparse_set: self.sparse_set, - last_run: self.last_run, - this_run: self.this_run, - marker: PhantomData, - } - } } // SAFETY: `Self::ReadOnly` is the same as `Self` unsafe impl WorldQuery for $name { - type Fetch<'w> = $fetch_name<'w, T>; + type Fetch<'w> = $fetch_name<'w>; type Item<'w> = bool; type ReadOnly = Self; type State = ComponentId; @@ -435,7 +423,6 @@ macro_rules! impl_tick_filter { .get(id) .debug_checked_unwrap() }), - marker: PhantomData, last_run, this_run, } From 7fef544bec0c8e553426dcdfbd17c16e8d8192e5 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Tue, 28 Mar 2023 19:44:06 -0400 Subject: [PATCH 04/12] remove a space --- crates/bevy_ecs/src/query/filter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 6eb695890c5fc..8f1a929e98f53 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -396,7 +396,7 @@ macro_rules! impl_tick_filter { #[derive(Clone)] $(#[$fetch_meta])* pub struct $fetch_name<'w> { - table_ticks: Option< ThinSlicePtr<'w, UnsafeCell>>, + table_ticks: Option>>, sparse_set: Option<&'w ComponentSparseSet>, last_run: Tick, this_run: Tick, From 7616798b9ac12de35049f5231261c281daaad623 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Tue, 28 Mar 2023 19:49:00 -0400 Subject: [PATCH 05/12] make it safe to clone `QueryIterationCursor` --- crates/bevy_ecs/src/query/iter.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index db46c10a1a589..74a500559234c 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -348,7 +348,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, const K: usize> match self.cursors[i].next(self.tables, self.archetypes, self.query_state) { Some(_) => { for j in (i + 1)..K { - self.cursors[j] = self.cursors[j - 1].clone_cursor(); + self.cursors[j] = self.cursors[j - 1].clone(); match self.cursors[j].next(self.tables, self.archetypes, self.query_state) { Some(_) => {} None if i > 0 => continue 'outer, @@ -460,14 +460,8 @@ struct QueryIterationCursor<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> { phantom: PhantomData, } -impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's, Q, F> { - /// This function is safe to call if `(Q, F): ReadOnlyWorldQuery` holds. - /// - /// # Safety - /// While calling this method on its own cannot cause UB it is marked `unsafe` as the caller must ensure - /// that the returned value is not used in any way that would cause two `QueryItem` for the same - /// `archetype_row` or `table_row` to be alive at the same time. - unsafe fn clone_cursor(&self) -> Self { +impl Clone for QueryIterationCursor<'_, '_, Q, F> { + fn clone(&self) -> Self { Self { table_id_iter: self.table_id_iter.clone(), archetype_id_iter: self.archetype_id_iter.clone(), From 0b4ca110b4022a0e9fd7c7d19bfbd0d7646bd30b Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Wed, 29 Mar 2023 09:22:27 -0400 Subject: [PATCH 06/12] use `Copy` to implement `Clone` --- crates/bevy_ecs/src/query/fetch.rs | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index ed56115a6883a..dc79c88df6241 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -514,12 +514,10 @@ pub struct ReadFetch<'w, T> { impl Clone for ReadFetch<'_, T> { fn clone(&self) -> Self { - Self { - table_components: self.table_components, - sparse_set: self.sparse_set, - } + *self } } +impl Copy for ReadFetch<'_, T> {} /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl WorldQuery for &T { @@ -661,14 +659,10 @@ pub struct RefFetch<'w, T> { impl Clone for RefFetch<'_, T> { fn clone(&self) -> Self { - Self { - table_data: self.table_data, - sparse_set: self.sparse_set, - last_run: self.last_run, - this_run: self.this_run, - } + *self } } +impl Copy for RefFetch<'_, T> {} /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { @@ -824,14 +818,10 @@ pub struct WriteFetch<'w, T> { impl Clone for WriteFetch<'_, T> { fn clone(&self) -> Self { - Self { - table_data: self.table_data, - sparse_set: self.sparse_set, - last_run: self.last_run, - this_run: self.this_run, - } + *self } } +impl Copy for WriteFetch<'_, T> {} /// SAFETY: access of `&T` is a subset of `&mut T` unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { From 942c3f2d0188fec6d6f011d4a1a7f914a2befcde Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Wed, 29 Mar 2023 09:57:54 -0400 Subject: [PATCH 07/12] Update crates/bevy_ecs/src/query/fetch.rs Co-authored-by: Alice Cecile --- crates/bevy_ecs/src/query/fetch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index dc79c88df6241..a98690a361c21 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -382,7 +382,7 @@ pub unsafe trait WorldQuery { /// `table_row` must be in the range of the current table and archetype. /// /// If this type does not implement [`ReadOnlyWorldQuery`], then the caller must ensure that - /// no two `Self::Item`s may exist for the same entity at any given time. + /// it is impossible for more than one `Self::Item`s to exist for the same entity at any given time. unsafe fn fetch<'w>( fetch: &mut Self::Fetch<'w>, entity: Entity, From 67476533a89309ce2866c4572a2be01bbfcc8cf7 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Wed, 29 Mar 2023 09:58:29 -0400 Subject: [PATCH 08/12] Update crates/bevy_ecs/src/query/fetch.rs --- crates/bevy_ecs/src/query/fetch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index a98690a361c21..8887134f9a8dd 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -382,7 +382,7 @@ pub unsafe trait WorldQuery { /// `table_row` must be in the range of the current table and archetype. /// /// If this type does not implement [`ReadOnlyWorldQuery`], then the caller must ensure that - /// it is impossible for more than one `Self::Item`s to exist for the same entity at any given time. + /// it is impossible for more than one `Self::Item` to exist for the same entity at any given time. unsafe fn fetch<'w>( fetch: &mut Self::Fetch<'w>, entity: Entity, From 2f2c74fd4cbc0f010344b1d85f7b94fcddf645e6 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 30 Mar 2023 08:10:31 -0400 Subject: [PATCH 09/12] remove an unnecessary marker field --- crates/bevy_ecs/src/query/iter.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 74a500559234c..04f05883144a9 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -457,7 +457,6 @@ struct QueryIterationCursor<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> { current_len: usize, // either table row or archetype index, depending on whether both `Q`'s and `F`'s fetches are dense current_row: usize, - phantom: PhantomData, } impl Clone for QueryIterationCursor<'_, '_, Q, F> { @@ -471,7 +470,6 @@ impl Clone for QueryIterationCursor<'_, '_ filter: self.filter.clone(), current_len: self.current_len, current_row: self.current_row, - phantom: PhantomData, } } } @@ -509,7 +507,6 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's, archetype_id_iter: query_state.matched_archetype_ids.iter(), current_len: 0, current_row: 0, - phantom: PhantomData, } } From 50c49016e0fa83c79ca3986209df00e5375554d6 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 30 Mar 2023 08:25:41 -0400 Subject: [PATCH 10/12] remove an unused import --- 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 04f05883144a9..b6417c579717a 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -6,7 +6,7 @@ use crate::{ query::{ArchetypeFilter, DebugCheckedUnwrap, QueryState, WorldQuery}, storage::{TableId, TableRow, Tables}, }; -use std::{borrow::Borrow, iter::FusedIterator, marker::PhantomData, mem::MaybeUninit}; +use std::{borrow::Borrow, iter::FusedIterator, mem::MaybeUninit}; use super::ReadOnlyWorldQuery; From b54ebdf45ed1cd607f35d01b853b8e320c523ae0 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Mon, 3 Apr 2023 08:42:35 -0400 Subject: [PATCH 11/12] fix a merge conflict --- crates/bevy_ecs/src/query/fetch.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 02926e33b499b..602b4f94c6479 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1384,15 +1384,7 @@ unsafe impl WorldQuery for PhantomData { fn shrink<'wlong: 'wshort, 'wshort>(_item: Self::Item<'wlong>) -> Self::Item<'wshort> {} - unsafe fn init_fetch<'w>( - _world: &'w World, - _state: &Self::State, - _last_run: Tick, - _this_run: Tick, - ) -> Self::Fetch<'w> { - } - - unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} + unsafe fn init_fetch(_world: &World, _state: &Self::State, _last_run: Tick, _this_run: Tick) {} // `PhantomData` does not match any components, so all components it matches // are stored in a Table (vacuous truth). From 144bf17821afd75c63b9e46a7b80e382f608c7dc Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Wed, 31 May 2023 09:31:59 -0400 Subject: [PATCH 12/12] tweak phrasing for the documentation --- crates/bevy_ecs/src/query/fetch.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 74f6d30a4c2e8..708738ef29517 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -392,8 +392,9 @@ pub unsafe trait 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. /// - /// If this type does not implement [`ReadOnlyWorldQuery`], then the caller must ensure that - /// it is impossible for more than one `Self::Item` to exist for the same entity at any given time. + /// If `update_component_access` includes any mutable accesses, then the caller must ensure + /// that `fetch` is called no more than once for each `entity`/`table_row` in each archetype. + /// If `Self` implements [`ReadOnlyWorldQuery`], then this can safely be called multiple times. unsafe fn fetch<'w>( fetch: &mut Self::Fetch<'w>, entity: Entity,