diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index c2d925a56b..987ee67dc4 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -237,6 +237,16 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { #marker_name: &'__w (), } + 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 { + #(#named_field_idents: self.#named_field_idents.clone(),)* + #marker_name: &(), + } + } + } + // 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 { @@ -275,17 +285,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { } } - unsafe fn clone_fetch<'__w>( - _fetch: &::Fetch<'__w> - ) -> ::Fetch<'__w> { - #fetch_struct_name { - #( - #named_field_idents: <#field_types>::clone_fetch(& _fetch. #named_field_idents), - )* - #marker_name: &(), - } - } - 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 0e11d42f6b..2401246322 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -318,7 +318,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; @@ -345,14 +345,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 @@ -404,6 +396,10 @@ 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 `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, @@ -483,8 +479,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>, @@ -554,10 +548,6 @@ unsafe impl<'a> WorldQuery for EntityRef<'a> { world.world() } - unsafe fn clone_fetch<'w>(world: &Self::Fetch<'w>) -> Self::Fetch<'w> { - world - } - #[inline] unsafe fn set_archetype<'w>( _fetch: &mut Self::Fetch<'w>, @@ -620,6 +610,13 @@ pub struct ReadFetch<'w, T> { sparse_set: Option<&'w ComponentSparseSet>, } +impl Clone for ReadFetch<'_, T> { + fn clone(&self) -> Self { + *self + } +} +impl Copy for ReadFetch<'_, T> {} + /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl WorldQuery for &T { type Fetch<'w> = ReadFetch<'w, T>; @@ -663,13 +660,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>, @@ -770,6 +760,13 @@ pub struct RefFetch<'w, T> { this_run: Tick, } +impl Clone for RefFetch<'_, T> { + fn clone(&self) -> Self { + *self + } +} +impl Copy for RefFetch<'_, T> {} + /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { type Fetch<'w> = RefFetch<'w, T>; @@ -812,15 +809,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>, @@ -933,6 +921,13 @@ pub struct WriteFetch<'w, T> { this_run: Tick, } +impl Clone for WriteFetch<'_, T> { + fn clone(&self) -> Self { + *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 { type Fetch<'w> = WriteFetch<'w, T>; @@ -975,15 +970,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>, @@ -1084,6 +1070,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>; @@ -1112,13 +1107,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>, @@ -1271,10 +1259,6 @@ unsafe impl WorldQuery for Has { false } - unsafe fn clone_fetch<'w>(fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> { - *fetch - } - #[inline] unsafe fn set_archetype<'w>( fetch: &mut Self::Fetch<'w>, @@ -1351,13 +1335,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)*; @@ -1461,13 +1438,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)*; @@ -1588,8 +1558,6 @@ unsafe impl WorldQuery for NopWorldQuery { ) { } - unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} - #[inline(always)] unsafe fn set_archetype( _fetch: &mut (), @@ -1651,8 +1619,6 @@ unsafe impl WorldQuery for PhantomData { ) -> Self::Fetch<'w> { } - unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} - // `PhantomData` does not match any components, so all components it matches // are stored in a Table (vacuous truth). const IS_DENSE: bool = true; diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index b3f039566e..021b010021 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -59,8 +59,6 @@ unsafe impl WorldQuery for With { ) { } - unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} - const IS_DENSE: bool = { match T::Storage::STORAGE_TYPE { StorageType::Table => true, @@ -162,8 +160,6 @@ unsafe impl WorldQuery for Without { ) { } - unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} - const IS_DENSE: bool = { match T::Storage::STORAGE_TYPE { StorageType::Table => true, @@ -259,6 +255,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)] @@ -288,18 +293,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,10 @@ macro_rules! impl_tick_filter { pub struct $name(PhantomData); #[doc(hidden)] + #[derive(Clone)] $(#[$fetch_meta])* - pub struct $fetch_name<'w, T> { - table_ticks: Option< ThinSlicePtr<'w, UnsafeCell>>, - marker: PhantomData, + pub struct $fetch_name<'w> { + table_ticks: Option>>, sparse_set: Option<&'w ComponentSparseSet>, last_run: Tick, this_run: Tick, @@ -414,7 +407,7 @@ macro_rules! impl_tick_filter { // 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; @@ -440,24 +433,11 @@ macro_rules! impl_tick_filter { .get(id) .debug_checked_unwrap() }), - marker: PhantomData, last_run, this_run, } } - 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 a148fc6ac4..142877efc5 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -6,7 +6,7 @@ use crate::{ storage::{TableId, TableRow, Tables}, world::unsafe_world_cell::UnsafeWorldCell, }; -use std::{borrow::Borrow, iter::FusedIterator, marker::PhantomData, mem::MaybeUninit}; +use std::{borrow::Borrow, iter::FusedIterator, mem::MaybeUninit}; use super::ReadOnlyWorldQuery; @@ -163,7 +163,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)); } } @@ -344,7 +346,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, @@ -453,28 +455,19 @@ 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<'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(), 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, } } } @@ -515,7 +508,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, } } @@ -593,8 +585,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; @@ -633,8 +628,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(),