From 4e5ae272c67ba7f52c3c25e4e8f2943d8dd9a4b5 Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 16 May 2022 14:45:43 -0700 Subject: [PATCH 01/11] Parallelize transform propagation --- crates/bevy_transform/Cargo.toml | 1 + crates/bevy_transform/src/systems.rs | 97 ++++++++++++++++------------ 2 files changed, 57 insertions(+), 41 deletions(-) diff --git a/crates/bevy_transform/Cargo.toml b/crates/bevy_transform/Cargo.toml index 6a85ed46064e5..aed0a7bcc0d60 100644 --- a/crates/bevy_transform/Cargo.toml +++ b/crates/bevy_transform/Cargo.toml @@ -15,3 +15,4 @@ bevy_ecs = { path = "../bevy_ecs", version = "0.8.0-dev", features = ["bevy_refl bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.8.0-dev"} bevy_math = { path = "../bevy_math", version = "0.8.0-dev" } bevy_reflect = { path = "../bevy_reflect", version = "0.8.0-dev", features = ["bevy"] } +bevy_tasks = { path = "../bevy_tasks", version = "0.8.0-dev" } diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index e7bd9789d3df7..05d5e530a374f 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -1,12 +1,15 @@ use crate::components::{GlobalTransform, Transform}; -use bevy_ecs::prelude::{Changed, Entity, Query, With, Without}; +use bevy_ecs::prelude::{Changed, Entity, Query, With, Without, Res}; +use bevy_tasks::prelude::ComputeTaskPool; use bevy_hierarchy::{Children, Parent}; /// Update [`GlobalTransform`] component of entities based on entity hierarchy and /// [`Transform`] component. pub fn transform_propagate_system( + task_pool: Res, mut root_query: Query< ( + Entity, Option<(&Children, Changed)>, &Transform, Changed, @@ -15,61 +18,71 @@ pub fn transform_propagate_system( ), Without, >, - mut transform_query: Query<( - &Transform, - Changed, - &mut GlobalTransform, - &Parent, - )>, + transform_query: Query< + (&Transform, Changed, &mut GlobalTransform), + With, + >, + parent_query: Query<&Parent>, children_query: Query<(&Children, Changed), (With, With)>, ) { - for (children, transform, transform_changed, mut global_transform, entity) in - root_query.iter_mut() - { - let mut changed = transform_changed; - if transform_changed { - *global_transform = GlobalTransform::from(*transform); - } + root_query.par_for_each_mut( + &*task_pool, + 64, + |(entity, children, transform, transform_changed, mut global_transform)| { + let mut changed = transform_changed; + if transform_changed { + *global_transform = GlobalTransform::from(*transform); + } - if let Some((children, changed_children)) = children { - // If our `Children` has changed, we need to recalculate everything below us - changed |= changed_children; - for child in children.iter() { - let _ = propagate_recursive( - &global_transform, - &mut transform_query, - &children_query, - *child, - entity, - changed, - ); + if let Some((children, changed_children)) = children { + // If our `Children` has changed, we need to recalculate everything below us + changed |= changed_children; + for child in children.iter() { + let _ = propagate_recursive( + &global_transform, + &transform_query, + &parent_query, + &children_query, + entity, + *child, + changed, + ); + } } - } - } + }, + ); } fn propagate_recursive( parent: &GlobalTransform, - transform_query: &mut Query<( - &Transform, - Changed, - &mut GlobalTransform, - &Parent, - )>, + transform_query: &Query< + (&Transform, Changed, &mut GlobalTransform), + With, + >, + parent_query: &Query<&Parent>, children_query: &Query<(&Children, Changed), (With, With)>, + expected_parent: Entity, entity: Entity, expected_parent: Entity, mut changed: bool, // We use a result here to use the `?` operator. Ideally we'd use a try block instead ) -> Result<(), ()> { - let global_matrix = { - let (transform, transform_changed, mut global_transform, child_parent) = - transform_query.get_mut(entity).map_err(drop)?; - // Note that for parallelising, this check cannot occur here, since there is an `&mut GlobalTransform` (in global_transform) + if let Ok(actual_parent) = parent_query.get(entity) { assert_eq!( - child_parent.0, expected_parent, + actual_parent.0, expected_parent, "Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle" ); + } else { + panic!("Propagated child for {:?} has no Parent component!", entity); + } + + // SAFE: With the check that each child has one and only one parent, each child must be globally unique within the + // hierarchy. Because of this, it is impossible for this query to have aliased mutable access to the same GlobalTransform. + // Any malformed hierarchy will cause a panic due to the above check. + let global_matrix = unsafe { + let (transform, transform_changed, mut global_transform) = + transform_query.get_unchecked(entity).map_err(drop)?; + changed |= transform_changed; if changed { *global_transform = parent.mul_transform(*transform); @@ -83,8 +96,10 @@ fn propagate_recursive( for child in children.iter() { let _ = propagate_recursive( &global_matrix, - transform_query, - children_query, + &transform_query, + &parent_query, + &children_query, + entity, *child, entity, changed, From ff0f0bc287f9758bcf181773235fa78274ae7589 Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 16 May 2022 15:06:49 -0700 Subject: [PATCH 02/11] build fixes --- crates/bevy_transform/src/lib.rs | 12 +++++ crates/bevy_transform/src/systems.rs | 77 +++++++++++++++------------- 2 files changed, 52 insertions(+), 37 deletions(-) diff --git a/crates/bevy_transform/src/lib.rs b/crates/bevy_transform/src/lib.rs index a88017c7d7d60..02d6328ed2013 100644 --- a/crates/bevy_transform/src/lib.rs +++ b/crates/bevy_transform/src/lib.rs @@ -93,12 +93,24 @@ impl Plugin for TransformPlugin { app.register_type::() .register_type::() // Adding these to startup ensures the first update is "correct" + .add_startup_system_to_stage( + StartupStage::PostStartup, + systems::transform_propagate_system_flat + .label(TransformSystem::TransformPropagate) + .after(HierarchySystem::ParentUpdate), + ) .add_startup_system_to_stage( StartupStage::PostStartup, systems::transform_propagate_system .label(TransformSystem::TransformPropagate) .after(HierarchySystem::ParentUpdate), ) + .add_system_to_stage( + CoreStage::PostUpdate, + systems::transform_propagate_system_flat + .label(TransformSystem::TransformPropagate) + .after(HierarchySystem::ParentUpdate), + ) .add_system_to_stage( CoreStage::PostUpdate, systems::transform_propagate_system diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 05d5e530a374f..46a4642cc69e3 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -1,7 +1,19 @@ use crate::components::{GlobalTransform, Transform}; -use bevy_ecs::prelude::{Changed, Entity, Query, With, Without, Res}; -use bevy_tasks::prelude::ComputeTaskPool; +use bevy_ecs::prelude::{Changed, Entity, Or, Query, Res, With, Without}; use bevy_hierarchy::{Children, Parent}; +use bevy_tasks::prelude::ComputeTaskPool; + +/// Update [`GlobalTransform`] component of entities that aren't in the hierarchy +pub fn transform_propagate_system_flat( + mut query: Query< + (&Transform, &mut GlobalTransform), + (Changed, Without, Without), + >, +) { + for (transform, mut global_transform) in query.iter_mut() { + *global_transform = GlobalTransform::from(*transform); + } +} /// Update [`GlobalTransform`] component of entities based on entity hierarchy and /// [`Transform`] component. @@ -10,44 +22,35 @@ pub fn transform_propagate_system( mut root_query: Query< ( Entity, - Option<(&Children, Changed)>, + &Children, &Transform, - Changed, + Or<(Changed, Changed)>, &mut GlobalTransform, - Entity, ), Without, >, - transform_query: Query< - (&Transform, Changed, &mut GlobalTransform), - With, - >, + transform_query: Query<(&Transform, Changed, &mut GlobalTransform), With>, parent_query: Query<&Parent>, children_query: Query<(&Children, Changed), (With, With)>, ) { root_query.par_for_each_mut( &*task_pool, 64, - |(entity, children, transform, transform_changed, mut global_transform)| { - let mut changed = transform_changed; - if transform_changed { + |(entity, children, transform, changed, mut global_transform)| { + if changed { *global_transform = GlobalTransform::from(*transform); } - if let Some((children, changed_children)) = children { - // If our `Children` has changed, we need to recalculate everything below us - changed |= changed_children; - for child in children.iter() { - let _ = propagate_recursive( - &global_transform, - &transform_query, - &parent_query, - &children_query, - entity, - *child, - changed, - ); - } + for child in children.iter() { + let _ = propagate_recursive( + &global_transform, + &transform_query, + &parent_query, + &children_query, + entity, + *child, + changed, + ); } }, ); @@ -55,15 +58,11 @@ pub fn transform_propagate_system( fn propagate_recursive( parent: &GlobalTransform, - transform_query: &Query< - (&Transform, Changed, &mut GlobalTransform), - With, - >, + transform_query: &Query<(&Transform, Changed, &mut GlobalTransform), With>, parent_query: &Query<&Parent>, children_query: &Query<(&Children, Changed), (With, With)>, expected_parent: Entity, entity: Entity, - expected_parent: Entity, mut changed: bool, // We use a result here to use the `?` operator. Ideally we'd use a try block instead ) -> Result<(), ()> { @@ -76,7 +75,7 @@ fn propagate_recursive( panic!("Propagated child for {:?} has no Parent component!", entity); } - // SAFE: With the check that each child has one and only one parent, each child must be globally unique within the + // SAFE: With the check that each child has one and only one parent, each child must be globally unique within the // hierarchy. Because of this, it is impossible for this query to have aliased mutable access to the same GlobalTransform. // Any malformed hierarchy will cause a panic due to the above check. let global_matrix = unsafe { @@ -96,12 +95,11 @@ fn propagate_recursive( for child in children.iter() { let _ = propagate_recursive( &global_matrix, - &transform_query, - &parent_query, - &children_query, + transform_query, + parent_query, + children_query, entity, *child, - entity, changed, ); } @@ -116,7 +114,7 @@ mod test { use bevy_math::vec3; use crate::components::{GlobalTransform, Transform}; - use crate::systems::transform_propagate_system; + use crate::systems::*; use crate::TransformBundle; use bevy_hierarchy::{ parent_update_system, BuildChildren, BuildWorldChildren, Children, Parent, @@ -128,6 +126,7 @@ mod test { let mut update_stage = SystemStage::parallel(); update_stage.add_system(parent_update_system); + update_stage.add_system(transform_propagate_system_flat); update_stage.add_system(transform_propagate_system); let mut schedule = Schedule::default(); @@ -173,6 +172,7 @@ mod test { let mut update_stage = SystemStage::parallel(); update_stage.add_system(parent_update_system); + update_stage.add_system(transform_propagate_system_flat); update_stage.add_system(transform_propagate_system); let mut schedule = Schedule::default(); @@ -216,6 +216,7 @@ mod test { let mut update_stage = SystemStage::parallel(); update_stage.add_system(parent_update_system); + update_stage.add_system(transform_propagate_system_flat); update_stage.add_system(transform_propagate_system); let mut schedule = Schedule::default(); @@ -301,6 +302,7 @@ mod test { let mut app = App::new(); app.add_system(parent_update_system); + app.add_system(transform_propagate_system_flat); app.add_system(transform_propagate_system); let translation = vec3(1.0, 0.0, 0.0); @@ -357,6 +359,7 @@ mod test { let mut app = App::new(); app.add_system(parent_update_system); + app.add_system(transform_propagate_system_flat); app.add_system(transform_propagate_system); let child = app From e9a485ec3fda8505c57f206c9d3e36af001c5faf Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 16 May 2022 20:04:21 -0700 Subject: [PATCH 03/11] Yeet Fetch State --- crates/bevy_ecs/macros/src/fetch.rs | 52 ++- crates/bevy_ecs/src/query/fetch.rs | 634 ++++++++++++--------------- crates/bevy_ecs/src/query/filter.rs | 266 +++++------ crates/bevy_ecs/src/query/state.rs | 31 +- crates/bevy_transform/src/systems.rs | 4 +- 5 files changed, 412 insertions(+), 575 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index f7d0433306e5b..8984019e7ac35 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -248,6 +248,31 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { unsafe fn archetype_filter_fetch(&mut self, _archetype_index: usize) -> bool { true #(&& self.#field_idents.archetype_filter_fetch(_archetype_index))* } + + fn init_state(world: &mut #path::world::World) -> Self::State { + #state_struct_name { + #(#field_idents: <<#field_types as #path::query::WorldQueryGats<'_>>::Fetch as #path::query::Fetch<'_>>::init_state(world),)* + #(#ignored_field_idents: Default::default(),)* + } + } + + fn update_component_access(state: &Self::State, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) { + ##(<<<#field_types as #path::query::WorldQueryGats<'_>>::Fetch as #path::query::Fetch<'_>>>::update_component_access(&state.#field_idents, _access);)* + } + + fn update_archetype_component_access(state: &Self::State, _archetype: &#path::archetype::Archetype, _access: &mut #path::query::Access<#path::archetype::ArchetypeComponentId>) { + // ##(<#field_types as Fetch<'_>>::update_archetype_component_access(&state.#field_idents, _access);)* + } + + fn matches_archetype(state: &Self::State, _archetype: &#path::archetype::Archetype) -> bool { + true + // true #(&& <#field_types as Fetch<'_>>::matches_archetype(&state.#field_idents, _archetype))* + } + + fn matches_table(state: &Self::State, _table: &#path::storage::Table) -> bool { + true + // true #(&& <#field_types as Fetch<'_>>::matches_table(&state.#field_idents,_table))* + } } } }; @@ -257,36 +282,9 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { let state_impl = quote! { #[doc(hidden)] #visibility struct #state_struct_name #user_impl_generics #user_where_clauses { - #(#field_idents: <#field_types as #path::query::WorldQuery>::State,)* #(#ignored_field_idents: #ignored_field_types,)* } - - // SAFETY: `update_component_access` and `update_archetype_component_access` are called for each item in the struct - unsafe impl #user_impl_generics #path::query::FetchState for #state_struct_name #user_ty_generics #user_where_clauses { - fn init(world: &mut #path::world::World) -> Self { - #state_struct_name { - #(#field_idents: <<#field_types as #path::query::WorldQuery>::State as #path::query::FetchState>::init(world),)* - #(#ignored_field_idents: Default::default(),)* - } - } - - fn update_component_access(&self, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) { - #(self.#field_idents.update_component_access(_access);)* - } - - fn update_archetype_component_access(&self, _archetype: &#path::archetype::Archetype, _access: &mut #path::query::Access<#path::archetype::ArchetypeComponentId>) { - #(self.#field_idents.update_archetype_component_access(_archetype, _access);)* - } - - fn matches_archetype(&self, _archetype: &#path::archetype::Archetype) -> bool { - true #(&& self.#field_idents.matches_archetype(_archetype))* - } - - fn matches_table(&self, _table: &#path::storage::Table) -> bool { - true #(&& self.#field_idents.matches_table(_table))* - } - } }; let read_only_fetch_impl = if fetch_struct_attributes.is_mutable { diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 5a89bc5280f63..44420a6a9846d 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -293,7 +293,7 @@ use std::{cell::UnsafeCell, marker::PhantomData}; /// # bevy_ecs::system::assert_is_system(my_system); /// ``` pub trait WorldQuery: for<'w> WorldQueryGats<'w, _State = Self::State> { - type State: FetchState; + type State: Send + Sync; /// This function manually implements variance for the query items. fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self>; @@ -312,7 +312,7 @@ pub type ROQueryItem<'w, Q> = <>::ReadOnlyFetch as Fetch pub trait WorldQueryGats<'world> { type Fetch: Fetch<'world, State = Self::_State>; type ReadOnlyFetch: Fetch<'world, State = Self::_State> + ReadOnlyFetch; - type _State: FetchState; + type _State: Send + Sync; } /// Types that implement this trait are responsible for fetching query items from tables or @@ -322,7 +322,7 @@ pub trait WorldQueryGats<'world> { /// [`WorldQuery::State`] types that are essential for fetching component data. pub trait Fetch<'world>: Sized { type Item; - type State: FetchState; + type State: Send + Sync; /// Creates a new instance of this fetch. /// @@ -411,28 +411,33 @@ pub trait Fetch<'world>: Sized { unsafe fn table_filter_fetch(&mut self, table_row: usize) -> bool { true } -} -/// State used to construct a Fetch. This will be cached inside [`QueryState`](crate::query::QueryState), -/// so it is best to move as much data / computation here as possible to reduce the cost of -/// constructing Fetch. -/// -/// # Safety -/// -/// Implementor must ensure that [`FetchState::update_component_access`] and -/// [`FetchState::update_archetype_component_access`] exactly reflects the results of -/// [`FetchState::matches_archetype`], [`FetchState::matches_table`], [`Fetch::archetype_fetch`], and -/// [`Fetch::table_fetch`]. -pub unsafe trait FetchState: Send + Sync + Sized { - fn init(world: &mut World) -> Self; - fn update_component_access(&self, access: &mut FilteredAccess); + fn init_state(world: &mut World) -> Self::State; + + #[allow(unused_variables)] + #[inline] + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) {} + + #[allow(unused_variables)] + #[inline] fn update_archetype_component_access( - &self, + state: &Self::State, archetype: &Archetype, access: &mut Access, - ); - fn matches_archetype(&self, archetype: &Archetype) -> bool; - fn matches_table(&self, table: &Table) -> bool; + ) { + } + + #[allow(unused_variables)] + #[inline] + fn matches_archetype(state: &Self::State, archetype: &Archetype) -> bool { + false + } + + #[allow(unused_variables)] + #[inline] + fn matches_table(state: &Self::State, table: &Table) -> bool { + false + } } /// A fetch that is read only. @@ -443,7 +448,7 @@ pub unsafe trait FetchState: Send + Sync + Sized { pub unsafe trait ReadOnlyFetch {} impl WorldQuery for Entity { - type State = EntityState; + type State = (); fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item @@ -460,45 +465,15 @@ pub struct EntityFetch<'w> { /// SAFETY: access is read only unsafe impl<'w> ReadOnlyFetch for EntityFetch<'w> {} -/// The [`FetchState`] of [`Entity`]. -#[doc(hidden)] -pub struct EntityState; - -// SAFETY: no component or archetype access -unsafe impl FetchState for EntityState { - fn init(_world: &mut World) -> Self { - Self - } - - fn update_component_access(&self, _access: &mut FilteredAccess) {} - - fn update_archetype_component_access( - &self, - _archetype: &Archetype, - _access: &mut Access, - ) { - } - - #[inline] - fn matches_archetype(&self, _archetype: &Archetype) -> bool { - true - } - - #[inline] - fn matches_table(&self, _table: &Table) -> bool { - true - } -} - impl<'w> WorldQueryGats<'w> for Entity { type Fetch = EntityFetch<'w>; type ReadOnlyFetch = EntityFetch<'w>; - type _State = EntityState; + type _State = (); } impl<'w> Fetch<'w> for EntityFetch<'w> { type Item = Entity; - type State = EntityState; + type State = (); const IS_DENSE: bool = true; @@ -506,7 +481,7 @@ impl<'w> Fetch<'w> for EntityFetch<'w> { unsafe fn init( _world: &'w World, - _state: &EntityState, + _state: &Self::State, _last_change_tick: u32, _change_tick: u32, ) -> EntityFetch<'w> { @@ -539,61 +514,25 @@ impl<'w> Fetch<'w> for EntityFetch<'w> { let entities = self.entities.unwrap_or_else(|| debug_checked_unreachable()); *entities.get(archetype_index) } -} - -impl WorldQuery for &T { - type State = ReadState; - - fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { - item - } -} -/// The [`FetchState`] of `&T`. -#[doc(hidden)] -pub struct ReadState { - component_id: ComponentId, - marker: PhantomData, -} + fn init_state(_world: &mut World) -> Self::State {} -// SAFETY: component access and archetype component access are properly updated to reflect that T is -// read -unsafe impl FetchState for ReadState { - fn init(world: &mut World) -> Self { - let component_id = world.init_component::(); - ReadState { - component_id, - marker: PhantomData, - } - } - - fn update_component_access(&self, access: &mut FilteredAccess) { - assert!( - !access.access().has_write(self.component_id), - "&{} conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.", - std::any::type_name::(), - ); - access.add_read(self.component_id); + #[inline] + fn matches_archetype(_state: &Self::State, _archetype: &Archetype) -> bool { + true } - fn update_archetype_component_access( - &self, - archetype: &Archetype, - access: &mut Access, - ) { - if let Some(archetype_component_id) = - archetype.get_archetype_component_id(self.component_id) - { - access.add_read(archetype_component_id); - } + #[inline] + fn matches_table(_state: &Self::State, _table: &Table) -> bool { + true } +} - fn matches_archetype(&self, archetype: &Archetype) -> bool { - archetype.contains(self.component_id) - } +impl WorldQuery for &T { + type State = ComponentId; - fn matches_table(&self, table: &Table) -> bool { - table.has_column(self.component_id) + fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { + item } } @@ -625,12 +564,12 @@ unsafe impl<'w, T: Component> ReadOnlyFetch for ReadFetch<'w, T> {} impl<'w, T: Component> WorldQueryGats<'w> for &T { type Fetch = ReadFetch<'w, T>; type ReadOnlyFetch = ReadFetch<'w, T>; - type _State = ReadState; + type _State = ComponentId; } impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> { type Item = &'w T; - type State = ReadState; + type State = ComponentId; const IS_DENSE: bool = { match T::Storage::STORAGE_TYPE { @@ -643,7 +582,7 @@ impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> { unsafe fn init( world: &'w World, - state: &ReadState, + state: &Self::State, _last_change_tick: u32, _change_tick: u32, ) -> ReadFetch<'w, T> { @@ -651,13 +590,8 @@ impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> { table_components: None, entity_table_rows: None, entities: None, - sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| { - world - .storages() - .sparse_sets - .get(state.component_id) - .unwrap() - }), + sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet) + .then(|| world.storages().sparse_sets.get(*state).unwrap()), } } @@ -671,9 +605,7 @@ impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> { match T::Storage::STORAGE_TYPE { StorageType::Table => { self.entity_table_rows = Some(archetype.entity_table_rows().into()); - let column = tables[archetype.table_id()] - .get_column(state.component_id) - .unwrap(); + let column = tables[archetype.table_id()].get_column(*state).unwrap(); self.table_components = Some(column.get_data_slice().into()); } StorageType::SparseSet => self.entities = Some(archetype.entities().into()), @@ -682,13 +614,7 @@ impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> { #[inline] unsafe fn set_table(&mut self, state: &Self::State, table: &'w Table) { - self.table_components = Some( - table - .get_column(state.component_id) - .unwrap() - .get_data_slice() - .into(), - ); + self.table_components = Some(table.get_column(*state).unwrap().get_data_slice().into()); } #[inline] @@ -723,10 +649,41 @@ impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> { .unwrap_or_else(|| debug_checked_unreachable()); components.get(table_row).deref() } + + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { + assert!( + !access.access().has_write(*state), + "&{} conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.", + std::any::type_name::(), + ); + access.add_read(*state); + } + + fn update_archetype_component_access( + state: &Self::State, + archetype: &Archetype, + access: &mut Access, + ) { + if let Some(archetype_component_id) = archetype.get_archetype_component_id(*state) { + access.add_read(archetype_component_id); + } + } + + fn matches_archetype(state: &Self::State, archetype: &Archetype) -> bool { + archetype.contains(*state) + } + + fn matches_table(state: &Self::State, table: &Table) -> bool { + table.has_column(*state) + } } impl WorldQuery for &mut T { - type State = WriteState; + type State = ComponentId; fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item @@ -786,63 +743,15 @@ impl Clone for ReadOnlyWriteFetch<'_, T> { } } -/// The [`FetchState`] of `&mut T`. -#[doc(hidden)] -pub struct WriteState { - component_id: ComponentId, - marker: PhantomData, -} - -// SAFETY: component access and archetype component access are properly updated to reflect that T is -// written -unsafe impl FetchState for WriteState { - fn init(world: &mut World) -> Self { - let component_id = world.init_component::(); - WriteState { - component_id, - marker: PhantomData, - } - } - - fn update_component_access(&self, access: &mut FilteredAccess) { - assert!( - !access.access().has_read(self.component_id), - "&mut {} conflicts with a previous access in this query. Mutable component access must be unique.", - std::any::type_name::(), - ); - access.add_write(self.component_id); - } - - fn update_archetype_component_access( - &self, - archetype: &Archetype, - access: &mut Access, - ) { - if let Some(archetype_component_id) = - archetype.get_archetype_component_id(self.component_id) - { - access.add_write(archetype_component_id); - } - } - - fn matches_archetype(&self, archetype: &Archetype) -> bool { - archetype.contains(self.component_id) - } - - fn matches_table(&self, table: &Table) -> bool { - table.has_column(self.component_id) - } -} - impl<'w, T: Component> WorldQueryGats<'w> for &mut T { type Fetch = WriteFetch<'w, T>; type ReadOnlyFetch = ReadOnlyWriteFetch<'w, T>; - type _State = WriteState; + type _State = ComponentId; } impl<'w, 's, T: Component> Fetch<'w> for WriteFetch<'w, T> { type Item = Mut<'w, T>; - type State = WriteState; + type State = ComponentId; const IS_DENSE: bool = { match T::Storage::STORAGE_TYPE { @@ -855,7 +764,7 @@ impl<'w, 's, T: Component> Fetch<'w> for WriteFetch<'w, T> { unsafe fn init( world: &'w World, - state: &WriteState, + state: &Self::State, last_change_tick: u32, change_tick: u32, ) -> Self { @@ -863,13 +772,8 @@ impl<'w, 's, T: Component> Fetch<'w> for WriteFetch<'w, T> { table_components: None, entities: None, entity_table_rows: None, - sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| { - world - .storages() - .sparse_sets - .get(state.component_id) - .unwrap() - }), + sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet) + .then(|| world.storages().sparse_sets.get(*state).unwrap()), table_ticks: None, last_change_tick, change_tick, @@ -886,9 +790,7 @@ impl<'w, 's, T: Component> Fetch<'w> for WriteFetch<'w, T> { match T::Storage::STORAGE_TYPE { StorageType::Table => { self.entity_table_rows = Some(archetype.entity_table_rows().into()); - let column = tables[archetype.table_id()] - .get_column(state.component_id) - .unwrap(); + let column = tables[archetype.table_id()].get_column(*state).unwrap(); self.table_components = Some(column.get_data_slice().into()); self.table_ticks = Some(column.get_ticks_slice().into()); } @@ -898,7 +800,7 @@ impl<'w, 's, T: Component> Fetch<'w> for WriteFetch<'w, T> { #[inline] unsafe fn set_table(&mut self, state: &Self::State, table: &'w Table) { - let column = table.get_column(state.component_id).unwrap(); + let column = table.get_column(*state).unwrap(); self.table_components = Some(column.get_data_slice().into()); self.table_ticks = Some(column.get_ticks_slice().into()); } @@ -957,11 +859,42 @@ impl<'w, 's, T: Component> Fetch<'w> for WriteFetch<'w, T> { }, } } + + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { + assert!( + !access.access().has_read(*state), + "&mut {} conflicts with a previous access in this query. Mutable component access must be unique.", + std::any::type_name::(), + ); + access.add_write(*state); + } + + fn update_archetype_component_access( + state: &Self::State, + archetype: &Archetype, + access: &mut Access, + ) { + if let Some(archetype_component_id) = archetype.get_archetype_component_id(*state) { + access.add_write(archetype_component_id); + } + } + + fn matches_archetype(state: &Self::State, archetype: &Archetype) -> bool { + archetype.contains(*state) + } + + fn matches_table(state: &Self::State, table: &Table) -> bool { + table.has_column(*state) + } } impl<'w, T: Component> Fetch<'w> for ReadOnlyWriteFetch<'w, T> { type Item = &'w T; - type State = WriteState; + type State = ComponentId; const IS_DENSE: bool = { match T::Storage::STORAGE_TYPE { @@ -974,7 +907,7 @@ impl<'w, T: Component> Fetch<'w> for ReadOnlyWriteFetch<'w, T> { unsafe fn init( world: &'w World, - state: &WriteState, + state: &Self::State, _last_change_tick: u32, _change_tick: u32, ) -> Self { @@ -982,13 +915,8 @@ impl<'w, T: Component> Fetch<'w> for ReadOnlyWriteFetch<'w, T> { table_components: None, entities: None, entity_table_rows: None, - sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| { - world - .storages() - .sparse_sets - .get(state.component_id) - .unwrap() - }), + sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet) + .then(|| world.storages().sparse_sets.get(*state).unwrap()), } } @@ -1002,9 +930,7 @@ impl<'w, T: Component> Fetch<'w> for ReadOnlyWriteFetch<'w, T> { match T::Storage::STORAGE_TYPE { StorageType::Table => { self.entity_table_rows = Some(archetype.entity_table_rows().into()); - let column = tables[archetype.table_id()] - .get_column(state.component_id) - .unwrap(); + let column = tables[archetype.table_id()].get_column(*state).unwrap(); self.table_components = Some(column.get_data_slice().into()); } StorageType::SparseSet => self.entities = Some(archetype.entities().into()), @@ -1013,13 +939,7 @@ impl<'w, T: Component> Fetch<'w> for ReadOnlyWriteFetch<'w, T> { #[inline] unsafe fn set_table(&mut self, state: &Self::State, table: &'w Table) { - self.table_components = Some( - table - .get_column(state.component_id) - .unwrap() - .get_data_slice() - .into(), - ); + self.table_components = Some(table.get_column(*state).unwrap().get_data_slice().into()); } #[inline] @@ -1054,10 +974,41 @@ impl<'w, T: Component> Fetch<'w> for ReadOnlyWriteFetch<'w, T> { .unwrap_or_else(|| debug_checked_unreachable()); components.get(table_row).deref() } + + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { + assert!( + !access.access().has_read(*state), + "&mut {} conflicts with a previous access in this query. Mutable component access must be unique.", + std::any::type_name::(), + ); + access.add_write(*state); + } + + fn update_archetype_component_access( + state: &Self::State, + archetype: &Archetype, + access: &mut Access, + ) { + if let Some(archetype_component_id) = archetype.get_archetype_component_id(*state) { + access.add_write(archetype_component_id); + } + } + + fn matches_archetype(state: &Self::State, archetype: &Archetype) -> bool { + archetype.contains(*state) + } + + fn matches_table(state: &Self::State, table: &Table) -> bool { + table.has_column(*state) + } } impl WorldQuery for Option { - type State = OptionState; + type State = T::State; fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item.map(T::shrink) @@ -1075,54 +1026,15 @@ pub struct OptionFetch { /// SAFETY: [`OptionFetch`] is read only because `T` is read only unsafe impl ReadOnlyFetch for OptionFetch {} -/// The [`FetchState`] of `Option`. -#[doc(hidden)] -pub struct OptionState { - state: T, -} - -// SAFETY: component access and archetype component access are properly updated according to the -// internal Fetch -unsafe impl FetchState for OptionState { - fn init(world: &mut World) -> Self { - Self { - state: T::init(world), - } - } - - fn update_component_access(&self, access: &mut FilteredAccess) { - self.state.update_component_access(access); - } - - fn update_archetype_component_access( - &self, - archetype: &Archetype, - access: &mut Access, - ) { - if self.state.matches_archetype(archetype) { - self.state - .update_archetype_component_access(archetype, access); - } - } - - fn matches_archetype(&self, _archetype: &Archetype) -> bool { - true - } - - fn matches_table(&self, _table: &Table) -> bool { - true - } -} - impl<'w, T: WorldQueryGats<'w>> WorldQueryGats<'w> for Option { type Fetch = OptionFetch; type ReadOnlyFetch = OptionFetch; - type _State = OptionState; + type _State = T::_State; } impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { type Item = Option; - type State = OptionState; + type State = T::State; const IS_DENSE: bool = T::IS_DENSE; @@ -1130,12 +1042,12 @@ impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { unsafe fn init( world: &'w World, - state: &OptionState, + state: &Self::State, last_change_tick: u32, change_tick: u32, ) -> Self { Self { - fetch: T::init(world, &state.state, last_change_tick, change_tick), + fetch: T::init(world, state, last_change_tick, change_tick), matches: false, } } @@ -1147,17 +1059,17 @@ impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { archetype: &'w Archetype, tables: &'w Tables, ) { - self.matches = state.state.matches_archetype(archetype); + self.matches = >::matches_archetype(state, archetype); if self.matches { - self.fetch.set_archetype(&state.state, archetype, tables); + self.fetch.set_archetype(state, archetype, tables); } } #[inline] unsafe fn set_table(&mut self, state: &Self::State, table: &'w Table) { - self.matches = state.state.matches_table(table); + self.matches = >::matches_table(state, table); if self.matches { - self.fetch.set_table(&state.state, table); + self.fetch.set_table(state, table); } } @@ -1178,6 +1090,32 @@ impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { None } } + + fn init_state(world: &mut World) -> Self::State { + T::init_state(world) + } + + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { + >::update_component_access(state, access) + } + + fn update_archetype_component_access( + state: &Self::State, + archetype: &Archetype, + access: &mut Access, + ) { + if >::matches_archetype(state, archetype) { + >::update_archetype_component_access(state, archetype, access); + } + } + + fn matches_archetype(_state: &Self::State, _archetype: &Archetype) -> bool { + true + } + + fn matches_table(_state: &Self::State, _table: &Table) -> bool { + true + } } /// [`WorldQuery`] that tracks changes and additions for component `T`. @@ -1245,61 +1183,13 @@ impl ChangeTrackers { } impl WorldQuery for ChangeTrackers { - type State = ChangeTrackersState; + type State = ComponentId; fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item } } -/// The [`FetchState`] of [`ChangeTrackers`]. -#[doc(hidden)] -pub struct ChangeTrackersState { - component_id: ComponentId, - marker: PhantomData, -} - -// SAFETY: component access and archetype component access are properly updated to reflect that T is -// read -unsafe impl FetchState for ChangeTrackersState { - fn init(world: &mut World) -> Self { - let component_id = world.init_component::(); - Self { - component_id, - marker: PhantomData, - } - } - - fn update_component_access(&self, access: &mut FilteredAccess) { - assert!( - !access.access().has_write(self.component_id), - "ChangeTrackers<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.", - std::any::type_name::() - ); - access.add_read(self.component_id); - } - - fn update_archetype_component_access( - &self, - archetype: &Archetype, - access: &mut Access, - ) { - if let Some(archetype_component_id) = - archetype.get_archetype_component_id(self.component_id) - { - access.add_read(archetype_component_id); - } - } - - fn matches_archetype(&self, archetype: &Archetype) -> bool { - archetype.contains(self.component_id) - } - - fn matches_table(&self, table: &Table) -> bool { - table.has_column(self.component_id) - } -} - /// The [`Fetch`] of [`ChangeTrackers`]. #[doc(hidden)] pub struct ChangeTrackersFetch<'w, T> { @@ -1335,12 +1225,12 @@ unsafe impl<'w, T: Component> ReadOnlyFetch for ChangeTrackersFetch<'w, T> {} impl<'w, T: Component> WorldQueryGats<'w> for ChangeTrackers { type Fetch = ChangeTrackersFetch<'w, T>; type ReadOnlyFetch = ChangeTrackersFetch<'w, T>; - type _State = ChangeTrackersState; + type _State = ComponentId; } impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> { type Item = ChangeTrackers; - type State = ChangeTrackersState; + type State = ComponentId; const IS_DENSE: bool = { match T::Storage::STORAGE_TYPE { @@ -1353,7 +1243,7 @@ impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> { unsafe fn init( world: &'w World, - state: &ChangeTrackersState, + state: &Self::State, last_change_tick: u32, change_tick: u32, ) -> ChangeTrackersFetch<'w, T> { @@ -1361,13 +1251,8 @@ impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> { table_ticks: None, entities: None, entity_table_rows: None, - sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| { - world - .storages() - .sparse_sets - .get(state.component_id) - .unwrap() - }), + sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet) + .then(|| world.storages().sparse_sets.get(*state).unwrap()), marker: PhantomData, last_change_tick, change_tick, @@ -1384,9 +1269,7 @@ impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> { match T::Storage::STORAGE_TYPE { StorageType::Table => { self.entity_table_rows = Some(archetype.entity_table_rows().into()); - let column = tables[archetype.table_id()] - .get_column(state.component_id) - .unwrap(); + let column = tables[archetype.table_id()].get_column(*state).unwrap(); self.table_ticks = Some(column.get_ticks_slice().into()); } StorageType::SparseSet => self.entities = Some(archetype.entities().into()), @@ -1395,13 +1278,7 @@ impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> { #[inline] unsafe fn set_table(&mut self, state: &Self::State, table: &'w Table) { - self.table_ticks = Some( - table - .get_column(state.component_id) - .unwrap() - .get_ticks_slice() - .into(), - ); + self.table_ticks = Some(table.get_column(*state).unwrap().get_ticks_slice().into()); } #[inline] @@ -1457,6 +1334,37 @@ impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> { change_tick: self.change_tick, } } + + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { + assert!( + !access.access().has_write(*state), + "ChangeTrackers<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.", + std::any::type_name::() + ); + access.add_read(*state); + } + + fn update_archetype_component_access( + state: &Self::State, + archetype: &Archetype, + access: &mut Access, + ) { + if let Some(archetype_component_id) = archetype.get_archetype_component_id(*state) { + access.add_read(archetype_component_id); + } + } + + fn matches_archetype(state: &Self::State, archetype: &Archetype) -> bool { + archetype.contains(*state) + } + + fn matches_table(state: &Self::State, table: &Table) -> bool { + table.has_column(*state) + } } macro_rules! impl_tuple_fetch { @@ -1525,34 +1433,30 @@ macro_rules! impl_tuple_fetch { let ($($name,)*) = self; true $(&& $name.archetype_filter_fetch(archetype_index))* } - } - // SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple - #[allow(non_snake_case)] - #[allow(clippy::unused_unit)] - unsafe impl<$($name: FetchState),*> FetchState for ($($name,)*) { - fn init(_world: &mut World) -> Self { - ($($name::init(_world),)*) + #[allow(clippy::unused_unit)] + fn init_state(_world: &mut World) -> Self::State { + ($(<$name as Fetch<'_>>::init_state(_world),)*) } - fn update_component_access(&self, _access: &mut FilteredAccess) { - let ($($name,)*) = self; - $($name.update_component_access(_access);)* + fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) { + let ($($name,)*) = _state; + $(<$name as Fetch>::update_component_access($name, _access);)* } - fn update_archetype_component_access(&self, _archetype: &Archetype, _access: &mut Access) { - let ($($name,)*) = self; - $($name.update_archetype_component_access(_archetype, _access);)* + fn update_archetype_component_access(_state: &Self::State, _archetype: &Archetype, _access: &mut Access) { + let ($($name,)*) = _state; + $(<$name as Fetch<'_>>::update_archetype_component_access($name, _archetype, _access);)* } - fn matches_archetype(&self, _archetype: &Archetype) -> bool { - let ($($name,)*) = self; - true $(&& $name.matches_archetype(_archetype))* + fn matches_archetype(_state: &Self::State, _archetype: &Archetype) -> bool { + let ($($name,)*) = _state; + true $(&& <$name as Fetch<'_>>::matches_archetype($name, _archetype))* } - fn matches_table(&self, _table: &Table) -> bool { - let ($($name,)*) = self; - true $(&& $name.matches_table(_table))* + fn matches_table(_state: &Self::State, _table: &Table) -> bool { + let ($($name,)*) = _state; + true $(&& <$name as Fetch<'_>>::matches_table($name, _table))* } } @@ -1612,7 +1516,7 @@ macro_rules! impl_anytuple_fetch { let ($($name,)*) = &mut self.0; let ($($state,)*) = &_state.0; $( - $name.1 = $state.matches_archetype(_archetype); + $name.1 = <$name as Fetch<'_>>::matches_archetype($state, _archetype); if $name.1 { $name.0.set_archetype($state, _archetype, _tables); } @@ -1624,7 +1528,7 @@ macro_rules! impl_anytuple_fetch { let ($($name,)*) = &mut self.0; let ($($state,)*) = &_state.0; $( - $name.1 = $state.matches_table(_table); + $name.1 = <$name as Fetch<'_>>::matches_table($state, _table); if $name.1 { $name.0.set_table($state, _table); } @@ -1636,7 +1540,7 @@ macro_rules! impl_anytuple_fetch { unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { let ($($name,)*) = &mut self.0; ($( - $name.1.then(|| $name.0.table_fetch(_table_row)), + $name.1.then(|| $name.0.table_fetch( _table_row)), )*) } @@ -1648,38 +1552,33 @@ macro_rules! impl_anytuple_fetch { $name.1.then(|| $name.0.archetype_fetch(_archetype_index)), )*) } - } - // SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple - #[allow(non_snake_case)] - #[allow(clippy::unused_unit)] - unsafe impl<$($name: FetchState),*> FetchState for AnyOf<($($name,)*)> { - fn init(_world: &mut World) -> Self { - AnyOf(($($name::init(_world),)*)) + fn init_state(_world: &mut World) -> Self::State { + AnyOf(($($name::init_state(_world),)*)) } - fn update_component_access(&self, _access: &mut FilteredAccess) { - let ($($name,)*) = &self.0; - $($name.update_component_access(_access);)* + fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) { + let ($($name,)*) = &_state.0; + $(<$name as Fetch<'_>>::update_component_access($name, _access);)* } - fn update_archetype_component_access(&self, _archetype: &Archetype, _access: &mut Access) { - let ($($name,)*) = &self.0; + fn update_archetype_component_access(_state: &Self::State, _archetype: &Archetype, _access: &mut Access) { + let ($($name,)*) = &_state.0; $( - if $name.matches_archetype(_archetype) { - $name.update_archetype_component_access(_archetype, _access); + if <$name as Fetch<'_>>::matches_archetype($name, _archetype) { + <$name as Fetch<'_>>::update_archetype_component_access($name, _archetype, _access); } )* } - fn matches_archetype(&self, _archetype: &Archetype) -> bool { - let ($($name,)*) = &self.0; - false $(|| $name.matches_archetype(_archetype))* + fn matches_archetype(_state: &Self::State, _archetype: &Archetype) -> bool { + let ($($name,)*) = &_state.0; + false $(|| <$name as Fetch<'_>>::matches_archetype($name, _archetype))* } - fn matches_table(&self, _table: &Table) -> bool { - let ($($name,)*) = &self.0; - false $(|| $name.matches_table(_table))* + fn matches_table(_state: &Self::State, _table: &Table) -> bool { + let ($($name,)*) = &_state.0; + false $(|| <$name as Fetch<'_>>::matches_table($name, _table))* } } @@ -1712,7 +1611,7 @@ pub struct NopFetch { state: PhantomData, } -impl<'w, State: FetchState> Fetch<'w> for NopFetch { +impl<'w, State: Send + Sync> Fetch<'w> for NopFetch { type Item = (); type State = State; @@ -1747,4 +1646,9 @@ impl<'w, State: FetchState> Fetch<'w> for NopFetch { #[inline(always)] unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item {} + + #[inline(always)] + fn init_state(_world: &mut World) -> Self::State { + unimplemented!(); + } } diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 63c83f6350b1a..533dcbc35dad7 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -3,8 +3,8 @@ use crate::{ component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType}, entity::Entity, query::{ - debug_checked_unreachable, Access, Fetch, FetchState, FilteredAccess, QueryFetch, - ROQueryFetch, WorldQuery, WorldQueryGats, + debug_checked_unreachable, Access, Fetch, FilteredAccess, QueryFetch, ROQueryFetch, + WorldQuery, WorldQueryGats, }, storage::{ComponentSparseSet, Table, Tables}, world::World, @@ -45,7 +45,7 @@ use super::ReadOnlyFetch; pub struct With(PhantomData); impl WorldQuery for With { - type State = WithState; + type State = ComponentId; fn shrink<'wlong: 'wshort, 'wshort>( item: super::QueryItem<'wlong, Self>, @@ -60,58 +60,19 @@ pub struct WithFetch { marker: PhantomData, } -/// The [`FetchState`] of [`With`]. -#[doc(hidden)] -pub struct WithState { - component_id: ComponentId, - marker: PhantomData, -} - -// SAFETY: no component access or archetype component access -unsafe impl FetchState for WithState { - fn init(world: &mut World) -> Self { - let component_id = world.init_component::(); - Self { - component_id, - marker: PhantomData, - } - } - - #[inline] - fn update_component_access(&self, access: &mut FilteredAccess) { - access.add_with(self.component_id); - } - - #[inline] - fn update_archetype_component_access( - &self, - _archetype: &Archetype, - _access: &mut Access, - ) { - } - - fn matches_archetype(&self, archetype: &Archetype) -> bool { - archetype.contains(self.component_id) - } - - fn matches_table(&self, table: &Table) -> bool { - table.has_column(self.component_id) - } -} - impl WorldQueryGats<'_> for With { type Fetch = WithFetch; type ReadOnlyFetch = WithFetch; - type _State = WithState; + type _State = ComponentId; } impl<'w, T: Component> Fetch<'w> for WithFetch { type Item = (); - type State = WithState; + type State = ComponentId; unsafe fn init( _world: &World, - _state: &WithState, + _state: &Self::State, _last_change_tick: u32, _change_tick: u32, ) -> Self { @@ -146,6 +107,23 @@ impl<'w, T: Component> Fetch<'w> for WithFetch { #[inline] unsafe fn table_fetch(&mut self, _table_row: usize) {} + + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + + #[inline] + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { + access.add_with(*state); + } + + fn matches_archetype(state: &Self::State, archetype: &Archetype) -> bool { + archetype.contains(*state) + } + + fn matches_table(state: &Self::State, table: &Table) -> bool { + table.has_column(*state) + } } // SAFETY: no component access or archetype component access @@ -188,7 +166,7 @@ impl Copy for WithFetch {} pub struct Without(PhantomData); impl WorldQuery for Without { - type State = WithoutState; + type State = ComponentId; fn shrink<'wlong: 'wshort, 'wshort>( item: super::QueryItem<'wlong, Self>, @@ -203,58 +181,19 @@ pub struct WithoutFetch { marker: PhantomData, } -/// The [`FetchState`] of [`Without`]. -#[doc(hidden)] -pub struct WithoutState { - component_id: ComponentId, - marker: PhantomData, -} - -// SAFETY: no component access or archetype component access -unsafe impl FetchState for WithoutState { - fn init(world: &mut World) -> Self { - let component_id = world.init_component::(); - Self { - component_id, - marker: PhantomData, - } - } - - #[inline] - fn update_component_access(&self, access: &mut FilteredAccess) { - access.add_without(self.component_id); - } - - #[inline] - fn update_archetype_component_access( - &self, - _archetype: &Archetype, - _access: &mut Access, - ) { - } - - fn matches_archetype(&self, archetype: &Archetype) -> bool { - !archetype.contains(self.component_id) - } - - fn matches_table(&self, table: &Table) -> bool { - !table.has_column(self.component_id) - } -} - impl WorldQueryGats<'_> for Without { type Fetch = WithoutFetch; type ReadOnlyFetch = WithoutFetch; - type _State = WithoutState; + type _State = ComponentId; } impl<'w, T: Component> Fetch<'w> for WithoutFetch { type Item = (); - type State = WithoutState; + type State = ComponentId; unsafe fn init( _world: &World, - _state: &WithoutState, + _state: &Self::State, _last_change_tick: u32, _change_tick: u32, ) -> Self { @@ -289,6 +228,23 @@ impl<'w, T: Component> Fetch<'w> for WithoutFetch { #[inline] unsafe fn table_fetch(&mut self, _table_row: usize) {} + + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + + #[inline] + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { + access.add_without(*state); + } + + fn matches_archetype(state: &Self::State, archetype: &Archetype) -> bool { + !archetype.contains(*state) + } + + fn matches_table(state: &Self::State, table: &Table) -> bool { + !table.has_column(*state) + } } // SAFETY: no component access or archetype component access @@ -390,7 +346,7 @@ macro_rules! impl_query_filter_tuple { let ($($filter,)*) = &mut self.0; let ($($state,)*) = &state.0; $( - $filter.matches = $state.matches_table(table); + $filter.matches = <$filter as Fetch<'_>>::matches_table($state, table); if $filter.matches { $filter.fetch.set_table($state, table); } @@ -402,7 +358,7 @@ macro_rules! impl_query_filter_tuple { let ($($filter,)*) = &mut self.0; let ($($state,)*) = &state.0; $( - $filter.matches = $state.matches_archetype(archetype); + $filter.matches = <$filter as Fetch<'_>>::matches_archetype($state, archetype); if $filter.matches { $filter.fetch.set_archetype($state, archetype, tables); } @@ -430,34 +386,29 @@ macro_rules! impl_query_filter_tuple { unsafe fn archetype_filter_fetch(&mut self, archetype_index: usize) -> bool { self.archetype_fetch(archetype_index) } - } - // SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple - #[allow(unused_variables)] - #[allow(non_snake_case)] - unsafe impl<$($filter: FetchState),*> FetchState for Or<($($filter,)*)> { - fn init(world: &mut World) -> Self { - Or(($($filter::init(world),)*)) + fn init_state(world: &mut World) -> Self::State { + Or(($($filter::init_state(world),)*)) } - fn update_component_access(&self, access: &mut FilteredAccess) { - let ($($filter,)*) = &self.0; - $($filter.update_component_access(access);)* + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { + let ($($filter,)*) = &state.0; + $(<$filter as Fetch<'_>>::update_component_access($filter, access);)* } - fn update_archetype_component_access(&self, archetype: &Archetype, access: &mut Access) { - let ($($filter,)*) = &self.0; - $($filter.update_archetype_component_access(archetype, access);)* + fn update_archetype_component_access(state: &Self::State, archetype: &Archetype, access: &mut Access) { + let ($($filter,)*) = &state.0; + $(<$filter as Fetch<'_>>::update_archetype_component_access($filter, archetype, access);)* } - fn matches_archetype(&self, archetype: &Archetype) -> bool { - let ($($filter,)*) = &self.0; - false $(|| $filter.matches_archetype(archetype))* + fn matches_archetype(state: &Self::State, archetype: &Archetype) -> bool { + let ($($filter,)*) = &state.0; + false $(|| <$filter as Fetch<'_>>::matches_archetype($filter, archetype))* } - fn matches_table(&self, table: &Table) -> bool { - let ($($filter,)*) = &self.0; - false $(|| $filter.matches_table(table))* + fn matches_table(state: &Self::State, table: &Table) -> bool { + let ($($filter,)*) = &state.0; + false $(|| <$filter as Fetch<'_>>::matches_table($filter, table))* } } @@ -472,8 +423,6 @@ macro_rules! impl_tick_filter { ( $(#[$meta:meta])* $name: ident, - $(#[$state_meta:meta])* - $state_name: ident, $(#[$fetch_meta:meta])* $fetch_name: ident, $is_detected: expr @@ -493,76 +442,31 @@ macro_rules! impl_tick_filter { change_tick: u32, } - #[doc(hidden)] - $(#[$state_meta])* - pub struct $state_name { - component_id: ComponentId, - marker: PhantomData, - } - impl WorldQuery for $name { - type State = $state_name; + type State = ComponentId; fn shrink<'wlong: 'wshort, 'wshort>(item: super::QueryItem<'wlong, Self>) -> super::QueryItem<'wshort, Self> { item } } - // SAFETY: this reads the T component. archetype component access and component access are updated to reflect that - unsafe impl FetchState for $state_name { - fn init(world: &mut World) -> Self { - Self { - component_id: world.init_component::(), - marker: PhantomData, - } - } - - #[inline] - fn update_component_access(&self, access: &mut FilteredAccess) { - if access.access().has_write(self.component_id) { - panic!("$state_name<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.", - std::any::type_name::()); - } - access.add_read(self.component_id); - } - - #[inline] - fn update_archetype_component_access( - &self, - archetype: &Archetype, - access: &mut Access, - ) { - if let Some(archetype_component_id) = archetype.get_archetype_component_id(self.component_id) { - access.add_read(archetype_component_id); - } - } - - fn matches_archetype(&self, archetype: &Archetype) -> bool { - archetype.contains(self.component_id) - } - - fn matches_table(&self, table: &Table) -> bool { - table.has_column(self.component_id) - } - } - impl<'w, T: Component> WorldQueryGats<'w> for $name { type Fetch = $fetch_name<'w, T>; type ReadOnlyFetch = $fetch_name<'w, T>; - type _State = $state_name; + type _State = ComponentId; } impl<'w, T: Component> Fetch<'w> for $fetch_name<'w, T> { - type State = $state_name; + type State = ComponentId; type Item = bool; - unsafe fn init(world: &'w World, state: & $state_name, last_change_tick: u32, change_tick: u32) -> Self { + unsafe fn init(world: &'w World, state: &Self::State, last_change_tick: u32, change_tick: u32) -> Self { Self { table_ticks: None, entities: None, entity_table_rows: None, sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet) - .then(|| world.storages().sparse_sets.get(state.component_id).unwrap()), + .then(|| world.storages().sparse_sets.get(*state).unwrap()), marker: PhantomData, last_change_tick, change_tick, @@ -579,7 +483,7 @@ macro_rules! impl_tick_filter { const IS_ARCHETYPAL: bool = false; unsafe fn set_table(&mut self, state: &Self::State, table: &'w Table) { - self.table_ticks = Some(table.get_column(state.component_id).unwrap().get_ticks_slice().into()); + self.table_ticks = Some(table.get_column(*state).unwrap().get_ticks_slice().into()); } unsafe fn set_archetype(&mut self, state: &Self::State, archetype: &'w Archetype, tables: &'w Tables) { @@ -587,7 +491,7 @@ macro_rules! impl_tick_filter { StorageType::Table => { self.entity_table_rows = Some(archetype.entity_table_rows().into()); let table = &tables[archetype.table_id()]; - self.table_ticks = Some(table.get_column(state.component_id).unwrap().get_ticks_slice().into()); + self.table_ticks = Some(table.get_column(*state).unwrap().get_ticks_slice().into()); } StorageType::SparseSet => self.entities = Some(archetype.entities().into()), } @@ -626,6 +530,38 @@ macro_rules! impl_tick_filter { unsafe fn archetype_filter_fetch(&mut self, archetype_index: usize) -> bool { self.archetype_fetch(archetype_index) } + + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + + #[inline] + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { + if access.access().has_write(*state) { + panic!("$name<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.", + std::any::type_name::()); + } + access.add_read(*state); + } + + #[inline] + fn update_archetype_component_access( + state: &Self::State, + archetype: &Archetype, + access: &mut Access, + ) { + if let Some(archetype_component_id) = archetype.get_archetype_component_id(*state) { + access.add_read(archetype_component_id); + } + } + + fn matches_archetype(state: &Self::State, archetype: &Archetype) -> bool { + archetype.contains(*state) + } + + fn matches_table(state: &Self::State, table: &Table) -> bool { + table.has_column(*state) + } } /// SAFETY: read-only access @@ -677,8 +613,6 @@ impl_tick_filter!( /// # bevy_ecs::system::assert_is_system(print_add_name_component); /// ``` Added, - /// The [`FetchState`] of [`Added`]. - AddedState, /// The [`Fetch`] of [`Added`]. AddedFetch, ComponentTicks::is_added @@ -717,8 +651,6 @@ impl_tick_filter!( /// # bevy_ecs::system::assert_is_system(print_moving_objects_system); /// ``` Changed, - /// The [`FetchState`] of [`Changed`]. - ChangedState, /// The [`Fetch`] of [`Changed`]. ChangedFetch, ComponentTicks::is_changed diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index ae23c1c512e8e..fa256e2911c76 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -3,10 +3,7 @@ use crate::{ component::ComponentId, entity::Entity, prelude::FromWorld, - query::{ - Access, Fetch, FetchState, FilteredAccess, NopFetch, QueryCombinationIter, QueryIter, - WorldQuery, - }, + query::{Access, Fetch, FilteredAccess, NopFetch, QueryCombinationIter, QueryIter, WorldQuery}, storage::TableId, world::{World, WorldId}, }; @@ -43,17 +40,17 @@ impl FromWorld for QueryState { 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 fetch_state = ::init(world); - let filter_state = ::init(world); + let fetch_state = QueryFetch::::init_state(world); + let filter_state = QueryFetch::::init_state(world); let mut component_access = FilteredAccess::default(); - fetch_state.update_component_access(&mut component_access); + QueryFetch::::update_component_access(&fetch_state, &mut component_access); // Use a temporary empty FilteredAccess for filters. This prevents them from conflicting with the // main Query's `fetch_state` access. Filters are allowed to conflict with the main query fetch // because they are evaluated *before* a specific reference is constructed. let mut filter_component_access = FilteredAccess::default(); - filter_state.update_component_access(&mut filter_component_access); + QueryFetch::::update_component_access(&filter_state, &mut filter_component_access); // Merge the temporary filter access with the main access. This ensures that filter access is // properly considered in a global "cross-query" context (both within systems and across systems). @@ -115,13 +112,19 @@ impl QueryState { /// Creates a new [`Archetype`]. pub fn new_archetype(&mut self, archetype: &Archetype) { - if self.fetch_state.matches_archetype(archetype) - && self.filter_state.matches_archetype(archetype) + if QueryFetch::::matches_archetype(&self.fetch_state, archetype) + && QueryFetch::::matches_archetype(&self.filter_state, archetype) { - self.fetch_state - .update_archetype_component_access(archetype, &mut self.archetype_component_access); - self.filter_state - .update_archetype_component_access(archetype, &mut self.archetype_component_access); + Q::Fetch::update_archetype_component_access( + &self.fetch_state, + archetype, + &mut self.archetype_component_access, + ); + F::Fetch::update_archetype_component_access( + &self.filter_state, + archetype, + &mut self.archetype_component_access, + ); let archetype_index = archetype.id().index(); if !self.matched_archetypes.contains(archetype_index) { self.matched_archetypes.grow(archetype_index + 1); diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 46a4642cc69e3..9ff110d8228d6 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -4,7 +4,7 @@ use bevy_hierarchy::{Children, Parent}; use bevy_tasks::prelude::ComputeTaskPool; /// Update [`GlobalTransform`] component of entities that aren't in the hierarchy -pub fn transform_propagate_system_flat( +pub(crate) fn transform_propagate_system_flat( mut query: Query< (&Transform, &mut GlobalTransform), (Changed, Without, Without), @@ -17,7 +17,7 @@ pub fn transform_propagate_system_flat( /// Update [`GlobalTransform`] component of entities based on entity hierarchy and /// [`Transform`] component. -pub fn transform_propagate_system( +pub(crate) fn transform_propagate_system( task_pool: Res, mut root_query: Query< ( From ae0b2d2314ffd63a04e6649f4b73afeabc467b54 Mon Sep 17 00:00:00 2001 From: james7132 Date: Wed, 18 May 2022 02:44:19 -0700 Subject: [PATCH 04/11] Fix WorldQuery derive macro --- crates/bevy_ecs/macros/src/fetch.rs | 10 +- .../ui/system_param_derive_readonly.stderr | 2 +- crates/bevy_transform/Cargo.toml | 1 - .../src/components/global_transform.rs | 3 + .../src/components/transform.rs | 3 + crates/bevy_transform/src/lib.rs | 12 --- crates/bevy_transform/src/systems.rs | 96 ++++++++----------- 7 files changed, 50 insertions(+), 77 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 8984019e7ac35..87503df144100 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -257,21 +257,19 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { } fn update_component_access(state: &Self::State, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) { - ##(<<<#field_types as #path::query::WorldQueryGats<'_>>::Fetch as #path::query::Fetch<'_>>>::update_component_access(&state.#field_idents, _access);)* + #(<<#field_types as #path::query::WorldQueryGats<'_>>::Fetch as #path::query::Fetch<'_>>::update_component_access(&state.#field_idents, _access);)* } fn update_archetype_component_access(state: &Self::State, _archetype: &#path::archetype::Archetype, _access: &mut #path::query::Access<#path::archetype::ArchetypeComponentId>) { - // ##(<#field_types as Fetch<'_>>::update_archetype_component_access(&state.#field_idents, _access);)* + #(<<#field_types as #path::query::WorldQueryGats<'_>>::Fetch as #path::query::Fetch<'_>>::update_archetype_component_access(&state.#field_idents, _archetype, _access);)* } fn matches_archetype(state: &Self::State, _archetype: &#path::archetype::Archetype) -> bool { - true - // true #(&& <#field_types as Fetch<'_>>::matches_archetype(&state.#field_idents, _archetype))* + true #(&& <<#field_types as #path::query::WorldQueryGats<'_>>::Fetch as #path::query::Fetch<'_>>::matches_archetype(&state.#field_idents, _archetype))* } fn matches_table(state: &Self::State, _table: &#path::storage::Table) -> bool { - true - // true #(&& <#field_types as Fetch<'_>>::matches_table(&state.#field_idents,_table))* + true #(&& <<#field_types as #path::query::WorldQueryGats<'_>>::Fetch as #path::query::Fetch<'_>>::matches_table(&state.#field_idents, _table))* } } } diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.stderr b/crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.stderr index 870e2fb3131c5..a428be317701c 100644 --- a/crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.stderr +++ b/crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.stderr @@ -14,7 +14,7 @@ error[E0277]: the trait bound `for<'x> WriteFetch<'x, Foo>: ReadOnlyFetch` is no | = note: required because of the requirements on the impl of `ReadOnlySystemParamFetch` for `QueryState<&'static mut Foo>` = note: 2 redundant requirements hidden - = note: required because of the requirements on the impl of `ReadOnlySystemParamFetch` for `_::FetchState<(QueryState<&'static mut Foo>,)>` + = note: required because of the requirements on the impl of `ReadOnlySystemParamFetch` for `FetchState<(QueryState<&'static mut Foo>,)>` note: required by a bound in `assert_readonly` --> tests/ui/system_param_derive_readonly.rs:23:32 | diff --git a/crates/bevy_transform/Cargo.toml b/crates/bevy_transform/Cargo.toml index aed0a7bcc0d60..6a85ed46064e5 100644 --- a/crates/bevy_transform/Cargo.toml +++ b/crates/bevy_transform/Cargo.toml @@ -15,4 +15,3 @@ bevy_ecs = { path = "../bevy_ecs", version = "0.8.0-dev", features = ["bevy_refl bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.8.0-dev"} bevy_math = { path = "../bevy_math", version = "0.8.0-dev" } bevy_reflect = { path = "../bevy_reflect", version = "0.8.0-dev", features = ["bevy"] } -bevy_tasks = { path = "../bevy_tasks", version = "0.8.0-dev" } diff --git a/crates/bevy_transform/src/components/global_transform.rs b/crates/bevy_transform/src/components/global_transform.rs index 6bddfead5ad74..be1d75cfcb46b 100644 --- a/crates/bevy_transform/src/components/global_transform.rs +++ b/crates/bevy_transform/src/components/global_transform.rs @@ -102,6 +102,7 @@ impl GlobalTransform { #[doc(hidden)] #[inline] + #[must_use] pub const fn with_translation(mut self, translation: Vec3) -> Self { self.translation = translation; self @@ -109,6 +110,7 @@ impl GlobalTransform { #[doc(hidden)] #[inline] + #[must_use] pub const fn with_rotation(mut self, rotation: Quat) -> Self { self.rotation = rotation; self @@ -116,6 +118,7 @@ impl GlobalTransform { #[doc(hidden)] #[inline] + #[must_use] pub const fn with_scale(mut self, scale: Vec3) -> Self { self.scale = scale; self diff --git a/crates/bevy_transform/src/components/transform.rs b/crates/bevy_transform/src/components/transform.rs index c822336d36c8b..bbca98e76961d 100644 --- a/crates/bevy_transform/src/components/transform.rs +++ b/crates/bevy_transform/src/components/transform.rs @@ -112,6 +112,7 @@ impl Transform { /// Returns this [`Transform`] with a new translation. #[inline] + #[must_use] pub const fn with_translation(mut self, translation: Vec3) -> Self { self.translation = translation; self @@ -119,6 +120,7 @@ impl Transform { /// Returns this [`Transform`] with a new rotation. #[inline] + #[must_use] pub const fn with_rotation(mut self, rotation: Quat) -> Self { self.rotation = rotation; self @@ -126,6 +128,7 @@ impl Transform { /// Returns this [`Transform`] with a new scale. #[inline] + #[must_use] pub const fn with_scale(mut self, scale: Vec3) -> Self { self.scale = scale; self diff --git a/crates/bevy_transform/src/lib.rs b/crates/bevy_transform/src/lib.rs index 02d6328ed2013..a88017c7d7d60 100644 --- a/crates/bevy_transform/src/lib.rs +++ b/crates/bevy_transform/src/lib.rs @@ -93,24 +93,12 @@ impl Plugin for TransformPlugin { app.register_type::() .register_type::() // Adding these to startup ensures the first update is "correct" - .add_startup_system_to_stage( - StartupStage::PostStartup, - systems::transform_propagate_system_flat - .label(TransformSystem::TransformPropagate) - .after(HierarchySystem::ParentUpdate), - ) .add_startup_system_to_stage( StartupStage::PostStartup, systems::transform_propagate_system .label(TransformSystem::TransformPropagate) .after(HierarchySystem::ParentUpdate), ) - .add_system_to_stage( - CoreStage::PostUpdate, - systems::transform_propagate_system_flat - .label(TransformSystem::TransformPropagate) - .after(HierarchySystem::ParentUpdate), - ) .add_system_to_stage( CoreStage::PostUpdate, systems::transform_propagate_system diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 9ff110d8228d6..e7bd9789d3df7 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -1,87 +1,75 @@ use crate::components::{GlobalTransform, Transform}; -use bevy_ecs::prelude::{Changed, Entity, Or, Query, Res, With, Without}; +use bevy_ecs::prelude::{Changed, Entity, Query, With, Without}; use bevy_hierarchy::{Children, Parent}; -use bevy_tasks::prelude::ComputeTaskPool; - -/// Update [`GlobalTransform`] component of entities that aren't in the hierarchy -pub(crate) fn transform_propagate_system_flat( - mut query: Query< - (&Transform, &mut GlobalTransform), - (Changed, Without, Without), - >, -) { - for (transform, mut global_transform) in query.iter_mut() { - *global_transform = GlobalTransform::from(*transform); - } -} /// Update [`GlobalTransform`] component of entities based on entity hierarchy and /// [`Transform`] component. -pub(crate) fn transform_propagate_system( - task_pool: Res, +pub fn transform_propagate_system( mut root_query: Query< ( - Entity, - &Children, + Option<(&Children, Changed)>, &Transform, - Or<(Changed, Changed)>, + Changed, &mut GlobalTransform, + Entity, ), Without, >, - transform_query: Query<(&Transform, Changed, &mut GlobalTransform), With>, - parent_query: Query<&Parent>, + mut transform_query: Query<( + &Transform, + Changed, + &mut GlobalTransform, + &Parent, + )>, children_query: Query<(&Children, Changed), (With, With)>, ) { - root_query.par_for_each_mut( - &*task_pool, - 64, - |(entity, children, transform, changed, mut global_transform)| { - if changed { - *global_transform = GlobalTransform::from(*transform); - } + for (children, transform, transform_changed, mut global_transform, entity) in + root_query.iter_mut() + { + let mut changed = transform_changed; + if transform_changed { + *global_transform = GlobalTransform::from(*transform); + } + if let Some((children, changed_children)) = children { + // If our `Children` has changed, we need to recalculate everything below us + changed |= changed_children; for child in children.iter() { let _ = propagate_recursive( &global_transform, - &transform_query, - &parent_query, + &mut transform_query, &children_query, - entity, *child, + entity, changed, ); } - }, - ); + } + } } fn propagate_recursive( parent: &GlobalTransform, - transform_query: &Query<(&Transform, Changed, &mut GlobalTransform), With>, - parent_query: &Query<&Parent>, + transform_query: &mut Query<( + &Transform, + Changed, + &mut GlobalTransform, + &Parent, + )>, children_query: &Query<(&Children, Changed), (With, With)>, - expected_parent: Entity, entity: Entity, + expected_parent: Entity, mut changed: bool, // We use a result here to use the `?` operator. Ideally we'd use a try block instead ) -> Result<(), ()> { - if let Ok(actual_parent) = parent_query.get(entity) { + let global_matrix = { + let (transform, transform_changed, mut global_transform, child_parent) = + transform_query.get_mut(entity).map_err(drop)?; + // Note that for parallelising, this check cannot occur here, since there is an `&mut GlobalTransform` (in global_transform) assert_eq!( - actual_parent.0, expected_parent, + child_parent.0, expected_parent, "Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle" ); - } else { - panic!("Propagated child for {:?} has no Parent component!", entity); - } - - // SAFE: With the check that each child has one and only one parent, each child must be globally unique within the - // hierarchy. Because of this, it is impossible for this query to have aliased mutable access to the same GlobalTransform. - // Any malformed hierarchy will cause a panic due to the above check. - let global_matrix = unsafe { - let (transform, transform_changed, mut global_transform) = - transform_query.get_unchecked(entity).map_err(drop)?; - changed |= transform_changed; if changed { *global_transform = parent.mul_transform(*transform); @@ -96,10 +84,9 @@ fn propagate_recursive( let _ = propagate_recursive( &global_matrix, transform_query, - parent_query, children_query, - entity, *child, + entity, changed, ); } @@ -114,7 +101,7 @@ mod test { use bevy_math::vec3; use crate::components::{GlobalTransform, Transform}; - use crate::systems::*; + use crate::systems::transform_propagate_system; use crate::TransformBundle; use bevy_hierarchy::{ parent_update_system, BuildChildren, BuildWorldChildren, Children, Parent, @@ -126,7 +113,6 @@ mod test { let mut update_stage = SystemStage::parallel(); update_stage.add_system(parent_update_system); - update_stage.add_system(transform_propagate_system_flat); update_stage.add_system(transform_propagate_system); let mut schedule = Schedule::default(); @@ -172,7 +158,6 @@ mod test { let mut update_stage = SystemStage::parallel(); update_stage.add_system(parent_update_system); - update_stage.add_system(transform_propagate_system_flat); update_stage.add_system(transform_propagate_system); let mut schedule = Schedule::default(); @@ -216,7 +201,6 @@ mod test { let mut update_stage = SystemStage::parallel(); update_stage.add_system(parent_update_system); - update_stage.add_system(transform_propagate_system_flat); update_stage.add_system(transform_propagate_system); let mut schedule = Schedule::default(); @@ -302,7 +286,6 @@ mod test { let mut app = App::new(); app.add_system(parent_update_system); - app.add_system(transform_propagate_system_flat); app.add_system(transform_propagate_system); let translation = vec3(1.0, 0.0, 0.0); @@ -359,7 +342,6 @@ mod test { let mut app = App::new(); app.add_system(parent_update_system); - app.add_system(transform_propagate_system_flat); app.add_system(transform_propagate_system); let child = app From 9ffb07c65166b8319f3deae139373ed415afbbfb Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 21 May 2022 16:26:53 -0700 Subject: [PATCH 05/11] Fix docs --- crates/bevy_ecs/src/query/fetch.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index bef07cfdf6055..aeb43eea659d0 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -54,7 +54,7 @@ use std::{cell::UnsafeCell, marker::PhantomData}; /// /// The derive macro implements [`WorldQuery`] for your type and declares an additional struct /// which will be used as an item for query iterators. The implementation also generates two other -/// structs that implement [`Fetch`] and [`FetchState`] and are used as [`WorldQuery::Fetch`](WorldQueryGats::Fetch) and +/// structs that implement [`Fetch`] and are used as [`WorldQuery::Fetch`](WorldQueryGats::Fetch) and /// [`WorldQuery::State`] associated types respectively. /// /// The derive macro requires every struct field to implement the `WorldQuery` trait. @@ -328,7 +328,7 @@ pub trait Fetch<'world>: Sized { /// /// # Safety /// - /// `state` must have been initialized (via [`FetchState::init`]) using the same `world` passed + /// `state` must have been initialized (via [`Fetch::init_state`]) using the same `world` passed /// in to this function. unsafe fn init( world: &'world World, From f4f824396d6df9b2d4a6fbcf04ad2cd7f0da84af Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 21 May 2022 16:53:32 -0700 Subject: [PATCH 06/11] Use better references --- crates/bevy_ecs/src/query/state.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index fa256e2911c76..97bff1da9482f 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -40,17 +40,17 @@ impl FromWorld for QueryState { 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 fetch_state = QueryFetch::::init_state(world); - let filter_state = QueryFetch::::init_state(world); + let fetch_state = Q::Fetch::init_state(world); + let filter_state = F::Fetch::init_state(world); let mut component_access = FilteredAccess::default(); - QueryFetch::::update_component_access(&fetch_state, &mut component_access); + Q::Fetch::update_component_access(&fetch_state, &mut component_access); // Use a temporary empty FilteredAccess for filters. This prevents them from conflicting with the // main Query's `fetch_state` access. Filters are allowed to conflict with the main query fetch // because they are evaluated *before* a specific reference is constructed. let mut filter_component_access = FilteredAccess::default(); - QueryFetch::::update_component_access(&filter_state, &mut filter_component_access); + F::Fetch::update_component_access(&filter_state, &mut filter_component_access); // Merge the temporary filter access with the main access. This ensures that filter access is // properly considered in a global "cross-query" context (both within systems and across systems). @@ -112,8 +112,8 @@ impl QueryState { /// Creates a new [`Archetype`]. pub fn new_archetype(&mut self, archetype: &Archetype) { - if QueryFetch::::matches_archetype(&self.fetch_state, archetype) - && QueryFetch::::matches_archetype(&self.filter_state, archetype) + if Q::Fetch::matches_archetype(&self.fetch_state, archetype) + && F::Fetch::matches_archetype(&self.filter_state, archetype) { Q::Fetch::update_archetype_component_access( &self.fetch_state, From caeb3788882b6969a674bbd8148db37d4bfa6007 Mon Sep 17 00:00:00 2001 From: james7132 Date: Tue, 31 May 2022 16:04:00 -0700 Subject: [PATCH 07/11] Review comments --- crates/bevy_ecs/macros/src/fetch.rs | 2 +- crates/bevy_ecs/src/query/fetch.rs | 64 ++++++++++++++++++++++------- crates/bevy_ecs/src/query/filter.rs | 27 ++++++++++-- 3 files changed, 73 insertions(+), 20 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 92900b86df95c..87906d87ff980 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -178,7 +178,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { #(#ignored_field_idents: #ignored_field_types,)* } - impl #user_impl_generics_with_world #path::query::Fetch<'__w> + unsafe impl #user_impl_generics_with_world #path::query::Fetch<'__w> for #fetch_struct_name #user_ty_generics_with_world #user_where_clauses_with_world { type Item = #item_struct_name #user_ty_generics_with_world; diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 4c2b29e847184..0430821d532cf 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -320,7 +320,13 @@ pub trait WorldQueryGats<'world> { /// /// Every type that implements [`WorldQuery`] have their associated [`WorldQuery::Fetch`](WorldQueryGats::Fetch) and /// [`WorldQuery::State`] types that are essential for fetching component data. -pub trait Fetch<'world>: Sized { +/// +/// # Safety +/// +/// Implementor must ensure that [`Fetch::update_component_access`] and [`Fetch::update_archetype_component_access`] +/// exactly reflects the results of [`Fetch::matches_component_set`], [`Fetch::archetype_fetch`], and +/// [`Fetch::table_fetch`]. +pub unsafe trait Fetch<'world>: Sized { type Item; type State: Send + Sync; @@ -415,17 +421,14 @@ pub trait Fetch<'world>: Sized { fn init_state(world: &mut World) -> Self::State; #[allow(unused_variables)] - #[inline] - fn update_component_access(state: &Self::State, access: &mut FilteredAccess) {} + fn update_component_access(state: &Self::State, access: &mut FilteredAccess); #[allow(unused_variables)] - #[inline] fn update_archetype_component_access( state: &Self::State, archetype: &Archetype, access: &mut Access, - ) { - } + ); fn matches_component_set( state: &Self::State, @@ -464,7 +467,8 @@ impl<'w> WorldQueryGats<'w> for Entity { type _State = (); } -impl<'w> Fetch<'w> for EntityFetch<'w> { +// SAFETY: no component or archetype access +unsafe impl<'w> Fetch<'w> for EntityFetch<'w> { type Item = Entity; type State = (); @@ -510,6 +514,15 @@ impl<'w> Fetch<'w> for EntityFetch<'w> { fn init_state(_world: &mut World) -> Self::State {} + fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) {} + + fn update_archetype_component_access( + _state: &Self::State, + _archetype: &Archetype, + _access: &mut Access, + ) { + } + #[inline] fn matches_component_set( _state: &Self::State, @@ -558,7 +571,9 @@ impl<'w, T: Component> WorldQueryGats<'w> for &T { type _State = ComponentId; } -impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> { +// SAFETY: component access and archetype component access are properly updated to reflect that T is +// read +unsafe impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> { type Item = &'w T; type State = ComponentId; @@ -739,7 +754,9 @@ impl<'w, T: Component> WorldQueryGats<'w> for &mut T { type _State = ComponentId; } -impl<'w, T: Component> Fetch<'w> for WriteFetch<'w, T> { +// SAFETY: component access and archetype component access are properly updated to reflect that T is +// written +unsafe impl<'w, T: Component> Fetch<'w> for WriteFetch<'w, T> { type Item = Mut<'w, T>; type State = ComponentId; @@ -881,7 +898,9 @@ impl<'w, T: Component> Fetch<'w> for WriteFetch<'w, T> { } } -impl<'w, T: Component> Fetch<'w> for ReadOnlyWriteFetch<'w, T> { +// SAFETY: component access and archetype component access are properly updated to reflect that T is +// read +unsafe impl<'w, T: Component> Fetch<'w> for ReadOnlyWriteFetch<'w, T> { type Item = &'w T; type State = ComponentId; @@ -1020,7 +1039,9 @@ impl<'w, T: WorldQueryGats<'w>> WorldQueryGats<'w> for Option { type _State = T::_State; } -impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { +// SAFETY: component access and archetype component access are properly updated according to the +// internal Fetch +unsafe impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { type Item = Option; type State = T::State; @@ -1221,7 +1242,9 @@ impl<'w, T: Component> WorldQueryGats<'w> for ChangeTrackers { type _State = ComponentId; } -impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> { +// SAFETY: component access and archetype component access are properly updated to reflect that T is +// read +unsafe impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> { type Item = ChangeTrackers; type State = ComponentId; @@ -1369,8 +1392,9 @@ macro_rules! impl_tuple_fetch { type _State = ($($name::_State,)*); } + // SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple #[allow(non_snake_case)] - impl<'w, $($name: Fetch<'w>),*> Fetch<'w> for ($($name,)*) { + unsafe impl<'w, $($name: Fetch<'w>),*> Fetch<'w> for ($($name,)*) { type Item = ($($name::Item,)*); type State = ($($name::State,)*); @@ -1483,8 +1507,9 @@ macro_rules! impl_anytuple_fetch { type _State = AnyOf<($($name::_State,)*)>; } + // SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple #[allow(non_snake_case)] - impl<'w, $($name: Fetch<'w>),*> Fetch<'w> for AnyOf<($(($name, bool),)*)> { + unsafe impl<'w, $($name: Fetch<'w>),*> Fetch<'w> for AnyOf<($(($name, bool),)*)> { type Item = ($(Option<$name::Item>,)*); type State = AnyOf<($($name::State,)*)>; @@ -1620,7 +1645,7 @@ pub struct NopFetch { state: PhantomData, } -impl<'w, State: Send + Sync> Fetch<'w> for NopFetch { +unsafe impl<'w, State: Send + Sync> Fetch<'w> for NopFetch { type Item = (); type State = State; @@ -1661,6 +1686,15 @@ impl<'w, State: Send + Sync> Fetch<'w> for NopFetch { unimplemented!(); } + fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) {} + + fn update_archetype_component_access( + _state: &Self::State, + _archetype: &Archetype, + _access: &mut Access, + ) { + } + fn matches_component_set( _state: &Self::State, _set_contains_id: &impl Fn(ComponentId) -> bool, diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index cfffd86a89a6f..422e803d27f2b 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -66,7 +66,8 @@ impl WorldQueryGats<'_> for With { type _State = ComponentId; } -impl<'w, T: Component> Fetch<'w> for WithFetch { +// SAFETY: no component access or archetype component access +unsafe impl<'w, T: Component> Fetch<'w> for WithFetch { type Item = (); type State = ComponentId; @@ -117,6 +118,14 @@ impl<'w, T: Component> Fetch<'w> for WithFetch { access.add_with(*state); } + #[inline] + fn update_archetype_component_access( + _state: &Self::State, + _archetype: &Archetype, + _access: &mut Access, + ) { + } + fn matches_component_set( state: &Self::State, set_contains_id: &impl Fn(ComponentId) -> bool, @@ -186,7 +195,8 @@ impl WorldQueryGats<'_> for Without { type _State = ComponentId; } -impl<'w, T: Component> Fetch<'w> for WithoutFetch { +// SAFETY: no component access or archetype component access +unsafe impl<'w, T: Component> Fetch<'w> for WithoutFetch { type Item = (); type State = ComponentId; @@ -237,6 +247,13 @@ impl<'w, T: Component> Fetch<'w> for WithoutFetch { access.add_without(*state); } + fn update_archetype_component_access( + _state: &Self::State, + _archetype: &Archetype, + _access: &mut Access, + ) { + } + fn matches_component_set( state: &Self::State, set_contains_id: &impl Fn(ComponentId) -> bool, @@ -320,9 +337,10 @@ macro_rules! impl_query_filter_tuple { type _State = Or<($($filter::_State,)*)>; } + // SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple #[allow(unused_variables)] #[allow(non_snake_case)] - impl<'w, $($filter: Fetch<'w>),*> Fetch<'w> for Or<($(OrFetch<'w, $filter>,)*)> { + unsafe impl<'w, $($filter: Fetch<'w>),*> Fetch<'w> for Or<($(OrFetch<'w, $filter>,)*)> { type State = Or<($(<$filter as Fetch<'w>>::State,)*)>; type Item = bool; @@ -476,7 +494,8 @@ macro_rules! impl_tick_filter { type _State = ComponentId; } - impl<'w, T: Component> Fetch<'w> for $fetch_name<'w, T> { + // SAFETY: this reads the T component. archetype component access and component access are updated to reflect that + unsafe impl<'w, T: Component> Fetch<'w> for $fetch_name<'w, T> { type State = ComponentId; type Item = bool; From 48f8640eff2f59261c060525fb217b78141c2764 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 19 Jun 2022 05:04:19 -0700 Subject: [PATCH 08/11] Fix up CI --- crates/bevy_ecs/src/query/fetch.rs | 2 +- crates/bevy_ecs/src/query/filter.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 495e245a2915c..cf174f1160643 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -359,7 +359,7 @@ pub trait WorldQueryGats<'world> { /// /// Implementor must ensure that [`Fetch::update_component_access`] and /// [`Fetch::update_archetype_component_access`] exactly reflects the results of -/// [`FetchState::matches_component_set`], [`Fetch::archetype_fetch`], and +/// [`Fetch::matches_component_set`], [`Fetch::archetype_fetch`], and /// [`Fetch::table_fetch`]. pub unsafe trait Fetch<'world>: Sized { type Item; diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index bdf73bce2c8af..0311d00ecb002 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -3,8 +3,8 @@ use crate::{ component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType}, entity::Entity, query::{ - debug_checked_unreachable, Access, Fetch, FilteredAccess, QueryFetch, - WorldQuery, WorldQueryGats, + debug_checked_unreachable, Access, Fetch, FilteredAccess, QueryFetch, WorldQuery, + WorldQueryGats, }, storage::{ComponentSparseSet, Table, Tables}, world::World, From 42abc956601984f7d1a1529d10f50f9348b79b6f Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 19 Jun 2022 05:20:26 -0700 Subject: [PATCH 09/11] Avoid the fish --- crates/bevy_ecs/macros/src/fetch.rs | 6 ++--- crates/bevy_ecs/src/query/fetch.rs | 39 ++++++++++++----------------- crates/bevy_ecs/src/query/filter.rs | 18 ++++++------- 3 files changed, 28 insertions(+), 35 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 2db778d543d77..8231fdf52b6dd 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -257,17 +257,17 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { fn init_state(world: &mut #path::world::World) -> Self::State { #state_struct_name { - #(#field_idents: <<#field_types as #path::query::WorldQueryGats<'_>>::Fetch as #path::query::Fetch<'_>>::init_state(world),)* + #(#field_idents: #path::query::#fetch_type_alias::<'static, #field_types>::init_state(world),)* #(#ignored_field_idents: Default::default(),)* } } fn update_component_access(state: &Self::State, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) { - #(<<#field_types as #path::query::WorldQueryGats<'_>>::Fetch as #path::query::Fetch<'_>>::update_component_access(&state.#field_idents, _access);)* + #(#path::query::#fetch_type_alias::<'static, #field_types>::update_component_access(&state.#field_idents, _access);)* } fn matches_component_set(state: &Self::State, _set_contains_id: &impl Fn(#path::component::ComponentId) -> bool) -> bool { - true #(&& <<#field_types as #path::query::WorldQueryGats<'_>>::Fetch as #path::query::Fetch<'_>>::matches_component_set(&state.#field_idents, _set_contains_id))* + true #(&& #path::query::#fetch_type_alias::<'static, #field_types>::matches_component_set(&state.#field_idents, _set_contains_id))* } fn update_archetype_component_access(state: &Self::State, _archetype: &#path::archetype::Archetype, _access: &mut #path::query::Access<#path::archetype::ArchetypeComponentId>) { diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index cf174f1160643..c634b30359cdf 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -472,13 +472,6 @@ pub unsafe trait Fetch<'world>: Sized { ) -> bool; } -/// A fetch that is read only. -/// -/// # Safety -/// -/// This must only be implemented for read-only fetches. -pub unsafe trait ReadOnlyFetch {} - /// SAFETY: no component or archetype access unsafe impl WorldQuery for Entity { type ReadOnly = Self; @@ -968,7 +961,7 @@ unsafe impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { archetype: &'w Archetype, tables: &'w Tables, ) { - self.matches = >::matches_component_set(state, &|id| archetype.contains(id)); + self.matches = T::matches_component_set(state, &|id| archetype.contains(id)); if self.matches { self.fetch.set_archetype(state, archetype, tables); } @@ -976,7 +969,7 @@ unsafe impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { #[inline] unsafe fn set_table(&mut self, state: &Self::State, table: &'w Table) { - self.matches = >::matches_component_set(state, &|id| table.has_column(id)); + self.matches = T::matches_component_set(state, &|id| table.has_column(id)); if self.matches { self.fetch.set_table(state, table); } @@ -1010,7 +1003,7 @@ unsafe impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { // regardless of whether it has a `U` component. If we dont do this the query will not conflict with // `Query<&mut V, Without>` which would be unsound. let mut intermediate = access.clone(); - >::update_component_access(state, &mut intermediate); + T::update_component_access(state, &mut intermediate); access.extend_access(&intermediate); } @@ -1019,8 +1012,8 @@ unsafe impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { archetype: &Archetype, access: &mut Access, ) { - if >::matches_component_set(state, &|id| archetype.contains(id)) { - >::update_archetype_component_access(state, archetype, access); + if T::matches_component_set(state, &|id| archetype.contains(id)) { + T::update_archetype_component_access(state, archetype, access); } } @@ -1352,22 +1345,22 @@ macro_rules! impl_tuple_fetch { #[allow(clippy::unused_unit)] fn init_state(_world: &mut World) -> Self::State { - ($(<$name as Fetch<'_>>::init_state(_world),)*) + ($($name::init_state(_world),)*) } fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) { let ($($name,)*) = _state; - $(<$name as Fetch>::update_component_access($name, _access);)* + $($name::update_component_access($name, _access);)* } fn update_archetype_component_access(_state: &Self::State, _archetype: &Archetype, _access: &mut Access) { let ($($name,)*) = _state; - $(<$name as Fetch<'_>>::update_archetype_component_access($name, _archetype, _access);)* + $($name::update_archetype_component_access($name, _archetype, _access);)* } fn matches_component_set(_state: &Self::State, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool { let ($($name,)*) = _state; - true $(&& <$name as Fetch<'_>>::matches_component_set($name, _set_contains_id))* + true $(&& $name::matches_component_set($name, _set_contains_id))* } } @@ -1429,7 +1422,7 @@ macro_rules! impl_anytuple_fetch { let ($($name,)*) = &mut self.0; let ($($state,)*) = &_state.0; $( - $name.1 = <$name as Fetch<'_>>::matches_component_set($state, &|id| _archetype.contains(id)); + $name.1 = $name::matches_component_set($state, &|id| _archetype.contains(id)); if $name.1 { $name.0.set_archetype($state, _archetype, _tables); } @@ -1441,7 +1434,7 @@ macro_rules! impl_anytuple_fetch { let ($($name,)*) = &mut self.0; let ($($state,)*) = &_state.0; $( - $name.1 = <$name as Fetch<'_>>::matches_component_set($state, &|id| _table.has_column(id)); + $name.1 = $name::matches_component_set($state, &|id| _table.has_column(id)); if $name.1 { $name.0.set_table($state, _table); } @@ -1490,11 +1483,11 @@ macro_rules! impl_anytuple_fetch { $( if _not_first { let mut intermediate = _access.clone(); - <$name as Fetch<'_>>::update_component_access($name, &mut intermediate); + $name::update_component_access($name, &mut intermediate); _intersected_access.extend_intersect_filter(&intermediate); _intersected_access.extend_access(&intermediate); } else { - <$name as Fetch<'_>>::update_component_access($name, &mut _intersected_access); + $name::update_component_access($name, &mut _intersected_access); _not_first = true; } )* @@ -1504,14 +1497,14 @@ macro_rules! impl_anytuple_fetch { fn matches_component_set(_state: &Self::State, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool { let ($($name,)*) = &_state.0; - false $(|| <$name as Fetch<'_>>::matches_component_set($name, _set_contains_id))* + false $(|| $name::matches_component_set($name, _set_contains_id))* } fn update_archetype_component_access(_state: &Self::State, _archetype: &Archetype, _access: &mut Access) { let ($($name,)*) = &_state.0; $( - if <$name as Fetch<'_>>::matches_component_set($name, &|id| _archetype.contains(id)) { - <$name as Fetch<'_>>::update_archetype_component_access($name, _archetype, _access); + if $name::matches_component_set($name, &|id| _archetype.contains(id)) { + $name::update_archetype_component_access($name, _archetype, _access); } )* } diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 0311d00ecb002..5924b4dd43e19 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -346,17 +346,17 @@ macro_rules! impl_query_filter_tuple { #[allow(unused_variables)] #[allow(non_snake_case)] unsafe impl<'w, $($filter: Fetch<'w>),*> Fetch<'w> for Or<($(OrFetch<'w, $filter>,)*)> { - type State = Or<($(<$filter as Fetch<'w>>::State,)*)>; + type State = Or<($($filter::State,)*)>; type Item = bool; const IS_DENSE: bool = true $(&& $filter::IS_DENSE)*; const IS_ARCHETYPAL: bool = true $(&& $filter::IS_ARCHETYPAL)*; - unsafe fn init(world: &'w World, state: & Or<($(<$filter as Fetch<'w>>::State,)*)>, last_change_tick: u32, change_tick: u32) -> Self { + unsafe fn init(world: &'w World, state: & Or<($($filter::State,)*)>, last_change_tick: u32, change_tick: u32) -> Self { let ($($filter,)*) = &state.0; Or(($(OrFetch { - fetch: <$filter as Fetch<'w>>::init(world, $filter, last_change_tick, change_tick), + fetch: $filter::init(world, $filter, last_change_tick, change_tick), matches: false, _marker: PhantomData, },)*)) @@ -367,7 +367,7 @@ macro_rules! impl_query_filter_tuple { let ($($filter,)*) = &mut self.0; let ($($state,)*) = &state.0; $( - $filter.matches = <$filter as Fetch<'_>>::matches_component_set($state, &|id| table.has_column(id)); + $filter.matches = $filter::matches_component_set($state, &|id| table.has_column(id)); if $filter.matches { $filter.fetch.set_table($state, table); } @@ -379,7 +379,7 @@ macro_rules! impl_query_filter_tuple { let ($($filter,)*) = &mut self.0; let ($($state,)*) = &state.0; $( - $filter.matches = <$filter as Fetch<'_>>::matches_component_set($state, &|id| archetype.contains(id)); + $filter.matches = $filter::matches_component_set($state, &|id| archetype.contains(id)); if $filter.matches { $filter.fetch.set_archetype($state, archetype, tables); } @@ -432,11 +432,11 @@ macro_rules! impl_query_filter_tuple { $( if _not_first { let mut intermediate = access.clone(); - <$filter as Fetch<'_>>::update_component_access($filter, &mut intermediate); + $filter::update_component_access($filter, &mut intermediate); _intersected_access.extend_intersect_filter(&intermediate); _intersected_access.extend_access(&intermediate); } else { - <$filter as Fetch<'_>>::update_component_access($filter, &mut _intersected_access); + $filter::update_component_access($filter, &mut _intersected_access); _not_first = true; } )* @@ -446,12 +446,12 @@ macro_rules! impl_query_filter_tuple { fn update_archetype_component_access(state: &Self::State, archetype: &Archetype, access: &mut Access) { let ($($filter,)*) = &state.0; - $(<$filter as Fetch<'_>>::update_archetype_component_access($filter, archetype, access);)* + $($filter::update_archetype_component_access($filter, archetype, access);)* } fn matches_component_set(state: &Self::State, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool { let ($($filter,)*) = &state.0; - false $(|| <$filter as Fetch<'_>>::matches_component_set($filter, _set_contains_id))* + false $(|| $filter::matches_component_set($filter, _set_contains_id))* } } From 52a217e8ea6b7dec49b70c8d60407cb9096163eb Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 19 Jun 2022 05:38:18 -0700 Subject: [PATCH 10/11] Update init_state to WorldQuery --- crates/bevy_ecs/src/query/fetch.rs | 63 +++++++++++++---------------- crates/bevy_ecs/src/query/filter.rs | 32 +++++++-------- crates/bevy_ecs/src/query/state.rs | 4 +- 3 files changed, 47 insertions(+), 52 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index c634b30359cdf..4de24d59053e0 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -323,6 +323,8 @@ pub unsafe trait WorldQuery: for<'w> WorldQueryGats<'w, _State = Self::State> { type ReadOnly: ReadOnlyWorldQuery; type State: Send + Sync; + fn init(world: &mut World) -> Self::State; + /// This function manually implements variance for the query items. fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self>; } @@ -453,8 +455,6 @@ pub unsafe trait Fetch<'world>: Sized { true } - fn init_state(world: &mut World) -> Self::State; - // This does not have a default body of `{}` because 99% of cases need to add accesses // and forgetting to do so would be unsound. fn update_component_access(state: &Self::State, access: &mut FilteredAccess); @@ -477,6 +477,8 @@ unsafe impl WorldQuery for Entity { type ReadOnly = Self; type State = (); + fn init(_world: &mut World) -> Self::State {} + fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item } @@ -542,8 +544,6 @@ unsafe impl<'w> Fetch<'w> for EntityFetch<'w> { *entities.get(archetype_index) } - fn init_state(_world: &mut World) -> Self::State {} - fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) {} fn update_archetype_component_access( @@ -567,6 +567,10 @@ unsafe impl WorldQuery for &T { type ReadOnly = Self; type State = ComponentId; + fn init(world: &mut World) -> Self::State { + world.init_component::() + } + fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item } @@ -687,10 +691,6 @@ unsafe impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> { components.get(table_row).deref() } - fn init_state(world: &mut World) -> Self::State { - world.init_component::() - } - fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { assert!( !access.access().has_write(*state), @@ -723,6 +723,10 @@ unsafe impl<'w, T: Component> WorldQuery for &'w mut T { type ReadOnly = &'w T; type State = ComponentId; + fn init(world: &mut World) -> Self::State { + world.init_component::() + } + fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item } @@ -875,10 +879,6 @@ unsafe impl<'w, T: Component> Fetch<'w> for WriteFetch<'w, T> { } } - fn init_state(world: &mut World) -> Self::State { - world.init_component::() - } - fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { assert!( !access.access().has_read(*state), @@ -911,6 +911,10 @@ unsafe impl WorldQuery for Option { type ReadOnly = Option; type State = T::State; + fn init(world: &mut World) -> Self::State { + T::init(world) + } + fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item.map(T::shrink) } @@ -993,10 +997,6 @@ unsafe impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { } } - fn init_state(world: &mut World) -> Self::State { - T::init_state(world) - } - fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { // We don't want to add the `with`/`without` of `T` as `Option` will match things regardless of // `T`'s filters. for example `Query<(Option<&U>, &mut V)>` will match every entity with a `V` component @@ -1094,6 +1094,10 @@ unsafe impl WorldQuery for ChangeTrackers { type ReadOnly = Self; type State = ComponentId; + fn init(world: &mut World) -> Self::State { + world.init_component::() + } + fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item } @@ -1245,10 +1249,6 @@ unsafe impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> { } } - fn init_state(world: &mut World) -> Self::State { - world.init_component::() - } - fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { assert!( !access.access().has_write(*state), @@ -1343,11 +1343,6 @@ macro_rules! impl_tuple_fetch { true $(&& $name.archetype_filter_fetch(archetype_index))* } - #[allow(clippy::unused_unit)] - fn init_state(_world: &mut World) -> Self::State { - ($($name::init_state(_world),)*) - } - fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) { let ($($name,)*) = _state; $($name::update_component_access($name, _access);)* @@ -1371,6 +1366,11 @@ macro_rules! impl_tuple_fetch { type ReadOnly = ($($name::ReadOnly,)*); type State = ($($name::State,)*); + #[allow(clippy::unused_unit)] + fn init(_world: &mut World) -> Self::State { + ($($name::init(_world),)*) + } + fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { let ($($name,)*) = item; ($( @@ -1459,10 +1459,6 @@ macro_rules! impl_anytuple_fetch { )*) } - fn init_state(_world: &mut World) -> Self::State { - AnyOf(($($name::init_state(_world),)*)) - } - fn update_component_access(state: &Self::State, _access: &mut FilteredAccess) { let ($($name,)*) = &state.0; @@ -1517,6 +1513,10 @@ macro_rules! impl_anytuple_fetch { type ReadOnly = AnyOf<($($name::ReadOnly,)*)>; type State = AnyOf<($($name::State,)*)>; + fn init(_world: &mut World) -> Self::State { + AnyOf(($($name::init(_world),)*)) + } + fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { let ($($name,)*) = item; ($( @@ -1578,11 +1578,6 @@ unsafe impl<'w, State: Send + Sync> Fetch<'w> for NopFetch { #[inline(always)] unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item {} - #[inline(always)] - fn init_state(_world: &mut World) -> Self::State { - unimplemented!(); - } - fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) {} fn update_archetype_component_access( diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 5924b4dd43e19..191040c68f3e5 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -49,6 +49,10 @@ unsafe impl WorldQuery for With { type ReadOnly = Self; type State = ComponentId; + fn init(world: &mut World) -> Self::State { + world.init_component::() + } + #[allow(clippy::semicolon_if_nothing_returned)] fn shrink<'wlong: 'wshort, 'wshort>( item: super::QueryItem<'wlong, Self>, @@ -111,10 +115,6 @@ unsafe impl<'w, T: Component> Fetch<'w> for WithFetch { #[inline] unsafe fn table_fetch(&mut self, _table_row: usize) {} - fn init_state(world: &mut World) -> Self::State { - world.init_component::() - } - #[inline] fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { access.add_with(*state); @@ -180,6 +180,10 @@ unsafe impl WorldQuery for Without { type ReadOnly = Self; type State = ComponentId; + fn init(world: &mut World) -> Self::State { + world.init_component::() + } + #[allow(clippy::semicolon_if_nothing_returned)] fn shrink<'wlong: 'wshort, 'wshort>( item: super::QueryItem<'wlong, Self>, @@ -242,10 +246,6 @@ unsafe impl<'w, T: Component> Fetch<'w> for WithoutFetch { #[inline] unsafe fn table_fetch(&mut self, _table_row: usize) {} - fn init_state(world: &mut World) -> Self::State { - world.init_component::() - } - #[inline] fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { access.add_without(*state); @@ -330,6 +330,10 @@ macro_rules! impl_query_filter_tuple { type ReadOnly = Or<($($filter::ReadOnly,)*)>; type State = Or<($($filter::State,)*)>; + fn init(world: &mut World) -> Self::State { + Or(($($filter::init(world),)*)) + } + fn shrink<'wlong: 'wshort, 'wshort>(item: super::QueryItem<'wlong, Self>) -> super::QueryItem<'wshort, Self> { item } @@ -408,10 +412,6 @@ macro_rules! impl_query_filter_tuple { self.archetype_fetch(archetype_index) } - fn init_state(world: &mut World) -> Self::State { - Or(($($filter::init_state(world),)*)) - } - fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { let ($($filter,)*) = &state.0; @@ -490,6 +490,10 @@ macro_rules! impl_tick_filter { type ReadOnly = Self; type State = ComponentId; + fn init(world: &mut World) -> Self::State { + world.init_component::() + } + fn shrink<'wlong: 'wshort, 'wshort>(item: super::QueryItem<'wlong, Self>) -> super::QueryItem<'wshort, Self> { item } @@ -576,10 +580,6 @@ macro_rules! impl_tick_filter { self.archetype_fetch(archetype_index) } - fn init_state(world: &mut World) -> Self::State { - world.init_component::() - } - #[inline] fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { if access.access().has_write(*state) { diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 0661ea2658331..42eeac0c7c98c 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -40,8 +40,8 @@ impl FromWorld for QueryState { 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 fetch_state = Q::Fetch::init_state(world); - let filter_state = F::Fetch::init_state(world); + let fetch_state = Q::init(world); + let filter_state = F::init(world); let mut component_access = FilteredAccess::default(); Q::Fetch::update_component_access(&fetch_state, &mut component_access); From 9ca044f435c3651dab8ebab711754a8a3c22dbd7 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 19 Jun 2022 06:14:26 -0700 Subject: [PATCH 11/11] Revert "Update init_state to WorldQuery" This reverts commit 52a217e8ea6b7dec49b70c8d60407cb9096163eb. --- crates/bevy_ecs/src/query/fetch.rs | 63 ++++++++++++++++------------- crates/bevy_ecs/src/query/filter.rs | 32 +++++++-------- crates/bevy_ecs/src/query/state.rs | 4 +- 3 files changed, 52 insertions(+), 47 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 4de24d59053e0..c634b30359cdf 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -323,8 +323,6 @@ pub unsafe trait WorldQuery: for<'w> WorldQueryGats<'w, _State = Self::State> { type ReadOnly: ReadOnlyWorldQuery; type State: Send + Sync; - fn init(world: &mut World) -> Self::State; - /// This function manually implements variance for the query items. fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self>; } @@ -455,6 +453,8 @@ pub unsafe trait Fetch<'world>: Sized { true } + fn init_state(world: &mut World) -> Self::State; + // This does not have a default body of `{}` because 99% of cases need to add accesses // and forgetting to do so would be unsound. fn update_component_access(state: &Self::State, access: &mut FilteredAccess); @@ -477,8 +477,6 @@ unsafe impl WorldQuery for Entity { type ReadOnly = Self; type State = (); - fn init(_world: &mut World) -> Self::State {} - fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item } @@ -544,6 +542,8 @@ unsafe impl<'w> Fetch<'w> for EntityFetch<'w> { *entities.get(archetype_index) } + fn init_state(_world: &mut World) -> Self::State {} + fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) {} fn update_archetype_component_access( @@ -567,10 +567,6 @@ unsafe impl WorldQuery for &T { type ReadOnly = Self; type State = ComponentId; - fn init(world: &mut World) -> Self::State { - world.init_component::() - } - fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item } @@ -691,6 +687,10 @@ unsafe impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> { components.get(table_row).deref() } + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { assert!( !access.access().has_write(*state), @@ -723,10 +723,6 @@ unsafe impl<'w, T: Component> WorldQuery for &'w mut T { type ReadOnly = &'w T; type State = ComponentId; - fn init(world: &mut World) -> Self::State { - world.init_component::() - } - fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item } @@ -879,6 +875,10 @@ unsafe impl<'w, T: Component> Fetch<'w> for WriteFetch<'w, T> { } } + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { assert!( !access.access().has_read(*state), @@ -911,10 +911,6 @@ unsafe impl WorldQuery for Option { type ReadOnly = Option; type State = T::State; - fn init(world: &mut World) -> Self::State { - T::init(world) - } - fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item.map(T::shrink) } @@ -997,6 +993,10 @@ unsafe impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { } } + fn init_state(world: &mut World) -> Self::State { + T::init_state(world) + } + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { // We don't want to add the `with`/`without` of `T` as `Option` will match things regardless of // `T`'s filters. for example `Query<(Option<&U>, &mut V)>` will match every entity with a `V` component @@ -1094,10 +1094,6 @@ unsafe impl WorldQuery for ChangeTrackers { type ReadOnly = Self; type State = ComponentId; - fn init(world: &mut World) -> Self::State { - world.init_component::() - } - fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { item } @@ -1249,6 +1245,10 @@ unsafe impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> { } } + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { assert!( !access.access().has_write(*state), @@ -1343,6 +1343,11 @@ macro_rules! impl_tuple_fetch { true $(&& $name.archetype_filter_fetch(archetype_index))* } + #[allow(clippy::unused_unit)] + fn init_state(_world: &mut World) -> Self::State { + ($($name::init_state(_world),)*) + } + fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) { let ($($name,)*) = _state; $($name::update_component_access($name, _access);)* @@ -1366,11 +1371,6 @@ macro_rules! impl_tuple_fetch { type ReadOnly = ($($name::ReadOnly,)*); type State = ($($name::State,)*); - #[allow(clippy::unused_unit)] - fn init(_world: &mut World) -> Self::State { - ($($name::init(_world),)*) - } - fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { let ($($name,)*) = item; ($( @@ -1459,6 +1459,10 @@ macro_rules! impl_anytuple_fetch { )*) } + fn init_state(_world: &mut World) -> Self::State { + AnyOf(($($name::init_state(_world),)*)) + } + fn update_component_access(state: &Self::State, _access: &mut FilteredAccess) { let ($($name,)*) = &state.0; @@ -1513,10 +1517,6 @@ macro_rules! impl_anytuple_fetch { type ReadOnly = AnyOf<($($name::ReadOnly,)*)>; type State = AnyOf<($($name::State,)*)>; - fn init(_world: &mut World) -> Self::State { - AnyOf(($($name::init(_world),)*)) - } - fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { let ($($name,)*) = item; ($( @@ -1578,6 +1578,11 @@ unsafe impl<'w, State: Send + Sync> Fetch<'w> for NopFetch { #[inline(always)] unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item {} + #[inline(always)] + fn init_state(_world: &mut World) -> Self::State { + unimplemented!(); + } + fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess) {} fn update_archetype_component_access( diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 191040c68f3e5..5924b4dd43e19 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -49,10 +49,6 @@ unsafe impl WorldQuery for With { type ReadOnly = Self; type State = ComponentId; - fn init(world: &mut World) -> Self::State { - world.init_component::() - } - #[allow(clippy::semicolon_if_nothing_returned)] fn shrink<'wlong: 'wshort, 'wshort>( item: super::QueryItem<'wlong, Self>, @@ -115,6 +111,10 @@ unsafe impl<'w, T: Component> Fetch<'w> for WithFetch { #[inline] unsafe fn table_fetch(&mut self, _table_row: usize) {} + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + #[inline] fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { access.add_with(*state); @@ -180,10 +180,6 @@ unsafe impl WorldQuery for Without { type ReadOnly = Self; type State = ComponentId; - fn init(world: &mut World) -> Self::State { - world.init_component::() - } - #[allow(clippy::semicolon_if_nothing_returned)] fn shrink<'wlong: 'wshort, 'wshort>( item: super::QueryItem<'wlong, Self>, @@ -246,6 +242,10 @@ unsafe impl<'w, T: Component> Fetch<'w> for WithoutFetch { #[inline] unsafe fn table_fetch(&mut self, _table_row: usize) {} + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + #[inline] fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { access.add_without(*state); @@ -330,10 +330,6 @@ macro_rules! impl_query_filter_tuple { type ReadOnly = Or<($($filter::ReadOnly,)*)>; type State = Or<($($filter::State,)*)>; - fn init(world: &mut World) -> Self::State { - Or(($($filter::init(world),)*)) - } - fn shrink<'wlong: 'wshort, 'wshort>(item: super::QueryItem<'wlong, Self>) -> super::QueryItem<'wshort, Self> { item } @@ -412,6 +408,10 @@ macro_rules! impl_query_filter_tuple { self.archetype_fetch(archetype_index) } + fn init_state(world: &mut World) -> Self::State { + Or(($($filter::init_state(world),)*)) + } + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { let ($($filter,)*) = &state.0; @@ -490,10 +490,6 @@ macro_rules! impl_tick_filter { type ReadOnly = Self; type State = ComponentId; - fn init(world: &mut World) -> Self::State { - world.init_component::() - } - fn shrink<'wlong: 'wshort, 'wshort>(item: super::QueryItem<'wlong, Self>) -> super::QueryItem<'wshort, Self> { item } @@ -580,6 +576,10 @@ macro_rules! impl_tick_filter { self.archetype_fetch(archetype_index) } + fn init_state(world: &mut World) -> Self::State { + world.init_component::() + } + #[inline] fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { if access.access().has_write(*state) { diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 42eeac0c7c98c..0661ea2658331 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -40,8 +40,8 @@ impl FromWorld for QueryState { 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 fetch_state = Q::init(world); - let filter_state = F::init(world); + let fetch_state = Q::Fetch::init_state(world); + let filter_state = F::Fetch::init_state(world); let mut component_access = FilteredAccess::default(); Q::Fetch::update_component_access(&fetch_state, &mut component_access);