diff --git a/crates/bevy_core/src/time/fixed_timestep.rs b/crates/bevy_core/src/time/fixed_timestep.rs index ca5262721f43b..3bbd6f18803b2 100644 --- a/crates/bevy_core/src/time/fixed_timestep.rs +++ b/crates/bevy_core/src/time/fixed_timestep.rs @@ -196,4 +196,8 @@ impl System for FixedTimestep { ); } } + + fn check_change_tick(&mut self, change_tick: u32) { + self.internal_system.check_change_tick(change_tick); + } } diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 9b4ccbfc2be6e..b35d8c054711c 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -270,11 +270,12 @@ pub fn impl_query_set(_input: TokenStream) -> TokenStream { #[inline] unsafe fn get_param( state: &'a mut Self, - _system_state: &'a SystemState, + system_state: &'a SystemState, world: &'a World, + change_tick: u32, ) -> Self::Item { let (#(#query,)*) = &state.0; - QuerySet((#(Query::new(world, #query),)*)) + QuerySet((#(Query::new(world, #query, system_state.last_change_tick, change_tick),)*)) } } @@ -402,9 +403,10 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { state: &'a mut Self, system_state: &'a #path::system::SystemState, world: &'a #path::world::World, + change_tick: u32, ) -> Self::Item { #struct_name { - #(#fields: <<#field_types as SystemParam>::Fetch as #path::system::SystemParamFetch>::get_param(&mut state.state.#field_indices, system_state, world),)* + #(#fields: <<#field_types as SystemParam>::Fetch as #path::system::SystemParamFetch>::get_param(&mut state.state.#field_indices, system_state, world, change_tick),)* #(#ignored_fields: <#ignored_field_types>::default(),)* } } diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 1bca3dbc4b233..6d5efe815a27c 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -1,6 +1,6 @@ use crate::{ bundle::BundleId, - component::{ComponentFlags, ComponentId, StorageType}, + component::{ComponentId, StorageType}, entity::{Entity, EntityLocation}, storage::{Column, SparseArray, SparseSet, SparseSetIndex, TableId}, }; @@ -36,9 +36,14 @@ impl ArchetypeId { } } +pub enum ComponentStatus { + Added, + Mutated, +} + pub struct FromBundle { pub archetype_id: ArchetypeId, - pub bundle_flags: Vec, + pub bundle_status: Vec, } #[derive(Default)] @@ -70,13 +75,13 @@ impl Edges { &mut self, bundle_id: BundleId, archetype_id: ArchetypeId, - bundle_flags: Vec, + bundle_status: Vec, ) { self.from_bundle.insert( bundle_id, FromBundle { archetype_id, - bundle_flags, + bundle_status, }, ); } diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 1c0e6a44caf76..c95e00e8c05d5 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1,7 +1,8 @@ pub use bevy_ecs_macros::Bundle; use crate::{ - component::{Component, ComponentFlags, ComponentId, Components, StorageType, TypeInfo}, + archetype::ComponentStatus, + component::{Component, ComponentId, ComponentTicks, Components, StorageType, TypeInfo}, entity::Entity, storage::{SparseSetIndex, SparseSets, Table}, }; @@ -123,6 +124,7 @@ pub struct BundleInfo { impl BundleInfo { /// # Safety /// table row must exist, entity must be valid + #[allow(clippy::clippy::too_many_arguments)] #[inline] pub(crate) unsafe fn write_components( &self, @@ -130,8 +132,9 @@ impl BundleInfo { entity: Entity, table: &Table, table_row: usize, - bundle_flags: &[ComponentFlags], + bundle_status: &[ComponentStatus], bundle: T, + change_tick: u32, ) { // NOTE: get_components calls this closure on each component in "bundle order". // bundle_info.component_ids are also in "bundle order" @@ -139,16 +142,38 @@ impl BundleInfo { bundle.get_components(|component_ptr| { // SAFE: component_id was initialized by get_dynamic_bundle_info let component_id = *self.component_ids.get_unchecked(bundle_component); - let flags = *bundle_flags.get_unchecked(bundle_component); + let component_status = bundle_status.get_unchecked(bundle_component); match self.storage_types[bundle_component] { StorageType::Table => { let column = table.get_column(component_id).unwrap(); column.set_unchecked(table_row, component_ptr); - column.get_flags_unchecked_mut(table_row).insert(flags); + let column_status = column.get_ticks_unchecked_mut(table_row); + match component_status { + ComponentStatus::Added => { + *column_status = ComponentTicks::new(change_tick); + } + ComponentStatus::Mutated => { + column_status.set_changed(change_tick); + } + } } StorageType::SparseSet => { let sparse_set = sparse_sets.get_mut(component_id).unwrap(); - sparse_set.insert(entity, component_ptr, flags); + match component_status { + ComponentStatus::Added => { + sparse_set.insert( + entity, + component_ptr, + ComponentTicks::new(change_tick), + ); + } + ComponentStatus::Mutated => { + sparse_set + .get_ticks(entity) + .unwrap() + .set_changed(change_tick); + } + } } } bundle_component += 1; diff --git a/crates/bevy_ecs/src/component/mod.rs b/crates/bevy_ecs/src/component/mod.rs index f2038a151ffc2..6892c0963fc10 100644 --- a/crates/bevy_ecs/src/component/mod.rs +++ b/crates/bevy_ecs/src/component/mod.rs @@ -3,7 +3,6 @@ mod type_info; pub use type_info::*; use crate::storage::SparseSetIndex; -use bitflags::bitflags; use std::{ alloc::Layout, any::{Any, TypeId}, @@ -303,9 +302,55 @@ impl Components { } } -bitflags! { - pub struct ComponentFlags: u8 { - const ADDED = 1; - const MUTATED = 2; +#[derive(Copy, Clone, Debug)] +pub struct ComponentTicks { + pub(crate) added: u32, + pub(crate) changed: u32, +} + +impl ComponentTicks { + #[inline] + pub fn is_added(&self, last_change_tick: u32, change_tick: u32) -> bool { + // The comparison is relative to `change_tick` so that we can detect changes over the whole + // `u32` range. Comparing directly the ticks would limit to half that due to overflow + // handling. + let component_delta = change_tick.wrapping_sub(self.added); + let system_delta = change_tick.wrapping_sub(last_change_tick); + + component_delta < system_delta + } + + #[inline] + pub fn is_changed(&self, last_change_tick: u32, change_tick: u32) -> bool { + let component_delta = change_tick.wrapping_sub(self.changed); + let system_delta = change_tick.wrapping_sub(last_change_tick); + + component_delta < system_delta + } + + pub(crate) fn new(change_tick: u32) -> Self { + Self { + added: change_tick, + changed: change_tick, + } + } + + pub(crate) fn check_ticks(&mut self, change_tick: u32) { + check_tick(&mut self.added, change_tick); + check_tick(&mut self.changed, change_tick); + } + + #[inline] + pub(crate) fn set_changed(&mut self, change_tick: u32) { + self.changed = change_tick; + } +} + +fn check_tick(last_change_tick: &mut u32, change_tick: u32) { + let tick_delta = change_tick.wrapping_sub(*last_change_tick); + const MAX_DELTA: u32 = (u32::MAX / 4) * 3; + // Clamp to max delta + if tick_delta > MAX_DELTA { + *last_change_tick = change_tick.wrapping_sub(MAX_DELTA); } } diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index e3afbdc113eb7..aa1be8d483bb5 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -16,7 +16,7 @@ pub mod prelude { pub use crate::{ bundle::Bundle, entity::Entity, - query::{Added, Changed, Flags, Mutated, Or, QueryState, With, WithBundle, Without}, + query::{Added, ChangeTrackers, Changed, Or, QueryState, With, WithBundle, Without}, schedule::{ AmbiguitySetLabel, ExclusiveSystemDescriptorCoercion, ParallelSystemDescriptorCoercion, Schedule, Stage, StageLabel, State, SystemLabel, SystemSet, SystemStage, @@ -35,7 +35,7 @@ mod tests { bundle::Bundle, component::{Component, ComponentDescriptor, StorageType, TypeInfo}, entity::Entity, - query::{Added, Changed, FilterFetch, Flags, Mutated, Or, With, Without, WorldQuery}, + query::{Added, ChangeTrackers, Changed, FilterFetch, With, Without, WorldQuery}, world::{Mut, World}, }; use bevy_tasks::TaskPool; @@ -702,13 +702,15 @@ mod tests { } #[test] - fn mutated_trackers() { + fn changed_trackers() { let mut world = World::default(); let e1 = world.spawn().insert_bundle((A(0), B(0))).id(); let e2 = world.spawn().insert_bundle((A(0), B(0))).id(); let e3 = world.spawn().insert_bundle((A(0), B(0))).id(); world.spawn().insert_bundle((A(0), B)); + world.clear_trackers(); + for (i, mut a) in world.query::<&mut A>().iter_mut(&mut world).enumerate() { if i % 2 == 0 { a.0 += 1; @@ -725,25 +727,25 @@ mod tests { .collect::>() } - assert_eq!(get_filtered::>(&mut world), vec![e1, e3]); + assert_eq!(get_filtered::>(&mut world), vec![e1, e3]); - // ensure changing an entity's archetypes also moves its mutated state + // ensure changing an entity's archetypes also moves its changed state world.entity_mut(e1).insert(C); - assert_eq!(get_filtered::>(&mut world), vec![e3, e1], "changed entities list should not change (although the order will due to archetype moves)"); + assert_eq!(get_filtered::>(&mut world), vec![e3, e1], "changed entities list should not change (although the order will due to archetype moves)"); - // spawning a new A entity should not change existing mutated state + // spawning a new A entity should not change existing changed state world.entity_mut(e1).insert_bundle((A(0), B)); assert_eq!( - get_filtered::>(&mut world), + get_filtered::>(&mut world), vec![e3, e1], "changed entities list should not change" ); - // removing an unchanged entity should not change mutated state + // removing an unchanged entity should not change changed state assert!(world.despawn(e2)); assert_eq!( - get_filtered::>(&mut world), + get_filtered::>(&mut world), vec![e3, e1], "changed entities list should not change" ); @@ -751,35 +753,34 @@ mod tests { // removing a changed entity should remove it from enumeration assert!(world.despawn(e1)); assert_eq!( - get_filtered::>(&mut world), + get_filtered::>(&mut world), vec![e3], "e1 should no longer be returned" ); world.clear_trackers(); - assert!(get_filtered::>(&mut world).is_empty()); + assert!(get_filtered::>(&mut world).is_empty()); let e4 = world.spawn().id(); world.entity_mut(e4).insert(A(0)); - assert!(get_filtered::>(&mut world).is_empty()); + assert_eq!(get_filtered::>(&mut world), vec![e4]); assert_eq!(get_filtered::>(&mut world), vec![e4]); world.entity_mut(e4).insert(A(1)); - assert_eq!(get_filtered::>(&mut world), vec![e4]); + assert_eq!(get_filtered::>(&mut world), vec![e4]); world.clear_trackers(); - // ensure inserting multiple components set mutated state for - // already existing components and set added state for - // non existing components even when changing archetype. + // ensure inserting multiple components set changed state for all components and set added + // state for non existing components even when changing archetype. world.entity_mut(e4).insert_bundle((A(0), B(0))); assert!(get_filtered::>(&mut world).is_empty()); - assert_eq!(get_filtered::>(&mut world), vec![e4]); + assert_eq!(get_filtered::>(&mut world), vec![e4]); assert_eq!(get_filtered::>(&mut world), vec![e4]); - assert!(get_filtered::>(&mut world).is_empty()); + assert_eq!(get_filtered::>(&mut world), vec![e4]); } #[test] @@ -801,48 +802,6 @@ mod tests { assert_eq!(e_mut.get::().unwrap(), &A(0)); } - #[test] - fn multiple_mutated_query() { - let mut world = World::default(); - world.spawn().insert_bundle((A(0), B(0))).id(); - let e2 = world.spawn().insert_bundle((A(0), B(0))).id(); - world.spawn().insert_bundle((A(0), B(0))); - - for mut a in world.query::<&mut A>().iter_mut(&mut world) { - a.0 += 1; - } - - for mut b in world.query::<&mut B>().iter_mut(&mut world).skip(1).take(1) { - b.0 += 1; - } - - let a_b_mutated = world - .query_filtered::, Mutated)>() - .iter(&world) - .collect::>(); - assert_eq!(a_b_mutated, vec![e2]); - } - - #[test] - fn or_mutated_query() { - let mut world = World::default(); - let e1 = world.spawn().insert_bundle((A(0), B(0))).id(); - let e2 = world.spawn().insert_bundle((A(0), B(0))).id(); - let _e3 = world.spawn().insert_bundle((A(0), B(0))).id(); - let e4 = world.spawn().insert(A(0)).id(); // ensure filters work for archetypes with only one of the Or filter items - - *world.entity_mut(e1).get_mut::().unwrap() = A(1); - *world.entity_mut(e2).get_mut::().unwrap() = B(1); - *world.entity_mut(e4).get_mut::().unwrap() = A(1); - - let a_b_mutated = world - .query_filtered::, Mutated)>>() - .iter(&world) - .collect::>(); - // e1 has mutated A, e3 has mutated B, e2 has mutated A and B, _e4 has no mutated component - assert_eq!(a_b_mutated, vec![e1, e2, e4]); - } - #[test] fn changed_query() { let mut world = World::default(); @@ -1060,30 +1019,27 @@ mod tests { } #[test] - fn flags_query() { + fn trackers_query() { let mut world = World::default(); let e1 = world.spawn().insert_bundle((A(0), B(0))).id(); world.spawn().insert(B(0)); - let mut flags_query = world.query::>>(); - let flags = flags_query.iter(&world).collect::>(); - let a_flags = flags[0].as_ref().unwrap(); - assert!(flags[1].is_none()); - assert!(a_flags.added()); - assert!(!a_flags.mutated()); - assert!(a_flags.changed()); + let mut trackers_query = world.query::>>(); + let trackers = trackers_query.iter(&world).collect::>(); + let a_trackers = trackers[0].as_ref().unwrap(); + assert!(trackers[1].is_none()); + assert!(a_trackers.is_added()); + assert!(a_trackers.is_changed()); world.clear_trackers(); - let flags = flags_query.iter(&world).collect::>(); - let a_flags = flags[0].as_ref().unwrap(); - assert!(!a_flags.added()); - assert!(!a_flags.mutated()); - assert!(!a_flags.changed()); + let trackers = trackers_query.iter(&world).collect::>(); + let a_trackers = trackers[0].as_ref().unwrap(); + assert!(!a_trackers.is_added()); + assert!(!a_trackers.is_changed()); *world.get_mut(e1).unwrap() = A(1); - let flags = flags_query.iter(&world).collect::>(); - let a_flags = flags[0].as_ref().unwrap(); - assert!(!a_flags.added()); - assert!(a_flags.mutated()); - assert!(a_flags.changed()); + let trackers = trackers_query.iter(&world).collect::>(); + let a_trackers = trackers[0].as_ref().unwrap(); + assert!(!a_trackers.is_added()); + assert!(a_trackers.is_changed()); } #[test] diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index e2b78036b120c..91f423be28725 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1,6 +1,6 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, - component::{Component, ComponentFlags, ComponentId, StorageType}, + component::{Component, ComponentId, ComponentTicks, StorageType}, entity::Entity, query::{Access, FilteredAccess}, storage::{ComponentSparseSet, Table, Tables}, @@ -26,7 +26,12 @@ pub trait Fetch<'w>: Sized { /// # Safety /// `state` must have been initialized (via [FetchState::init]) using the same `world` passed in /// to this function. - unsafe fn init(world: &World, state: &Self::State) -> Self; + unsafe fn init( + world: &World, + state: &Self::State, + last_change_tick: u32, + change_tick: u32, + ) -> Self; /// Returns true if (and only if) every table of every archetype matched by this Fetch contains /// all of the matched components. This is used to select a more efficient "table iterator" @@ -140,7 +145,12 @@ impl<'w> Fetch<'w> for EntityFetch { true } - unsafe fn init(_world: &World, _state: &Self::State) -> Self { + unsafe fn init( + _world: &World, + _state: &Self::State, + _last_change_tick: u32, + _change_tick: u32, + ) -> Self { Self { entities: std::ptr::null::(), } @@ -243,7 +253,12 @@ impl<'w, T: Component> Fetch<'w> for ReadFetch { } } - unsafe fn init(world: &World, state: &Self::State) -> Self { + unsafe fn init( + world: &World, + state: &Self::State, + _last_change_tick: u32, + _change_tick: u32, + ) -> Self { let mut value = Self { storage_type: state.storage_type, table_components: NonNull::dangling(), @@ -317,10 +332,12 @@ impl WorldQuery for &mut T { pub struct WriteFetch { storage_type: StorageType, table_components: NonNull, - table_flags: *mut ComponentFlags, + table_ticks: *mut ComponentTicks, entities: *const Entity, entity_table_rows: *const usize, sparse_set: *const ComponentSparseSet, + last_change_tick: u32, + change_tick: u32, } pub struct WriteState { @@ -382,14 +399,21 @@ impl<'w, T: Component> Fetch<'w> for WriteFetch { } } - unsafe fn init(world: &World, state: &Self::State) -> Self { + unsafe fn init( + world: &World, + state: &Self::State, + last_change_tick: u32, + change_tick: u32, + ) -> Self { let mut value = Self { storage_type: state.storage_type, table_components: NonNull::dangling(), entities: ptr::null::(), entity_table_rows: ptr::null::(), sparse_set: ptr::null::(), - table_flags: ptr::null_mut::(), + table_ticks: ptr::null_mut::(), + last_change_tick, + change_tick, }; if state.storage_type == StorageType::SparseSet { value.sparse_set = world @@ -415,7 +439,7 @@ impl<'w, T: Component> Fetch<'w> for WriteFetch { .get_column(state.component_id) .unwrap(); self.table_components = column.get_ptr().cast::(); - self.table_flags = column.get_flags_mut_ptr(); + self.table_ticks = column.get_ticks_mut_ptr(); } StorageType::SparseSet => self.entities = archetype.entities().as_ptr(), } @@ -425,7 +449,7 @@ impl<'w, T: Component> Fetch<'w> for WriteFetch { unsafe fn set_table(&mut self, state: &Self::State, table: &Table) { let column = table.get_column(state.component_id).unwrap(); self.table_components = column.get_ptr().cast::(); - self.table_flags = column.get_flags_mut_ptr(); + self.table_ticks = column.get_ticks_mut_ptr(); } #[inline] @@ -435,15 +459,20 @@ impl<'w, T: Component> Fetch<'w> for WriteFetch { let table_row = *self.entity_table_rows.add(archetype_index); Mut { value: &mut *self.table_components.as_ptr().add(table_row), - flags: &mut *self.table_flags.add(table_row), + component_ticks: &mut *self.table_ticks.add(table_row), + change_tick: self.change_tick, + last_change_tick: self.last_change_tick, } } StorageType::SparseSet => { let entity = *self.entities.add(archetype_index); - let (component, flags) = (*self.sparse_set).get_with_flags(entity).unwrap(); + let (component, component_ticks) = + (*self.sparse_set).get_with_ticks(entity).unwrap(); Mut { value: &mut *component.cast::(), - flags: &mut *flags, + component_ticks: &mut *component_ticks, + change_tick: self.change_tick, + last_change_tick: self.last_change_tick, } } } @@ -453,7 +482,9 @@ impl<'w, T: Component> Fetch<'w> for WriteFetch { unsafe fn table_fetch(&mut self, table_row: usize) -> Self::Item { Mut { value: &mut *self.table_components.as_ptr().add(table_row), - flags: &mut *self.table_flags.add(table_row), + component_ticks: &mut *self.table_ticks.add(table_row), + change_tick: self.change_tick, + last_change_tick: self.last_change_tick, } } } @@ -517,9 +548,14 @@ impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { self.fetch.is_dense() } - unsafe fn init(world: &World, state: &Self::State) -> Self { + unsafe fn init( + world: &World, + state: &Self::State, + last_change_tick: u32, + change_tick: u32, + ) -> Self { Self { - fetch: T::init(world, &state.state), + fetch: T::init(world, &state.state, last_change_tick, change_tick), matches: false, } } @@ -564,45 +600,44 @@ impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch { } } -/// Flags on component `T` that happened since the start of the frame. +/// Change trackers for component `T` #[derive(Clone)] -pub struct Flags { - flags: ComponentFlags, +pub struct ChangeTrackers { + pub(crate) component_ticks: ComponentTicks, + pub(crate) last_change_tick: u32, + pub(crate) change_tick: u32, marker: PhantomData, } -impl std::fmt::Debug for Flags { +impl std::fmt::Debug for ChangeTrackers { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("Flags") - .field("added", &self.added()) - .field("mutated", &self.mutated()) + f.debug_struct("ChangeTrackers") + .field("component_ticks", &self.component_ticks) + .field("last_change_tick", &self.last_change_tick) + .field("change_tick", &self.change_tick) .finish() } } -impl Flags { - /// Has this component been added since the start of the frame. - pub fn added(&self) -> bool { - self.flags.contains(ComponentFlags::ADDED) - } - - /// Has this component been mutated since the start of the frame. - pub fn mutated(&self) -> bool { - self.flags.contains(ComponentFlags::MUTATED) +impl ChangeTrackers { + /// Has this component been added since the last execution of this system. + pub fn is_added(&self) -> bool { + self.component_ticks + .is_added(self.last_change_tick, self.change_tick) } - /// Has this component been either mutated or added since the start of the frame. - pub fn changed(&self) -> bool { - self.flags - .intersects(ComponentFlags::ADDED | ComponentFlags::MUTATED) + /// Has this component been changed since the last execution of this system. + pub fn is_changed(&self) -> bool { + self.component_ticks + .is_changed(self.last_change_tick, self.change_tick) } } -impl WorldQuery for Flags { - type Fetch = FlagsFetch; - type State = FlagsState; +impl WorldQuery for ChangeTrackers { + type Fetch = ChangeTrackersFetch; + type State = ChangeTrackersState; } -pub struct FlagsState { +pub struct ChangeTrackersState { component_id: ComponentId, storage_type: StorageType, marker: PhantomData, @@ -610,7 +645,7 @@ pub struct FlagsState { // SAFE: component access and archetype component access are properly updated to reflect that T is // read -unsafe impl FetchState for FlagsState { +unsafe impl FetchState for ChangeTrackersState { fn init(world: &mut World) -> Self { let component_info = world.components.get_or_insert_info::(); Self { @@ -645,21 +680,23 @@ unsafe impl FetchState for FlagsState { } } -pub struct FlagsFetch { +pub struct ChangeTrackersFetch { storage_type: StorageType, - table_flags: *const ComponentFlags, + table_ticks: *const ComponentTicks, entity_table_rows: *const usize, entities: *const Entity, sparse_set: *const ComponentSparseSet, marker: PhantomData, + last_change_tick: u32, + change_tick: u32, } /// SAFE: access is read only -unsafe impl ReadOnlyFetch for FlagsFetch {} +unsafe impl ReadOnlyFetch for ChangeTrackersFetch {} -impl<'w, T: Component> Fetch<'w> for FlagsFetch { - type Item = Flags; - type State = FlagsState; +impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch { + type Item = ChangeTrackers; + type State = ChangeTrackersState; #[inline] fn is_dense(&self) -> bool { @@ -669,14 +706,21 @@ impl<'w, T: Component> Fetch<'w> for FlagsFetch { } } - unsafe fn init(world: &World, state: &Self::State) -> Self { + unsafe fn init( + world: &World, + state: &Self::State, + last_change_tick: u32, + change_tick: u32, + ) -> Self { let mut value = Self { storage_type: state.storage_type, - table_flags: ptr::null::(), + table_ticks: ptr::null::(), entities: ptr::null::(), entity_table_rows: ptr::null::(), sparse_set: ptr::null::(), marker: PhantomData, + last_change_tick, + change_tick, }; if state.storage_type == StorageType::SparseSet { value.sparse_set = world @@ -701,7 +745,7 @@ impl<'w, T: Component> Fetch<'w> for FlagsFetch { let column = tables[archetype.table_id()] .get_column(state.component_id) .unwrap(); - self.table_flags = column.get_flags_mut_ptr().cast::(); + self.table_ticks = column.get_ticks_mut_ptr().cast::(); } StorageType::SparseSet => self.entities = archetype.entities().as_ptr(), } @@ -709,11 +753,11 @@ impl<'w, T: Component> Fetch<'w> for FlagsFetch { #[inline] unsafe fn set_table(&mut self, state: &Self::State, table: &Table) { - self.table_flags = table + self.table_ticks = table .get_column(state.component_id) .unwrap() - .get_flags_mut_ptr() - .cast::(); + .get_ticks_mut_ptr() + .cast::(); } #[inline] @@ -721,16 +765,20 @@ impl<'w, T: Component> Fetch<'w> for FlagsFetch { match self.storage_type { StorageType::Table => { let table_row = *self.entity_table_rows.add(archetype_index); - Flags { - flags: *self.table_flags.add(table_row), + ChangeTrackers { + component_ticks: *self.table_ticks.add(table_row), marker: PhantomData, + last_change_tick: self.last_change_tick, + change_tick: self.change_tick, } } StorageType::SparseSet => { let entity = *self.entities.add(archetype_index); - Flags { - flags: *(*self.sparse_set).get_flags(entity).unwrap(), + ChangeTrackers { + component_ticks: *(*self.sparse_set).get_ticks(entity).unwrap(), marker: PhantomData, + last_change_tick: self.last_change_tick, + change_tick: self.change_tick, } } } @@ -738,9 +786,11 @@ impl<'w, T: Component> Fetch<'w> for FlagsFetch { #[inline] unsafe fn table_fetch(&mut self, table_row: usize) -> Self::Item { - Flags { - flags: *self.table_flags.add(table_row), + ChangeTrackers { + component_ticks: *self.table_ticks.add(table_row), marker: PhantomData, + last_change_tick: self.last_change_tick, + change_tick: self.change_tick, } } } @@ -752,9 +802,9 @@ macro_rules! impl_tuple_fetch { type Item = ($($name::Item,)*); type State = ($($name::State,)*); - unsafe fn init(_world: &World, state: &Self::State) -> Self { + unsafe fn init(_world: &World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { let ($($name,)*) = state; - ($($name::init(_world, $name),)*) + ($($name::init(_world, $name, _last_change_tick, _change_tick),)*) } diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index a6dd1c6d82851..ed611b5b7b323 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -1,7 +1,7 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, bundle::Bundle, - component::{Component, ComponentFlags, ComponentId, StorageType}, + component::{Component, ComponentId, ComponentTicks, StorageType}, entity::Entity, query::{Access, Fetch, FetchState, FilteredAccess, WorldQuery}, storage::{ComponentSparseSet, Table, Tables}, @@ -50,8 +50,7 @@ where } } -/// Filter that retrieves components of type `T` that have either been mutated or added since the -/// start of the frame. +/// Filter that selects entities with a component `T` pub struct With(PhantomData); impl WorldQuery for With { @@ -106,7 +105,12 @@ impl<'a, T: Component> Fetch<'a> for WithFetch { type Item = bool; type State = WithState; - unsafe fn init(_world: &World, state: &Self::State) -> Self { + unsafe fn init( + _world: &World, + state: &Self::State, + _last_change_tick: u32, + _change_tick: u32, + ) -> Self { Self { storage_type: state.storage_type, marker: PhantomData, @@ -141,8 +145,7 @@ impl<'a, T: Component> Fetch<'a> for WithFetch { } } -/// Filter that retrieves components of type `T` that have either been mutated or added since the -/// start of the frame. +/// Filter that selects entities without a component `T` pub struct Without(PhantomData); impl WorldQuery for Without { @@ -198,7 +201,12 @@ impl<'a, T: Component> Fetch<'a> for WithoutFetch { type Item = bool; type State = WithoutState; - unsafe fn init(_world: &World, state: &Self::State) -> Self { + unsafe fn init( + _world: &World, + state: &Self::State, + _last_change_tick: u32, + _change_tick: u32, + ) -> Self { Self { storage_type: state.storage_type, marker: PhantomData, @@ -288,7 +296,12 @@ impl<'a, T: Bundle> Fetch<'a> for WithBundleFetch { type Item = bool; type State = WithBundleState; - unsafe fn init(_world: &World, state: &Self::State) -> Self { + unsafe fn init( + _world: &World, + state: &Self::State, + _last_change_tick: u32, + _change_tick: u32, + ) -> Self { Self { is_dense: state.is_dense, marker: PhantomData, @@ -361,10 +374,10 @@ macro_rules! impl_query_filter_tuple { type State = Or<($(<$filter as Fetch<'a>>::State,)*)>; type Item = bool; - unsafe fn init(world: &World, state: &Self::State) -> Self { + unsafe fn init(world: &World, state: &Self::State, last_change_tick: u32, change_tick: u32) -> Self { let ($($filter,)*) = &state.0; Or(($(OrFetch { - fetch: $filter::init(world, $filter), + fetch: $filter::init(world, $filter, last_change_tick, change_tick), matches: false, },)*)) } @@ -445,20 +458,22 @@ macro_rules! impl_query_filter_tuple { all_tuples!(impl_query_filter_tuple, 0, 15, F, S); -macro_rules! impl_flag_filter { +macro_rules! impl_tick_filter { ( $(#[$meta:meta])* - $name: ident, $state_name: ident, $fetch_name: ident, $($flags: expr),+) => { + $name: ident, $state_name: ident, $fetch_name: ident, $is_detected: expr) => { $(#[$meta])* pub struct $name(PhantomData); pub struct $fetch_name { storage_type: StorageType, - table_flags: *mut ComponentFlags, + table_ticks: *mut ComponentTicks, entity_table_rows: *const usize, marker: PhantomData, entities: *const Entity, sparse_set: *const ComponentSparseSet, + last_change_tick: u32, + change_tick: u32, } pub struct $state_name { @@ -513,14 +528,16 @@ macro_rules! impl_flag_filter { type State = $state_name; type Item = bool; - unsafe fn init(world: &World, state: &Self::State) -> Self { + unsafe fn init(world: &World, state: &Self::State, last_change_tick: u32, change_tick: u32) -> Self { let mut value = Self { storage_type: state.storage_type, - table_flags: ptr::null_mut::(), + table_ticks: ptr::null_mut::(), entities: ptr::null::(), entity_table_rows: ptr::null::(), sparse_set: ptr::null::(), marker: PhantomData, + last_change_tick, + change_tick, }; if state.storage_type == StorageType::SparseSet { value.sparse_set = world @@ -537,9 +554,9 @@ macro_rules! impl_flag_filter { } unsafe fn set_table(&mut self, state: &Self::State, table: &Table) { - self.table_flags = table + self.table_ticks = table .get_column(state.component_id).unwrap() - .get_flags_mut_ptr(); + .get_ticks_mut_ptr(); } unsafe fn set_archetype(&mut self, state: &Self::State, archetype: &Archetype, tables: &Tables) { @@ -547,28 +564,28 @@ macro_rules! impl_flag_filter { StorageType::Table => { self.entity_table_rows = archetype.entity_table_rows().as_ptr(); let table = &tables[archetype.table_id()]; - self.table_flags = table + self.table_ticks = table .get_column(state.component_id).unwrap() - .get_flags_mut_ptr(); + .get_ticks_mut_ptr(); } StorageType::SparseSet => self.entities = archetype.entities().as_ptr(), } } unsafe fn table_fetch(&mut self, table_row: usize) -> bool { - false $(|| (*self.table_flags.add(table_row)).contains($flags))+ + $is_detected(&*self.table_ticks.add(table_row), self.last_change_tick, self.change_tick) } unsafe fn archetype_fetch(&mut self, archetype_index: usize) -> bool { match self.storage_type { StorageType::Table => { let table_row = *self.entity_table_rows.add(archetype_index); - false $(|| (*self.table_flags.add(table_row)).contains($flags))+ + $is_detected(&*self.table_ticks.add(table_row), self.last_change_tick, self.change_tick) } StorageType::SparseSet => { let entity = *self.entities.add(archetype_index); - let flags = (*(*self.sparse_set).get_flags(entity).unwrap()); - false $(|| flags.contains($flags))+ + let ticks = (*(*self.sparse_set).get_ticks(entity).unwrap()); + $is_detected(&ticks, self.last_change_tick, self.change_tick) } } } @@ -576,16 +593,15 @@ macro_rules! impl_flag_filter { }; } -impl_flag_filter!( - /// Filter that retrieves components of type `T` that have been added since the start of the - /// frame +impl_tick_filter!( + /// Filter that retrieves components of type `T` that have been added since the last execution + /// of this system /// - /// This filter is useful as a performance optimization as it means that the query contains - /// fewer items for a system to iterate over. + /// This filter is useful to do one-time post-processing on components. /// /// Because the ordering of systems can change and this filter is only effective on changes - /// before the query executes you need to use explicit dependency ordering or ordered - /// stages for these query filters to be useful. + /// before the query executes you need to use explicit dependency ordering or ordered stages to + /// avoid frame delays. /// /// /// Example: @@ -606,57 +622,37 @@ impl_flag_filter!( Added, AddedState, AddedFetch, - ComponentFlags::ADDED + ComponentTicks::is_added ); -impl_flag_filter!( - /// Filter that retrieves components of type `T` that have been mutated since the start of the - /// frame. Added components do not count as mutated. +impl_tick_filter!( + /// Filter that retrieves components of type `T` that have been changed since the last + /// execution of this system /// - /// This filter is useful as a performance optimization as it means that the query contains - /// fewer items for a system to iterate over. + /// This filter is useful for synchronizing components, and as a performance optimization as it + /// means that the query contains fewer items for a system to iterate over. /// /// Because the ordering of systems can change and this filter is only effective on changes /// before the query executes you need to use explicit dependency ordering or ordered - /// stages for these query filters to be useful. + /// stages to avoid frame delays. /// /// Example: /// ``` /// # use bevy_ecs::system::Query; - /// # use bevy_ecs::query::Mutated; + /// # use bevy_ecs::query::Changed; /// # /// # #[derive(Debug)] /// # struct Name {}; /// # struct Transform {}; /// # - /// fn print_moving_objects_system(query: Query<&Name, Mutated>) { + /// fn print_moving_objects_system(query: Query<&Name, Changed>) { /// for name in query.iter() { - /// println!("Entity Moved: {:?}", name) + /// println!("Entity Moved: {:?}", name); /// } /// } /// ``` - Mutated, - MutatedState, - MutatedFetch, - ComponentFlags::MUTATED -); - -impl_flag_filter!( - /// Filter that retrieves components of type `T` that have been added or mutated since the - /// start of the frame - /// - /// This filter is useful as a performance optimization as it means that the query contains - /// fewer items for a system to iterate over. - /// - /// Because the ordering of systems can change and this filter is only effective on changes - /// before the query executes you need to use explicit dependency ordering or ordered - /// stages for these query filters to be useful. - /// - /// Also see the documentation for [`Mutated`] and [`Added`] as this filter is a logical OR - /// of them. Changed, ChangedState, ChangedFetch, - ComponentFlags::ADDED, - ComponentFlags::MUTATED + ComponentTicks::is_changed ); diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 20995633154b4..4186ce45c455f 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -26,9 +26,24 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> QueryIter<'w, 's, Q, F> where F::Fetch: FilterFetch, { - pub(crate) unsafe fn new(world: &'w World, query_state: &'s QueryState) -> Self { - let fetch = ::init(world, &query_state.fetch_state); - let filter = ::init(world, &query_state.filter_state); + pub(crate) unsafe fn new( + world: &'w World, + query_state: &'s QueryState, + last_change_tick: u32, + change_tick: u32, + ) -> Self { + let fetch = ::init( + world, + &query_state.fetch_state, + last_change_tick, + change_tick, + ); + let filter = ::init( + world, + &query_state.filter_state, + last_change_tick, + change_tick, + ); QueryIter { is_dense: fetch.is_dense() && filter.is_dense(), world, diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 9863e1f52e290..07295430105cd 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -132,7 +132,12 @@ where entity: Entity, ) -> Result<>::Item, QueryEntityError> { self.validate_world_and_update_archetypes(world); - self.get_unchecked_manual(world, entity) + self.get_unchecked_manual( + world, + entity, + world.last_change_tick(), + world.read_change_tick(), + ) } /// # Safety @@ -142,6 +147,8 @@ where &self, world: &'w World, entity: Entity, + last_change_tick: u32, + change_tick: u32, ) -> Result<>::Item, QueryEntityError> { let location = world .entities @@ -154,8 +161,10 @@ where return Err(QueryEntityError::QueryDoesNotMatch); } let archetype = &world.archetypes[location.archetype_id]; - let mut fetch = ::init(world, &self.fetch_state); - let mut filter = ::init(world, &self.filter_state); + let mut fetch = + ::init(world, &self.fetch_state, last_change_tick, change_tick); + let mut filter = + ::init(world, &self.filter_state, last_change_tick, change_tick); fetch.set_archetype(&self.fetch_state, archetype, &world.storages().tables); filter.set_archetype(&self.filter_state, archetype, &world.storages().tables); @@ -190,7 +199,7 @@ where world: &'w World, ) -> QueryIter<'w, 's, Q, F> { self.validate_world_and_update_archetypes(world); - self.iter_unchecked_manual(world) + self.iter_unchecked_manual(world, world.last_change_tick(), world.read_change_tick()) } /// # Safety @@ -202,8 +211,10 @@ where pub(crate) unsafe fn iter_unchecked_manual<'w, 's>( &'s self, world: &'w World, + last_change_tick: u32, + change_tick: u32, ) -> QueryIter<'w, 's, Q, F> { - QueryIter::new(world, self) + QueryIter::new(world, self, last_change_tick, change_tick) } #[inline] @@ -242,7 +253,12 @@ where func: impl FnMut(>::Item), ) { self.validate_world_and_update_archetypes(world); - self.for_each_unchecked_manual(world, func); + self.for_each_unchecked_manual( + world, + func, + world.last_change_tick(), + world.read_change_tick(), + ); } #[inline] @@ -287,7 +303,14 @@ where func: impl Fn(>::Item) + Send + Sync + Clone, ) { self.validate_world_and_update_archetypes(world); - self.par_for_each_unchecked_manual(world, task_pool, batch_size, func); + self.par_for_each_unchecked_manual( + world, + task_pool, + batch_size, + func, + world.last_change_tick(), + world.read_change_tick(), + ); } /// # Safety @@ -299,9 +322,13 @@ where &'s self, world: &'w World, mut func: impl FnMut(>::Item), + last_change_tick: u32, + change_tick: u32, ) { - let mut fetch = ::init(world, &self.fetch_state); - let mut filter = ::init(world, &self.filter_state); + let mut fetch = + ::init(world, &self.fetch_state, last_change_tick, change_tick); + let mut filter = + ::init(world, &self.filter_state, last_change_tick, change_tick); if fetch.is_dense() && filter.is_dense() { let tables = &world.storages().tables; for table_id in self.matched_table_ids.iter() { @@ -346,10 +373,14 @@ where task_pool: &TaskPool, batch_size: usize, func: impl Fn(>::Item) + Send + Sync + Clone, + last_change_tick: u32, + change_tick: u32, ) { task_pool.scope(|scope| { - let fetch = ::init(world, &self.fetch_state); - let filter = ::init(world, &self.filter_state); + let fetch = + ::init(world, &self.fetch_state, last_change_tick, change_tick); + let filter = + ::init(world, &self.filter_state, last_change_tick, change_tick); if fetch.is_dense() && filter.is_dense() { let tables = &world.storages().tables; @@ -359,8 +390,18 @@ where while offset < table.len() { let func = func.clone(); scope.spawn(async move { - let mut fetch = ::init(world, &self.fetch_state); - let mut filter = ::init(world, &self.filter_state); + let mut fetch = ::init( + world, + &self.fetch_state, + last_change_tick, + change_tick, + ); + let mut filter = ::init( + world, + &self.filter_state, + last_change_tick, + change_tick, + ); let tables = &world.storages().tables; let table = &tables[*table_id]; fetch.set_table(&self.fetch_state, table); @@ -385,8 +426,18 @@ where while offset < archetype.len() { let func = func.clone(); scope.spawn(async move { - let mut fetch = ::init(world, &self.fetch_state); - let mut filter = ::init(world, &self.filter_state); + let mut fetch = ::init( + world, + &self.fetch_state, + last_change_tick, + change_tick, + ); + let mut filter = ::init( + world, + &self.filter_state, + last_change_tick, + change_tick, + ); let tables = &world.storages().tables; let archetype = &world.archetypes[*archetype_id]; fetch.set_archetype(&self.fetch_state, archetype, tables); diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index ed5ab878dbd93..3e79161578a39 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -1,7 +1,7 @@ use std::ops::{Deref, DerefMut}; use crate::{ - component::{Component, ComponentFlags}, + component::{Component, ComponentTicks}, entity::{Entity, EntityMap, MapEntities, MapEntitiesError}, world::{FromWorld, World}, }; @@ -101,10 +101,12 @@ impl FromType for ReflectComponent { reflect_component_mut: |world, entity| unsafe { world .get_entity(entity)? - .get_unchecked_mut::() + .get_unchecked_mut::(world.last_change_tick(), world.read_change_tick()) .map(|c| ReflectMut { value: c.value as &mut dyn Reflect, - flags: c.flags, + component_ticks: c.component_ticks, + last_change_tick: c.last_change_tick, + change_tick: c.change_tick, }) }, } @@ -114,7 +116,9 @@ impl FromType for ReflectComponent { /// Unique borrow of a Reflected component pub struct ReflectMut<'a> { pub(crate) value: &'a mut dyn Reflect, - pub(crate) flags: &'a mut ComponentFlags, + pub(crate) component_ticks: &'a mut ComponentTicks, + pub(crate) last_change_tick: u32, + pub(crate) change_tick: u32, } impl<'a> Deref for ReflectMut<'a> { @@ -129,11 +133,27 @@ impl<'a> Deref for ReflectMut<'a> { impl<'a> DerefMut for ReflectMut<'a> { #[inline] fn deref_mut(&mut self) -> &mut dyn Reflect { - self.flags.insert(ComponentFlags::MUTATED); + self.component_ticks.set_changed(self.change_tick); self.value } } +impl<'a> ReflectMut<'a> { + /// Returns true if (and only if) this component been added since the last execution of this + /// system. + pub fn is_added(&self) -> bool { + self.component_ticks + .is_added(self.last_change_tick, self.change_tick) + } + + /// Returns true if (and only if) this component been changed since the last execution of this + /// system. + pub fn is_changed(&self) -> bool { + self.component_ticks + .is_changed(self.last_change_tick, self.change_tick) + } +} + impl_reflect_value!(Entity(Hash, PartialEq, Serialize, Deserialize)); #[derive(Clone)] diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 5074b68d5a283..faee3ccb27045 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -333,4 +333,6 @@ impl System for RunOnce { fn apply_buffers(&mut self, _world: &mut World) {} fn initialize(&mut self, _world: &mut World) {} + + fn check_change_tick(&mut self, _change_tick: u32) {} } diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index e7c604ee4dfcb..e2a83b80813f0 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -80,6 +80,8 @@ pub struct SystemStage { uninitialized_at_end: Vec, /// Newly inserted systems that will be initialized at the next opportunity. uninitialized_parallel: Vec, + /// Saves the value of the World change_tick during the last tick check + last_tick_check: u32, } impl SystemStage { @@ -102,6 +104,7 @@ impl SystemStage { uninitialized_at_start: vec![], uninitialized_before_commands: vec![], uninitialized_at_end: vec![], + last_tick_check: Default::default(), } } @@ -198,6 +201,16 @@ impl SystemStage { // TODO: consider exposing fn add_system_to_set(&mut self, system: impl Into, set: usize) -> &mut Self { + // This assertion is there to document that a maximum of `u32::MAX / 8` systems should be + // added to a stage to guarantee that change detection has no false positive, but it + // can be circumvented using exclusive or chained systems + assert!( + self.exclusive_at_start.len() + + self.exclusive_before_commands.len() + + self.exclusive_at_end.len() + + self.parallel.len() + < (u32::MAX / 8) as usize + ); self.systems_modified = true; match system.into() { SystemDescriptor::Exclusive(descriptor) => { @@ -361,6 +374,37 @@ impl SystemStage { info!("{}", string); } } + + /// Checks for old component and system change ticks + fn check_change_ticks(&mut self, world: &mut World) { + let change_tick = world.change_tick(); + let time_since_last_check = change_tick.wrapping_sub(self.last_tick_check); + // Only check after at least `u32::MAX / 8` counts, and at most `u32::MAX / 4` counts + // since the max number of [System] in a [SystemStage] is limited to `u32::MAX / 8` + // and this function is called at the end of each [SystemStage] loop + const MIN_TIME_SINCE_LAST_CHECK: u32 = u32::MAX / 8; + + if time_since_last_check > MIN_TIME_SINCE_LAST_CHECK { + // Check all system change ticks + for exclusive_system in &mut self.exclusive_at_start { + exclusive_system.system_mut().check_change_tick(change_tick); + } + for exclusive_system in &mut self.exclusive_before_commands { + exclusive_system.system_mut().check_change_tick(change_tick); + } + for exclusive_system in &mut self.exclusive_at_end { + exclusive_system.system_mut().check_change_tick(change_tick); + } + for parallel_system in &mut self.parallel { + parallel_system.system_mut().check_change_tick(change_tick); + } + + // Check component ticks + world.check_change_ticks(); + + self.last_tick_check = change_tick; + } + } } enum DependencyGraphError { @@ -655,6 +699,9 @@ impl Stage for SystemStage { } } + // Check for old component and system change ticks + self.check_change_ticks(world); + // Reevaluate system sets' run criteria. has_work = false; for system_set in self.system_sets.iter_mut() { @@ -678,6 +725,9 @@ impl Stage for SystemStage { #[cfg(test)] mod tests { use crate::{ + entity::Entity, + query::ChangeTrackers, + query::Changed, schedule::{ BoxedSystemLabel, ExclusiveSystemDescriptorCoercion, ParallelSystemDescriptorCoercion, ShouldRun, SingleThreadedExecutor, Stage, SystemSet, SystemStage, @@ -1673,4 +1723,70 @@ mod tests { stage.run(&mut world); assert_eq!(*world.get_resource::().unwrap(), 1); } + + #[test] + fn change_ticks_wrapover() { + const MIN_TIME_SINCE_LAST_CHECK: u32 = u32::MAX / 8; + const MAX_DELTA: u32 = (u32::MAX / 4) * 3; + + let mut world = World::new(); + world.spawn().insert(0usize); + *world.change_tick.get_mut() += MAX_DELTA + 1; + + let mut stage = SystemStage::parallel(); + fn work() {} + stage.add_system(work.system()); + + // Overflow twice + for _ in 0..10 { + stage.run(&mut world); + for tracker in world.query::>().iter(&world) { + let time_since_last_check = tracker + .change_tick + .wrapping_sub(tracker.component_ticks.added); + assert!(time_since_last_check <= MAX_DELTA); + let time_since_last_check = tracker + .change_tick + .wrapping_sub(tracker.component_ticks.changed); + assert!(time_since_last_check <= MAX_DELTA); + } + let change_tick = world.change_tick.get_mut(); + *change_tick = change_tick.wrapping_add(MIN_TIME_SINCE_LAST_CHECK + 1); + } + } + + #[test] + fn change_query_wrapover() { + struct C; + let mut world = World::new(); + + // Spawn entities at various ticks + let component_ticks = [0, u32::MAX / 4, u32::MAX / 2, u32::MAX / 4 * 3, u32::MAX]; + let ids = component_ticks + .iter() + .map(|tick| { + *world.change_tick.get_mut() = *tick; + world.spawn().insert(C).id() + }) + .collect::>(); + + let test_cases = [ + // normal + (0, u32::MAX / 2, vec![ids[1], ids[2]]), + // just wrapped over + (u32::MAX / 2, 0, vec![ids[0], ids[3], ids[4]]), + ]; + for (last_change_tick, change_tick, changed_entities) in test_cases.iter() { + *world.change_tick.get_mut() = *change_tick; + world.last_change_tick = *last_change_tick; + + assert_eq!( + world + .query_filtered::>() + .iter(&world) + .collect::>(), + *changed_entities + ); + } + } } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 67c3180cfdb14..165787050d65d 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -1,5 +1,5 @@ use crate::{ - component::{ComponentFlags, ComponentId, ComponentInfo}, + component::{ComponentId, ComponentInfo, ComponentTicks}, entity::Entity, storage::BlobVec, }; @@ -87,7 +87,7 @@ impl SparseArray { #[derive(Debug)] pub struct ComponentSparseSet { dense: BlobVec, - flags: UnsafeCell>, + ticks: UnsafeCell>, entities: Vec, sparse: SparseArray, } @@ -96,7 +96,7 @@ impl ComponentSparseSet { pub fn new(component_info: &ComponentInfo, capacity: usize) -> Self { Self { dense: BlobVec::new(component_info.layout(), component_info.drop(), capacity), - flags: UnsafeCell::new(Vec::with_capacity(capacity)), + ticks: UnsafeCell::new(Vec::with_capacity(capacity)), entities: Vec::with_capacity(capacity), sparse: Default::default(), } @@ -117,22 +117,25 @@ impl ComponentSparseSet { /// the value when needed. /// /// # Safety - /// The `value` pointer must point to a valid address that matches the `Layout` inside the - /// `ComponentInfo` given when constructing this sparse set. - pub unsafe fn insert(&mut self, entity: Entity, value: *mut u8, flags: ComponentFlags) { + /// The `value` pointer must point to a valid address that matches the `Layout` + /// inside the `ComponentInfo` given when constructing this sparse set. + pub unsafe fn insert( + &mut self, + entity: Entity, + value: *mut u8, + component_ticks: ComponentTicks, + ) { let dense = &mut self.dense; let entities = &mut self.entities; - let flag_list = self.flags.get_mut(); + let ticks_list = self.ticks.get_mut(); let dense_index = *self.sparse.get_or_insert_with(entity, move || { - flag_list.push(ComponentFlags::empty()); + ticks_list.push(component_ticks); entities.push(entity); dense.push_uninit() }); // SAFE: dense_index exists thanks to the call above self.dense.set_unchecked(dense_index, value); - (*self.flags.get()) - .get_unchecked_mut(dense_index) - .insert(flags); + *(*self.ticks.get()).get_unchecked_mut(dense_index) = component_ticks; } #[inline] @@ -153,14 +156,14 @@ impl ComponentSparseSet { /// # Safety /// ensure the same entity is not accessed twice at the same time #[inline] - pub unsafe fn get_with_flags(&self, entity: Entity) -> Option<(*mut u8, *mut ComponentFlags)> { - let flags = &mut *self.flags.get(); + pub unsafe fn get_with_ticks(&self, entity: Entity) -> Option<(*mut u8, *mut ComponentTicks)> { + let ticks = &mut *self.ticks.get(); self.sparse.get(entity).map(move |dense_index| { let dense_index = *dense_index; // SAFE: if the sparse index points to something in the dense vec, it exists ( self.dense.get_unchecked(dense_index), - flags.get_unchecked_mut(dense_index) as *mut ComponentFlags, + ticks.get_unchecked_mut(dense_index) as *mut ComponentTicks, ) }) } @@ -168,12 +171,12 @@ impl ComponentSparseSet { /// # Safety /// ensure the same entity is not accessed twice at the same time #[inline] - pub unsafe fn get_flags(&self, entity: Entity) -> Option<&mut ComponentFlags> { - let flags = &mut *self.flags.get(); + pub unsafe fn get_ticks(&self, entity: Entity) -> Option<&mut ComponentTicks> { + let ticks = &mut *self.ticks.get(); self.sparse.get(entity).map(move |dense_index| { let dense_index = *dense_index; // SAFE: if the sparse index points to something in the dense vec, it exists - flags.get_unchecked_mut(dense_index) + ticks.get_unchecked_mut(dense_index) }) } @@ -182,9 +185,9 @@ impl ComponentSparseSet { /// returned). pub fn remove_and_forget(&mut self, entity: Entity) -> Option<*mut u8> { self.sparse.remove(entity).map(|dense_index| { - // SAFE: unique access to flags + // SAFE: unique access to ticks unsafe { - (*self.flags.get()).swap_remove(dense_index); + (*self.ticks.get()).swap_remove(dense_index); } self.entities.swap_remove(dense_index); let is_last = dense_index == self.dense.len() - 1; @@ -200,7 +203,7 @@ impl ComponentSparseSet { pub fn remove(&mut self, entity: Entity) -> bool { if let Some(dense_index) = self.sparse.remove(entity) { - self.flags.get_mut().swap_remove(dense_index); + self.ticks.get_mut().swap_remove(dense_index); self.entities.swap_remove(dense_index); let is_last = dense_index == self.dense.len() - 1; // SAFE: if the sparse index points to something in the dense vec, it exists @@ -215,10 +218,10 @@ impl ComponentSparseSet { } } - pub(crate) fn clear_flags(&mut self) { - let flags = self.flags.get_mut().iter_mut(); - for component_flags in flags { - *component_flags = ComponentFlags::empty(); + pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { + let ticks = self.ticks.get_mut().iter_mut(); + for component_ticks in ticks { + component_ticks.check_ticks(change_tick); } } } @@ -445,9 +448,9 @@ impl SparseSets { self.sets.get_mut(component_id) } - pub(crate) fn clear_flags(&mut self) { + pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { for set in self.sets.values_mut() { - set.clear_flags(); + set.check_change_ticks(change_tick); } } } diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 71ce7d1bcd39a..bcb778a088f01 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -1,6 +1,6 @@ use crate::{ archetype::ArchetypeId, - component::{ComponentFlags, ComponentId, ComponentInfo, Components}, + component::{ComponentId, ComponentInfo, ComponentTicks, Components}, entity::Entity, storage::{BlobVec, SparseSet}, }; @@ -35,7 +35,7 @@ impl TableId { pub struct Column { pub(crate) component_id: ComponentId, pub(crate) data: BlobVec, - pub(crate) flags: UnsafeCell>, + pub(crate) ticks: UnsafeCell>, } impl Column { @@ -44,7 +44,7 @@ impl Column { Column { component_id: component_info.id(), data: BlobVec::new(component_info.layout(), component_info.drop(), capacity), - flags: UnsafeCell::new(Vec::with_capacity(capacity)), + ticks: UnsafeCell::new(Vec::with_capacity(capacity)), } } @@ -69,36 +69,36 @@ impl Column { /// # Safety /// Assumes data has already been allocated for the given row/column. - /// Allows aliased mutable accesses to the row's ComponentFlags. Caller must ensure that this - /// does not happen. + /// Allows aliased mutable accesses to the row's [ComponentTicks]. + /// Caller must ensure that this does not happen. #[inline] #[allow(clippy::mut_from_ref)] - pub unsafe fn get_flags_unchecked_mut(&self, row: usize) -> &mut ComponentFlags { + pub unsafe fn get_ticks_unchecked_mut(&self, row: usize) -> &mut ComponentTicks { debug_assert!(row < self.len()); - (*self.flags.get()).get_unchecked_mut(row) + (*self.ticks.get()).get_unchecked_mut(row) } #[inline] pub(crate) unsafe fn swap_remove_unchecked(&mut self, row: usize) { self.data.swap_remove_and_drop_unchecked(row); - (*self.flags.get()).swap_remove(row); + (*self.ticks.get()).swap_remove(row); } #[inline] pub(crate) unsafe fn swap_remove_and_forget_unchecked( &mut self, row: usize, - ) -> (*mut u8, ComponentFlags) { + ) -> (*mut u8, ComponentTicks) { let data = self.data.swap_remove_and_forget_unchecked(row); - let flags = (*self.flags.get()).swap_remove(row); - (data, flags) + let ticks = (*self.ticks.get()).swap_remove(row); + (data, ticks) } /// # Safety /// allocated value must be immediately set at the returned row pub(crate) unsafe fn push_uninit(&mut self) -> usize { let row = self.data.push_uninit(); - (*self.flags.get()).push(ComponentFlags::empty()); + (*self.ticks.get()).push(ComponentTicks::new(0)); row } @@ -107,8 +107,8 @@ impl Column { self.data.reserve(additional); // SAFE: unique access to self unsafe { - let flags = &mut (*self.flags.get()); - flags.reserve(additional); + let ticks = &mut (*self.ticks.get()); + ticks.reserve(additional); } } @@ -122,8 +122,8 @@ impl Column { /// # Safety /// must ensure rust mutability rules are not violated #[inline] - pub unsafe fn get_flags_mut_ptr(&self) -> *mut ComponentFlags { - (*self.flags.get()).as_mut_ptr() + pub unsafe fn get_ticks_mut_ptr(&self) -> *mut ComponentTicks { + (*self.ticks.get()).as_mut_ptr() } /// # Safety @@ -137,16 +137,16 @@ impl Column { /// # Safety /// must ensure rust mutability rules are not violated #[inline] - pub unsafe fn get_flags_unchecked(&self, row: usize) -> *mut ComponentFlags { - debug_assert!(row < (*self.flags.get()).len()); - self.get_flags_mut_ptr().add(row) + pub unsafe fn get_ticks_unchecked(&self, row: usize) -> *mut ComponentTicks { + debug_assert!(row < (*self.ticks.get()).len()); + self.get_ticks_mut_ptr().add(row) } #[inline] - pub(crate) fn clear_flags(&mut self) { - let flags = unsafe { (*self.flags.get()).iter_mut() }; - for component_flags in flags { - *component_flags = ComponentFlags::empty(); + pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { + let ticks = unsafe { (*self.ticks.get()).iter_mut() }; + for component_ticks in ticks { + component_ticks.check_ticks(change_tick); } } } @@ -230,10 +230,10 @@ impl Table { let is_last = row == self.entities.len() - 1; let new_row = new_table.allocate(self.entities.swap_remove(row)); for column in self.columns.values_mut() { - let (data, flags) = column.swap_remove_and_forget_unchecked(row); + let (data, ticks) = column.swap_remove_and_forget_unchecked(row); if let Some(new_column) = new_table.get_column_mut(column.component_id) { new_column.set_unchecked(new_row, data); - *new_column.get_flags_unchecked_mut(new_row) = flags; + *new_column.get_ticks_unchecked_mut(new_row) = ticks; } } TableMoveResult { @@ -262,9 +262,9 @@ impl Table { let new_row = new_table.allocate(self.entities.swap_remove(row)); for column in self.columns.values_mut() { if let Some(new_column) = new_table.get_column_mut(column.component_id) { - let (data, flags) = column.swap_remove_and_forget_unchecked(row); + let (data, ticks) = column.swap_remove_and_forget_unchecked(row); new_column.set_unchecked(new_row, data); - *new_column.get_flags_unchecked_mut(new_row) = flags; + *new_column.get_ticks_unchecked_mut(new_row) = ticks; } else { column.swap_remove_unchecked(row); } @@ -295,9 +295,9 @@ impl Table { let new_row = new_table.allocate(self.entities.swap_remove(row)); for column in self.columns.values_mut() { let new_column = new_table.get_column_mut(column.component_id).unwrap(); - let (data, flags) = column.swap_remove_and_forget_unchecked(row); + let (data, ticks) = column.swap_remove_and_forget_unchecked(row); new_column.set_unchecked(new_row, data); - *new_column.get_flags_unchecked_mut(new_row) = flags; + *new_column.get_ticks_unchecked_mut(new_row) = ticks; } TableMoveResult { new_row, @@ -346,7 +346,7 @@ impl Table { self.entities.push(entity); for column in self.columns.values_mut() { column.data.set_len(self.entities.len()); - (*column.flags.get()).push(ComponentFlags::empty()); + (*column.ticks.get()).push(ComponentTicks::new(0)); } index } @@ -366,9 +366,9 @@ impl Table { self.entities.is_empty() } - pub(crate) fn clear_flags(&mut self) { + pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { for column in self.columns.values_mut() { - column.clear_flags(); + column.check_change_ticks(change_tick); } } @@ -454,9 +454,9 @@ impl Tables { self.tables.iter() } - pub(crate) fn clear_flags(&mut self) { + pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { for table in self.tables.iter_mut() { - table.clear_flags(); + table.check_change_ticks(change_tick); } } } diff --git a/crates/bevy_ecs/src/system/exclusive_system.rs b/crates/bevy_ecs/src/system/exclusive_system.rs index 9f67d6c56d528..2196c2665ec08 100644 --- a/crates/bevy_ecs/src/system/exclusive_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_system.rs @@ -1,5 +1,5 @@ use crate::{ - system::{BoxedSystem, IntoSystem, System, SystemId}, + system::{check_system_change_tick, BoxedSystem, IntoSystem, System, SystemId}, world::World, }; use std::borrow::Cow; @@ -12,12 +12,15 @@ pub trait ExclusiveSystem: Send + Sync + 'static { fn run(&mut self, world: &mut World); fn initialize(&mut self, world: &mut World); + + fn check_change_tick(&mut self, change_tick: u32); } pub struct ExclusiveSystemFn { func: Box, name: Cow<'static, str>, id: SystemId, + last_change_tick: u32, } impl ExclusiveSystem for ExclusiveSystemFn { @@ -30,10 +33,25 @@ impl ExclusiveSystem for ExclusiveSystemFn { } fn run(&mut self, world: &mut World) { + // The previous value is saved in case this exclusive system is run by another exclusive + // system + let saved_last_tick = world.last_change_tick; + world.last_change_tick = self.last_change_tick; + (self.func)(world); + + let change_tick = world.change_tick.get_mut(); + self.last_change_tick = *change_tick; + *change_tick += 1; + + world.last_change_tick = saved_last_tick; } fn initialize(&mut self, _: &mut World) {} + + fn check_change_tick(&mut self, change_tick: u32) { + check_system_change_tick(&mut self.last_change_tick, change_tick, self.name.as_ref()); + } } pub trait IntoExclusiveSystem { @@ -49,6 +67,7 @@ where func: Box::new(self), name: core::any::type_name::().into(), id: SystemId::new(), + last_change_tick: 0, } } } @@ -74,6 +93,10 @@ impl ExclusiveSystem for ExclusiveSystemCoerced { fn initialize(&mut self, world: &mut World) { self.system.initialize(world); } + + fn check_change_tick(&mut self, change_tick: u32) { + self.system.check_change_tick(change_tick); + } } impl IntoExclusiveSystem<(Params, SystemType), ExclusiveSystemCoerced> for S diff --git a/crates/bevy_ecs/src/system/into_system.rs b/crates/bevy_ecs/src/system/into_system.rs index ed9c707ae3e6b..57ec81aff8d8c 100644 --- a/crates/bevy_ecs/src/system/into_system.rs +++ b/crates/bevy_ecs/src/system/into_system.rs @@ -2,7 +2,9 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, component::ComponentId, query::{Access, FilteredAccessSet}, - system::{System, SystemId, SystemParam, SystemParamFetch, SystemParamState}, + system::{ + check_system_change_tick, System, SystemId, SystemParam, SystemParamFetch, SystemParamState, + }, world::World, }; use bevy_ecs_macros::all_tuples; @@ -16,6 +18,7 @@ pub struct SystemState { // NOTE: this must be kept private. making a SystemState non-send is irreversible to prevent // SystemParams from overriding each other is_send: bool, + pub(crate) last_change_tick: u32, } impl SystemState { @@ -26,6 +29,7 @@ impl SystemState { component_access_set: FilteredAccessSet::default(), is_send: true, id: SystemId::new(), + last_change_tick: 0, } } @@ -139,12 +143,16 @@ where #[inline] unsafe fn run_unsafe(&mut self, input: Self::In, world: &World) -> Self::Out { - self.func.run( + let change_tick = world.increment_change_tick(); + let out = self.func.run( input, self.param_state.as_mut().unwrap(), &self.system_state, world, - ) + change_tick, + ); + self.system_state.last_change_tick = change_tick; + out } #[inline] @@ -161,6 +169,15 @@ where self.config.take().unwrap(), )); } + + #[inline] + fn check_change_tick(&mut self, change_tick: u32) { + check_system_change_tick( + &mut self.system_state.last_change_tick, + change_tick, + self.system_state.name.as_ref(), + ); + } } pub trait SystemParamFunction: Send + Sync + 'static { @@ -170,6 +187,7 @@ pub trait SystemParamFunction: Send + Sync state: &mut Param::Fetch, system_state: &SystemState, world: &World, + change_tick: u32, ) -> Out; } @@ -183,9 +201,9 @@ macro_rules! impl_system_function { FnMut($(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out + Send + Sync + 'static, Out: 'static { #[inline] - fn run(&mut self, _input: (), state: &mut <($($param,)*) as SystemParam>::Fetch, system_state: &SystemState, world: &World) -> Out { + fn run(&mut self, _input: (), state: &mut <($($param,)*) as SystemParam>::Fetch, system_state: &SystemState, world: &World, change_tick: u32) -> Out { unsafe { - let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_state, world); + let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_state, world, change_tick); self($($param),*) } } @@ -199,9 +217,9 @@ macro_rules! impl_system_function { FnMut(In, $(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out + Send + Sync + 'static, Out: 'static { #[inline] - fn run(&mut self, input: Input, state: &mut <($($param,)*) as SystemParam>::Fetch, system_state: &SystemState, world: &World) -> Out { + fn run(&mut self, input: Input, state: &mut <($($param,)*) as SystemParam>::Fetch, system_state: &SystemState, world: &World, change_tick: u32) -> Out { unsafe { - let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_state, world); + let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_state, world, change_tick); self(In(input), $($param),*) } } diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index 6ad24782b7fbc..c396b84a12800 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -24,7 +24,7 @@ mod tests { bundle::Bundles, component::Components, entity::{Entities, Entity}, - query::{Added, Changed, Mutated, Or, With, Without}, + query::{Added, Changed, Or, With, Without}, schedule::{Schedule, Stage, SystemStage}, system::{ IntoExclusiveSystem, IntoSystem, Local, Query, QuerySet, RemovedComponents, Res, @@ -132,16 +132,13 @@ mod tests { set: QuerySet<( Query<(), Or<(Changed, Changed)>>, Query<(), Or<(Added, Added)>>, - Query<(), Or<(Mutated, Mutated)>>, )>, ) { let changed = set.q0().iter().count(); let added = set.q1().iter().count(); - let mutated = set.q2().iter().count(); assert_eq!(changed, 1); assert_eq!(added, 1); - assert_eq!(mutated, 0); *ran = true; } @@ -164,11 +161,11 @@ mod tests { mut changed: ResMut, mut added: ResMut, ) { - if value.added() { + if value.is_added() { added.0 += 1; } - if value.changed() { + if value.is_changed() { changed.0 += 1; } } diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 8a3fb23c72e2e..11d4ae2d45534 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -17,6 +17,8 @@ where { pub(crate) world: &'w World, pub(crate) state: &'w QueryState, + pub(crate) last_change_tick: u32, + pub(crate) change_tick: u32, } impl<'w, Q: WorldQuery, F: WorldQuery> Query<'w, Q, F> @@ -27,8 +29,18 @@ where /// This will create a Query that could violate memory safety rules. Make sure that this is only /// called in ways that ensure the Queries have unique mutable access. #[inline] - pub(crate) unsafe fn new(world: &'w World, state: &'w QueryState) -> Self { - Self { world, state } + pub(crate) unsafe fn new( + world: &'w World, + state: &'w QueryState, + last_change_tick: u32, + change_tick: u32, + ) -> Self { + Self { + world, + state, + last_change_tick, + change_tick, + } } /// Iterates over the query results. This can only be called for read-only queries @@ -37,17 +49,23 @@ where where Q::Fetch: ReadOnlyFetch, { - // SAFE: system runs without conflicts with other systems. same-system queries have runtime - // borrow checks when they conflict - unsafe { self.state.iter_unchecked_manual(self.world) } + // SAFE: system runs without conflicts with other systems. + // same-system queries have runtime borrow checks when they conflict + unsafe { + self.state + .iter_unchecked_manual(self.world, self.last_change_tick, self.change_tick) + } } /// Iterates over the query results #[inline] pub fn iter_mut(&mut self) -> QueryIter<'_, '_, Q, F> { - // SAFE: system runs without conflicts with other systems. same-system queries have runtime - // borrow checks when they conflict - unsafe { self.state.iter_unchecked_manual(self.world) } + // SAFE: system runs without conflicts with other systems. + // same-system queries have runtime borrow checks when they conflict + unsafe { + self.state + .iter_unchecked_manual(self.world, self.last_change_tick, self.change_tick) + } } /// Iterates over the query results @@ -57,9 +75,10 @@ where /// mutable references to the same component #[inline] pub unsafe fn iter_unsafe(&self) -> QueryIter<'_, '_, Q, F> { - // SEMI-SAFE: system runs without conflicts with other systems. same-system queries have - // runtime borrow checks when they conflict - self.state.iter_unchecked_manual(self.world) + // SEMI-SAFE: system runs without conflicts with other systems. + // same-system queries have runtime borrow checks when they conflict + self.state + .iter_unchecked_manual(self.world, self.last_change_tick, self.change_tick) } /// Runs `f` on each query result. This is faster than the equivalent iter() method, but cannot @@ -69,18 +88,32 @@ where where Q::Fetch: ReadOnlyFetch, { - // SAFE: system runs without conflicts with other systems. same-system queries have runtime - // borrow checks when they conflict - unsafe { self.state.for_each_unchecked_manual(self.world, f) }; + // SAFE: system runs without conflicts with other systems. + // same-system queries have runtime borrow checks when they conflict + unsafe { + self.state.for_each_unchecked_manual( + self.world, + f, + self.last_change_tick, + self.change_tick, + ) + }; } /// Runs `f` on each query result. This is faster than the equivalent iter() method, but cannot /// be chained like a normal iterator. #[inline] - pub fn for_each_mut(&mut self, f: impl FnMut(>::Item)) { + pub fn for_each_mut(&self, f: impl FnMut(>::Item)) { // SAFE: system runs without conflicts with other systems. same-system queries have runtime // borrow checks when they conflict - unsafe { self.state.for_each_unchecked_manual(self.world, f) }; + unsafe { + self.state.for_each_unchecked_manual( + self.world, + f, + self.last_change_tick, + self.change_tick, + ) + }; } /// Runs `f` on each query result in parallel using the given task pool. @@ -96,8 +129,14 @@ where // SAFE: system runs without conflicts with other systems. same-system queries have runtime // borrow checks when they conflict unsafe { - self.state - .par_for_each_unchecked_manual(self.world, task_pool, batch_size, f) + self.state.par_for_each_unchecked_manual( + self.world, + task_pool, + batch_size, + f, + self.last_change_tick, + self.change_tick, + ) }; } @@ -112,8 +151,14 @@ where // SAFE: system runs without conflicts with other systems. same-system queries have runtime // borrow checks when they conflict unsafe { - self.state - .par_for_each_unchecked_manual(self.world, task_pool, batch_size, f) + self.state.par_for_each_unchecked_manual( + self.world, + task_pool, + batch_size, + f, + self.last_change_tick, + self.change_tick, + ) }; } @@ -123,9 +168,16 @@ where where Q::Fetch: ReadOnlyFetch, { - // SAFE: system runs without conflicts with other systems. same-system queries have runtime - // borrow checks when they conflict - unsafe { self.state.get_unchecked_manual(self.world, entity) } + // SAFE: system runs without conflicts with other systems. + // same-system queries have runtime borrow checks when they conflict + unsafe { + self.state.get_unchecked_manual( + self.world, + entity, + self.last_change_tick, + self.change_tick, + ) + } } /// Gets the query result for the given `entity` @@ -134,9 +186,16 @@ where &mut self, entity: Entity, ) -> Result<::Item, QueryEntityError> { - // // SAFE: system runs without conflicts with other systems. same-system queries have - // runtime borrow checks when they conflict - unsafe { self.state.get_unchecked_manual(self.world, entity) } + // SAFE: system runs without conflicts with other systems. + // same-system queries have runtime borrow checks when they conflict + unsafe { + self.state.get_unchecked_manual( + self.world, + entity, + self.last_change_tick, + self.change_tick, + ) + } } /// Gets the query result for the given `entity` @@ -149,9 +208,10 @@ where &self, entity: Entity, ) -> Result<::Item, QueryEntityError> { - // SEMI-SAFE: system runs without conflicts with other systems. same-system queries have - // runtime borrow checks when they conflict - self.state.get_unchecked_manual(self.world, entity) + // SEMI-SAFE: system runs without conflicts with other systems. + // same-system queries have runtime borrow checks when they conflict + self.state + .get_unchecked_manual(self.world, entity, self.last_change_tick, self.change_tick) } /// Gets a reference to the entity's component of the given type. This will fail if the entity @@ -225,7 +285,7 @@ where .has_write(archetype_component) { entity_ref - .get_unchecked_mut::() + .get_unchecked_mut::(self.last_change_tick, self.change_tick) .ok_or(QueryComponentError::MissingComponent) } else { Err(QueryComponentError::MissingWriteAccess) diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index 528ff003f32ae..824cbc5e847b6 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -1,3 +1,5 @@ +use bevy_utils::tracing::warn; + use crate::{ archetype::{Archetype, ArchetypeComponentId}, component::ComponentId, @@ -47,6 +49,24 @@ pub trait System: Send + Sync + 'static { } fn apply_buffers(&mut self, world: &mut World); fn initialize(&mut self, _world: &mut World); + fn check_change_tick(&mut self, change_tick: u32); } pub type BoxedSystem = Box>; + +pub(crate) fn check_system_change_tick( + last_change_tick: &mut u32, + change_tick: u32, + system_name: &str, +) { + let tick_delta = change_tick.wrapping_sub(*last_change_tick); + const MAX_DELTA: u32 = (u32::MAX / 4) * 3; + // Clamp to max delta + if tick_delta > MAX_DELTA { + warn!( + "Too many intervening systems have run since the last time System '{}' was last run; it may fail to detect changes.", + system_name + ); + *last_change_tick = change_tick.wrapping_sub(MAX_DELTA); + } +} diff --git a/crates/bevy_ecs/src/system/system_chaining.rs b/crates/bevy_ecs/src/system/system_chaining.rs index c620131d98039..951d07ff13221 100644 --- a/crates/bevy_ecs/src/system/system_chaining.rs +++ b/crates/bevy_ecs/src/system/system_chaining.rs @@ -68,6 +68,11 @@ impl> System for ChainSystem self.component_access .extend(self.system_b.component_access()); } + + fn check_change_tick(&mut self, change_tick: u32) { + self.system_a.check_change_tick(change_tick); + self.system_b.check_change_tick(change_tick); + } } pub trait IntoChainSystem: System + Sized diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 60d8bbe9e0aa9..f376f8c9ea8bb 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1,7 +1,7 @@ use crate::{ archetype::{Archetype, Archetypes}, bundle::Bundles, - component::{Component, ComponentFlags, ComponentId, Components}, + component::{Component, ComponentId, ComponentTicks, Components}, entity::{Entities, Entity}, query::{FilterFetch, FilteredAccess, FilteredAccessSet, QueryState, WorldQuery}, system::{CommandQueue, Commands, Query, SystemState}, @@ -37,10 +37,10 @@ pub trait SystemParam: Sized { } /// # Safety -/// it is the implementors responsibility to ensure `system_state` is populated with the _exact_ -/// [World] access used by the SystemParamState (and associated FetchSystemParam). Additionally, it -/// is the implementor's responsibility to ensure there is no conflicting access across all -/// SystemParams. +/// It is the implementor's responsibility to ensure `system_state` is populated with the _exact_ +/// [World] access used by the SystemParamState (and associated FetchSystemParam). +/// Additionally, it is the implementor's responsibility to ensure there is no +/// conflicting access across all SystemParams. pub unsafe trait SystemParamState: Send + Sync + 'static { type Config: Default + Send + Sync; fn init(world: &mut World, system_state: &mut SystemState, config: Self::Config) -> Self; @@ -59,6 +59,7 @@ pub trait SystemParamFetch<'a>: SystemParamState { state: &'a mut Self, system_state: &'a SystemState, world: &'a World, + change_tick: u32, ) -> Self::Item; } @@ -115,10 +116,11 @@ where #[inline] unsafe fn get_param( state: &'a mut Self, - _system_state: &'a SystemState, + system_state: &'a SystemState, world: &'a World, + change_tick: u32, ) -> Self::Item { - Query::new(world, state) + Query::new(world, state, system_state.last_change_tick, change_tick) } } @@ -155,25 +157,23 @@ impl_query_set!(); /// Use `Option>` if the resource might not always exist. pub struct Res<'w, T> { value: &'w T, - flags: ComponentFlags, + ticks: &'w ComponentTicks, + last_change_tick: u32, + change_tick: u32, } impl<'w, T: Component> Res<'w, T> { - /// Returns true if (and only if) this resource been added since the start of the frame. - pub fn added(&self) -> bool { - self.flags.contains(ComponentFlags::ADDED) - } - - /// Returns true if (and only if) this resource been mutated since the start of the frame. - pub fn mutated(&self) -> bool { - self.flags.contains(ComponentFlags::MUTATED) + /// Returns true if (and only if) this resource been added since the last execution of this + /// system. + pub fn is_added(&self) -> bool { + self.ticks.is_added(self.last_change_tick, self.change_tick) } - /// Returns true if (and only if) this resource been either mutated or added since the start of - /// the frame. - pub fn changed(&self) -> bool { - self.flags - .intersects(ComponentFlags::ADDED | ComponentFlags::MUTATED) + /// Returns true if (and only if) this resource been changed since the last execution of this + /// system. + pub fn is_changed(&self) -> bool { + self.ticks + .is_changed(self.last_change_tick, self.change_tick) } } @@ -229,8 +229,9 @@ impl<'a, T: Component> SystemParamFetch<'a> for ResState { #[inline] unsafe fn get_param( state: &'a mut Self, - _system_state: &'a SystemState, + system_state: &'a SystemState, world: &'a World, + change_tick: u32, ) -> Self::Item { let column = world .get_populated_resource_column(state.component_id) @@ -242,7 +243,9 @@ impl<'a, T: Component> SystemParamFetch<'a> for ResState { }); Res { value: &*column.get_ptr().as_ptr().cast::(), - flags: *column.get_flags_mut_ptr(), + ticks: &*column.get_ticks_mut_ptr(), + last_change_tick: system_state.last_change_tick, + change_tick, } } } @@ -267,14 +270,17 @@ impl<'a, T: Component> SystemParamFetch<'a> for OptionResState { #[inline] unsafe fn get_param( state: &'a mut Self, - _system_state: &'a SystemState, + system_state: &'a SystemState, world: &'a World, + change_tick: u32, ) -> Self::Item { world .get_populated_resource_column(state.0.component_id) .map(|column| Res { value: &*column.get_ptr().as_ptr().cast::(), - flags: *column.get_flags_mut_ptr(), + ticks: &*column.get_ticks_mut_ptr(), + last_change_tick: system_state.last_change_tick, + change_tick, }) } } @@ -286,25 +292,23 @@ impl<'a, T: Component> SystemParamFetch<'a> for OptionResState { /// Use `Option>` if the resource might not always exist. pub struct ResMut<'w, T> { value: &'w mut T, - flags: &'w mut ComponentFlags, + ticks: &'w mut ComponentTicks, + last_change_tick: u32, + change_tick: u32, } impl<'w, T: Component> ResMut<'w, T> { - /// Returns true if (and only if) this resource been added since the start of the frame. - pub fn added(&self) -> bool { - self.flags.contains(ComponentFlags::ADDED) + /// Returns true if (and only if) this resource been added since the last execution of this + /// system. + pub fn is_added(&self) -> bool { + self.ticks.is_added(self.last_change_tick, self.change_tick) } - /// Returns true if (and only if) this resource been mutated since the start of the frame. - pub fn mutated(&self) -> bool { - self.flags.contains(ComponentFlags::MUTATED) - } - - /// Returns true if (and only if) this resource been either mutated or added since the start of - /// the frame. - pub fn changed(&self) -> bool { - self.flags - .intersects(ComponentFlags::ADDED | ComponentFlags::MUTATED) + /// Returns true if (and only if) this resource been changed since the last execution of this + /// system. + pub fn is_changed(&self) -> bool { + self.ticks + .is_changed(self.last_change_tick, self.change_tick) } } @@ -318,7 +322,7 @@ impl<'w, T: Component> Deref for ResMut<'w, T> { impl<'w, T: Component> DerefMut for ResMut<'w, T> { fn deref_mut(&mut self) -> &mut Self::Target { - self.flags.insert(ComponentFlags::MUTATED); + self.ticks.set_changed(self.change_tick); self.value } } @@ -371,8 +375,9 @@ impl<'a, T: Component> SystemParamFetch<'a> for ResMutState { #[inline] unsafe fn get_param( state: &'a mut Self, - _system_state: &'a SystemState, + system_state: &'a SystemState, world: &'a World, + change_tick: u32, ) -> Self::Item { let value = world .get_resource_unchecked_mut_with_id(state.component_id) @@ -384,7 +389,9 @@ impl<'a, T: Component> SystemParamFetch<'a> for ResMutState { }); ResMut { value: value.value, - flags: value.flags, + ticks: value.component_ticks, + last_change_tick: system_state.last_change_tick, + change_tick, } } } @@ -409,14 +416,17 @@ impl<'a, T: Component> SystemParamFetch<'a> for OptionResMutState { #[inline] unsafe fn get_param( state: &'a mut Self, - _system_state: &'a SystemState, + system_state: &'a SystemState, world: &'a World, + change_tick: u32, ) -> Self::Item { world .get_resource_unchecked_mut_with_id(state.0.component_id) .map(|value| ResMut { value: value.value, - flags: value.flags, + ticks: value.component_ticks, + last_change_tick: system_state.last_change_tick, + change_tick, }) } } @@ -446,6 +456,7 @@ impl<'a> SystemParamFetch<'a> for CommandQueue { state: &'a mut Self, _system_state: &'a SystemState, world: &'a World, + _change_tick: u32, ) -> Self::Item { Commands::new(state, world) } @@ -492,6 +503,7 @@ impl<'a, T: Component + FromWorld> SystemParamFetch<'a> for LocalState { state: &'a mut Self, _system_state: &'a SystemState, _world: &'a World, + _change_tick: u32, ) -> Self::Item { Local(&mut state.0) } @@ -539,6 +551,7 @@ impl<'a, T: Component> SystemParamFetch<'a> for RemovedComponentsState { state: &'a mut Self, _system_state: &'a SystemState, world: &'a World, + _change_tick: u32, ) -> Self::Item { RemovedComponents { world, @@ -551,6 +564,24 @@ impl<'a, T: Component> SystemParamFetch<'a> for RemovedComponentsState { /// Shared borrow of a NonSend resource pub struct NonSend<'w, T> { pub(crate) value: &'w T, + ticks: ComponentTicks, + last_change_tick: u32, + change_tick: u32, +} + +impl<'w, T: Component> NonSend<'w, T> { + /// Returns true if (and only if) this resource been added since the last execution of this + /// system. + pub fn is_added(&self) -> bool { + self.ticks.is_added(self.last_change_tick, self.change_tick) + } + + /// Returns true if (and only if) this resource been changed since the last execution of this + /// system. + pub fn is_changed(&self) -> bool { + self.ticks + .is_changed(self.last_change_tick, self.change_tick) + } } impl<'w, T: 'static> Deref for NonSend<'w, T> { @@ -607,18 +638,24 @@ impl<'a, T: 'static> SystemParamFetch<'a> for NonSendState { #[inline] unsafe fn get_param( state: &'a mut Self, - _system_state: &'a SystemState, + system_state: &'a SystemState, world: &'a World, + change_tick: u32, ) -> Self::Item { + world.validate_non_send_access::(); + let column = world + .get_populated_resource_column(state.component_id) + .unwrap_or_else(|| { + panic!( + "Requested non-send resource does not exist: {}", + std::any::type_name::() + ) + }); NonSend { - value: world - .get_non_send_with_id::(state.component_id) - .unwrap_or_else(|| { - panic!( - "Requested non-send resource does not exist: {}", - std::any::type_name::() - ) - }), + value: &*column.get_ptr().as_ptr().cast::(), + ticks: *column.get_ticks_mut_ptr(), + last_change_tick: system_state.last_change_tick, + change_tick, } } } @@ -626,7 +663,24 @@ impl<'a, T: 'static> SystemParamFetch<'a> for NonSendState { /// Unique borrow of a NonSend resource pub struct NonSendMut<'a, T: 'static> { pub(crate) value: &'a mut T, - pub(crate) flags: &'a mut ComponentFlags, + ticks: &'a mut ComponentTicks, + last_change_tick: u32, + change_tick: u32, +} + +impl<'w, T: Component> NonSendMut<'w, T> { + /// Returns true if (and only if) this resource been added since the last execution of this + /// system. + pub fn is_added(&self) -> bool { + self.ticks.is_added(self.last_change_tick, self.change_tick) + } + + /// Returns true if (and only if) this resource been changed since the last execution of this + /// system. + pub fn is_changed(&self) -> bool { + self.ticks + .is_changed(self.last_change_tick, self.change_tick) + } } impl<'a, T: 'static> Deref for NonSendMut<'a, T> { @@ -641,7 +695,7 @@ impl<'a, T: 'static> Deref for NonSendMut<'a, T> { impl<'a, T: 'static> DerefMut for NonSendMut<'a, T> { #[inline] fn deref_mut(&mut self) -> &mut T { - self.flags.insert(ComponentFlags::MUTATED); + self.ticks.set_changed(self.change_tick); self.value } } @@ -702,11 +756,13 @@ impl<'a, T: 'static> SystemParamFetch<'a> for NonSendMutState { #[inline] unsafe fn get_param( state: &'a mut Self, - _system_state: &'a SystemState, + system_state: &'a SystemState, world: &'a World, + change_tick: u32, ) -> Self::Item { - let value = world - .get_non_send_unchecked_mut_with_id(state.component_id) + world.validate_non_send_access::(); + let column = world + .get_populated_resource_column(state.component_id) .unwrap_or_else(|| { panic!( "Requested non-send resource does not exist: {}", @@ -714,8 +770,10 @@ impl<'a, T: 'static> SystemParamFetch<'a> for NonSendMutState { ) }); NonSendMut { - value: value.value, - flags: value.flags, + value: &mut *column.get_ptr().as_ptr().cast::(), + ticks: &mut *column.get_ticks_mut_ptr(), + last_change_tick: system_state.last_change_tick, + change_tick, } } } @@ -745,6 +803,7 @@ impl<'a> SystemParamFetch<'a> for ArchetypesState { _state: &'a mut Self, _system_state: &'a SystemState, world: &'a World, + _change_tick: u32, ) -> Self::Item { world.archetypes() } @@ -773,6 +832,7 @@ impl<'a> SystemParamFetch<'a> for ComponentsState { _state: &'a mut Self, _system_state: &'a SystemState, world: &'a World, + _change_tick: u32, ) -> Self::Item { world.components() } @@ -801,6 +861,7 @@ impl<'a> SystemParamFetch<'a> for EntitiesState { _state: &'a mut Self, _system_state: &'a SystemState, world: &'a World, + _change_tick: u32, ) -> Self::Item { world.entities() } @@ -829,11 +890,48 @@ impl<'a> SystemParamFetch<'a> for BundlesState { _state: &'a mut Self, _system_state: &'a SystemState, world: &'a World, + _change_tick: u32, ) -> Self::Item { world.bundles() } } +#[derive(Debug)] +pub struct SystemChangeTick { + pub last_change_tick: u32, + pub change_tick: u32, +} + +impl SystemParam for SystemChangeTick { + type Fetch = SystemChangeTickState; +} + +pub struct SystemChangeTickState {} + +unsafe impl SystemParamState for SystemChangeTickState { + type Config = (); + + fn init(_world: &mut World, _system_state: &mut SystemState, _config: Self::Config) -> Self { + Self {} + } +} + +impl<'a> SystemParamFetch<'a> for SystemChangeTickState { + type Item = SystemChangeTick; + + unsafe fn get_param( + _state: &mut Self, + system_state: &SystemState, + _world: &World, + change_tick: u32, + ) -> Self::Item { + SystemChangeTick { + last_change_tick: system_state.last_change_tick, + change_tick, + } + } +} + macro_rules! impl_system_param_tuple { ($($param: ident),*) => { impl<$($param: SystemParam),*> SystemParam for ($($param,)*) { @@ -849,10 +947,11 @@ macro_rules! impl_system_param_tuple { state: &'a mut Self, system_state: &'a SystemState, world: &'a World, + change_tick: u32, ) -> Self::Item { let ($($param,)*) = state; - ($($param::get_param($param, system_state, world),)*) + ($($param::get_param($param, system_state, world, change_tick),)*) } } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 71aa5aac925ea..a0b64c00779c6 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1,7 +1,7 @@ use crate::{ - archetype::{Archetype, ArchetypeId, Archetypes}, + archetype::{Archetype, ArchetypeId, Archetypes, ComponentStatus}, bundle::{Bundle, BundleInfo}, - component::{Component, ComponentFlags, ComponentId, Components, StorageType}, + component::{Component, ComponentId, ComponentTicks, Components, StorageType}, entity::{Entity, EntityLocation}, storage::{SparseSet, Storages}, world::{Mut, World}, @@ -72,11 +72,17 @@ impl<'w> EntityRef<'w> { /// This allows aliased mutability. You must make sure this call does not result in multiple /// mutable references to the same component #[inline] - pub unsafe fn get_unchecked_mut(&self) -> Option> { - get_component_and_flags_with_type(self.world, TypeId::of::(), self.entity, self.location) - .map(|(value, flags)| Mut { + pub unsafe fn get_unchecked_mut( + &self, + last_change_tick: u32, + change_tick: u32, + ) -> Option> { + get_component_and_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) + .map(|(value, ticks)| Mut { value: &mut *value.cast::(), - flags: &mut *flags, + component_ticks: &mut *ticks, + last_change_tick, + change_tick, }) } } @@ -147,15 +153,17 @@ impl<'w> EntityMut<'w> { // SAFE: world access is unique, entity location is valid, and returned component is of type // T unsafe { - get_component_and_flags_with_type( + get_component_and_ticks_with_type( self.world, TypeId::of::(), self.entity, self.location, ) - .map(|(value, flags)| Mut { + .map(|(value, ticks)| Mut { value: &mut *value.cast::(), - flags: &mut *flags, + component_ticks: &mut *ticks, + last_change_tick: self.world.last_change_tick(), + change_tick: self.world.change_tick(), }) } } @@ -165,10 +173,12 @@ impl<'w> EntityMut<'w> { /// mutable references to the same component #[inline] pub unsafe fn get_unchecked_mut(&self) -> Option> { - get_component_and_flags_with_type(self.world, TypeId::of::(), self.entity, self.location) - .map(|(value, flags)| Mut { + get_component_and_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) + .map(|(value, ticks)| Mut { value: &mut *value.cast::(), - flags: &mut *flags, + component_ticks: &mut *ticks, + last_change_tick: self.world.last_change_tick(), + change_tick: self.world.read_change_tick(), }) } @@ -176,6 +186,7 @@ impl<'w> EntityMut<'w> { // TODO: move relevant methods to World (add/remove bundle) pub fn insert_bundle(&mut self, bundle: T) -> &mut Self { let entity = self.entity; + let change_tick = self.world.change_tick(); let entities = &mut self.world.entities; let archetypes = &mut self.world.archetypes; let components = &mut self.world.components; @@ -225,8 +236,6 @@ impl<'w> EntityMut<'w> { // if an entity was moved into this entity's table spot, update its table row if let Some(swapped_entity) = move_result.swapped_entity { let swapped_location = entities.get(swapped_entity).unwrap(); - // SAFE: entity is live and is therefore contained in an archetype that - // exists archetypes[swapped_location.archetype_id] .set_entity_table_row(swapped_location.index, old_table_row); } @@ -250,8 +259,9 @@ impl<'w> EntityMut<'w> { entity, table, table_row, - &from_bundle.bundle_flags, + &from_bundle.bundle_status, bundle, + change_tick, ) }; self @@ -524,12 +534,12 @@ unsafe fn get_component( /// # Safety /// Caller must ensure that `component_id` is valid #[inline] -unsafe fn get_component_and_flags( +unsafe fn get_component_and_ticks( world: &World, component_id: ComponentId, entity: Entity, location: EntityLocation, -) -> Option<(*mut u8, *mut ComponentFlags)> { +) -> Option<(*mut u8, *mut ComponentTicks)> { let archetype = &world.archetypes[location.archetype_id]; let component_info = world.components.get_info_unchecked(component_id); match component_info.storage_type() { @@ -540,14 +550,14 @@ unsafe fn get_component_and_flags( // SAFE: archetypes only store valid table_rows and the stored component type is T Some(( components.get_unchecked(table_row), - components.get_flags_unchecked(table_row), + components.get_ticks_unchecked(table_row), )) } StorageType::SparseSet => world .storages .sparse_sets .get(component_id) - .and_then(|sparse_set| sparse_set.get_with_flags(entity)), + .and_then(|sparse_set| sparse_set.get_with_ticks(entity)), } } @@ -601,14 +611,14 @@ unsafe fn get_component_with_type( /// # Safety /// `entity_location` must be within bounds of an archetype that exists. -pub(crate) unsafe fn get_component_and_flags_with_type( +pub(crate) unsafe fn get_component_and_ticks_with_type( world: &World, type_id: TypeId, entity: Entity, location: EntityLocation, -) -> Option<(*mut u8, *mut ComponentFlags)> { +) -> Option<(*mut u8, *mut ComponentTicks)> { let component_id = world.components.get_id(type_id)?; - get_component_and_flags(world, component_id, entity, location) + get_component_and_ticks(world, component_id, entity, location) } fn contains_component_with_type(world: &World, type_id: TypeId, location: EntityLocation) -> bool { @@ -648,14 +658,14 @@ pub(crate) unsafe fn add_bundle_to_archetype( } let mut new_table_components = Vec::new(); let mut new_sparse_set_components = Vec::new(); - let mut tracking_flags = Vec::with_capacity(bundle_info.component_ids.len()); + let mut bundle_status = Vec::with_capacity(bundle_info.component_ids.len()); let current_archetype = &mut archetypes[archetype_id]; for component_id in bundle_info.component_ids.iter().cloned() { if current_archetype.contains(component_id) { - tracking_flags.push(ComponentFlags::MUTATED); + bundle_status.push(ComponentStatus::Mutated); } else { - tracking_flags.push(ComponentFlags::ADDED); + bundle_status.push(ComponentStatus::Added); let component_info = components.get_info_unchecked(component_id); match component_info.storage_type() { StorageType::Table => new_table_components.push(component_id), @@ -671,7 +681,7 @@ pub(crate) unsafe fn add_bundle_to_archetype( let edges = current_archetype.edges_mut(); // the archetype does not change when we add this bundle edges.set_add_bundle(bundle_info.id, archetype_id); - edges.set_from_bundle(bundle_info.id, archetype_id, tracking_flags); + edges.set_from_bundle(bundle_info.id, archetype_id, bundle_status); archetype_id } else { let table_id; @@ -715,7 +725,7 @@ pub(crate) unsafe fn add_bundle_to_archetype( archetypes[new_archetype_id].edges_mut().set_from_bundle( bundle_info.id, new_archetype_id, - tracking_flags, + bundle_status, ); new_archetype_id } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 952bb3ff4e7ee..0c7111e66d56e 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -12,14 +12,18 @@ use crate::{ archetype::{ArchetypeComponentId, ArchetypeComponentInfo, ArchetypeId, Archetypes}, bundle::{Bundle, Bundles}, component::{ - Component, ComponentDescriptor, ComponentFlags, ComponentId, Components, ComponentsError, + Component, ComponentDescriptor, ComponentId, ComponentTicks, Components, ComponentsError, StorageType, }, entity::{Entities, Entity}, query::{FilterFetch, QueryState, WorldQuery}, storage::{Column, SparseSet, Storages}, }; -use std::{any::TypeId, fmt}; +use std::{ + any::TypeId, + fmt, + sync::atomic::{AtomicU32, Ordering}, +}; #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct WorldId(u64); @@ -30,11 +34,11 @@ impl Default for WorldId { } } -/// [World] stores and exposes operations on [entities](Entity), [components](Component), and their -/// associated metadata. Each [Entity] has a set of components. Each component can have up to one -/// instance of each component type. Entity components can be created, updated, removed, and queried -/// using a given [World]. -#[derive(Default)] +/// [World] stores and exposes operations on [entities](Entity), [components](Component), +/// and their associated metadata. +/// Each [Entity] has a set of components. Each component can have up to one instance of each +/// component type. Entity components can be created, updated, removed, and queried using a given +/// [World]. pub struct World { id: WorldId, pub(crate) entities: Entities, @@ -46,6 +50,28 @@ pub struct World { /// Access cache used by [WorldCell]. pub(crate) archetype_component_access: ArchetypeComponentAccess, main_thread_validator: MainThreadValidator, + pub(crate) change_tick: AtomicU32, + pub(crate) last_change_tick: u32, +} + +impl Default for World { + fn default() -> Self { + Self { + id: Default::default(), + entities: Default::default(), + components: Default::default(), + archetypes: Default::default(), + storages: Default::default(), + bundles: Default::default(), + removed_components: Default::default(), + archetype_component_access: Default::default(), + main_thread_validator: Default::default(), + // Default value is `1`, and `last_change_tick`s default to `0`, such that changes + // are detected on first system runs and for direct world queries. + change_tick: AtomicU32::new(1), + last_change_tick: 0, + } + } } impl World { @@ -378,22 +404,17 @@ impl World { .unwrap_or(false) } - /// Clears all component tracker state, such as "added", "mutated", and "removed". + /// Clears component tracker state pub fn clear_trackers(&mut self) { - self.storages.tables.clear_flags(); - self.storages.sparse_sets.clear_flags(); for entities in self.removed_components.values_mut() { entities.clear(); } - let resource_archetype = self.archetypes.resource_mut(); - for column in resource_archetype.unique_components.values_mut() { - column.clear_flags(); - } + self.last_change_tick = self.increment_change_tick(); } /// Returns [QueryState] for the given [WorldQuery], which is used to efficiently - /// run queries on the [World]. + /// run queries on the [World] by storing and reusing the [QueryState]. /// ``` /// use bevy_ecs::{entity::Entity, world::World}; /// @@ -428,8 +449,8 @@ impl World { QueryState::new(self) } - /// Returns [QueryState] for the given [WorldQuery] and filter, which is used to efficiently - /// run filtered queries on the [World]. + /// Returns [QueryState] for the given filtered [WorldQuery], which is used to efficiently + /// run queries on the [World] by storing and reusing the [QueryState]. /// ``` /// use bevy_ecs::{entity::Entity, world::World, query::With}; /// @@ -453,8 +474,8 @@ impl World { QueryState::new(self) } - /// Returns an iterator of entities that had components of type `T` removed since the last call - /// to [World::clear_trackers]. + /// Returns an iterator of entities that had components of type `T` removed + /// since the last call to [World::clear_trackers]. pub fn removed(&self) -> std::iter::Cloned> { if let Some(component_id) = self.components.get_id(TypeId::of::()) { self.removed_with_id(component_id) @@ -621,7 +642,7 @@ impl World { .components .get_resource_id(TypeId::of::()) .unwrap_or_else(|| panic!("resource does not exist: {}", std::any::type_name::())); - let (ptr, mut flags) = { + let (ptr, mut ticks) = { let resource_archetype = self.archetypes.resource_mut(); let unique_components = resource_archetype.unique_components_mut(); let column = unique_components.get_mut(component_id).unwrap_or_else(|| { @@ -637,7 +658,9 @@ impl World { // SAFE: pointer is of type T let value = Mut { value: unsafe { &mut *ptr.cast::() }, - flags: &mut flags, + component_ticks: &mut ticks, + last_change_tick: self.last_change_tick(), + change_tick: self.change_tick(), }; let result = f(value, self); let resource_archetype = self.archetypes.resource_mut(); @@ -650,7 +673,7 @@ impl World { // SAFE: row was just allocated above unsafe { column.set_unchecked(row, ptr) }; // SAFE: row was just allocated above - unsafe { *column.get_flags_unchecked_mut(row) = flags }; + unsafe { *column.get_ticks_unchecked_mut(row) = ticks }; result } @@ -676,7 +699,9 @@ impl World { let column = self.get_populated_resource_column(component_id)?; Some(Mut { value: &mut *column.get_ptr().as_ptr().cast::(), - flags: &mut *column.get_flags_mut_ptr(), + component_ticks: &mut *column.get_ticks_mut_ptr(), + last_change_tick: self.last_change_tick(), + change_tick: self.read_change_tick(), }) } @@ -707,6 +732,7 @@ impl World { /// `component_id` must be valid and correspond to a resource component of type T #[inline] unsafe fn insert_resource_with_id(&mut self, component_id: ComponentId, mut value: T) { + let change_tick = self.change_tick(); let column = self.initialize_resource_internal(component_id); if column.is_empty() { // SAFE: column is of type T and has been allocated above @@ -716,15 +742,12 @@ impl World { // SAFE: index was just allocated above column.set_unchecked(row, data); std::mem::forget(value); - column - .get_flags_unchecked_mut(row) - .set(ComponentFlags::ADDED, true); + // SAFE: index was just allocated above + *column.get_ticks_unchecked_mut(row) = ComponentTicks::new(change_tick); } else { // SAFE: column is of type T and has already been allocated *column.get_unchecked(0).cast::() = value; - column - .get_flags_unchecked_mut(0) - .set(ComponentFlags::MUTATED, true); + column.get_ticks_unchecked_mut(0).set_changed(change_tick); } } @@ -788,7 +811,7 @@ impl World { }) } - fn validate_non_send_access(&self) { + pub(crate) fn validate_non_send_access(&self) { if !self.main_thread_validator.is_main_thread() { panic!( "attempted to access NonSend resource {} off of the main thread", @@ -812,6 +835,38 @@ impl World { }); } } + + #[inline] + pub fn increment_change_tick(&self) -> u32 { + self.change_tick.fetch_add(1, Ordering::AcqRel) + } + + #[inline] + pub fn read_change_tick(&self) -> u32 { + self.change_tick.load(Ordering::Acquire) + } + + #[inline] + pub fn change_tick(&mut self) -> u32 { + *self.change_tick.get_mut() + } + + #[inline] + pub fn last_change_tick(&self) -> u32 { + self.last_change_tick + } + + pub fn check_change_ticks(&mut self) { + // Iterate over all component change ticks, clamping their age to max age + // PERF: parallelize + let change_tick = self.change_tick(); + self.storages.tables.check_change_ticks(change_tick); + self.storages.sparse_sets.check_change_ticks(change_tick); + let resource_archetype = self.archetypes.resource_mut(); + for column in resource_archetype.unique_components.values_mut() { + column.check_change_ticks(change_tick); + } + } } impl fmt::Debug for World { diff --git a/crates/bevy_ecs/src/world/pointer.rs b/crates/bevy_ecs/src/world/pointer.rs index a0e270274feb7..c7add3a58f817 100644 --- a/crates/bevy_ecs/src/world/pointer.rs +++ b/crates/bevy_ecs/src/world/pointer.rs @@ -1,10 +1,12 @@ -use crate::component::ComponentFlags; +use crate::component::ComponentTicks; use std::ops::{Deref, DerefMut}; /// Unique borrow of an entity's component pub struct Mut<'a, T> { pub(crate) value: &'a mut T, - pub(crate) flags: &'a mut ComponentFlags, + pub(crate) component_ticks: &'a mut ComponentTicks, + pub(crate) last_change_tick: u32, + pub(crate) change_tick: u32, } impl<'a, T> Deref for Mut<'a, T> { @@ -19,7 +21,7 @@ impl<'a, T> Deref for Mut<'a, T> { impl<'a, T> DerefMut for Mut<'a, T> { #[inline] fn deref_mut(&mut self) -> &mut T { - self.flags.insert(ComponentFlags::MUTATED); + self.component_ticks.set_changed(self.change_tick); self.value } } @@ -31,20 +33,17 @@ impl<'a, T: core::fmt::Debug> core::fmt::Debug for Mut<'a, T> { } impl<'w, T> Mut<'w, T> { - /// Returns true if (and only if) this component been added since the start of the frame. - pub fn added(&self) -> bool { - self.flags.contains(ComponentFlags::ADDED) + /// Returns true if (and only if) this component been added since the last execution of this + /// system. + pub fn is_added(&self) -> bool { + self.component_ticks + .is_added(self.last_change_tick, self.change_tick) } - /// Returns true if (and only if) this component been mutated since the start of the frame. - pub fn mutated(&self) -> bool { - self.flags.contains(ComponentFlags::MUTATED) - } - - /// Returns true if (and only if) this component been either mutated or added since the start of - /// the frame. - pub fn changed(&self) -> bool { - self.flags - .intersects(ComponentFlags::ADDED | ComponentFlags::MUTATED) + /// Returns true if (and only if) this component been changed + /// since the last execution of this system. + pub fn is_changed(&self) -> bool { + self.component_ticks + .is_changed(self.last_change_tick, self.change_tick) } } diff --git a/crates/bevy_ecs/src/world/spawn_batch.rs b/crates/bevy_ecs/src/world/spawn_batch.rs index 105c8080ea62e..6af5d53b1e4ef 100644 --- a/crates/bevy_ecs/src/world/spawn_batch.rs +++ b/crates/bevy_ecs/src/world/spawn_batch.rs @@ -17,6 +17,7 @@ where table: &'w mut Table, sparse_sets: &'w mut SparseSets, bundle_info: &'w BundleInfo, + change_tick: u32, } impl<'w, I> SpawnBatchIter<'w, I> @@ -57,6 +58,7 @@ where table, sparse_sets: &mut world.storages.sparse_sets, bundle_info, + change_tick: *world.change_tick.get_mut(), } } } @@ -96,8 +98,9 @@ where entity, self.table, table_row, - &from_bundle.bundle_flags, + &from_bundle.bundle_status, bundle, + self.change_tick, ); self.entities.meta[entity.id as usize].location = location; } diff --git a/crates/bevy_render/src/mesh/mesh.rs b/crates/bevy_render/src/mesh/mesh.rs index 68d42de2d7bd2..f977f25647954 100644 --- a/crates/bevy_render/src/mesh/mesh.rs +++ b/crates/bevy_render/src/mesh/mesh.rs @@ -234,8 +234,8 @@ pub struct Mesh { impl Mesh { /// Per vertex coloring. Use in conjunction with [`Mesh::set_attribute`] pub const ATTRIBUTE_COLOR: &'static str = "Vertex_Color"; - /// The direction the vertex normal is facing in. Use in conjunction with - /// [`Mesh::set_attribute`] + /// The direction the vertex normal is facing in. + /// Use in conjunction with [`Mesh::set_attribute`] pub const ATTRIBUTE_NORMAL: &'static str = "Vertex_Normal"; /// Where the vertex is located in space. Use in conjunction with [`Mesh::set_attribute`] pub const ATTRIBUTE_POSITION: &'static str = "Vertex_Position"; diff --git a/examples/ecs/change_detection.rs b/examples/ecs/change_detection.rs index c034376e18af4..46684a889f8fc 100644 --- a/examples/ecs/change_detection.rs +++ b/examples/ecs/change_detection.rs @@ -8,7 +8,7 @@ fn main() { .add_startup_system(setup.system()) .add_system(change_component.system()) .add_system(change_detection.system()) - .add_system(flags_monitoring.system()) + .add_system(tracker_monitoring.system()) .run(); } @@ -29,7 +29,7 @@ fn change_component(time: Res