diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index 000c2121a7d1c..8377d9b97e307 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -27,6 +27,7 @@ fixedbitset = "0.4" fxhash = "0.2" downcast-rs = "1.2" serde = { version = "1", features = ["derive"] } +cfg-if = "1.0" [dev-dependencies] rand = "0.8" diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 4d04d1812b962..c7369b1ed8c16 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -224,8 +224,8 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { unsafe fn init_fetch<'__w>( _world: &'__w #path::world::World, state: &Self::State, - _last_change_tick: u32, - _change_tick: u32 + _last_change_tick: #path::change_detection::Tick, + _change_tick: #path::change_detection::Tick ) -> ::Fetch<'__w> { #fetch_struct_name { #(#field_idents: diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index f67e58cf19be6..ea5cdeb7247f7 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -292,7 +292,7 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream { state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { ParamSet { param_states: &mut state.0, @@ -437,7 +437,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { state: &'s mut Self, system_meta: &#path::system::SystemMeta, world: &'w #path::world::World, - change_tick: u32, + change_tick: #path::change_detection::Tick, ) -> Self::Item { #struct_name { #(#fields: <<#field_types as #path::system::SystemParam>::Fetch as #path::system::SystemParamFetch>::get_param(&mut state.state.#field_indices, system_meta, world, change_tick),)* diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 82c8893feb8f0..aacb7390460ea 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -9,7 +9,8 @@ use crate::{ Archetype, ArchetypeId, Archetypes, BundleComponentStatus, ComponentStatus, SpawnBundleStatus, }, - component::{Component, ComponentId, Components, StorageType, Tick}, + change_detection::SmallTick, + component::{Component, ComponentId, Components, StorageType}, entity::{Entities, Entity, EntityLocation}, storage::{SparseSetIndex, SparseSets, Storages, Table}, }; @@ -280,7 +281,7 @@ impl BundleInfo { components: &mut Components, storages: &'a mut Storages, archetype_id: ArchetypeId, - change_tick: u32, + change_tick: SmallTick, ) -> BundleInserter<'a, 'b> { let new_archetype_id = self.add_bundle_to_archetype(archetypes, storages, components, archetype_id); @@ -339,7 +340,7 @@ impl BundleInfo { archetypes: &'a mut Archetypes, components: &mut Components, storages: &'a mut Storages, - change_tick: u32, + change_tick: SmallTick, ) -> BundleSpawner<'a, 'b> { let new_archetype_id = self.add_bundle_to_archetype(archetypes, storages, components, ArchetypeId::EMPTY); @@ -380,7 +381,7 @@ impl BundleInfo { bundle_component_status: &S, entity: Entity, table_row: usize, - change_tick: u32, + change_tick: SmallTick, bundle: T, ) { // NOTE: get_components calls this closure on each component in "bundle order". @@ -394,7 +395,7 @@ impl BundleInfo { // SAFETY: bundle_component is a valid index for this bundle match bundle_component_status.get_status(bundle_component) { ComponentStatus::Added => { - column.initialize(table_row, component_ptr, Tick::new(change_tick)); + column.initialize(table_row, component_ptr, change_tick); } ComponentStatus::Mutated => { column.replace(table_row, component_ptr, change_tick); @@ -502,7 +503,7 @@ pub(crate) struct BundleInserter<'a, 'b> { sparse_sets: &'a mut SparseSets, result: InsertBundleResult<'a>, archetypes_ptr: *mut Archetype, - change_tick: u32, + change_tick: SmallTick, } pub(crate) enum InsertBundleResult<'a> { @@ -637,7 +638,7 @@ pub(crate) struct BundleSpawner<'a, 'b> { bundle_info: &'b BundleInfo, table: &'a mut Table, sparse_sets: &'a mut SparseSets, - change_tick: u32, + change_tick: SmallTick, } impl<'a, 'b> BundleSpawner<'a, 'b> { diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index 0831f316ef4f8..693138726052e 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -1,12 +1,12 @@ //! Types that detect when their internal data mutate. -use crate::{ - component::{Tick, TickCells}, - ptr::PtrMut, - system::Resource, +use crate::{component::TickCells, ptr::PtrMut, system::Resource}; +use std::{ + fmt::{self, Display}, + num::NonZeroU64, + ops::{Deref, DerefMut}, + sync::atomic::Ordering::Relaxed, }; -use bevy_ptr::UnsafeCellDeref; -use std::ops::{Deref, DerefMut}; /// The (arbitrarily chosen) minimum number of world tick increments between `check_tick` scans. /// @@ -23,6 +23,261 @@ pub const CHECK_TICK_THRESHOLD: u32 = 518_400_000; /// Changes stop being detected once they become this old. pub const MAX_CHANGE_AGE: u32 = u32::MAX - (2 * CHECK_TICK_THRESHOLD - 1); +use cfg_if::cfg_if; + +cfg_if! { + if #[cfg(target_has_atomic = "64")] { + type AtomicTick = core::sync::atomic::AtomicU64; + type AtomicTickValue = u64; + } else { + type AtomicTick = core::sync::atomic::AtomicU32; + type AtomicTickValue = u32; + const ATOMIC_PANIC_THRESHOLD: u32 = u32::MAX; + } +} + +// This base value ensures that tick value are never zero. +// Yield nonsense instead of UB after u64 increment overflows (virtually impossible). +const TICK_BASE_VALUE: u64 = 1 + (u64::MAX >> 1); + +/// Tick value. +/// Ticks are cheap to copy, comparable and has total order. +#[repr(transparent)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct Tick(pub NonZeroU64); + +impl Display for Tick { + #[inline(always)] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + Display::fmt(&self.0.get(), f) + } +} + +impl Default for Tick { + #[inline(always)] + fn default() -> Self { + Tick::new() + } +} + +impl Tick { + #[inline(always)] + pub const fn new() -> Self { + // Safety: + // `TICK_BASE_VALUE` is nonzero + Tick(unsafe { NonZeroU64::new_unchecked(TICK_BASE_VALUE) }) + } +} + +/// Tick value with smaller range. +/// Unlike [`Tick`] this value contains only lower bits of tick value +/// in order to reduce space required to store it and memory bandwidth +/// required to read it. +/// +/// [`SmallTick`] values should be periodically updated to not fall +/// too far behind [`TickCounter`]'s last tick. +/// When [`SmallTick`] value gets too old it is periodically updated to +/// [`TickCounter::current() - SMALL_TICK_AGE_THRESHOLD`]. +/// +/// This way [`SmallTick::is_later_than`] with target older +/// than `SMALL_TICK_AGE_THRESHOLD` may return false positive. +/// Which is acceptable tradeoff given that too old targets should +/// not occur outside some edge cases. +/// +#[repr(transparent)] +#[derive(Clone, Copy, Debug)] +pub struct SmallTick(pub u32); + +impl Default for SmallTick { + #[inline(always)] + fn default() -> Self { + SmallTick::new() + } +} + +impl SmallTick { + #[inline(always)] + pub const fn new() -> Self { + SmallTick(0) + } +} + +impl From for SmallTick { + #[inline(always)] + fn from(tick: Tick) -> Self { + // Taking only lower 32 bits. + SmallTick(tick.0.get() as u32) + } +} + +impl SmallTick { + // Returns true if this `SmallTick` is "newer" (or later) than `target` tick. + // uses `change_tick` tick to restore upper 32 bits. + // `change_tick` must be newer than `target`.x + // It returns false positive if `target` is + // `2^32` or more earlier than `change_tick`. + #[inline] + pub fn is_newer_than(&self, target: Tick, change_tick: Tick) -> bool { + debug_assert!(target.0.get() <= change_tick.0.get()); + + let target_age = change_tick.0.get() - target.0.get(); + if target_age > u32::MAX as u64 { + return true; + } + + let change_tick = change_tick.0.get() as u32; + + let age = change_tick.wrapping_sub(self.0); + + age < target_age as u32 + } + + #[inline] + pub fn set_changed(&mut self, change_tick: Tick) { + self.0 = change_tick.0.get() as u32; + } + + /// Maintain this tick value to keep its age from overflowing. + /// This method should be called frequently enough. + /// + /// Preferably calls this for all [`SmallTick`] values when + /// [`TickCounter::maintain`] returns `true`. + #[inline(always)] + pub fn check_tick(&mut self, change_tick: Tick) { + let age = (change_tick.0.get() as u32).wrapping_sub(self.0); + if age > MAX_CHANGE_AGE { + self.0 = (change_tick.0.get() as u32).wrapping_sub(MAX_CHANGE_AGE); + } + } + + #[cfg(test)] + fn needs_check(&self, change_tick: Tick) -> bool { + let age = (change_tick.0.get() as u32).wrapping_sub(self.0); + age > MAX_CHANGE_AGE + } +} + +/// Atomic counter for ticks. +/// Allows incrementing ticks atomically. +/// Yields [`Tick`] values. +/// +/// On system without 64-bit atomics requires calls to +/// [`TickCounter::maintain`] to keep 32-bit atomic value from overflowing. +/// If overflow do occur then one of the calls to [`TickCounter::next`] would panic +/// and successful calls to [`TickCounter::next`] and [`TickCounter::current`] would return +/// out-of-order [`Tick`] values. +/// Make sure to configure your code to either require `cfg(target_has_atomic = "64")` +/// or arrange calls to [`TickCounter::maintain`] frequently enough. +pub struct TickCounter { + #[cfg(not(target_has_atomic = "64"))] + offset: NonZeroU64, + atomic: AtomicTick, + + /// Tick when last maintenance was recommended. + last_maintenance: u64, +} + +impl Default for TickCounter { + #[inline(always)] + fn default() -> Self { + TickCounter::new() + } +} + +// #[allow(clippy::useless_conversion)] +impl TickCounter { + #[inline] + pub fn new() -> TickCounter { + cfg_if! { + if #[cfg(target_has_atomic = "64")] { + TickCounter { + atomic: AtomicTick::new(1), + last_maintenance: 0, + } + } else { + TickCounter { + atomic: AtomicTick::new(0), + offset: unsafe { + // # Safety + // duh + NonZeroU64::new_unchecked(1) + }, + last_maintenance: 0, + } + } + } + } + + #[inline(always)] + pub fn maintain(&mut self) -> bool { + #[cfg(not(target_has_atomic = "64"))] + { + self.offset += std::mem::replace(self.atomic.get_mut(), 0) as u64; + } + + let atomic = *self.atomic.get_mut(); + let tick_index = self.tick_index(atomic); + if tick_index > self.last_maintenance + CHECK_TICK_THRESHOLD as u64 { + self.last_maintenance = tick_index; + true + } else { + false + } + } + + #[inline(always)] + pub fn current(&self) -> Tick { + let atomic = self.atomic.load(Relaxed); + self.make_tick(atomic) + } + + #[inline(always)] + pub fn current_mut(&mut self) -> Tick { + let atomic = *self.atomic.get_mut(); + self.make_tick(atomic) + } + + #[inline(always)] + pub fn next(&self) -> Tick { + let atomic = self.atomic.fetch_add(1, Relaxed); + self.make_tick(atomic) + } + + #[inline] + fn tick_index(&self, atomic: AtomicTickValue) -> u64 { + cfg_if! { + if #[cfg(target_has_atomic = "64")] { + atomic + } else { + if atomic >= ATOMIC_PANIC_THRESHOLD { + panic!("Too many `TickCounter::next` calls between `TickCounter::maintain` calls") + } + self.offset + atomic as u64 + } + } + } + + #[inline(always)] + fn make_tick(&self, atomic: AtomicTickValue) -> Tick { + let index = self.tick_index(atomic); + // Safety: + // `TICK_BASE_VALUE` is nonzero + Tick(unsafe { NonZeroU64::new(TICK_BASE_VALUE | index).unwrap_unchecked() }) + } + + #[cfg(test)] + fn skip(&mut self, n: u64) { + #[cfg(target_has_atomic = "64")] + { + *self.atomic.get_mut() += n; + } + #[cfg(not(target_has_atomic = "64"))] + { + self.offset += n; + } + } +} + /// Types that implement reliable change detection. /// /// ## Example @@ -73,7 +328,7 @@ pub trait DetectChanges { /// For comparison, the previous change tick of a system can be read using the /// [`SystemChangeTick`](crate::system::SystemChangeTick) /// [`SystemParam`](crate::system::SystemParam). - fn last_changed(&self) -> u32; + fn last_changed(&self) -> Tick; /// Manually sets the change tick recording the previous time this data was mutated. /// @@ -81,7 +336,7 @@ pub trait DetectChanges { /// This is a complex and error-prone operation, primarily intended for use with rollback networking strategies. /// If you merely want to flag this data as changed, use [`set_changed`](DetectChanges::set_changed) instead. /// If you want to avoid triggering change detection, use [`bypass_change_detection`](DetectChanges::bypass_change_detection) instead. - fn set_last_changed(&mut self, last_change_tick: u32); + fn set_last_changed(&mut self, last_change_tick: Tick); /// Manually bypasses change detection, allowing you to mutate the underlying value without updating the change tick. /// @@ -101,31 +356,31 @@ macro_rules! change_detection_impl { fn is_added(&self) -> bool { self.ticks .added - .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick) + .is_newer_than(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] fn is_changed(&self) -> bool { self.ticks .changed - .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick) + .is_newer_than(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] fn set_changed(&mut self) { self.ticks .changed - .set_changed(self.ticks.change_tick); + .set_changed(self.ticks.change_tick.into()); } #[inline] - fn last_changed(&self) -> u32 { + fn last_changed(&self) -> Tick { self.ticks.last_change_tick } #[inline] - fn set_last_changed(&mut self, last_change_tick: u32) { - self.ticks.last_change_tick = last_change_tick + fn set_last_changed(&mut self, last_change_tick: Tick) { + self.ticks.last_change_tick = last_change_tick; } #[inline] @@ -229,10 +484,10 @@ macro_rules! impl_debug { } pub(crate) struct Ticks<'a> { - pub(crate) added: &'a mut Tick, - pub(crate) changed: &'a mut Tick, - pub(crate) last_change_tick: u32, - pub(crate) change_tick: u32, + pub(crate) added: &'a mut SmallTick, + pub(crate) changed: &'a mut SmallTick, + pub(crate) last_change_tick: Tick, + pub(crate) change_tick: Tick, } impl<'a> Ticks<'a> { @@ -241,12 +496,12 @@ impl<'a> Ticks<'a> { #[inline] pub(crate) unsafe fn from_tick_cells( cells: TickCells<'a>, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Self { Self { - added: cells.added.deref_mut(), - changed: cells.changed.deref_mut(), + added: &mut *cells.added.get(), + changed: &mut *cells.changed.get(), last_change_tick, change_tick, } @@ -406,28 +661,30 @@ impl<'a> DetectChanges for MutUntyped<'a> { fn is_added(&self) -> bool { self.ticks .added - .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick) + .is_newer_than(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] fn is_changed(&self) -> bool { self.ticks .changed - .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick) + .is_newer_than(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] fn set_changed(&mut self) { - self.ticks.changed.set_changed(self.ticks.change_tick); + self.ticks + .changed + .set_changed(self.ticks.change_tick.into()); } #[inline] - fn last_changed(&self) -> u32 { + fn last_changed(&self) -> Tick { self.ticks.last_change_tick } #[inline] - fn set_last_changed(&mut self, last_change_tick: u32) { + fn set_last_changed(&mut self, last_change_tick: Tick) { self.ticks.last_change_tick = last_change_tick; } @@ -447,12 +704,16 @@ impl std::fmt::Debug for MutUntyped<'_> { #[cfg(test)] mod tests { + use std::num::NonZeroU64; + use bevy_ecs_macros::Resource; use crate::{ self as bevy_ecs, - change_detection::{Mut, NonSendMut, ResMut, Ticks, CHECK_TICK_THRESHOLD, MAX_CHANGE_AGE}, - component::{Component, ComponentTicks, Tick}, + change_detection::{ + Mut, NonSendMut, ResMut, SmallTick, Tick, Ticks, CHECK_TICK_THRESHOLD, MAX_CHANGE_AGE, + }, + component::{Component, ComponentTicks}, query::ChangeTrackers, system::{IntoSystem, Query, System}, world::World, @@ -488,37 +749,14 @@ mod tests { // The spawn will be detected since it happened after the system "last ran". assert!(change_detected_system.run((), &mut world)); - // world: 1 + MAX_CHANGE_AGE - let change_tick = world.change_tick.get_mut(); - *change_tick = change_tick.wrapping_add(MAX_CHANGE_AGE); - - // Both the system and component appeared `MAX_CHANGE_AGE` ticks ago. - // Since we clamp things to `MAX_CHANGE_AGE` for determinism, - // `ComponentTicks::is_changed` will now see `MAX_CHANGE_AGE > MAX_CHANGE_AGE` - // and return `false`. - assert!(!change_expired_system.run((), &mut world)); - } - - #[test] - fn change_tick_wraparound() { - fn change_detected(query: Query>) -> bool { - query.single().is_changed() - } - - let mut world = World::new(); - world.last_change_tick = u32::MAX; - *world.change_tick.get_mut() = 0; - - // component added: 0, changed: 0 - world.spawn(C); - - // system last ran: u32::MAX - let mut change_detected_system = IntoSystem::into_system(change_detected); - change_detected_system.initialize(&mut world); + // world: MAX_CHANGE_AGE + 1 + world.change_tick.skip(MAX_CHANGE_AGE as u64); - // Since the world is always ahead, as long as changes can't get older than `u32::MAX` (which we ensure), - // the wrapping difference will always be positive, so wraparound doesn't matter. - assert!(change_detected_system.run((), &mut world)); + // Both the system and component appeared `MAX_CHANGE_AGE + 1` ticks ago. + // Since we clamp component ticks to `MAX_CHANGE_AGE` for determinism, + // `ComponentTicks::is_changed` will now see `MAX_CHANGE_AGE + 1 > MAX_CHANGE_AGE` + // and return `true`. + assert!(change_expired_system.run((), &mut world)); } #[test] @@ -529,39 +767,37 @@ mod tests { world.spawn(C); // a bunch of stuff happens, the component is now older than `MAX_CHANGE_AGE` - *world.change_tick.get_mut() += MAX_CHANGE_AGE + CHECK_TICK_THRESHOLD; + world + .change_tick + .skip(MAX_CHANGE_AGE as u64 + CHECK_TICK_THRESHOLD as u64); let change_tick = world.change_tick(); let mut query = world.query::>(); for tracker in query.iter(&world) { - let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added.tick); - let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed.tick); - assert!(ticks_since_insert > MAX_CHANGE_AGE); - assert!(ticks_since_change > MAX_CHANGE_AGE); + assert!(tracker.component_ticks.added.needs_check(change_tick)); + assert!(tracker.component_ticks.changed.needs_check(change_tick)); } // scan change ticks and clamp those at risk of overflow world.check_change_ticks(); for tracker in query.iter(&world) { - let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added.tick); - let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed.tick); - assert!(ticks_since_insert == MAX_CHANGE_AGE); - assert!(ticks_since_change == MAX_CHANGE_AGE); + assert!(!tracker.component_ticks.added.needs_check(change_tick)); + assert!(!tracker.component_ticks.changed.needs_check(change_tick)); } } #[test] fn mut_from_res_mut() { let mut component_ticks = ComponentTicks { - added: Tick::new(1), - changed: Tick::new(2), + added: SmallTick(1), + changed: SmallTick(2), }; let ticks = Ticks { added: &mut component_ticks.added, changed: &mut component_ticks.changed, - last_change_tick: 3, - change_tick: 4, + last_change_tick: Tick(NonZeroU64::new(3).unwrap()), + change_tick: Tick(NonZeroU64::new(4).unwrap()), }; let mut res = R {}; let res_mut = ResMut { @@ -570,23 +806,23 @@ mod tests { }; let into_mut: Mut = res_mut.into(); - assert_eq!(1, into_mut.ticks.added.tick); - assert_eq!(2, into_mut.ticks.changed.tick); - assert_eq!(3, into_mut.ticks.last_change_tick); - assert_eq!(4, into_mut.ticks.change_tick); + assert_eq!(1, into_mut.ticks.added.0); + assert_eq!(2, into_mut.ticks.changed.0); + assert_eq!(3, into_mut.ticks.last_change_tick.0.get()); + assert_eq!(4, into_mut.ticks.change_tick.0.get()); } #[test] fn mut_from_non_send_mut() { let mut component_ticks = ComponentTicks { - added: Tick::new(1), - changed: Tick::new(2), + added: SmallTick(1), + changed: SmallTick(2), }; let ticks = Ticks { added: &mut component_ticks.added, changed: &mut component_ticks.changed, - last_change_tick: 3, - change_tick: 4, + last_change_tick: Tick(NonZeroU64::new(3).unwrap()), + change_tick: Tick(NonZeroU64::new(4).unwrap()), }; let mut res = R {}; let non_send_mut = NonSendMut { @@ -595,10 +831,10 @@ mod tests { }; let into_mut: Mut = non_send_mut.into(); - assert_eq!(1, into_mut.ticks.added.tick); - assert_eq!(2, into_mut.ticks.changed.tick); - assert_eq!(3, into_mut.ticks.last_change_tick); - assert_eq!(4, into_mut.ticks.change_tick); + assert_eq!(1, into_mut.ticks.added.0); + assert_eq!(2, into_mut.ticks.changed.0); + assert_eq!(3, into_mut.ticks.last_change_tick.0.get()); + assert_eq!(4, into_mut.ticks.change_tick.0.get()); } #[test] @@ -606,11 +842,14 @@ mod tests { use super::*; struct Outer(i64); - let (last_change_tick, change_tick) = (2, 3); let mut component_ticks = ComponentTicks { - added: Tick::new(1), - changed: Tick::new(2), + added: SmallTick(1), + changed: SmallTick(2), }; + let (last_change_tick, change_tick) = ( + Tick(NonZeroU64::new(2).unwrap()), + Tick(NonZeroU64::new(3).unwrap()), + ); let ticks = Ticks { added: &mut component_ticks.added, changed: &mut component_ticks.changed, @@ -636,3 +875,73 @@ mod tests { assert!(component_ticks.is_changed(last_change_tick, change_tick)); } } + +#[test] +fn test_later() { + let counter = TickCounter::new(); + let target = counter.next(); + let small_tick = SmallTick::from(counter.next()); + + assert!(small_tick.is_newer_than(target, counter.current())); +} + +#[test] +fn test_earlier() { + let counter = TickCounter::new(); + let small_tick = SmallTick::from(counter.next()); + let target = counter.next(); + + assert!(!small_tick.is_newer_than(target, counter.current())); +} + +#[test] +fn old_tick() { + let mut counter = TickCounter::new(); + let mut small_tick = SmallTick::from(counter.next()); + + counter.skip(MAX_CHANGE_AGE.into()); + + if counter.maintain() { + small_tick.check_tick(counter.current_mut()); + } + + let target = counter.next(); + + // Old small tick age is bumped to `MAX_CHANGE_AGE` + // which should not cause false positive for fresh enough targets. + assert!(!small_tick.is_newer_than(target, counter.current())); +} + +#[test] +fn old_tick_false_positive() { + let mut counter = TickCounter::new(); + let mut small_tick = SmallTick::from(counter.next()); + let target = counter.next(); + + counter.skip(MAX_CHANGE_AGE.into()); + counter.skip(1); + + if counter.maintain() { + small_tick.check_tick(counter.current_mut()); + } + + // Old small tick age is bumped to `MAX_CHANGE_AGE` + // and target older than this threshold causes false positive here. + assert!(small_tick.is_newer_than(target, counter.current())); +} + +#[test] +fn old_target() { + let mut counter = TickCounter::new(); + let target = counter.next(); + + counter.skip(MAX_CHANGE_AGE.into()); + counter.skip(1); + + counter.maintain(); + + let small_tick = SmallTick::from(counter.next()); + + // Old target would never be considered newer than small tick. + assert!(small_tick.is_newer_than(target, counter.current())); +} diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 442fc60ae03a7..ce5a90375482c 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1,7 +1,7 @@ //! Types for declaring and storing [`Component`]s. use crate::{ - change_detection::MAX_CHANGE_AGE, + change_detection::{SmallTick, Tick}, storage::{SparseSetIndex, Storages}, system::Resource, }; @@ -518,67 +518,11 @@ impl Components { } } -/// Used to track changes in state between system runs, e.g. components being added or accessed mutably. -#[derive(Copy, Clone, Debug)] -pub struct Tick { - pub(crate) tick: u32, -} - -impl Tick { - pub const fn new(tick: u32) -> Self { - Self { tick } - } - - #[inline] - /// Returns `true` if the tick is older than the system last's run. - pub fn is_older_than(&self, last_change_tick: u32, change_tick: u32) -> bool { - // This works even with wraparound because the world tick (`change_tick`) is always "newer" than - // `last_change_tick` and `self.tick`, and we scan periodically to clamp `ComponentTicks` values - // so they never get older than `u32::MAX` (the difference would overflow). - // - // The clamp here ensures determinism (since scans could differ between app runs). - let ticks_since_insert = change_tick.wrapping_sub(self.tick).min(MAX_CHANGE_AGE); - let ticks_since_system = change_tick - .wrapping_sub(last_change_tick) - .min(MAX_CHANGE_AGE); - - ticks_since_system > ticks_since_insert - } - - pub(crate) fn check_tick(&mut self, change_tick: u32) { - let age = change_tick.wrapping_sub(self.tick); - // This comparison assumes that `age` has not overflowed `u32::MAX` before, which will be true - // so long as this check always runs before that can happen. - if age > MAX_CHANGE_AGE { - self.tick = change_tick.wrapping_sub(MAX_CHANGE_AGE); - } - } - - /// Manually sets the change tick. - /// - /// This is normally done automatically via the [`DerefMut`](std::ops::DerefMut) implementation - /// on [`Mut`](crate::change_detection::Mut), [`ResMut`](crate::change_detection::ResMut), etc. - /// However, components and resources that make use of interior mutability might require manual updates. - /// - /// # Example - /// ```rust,no_run - /// # use bevy_ecs::{world::World, component::ComponentTicks}; - /// let world: World = unimplemented!(); - /// let component_ticks: ComponentTicks = unimplemented!(); - /// - /// component_ticks.set_changed(world.read_change_tick()); - /// ``` - #[inline] - pub fn set_changed(&mut self, change_tick: u32) { - self.tick = change_tick; - } -} - /// Wrapper around [`Tick`]s for a single component #[derive(Copy, Clone, Debug)] pub struct TickCells<'a> { - pub added: &'a UnsafeCell, - pub changed: &'a UnsafeCell, + pub added: &'a UnsafeCell, + pub changed: &'a UnsafeCell, } impl<'a> TickCells<'a> { @@ -596,27 +540,27 @@ impl<'a> TickCells<'a> { /// Records when a component was added and when it was last mutably dereferenced (or added). #[derive(Copy, Clone, Debug)] pub struct ComponentTicks { - pub(crate) added: Tick, - pub(crate) changed: Tick, + pub(crate) added: SmallTick, + pub(crate) changed: SmallTick, } impl ComponentTicks { #[inline] /// Returns `true` if the component was added after the system last ran. - pub fn is_added(&self, last_change_tick: u32, change_tick: u32) -> bool { - self.added.is_older_than(last_change_tick, change_tick) + pub fn is_added(&self, last_change_tick: Tick, change_tick: Tick) -> bool { + self.added.is_newer_than(last_change_tick, change_tick) } #[inline] /// Returns `true` if the component was added or mutably dereferenced after the system last ran. - pub fn is_changed(&self, last_change_tick: u32, change_tick: u32) -> bool { - self.changed.is_older_than(last_change_tick, change_tick) + pub fn is_changed(&self, last_change_tick: Tick, change_tick: Tick) -> bool { + self.changed.is_newer_than(last_change_tick, change_tick) } - pub(crate) fn new(change_tick: u32) -> Self { + pub(crate) fn new(change_tick: SmallTick) -> Self { Self { - added: Tick::new(change_tick), - changed: Tick::new(change_tick), + added: change_tick, + changed: change_tick, } } @@ -635,7 +579,7 @@ impl ComponentTicks { /// component_ticks.set_changed(world.read_change_tick()); /// ``` #[inline] - pub fn set_changed(&mut self, change_tick: u32) { + pub fn set_changed(&mut self, change_tick: Tick) { self.changed.set_changed(change_tick); } } diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index a91ed6634f1b6..bae920abd0059 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -28,7 +28,7 @@ pub mod prelude { #[doc(hidden)] pub use crate::{ bundle::Bundle, - change_detection::DetectChanges, + change_detection::{DetectChanges, Tick}, component::Component, entity::Entity, event::{EventReader, EventWriter, Events}, diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 9e62594e3c44e..0b6c2c74c0cdf 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1,7 +1,7 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, - change_detection::Ticks, - component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType, Tick}, + change_detection::{SmallTick, Tick, Ticks}, + component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType}, entity::Entity, query::{Access, DebugCheckedUnwrap, FilteredAccess}, storage::{ComponentSparseSet, Table}, @@ -333,8 +333,8 @@ pub unsafe trait WorldQuery { unsafe fn init_fetch<'w>( world: &'w World, state: &Self::State, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Self::Fetch<'w>; /// While this function can be called for any query, it is always safe to call if `Self: ReadOnlyWorldQuery` holds. @@ -460,8 +460,8 @@ unsafe impl WorldQuery for Entity { unsafe fn init_fetch<'w>( _world: &'w World, _state: &Self::State, - _last_change_tick: u32, - _change_tick: u32, + _last_change_tick: Tick, + _change_tick: Tick, ) -> Self::Fetch<'w> { } @@ -542,8 +542,8 @@ unsafe impl WorldQuery for &T { unsafe fn init_fetch<'w>( world: &'w World, &component_id: &ComponentId, - _last_change_tick: u32, - _change_tick: u32, + _last_change_tick: Tick, + _change_tick: Tick, ) -> ReadFetch<'w, T> { ReadFetch { table_components: None, @@ -654,14 +654,14 @@ pub struct WriteFetch<'w, T> { // T::Storage = TableStorage table_data: Option<( ThinSlicePtr<'w, UnsafeCell>, - ThinSlicePtr<'w, UnsafeCell>, - ThinSlicePtr<'w, UnsafeCell>, + ThinSlicePtr<'w, UnsafeCell>, + ThinSlicePtr<'w, UnsafeCell>, )>, // T::Storage = SparseStorage sparse_set: Option<&'w ComponentSparseSet>, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, } /// SAFETY: access of `&T` is a subset of `&mut T` @@ -687,8 +687,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { unsafe fn init_fetch<'w>( world: &'w World, &component_id: &ComponentId, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> WriteFetch<'w, T> { WriteFetch { table_data: None, @@ -831,8 +831,8 @@ unsafe impl WorldQuery for Option { unsafe fn init_fetch<'w>( world: &'w World, state: &T::State, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> OptionFetch<'w, T> { OptionFetch { fetch: T::init_fetch(world, state, last_change_tick, change_tick), @@ -948,8 +948,8 @@ unsafe impl ReadOnlyWorldQuery for Option {} /// ``` pub struct ChangeTrackers { pub(crate) component_ticks: ComponentTicks, - pub(crate) last_change_tick: u32, - pub(crate) change_tick: u32, + pub(crate) last_change_tick: Tick, + pub(crate) change_tick: Tick, marker: PhantomData, } @@ -992,14 +992,14 @@ impl ChangeTrackers { #[doc(hidden)] pub struct ChangeTrackersFetch<'w, T> { // T::Storage = TableStorage - table_added: Option>>, - table_changed: Option>>, + table_added: Option>>, + table_changed: Option>>, // T::Storage = SparseStorage sparse_set: Option<&'w ComponentSparseSet>, marker: PhantomData, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, } // SAFETY: `ROQueryFetch` is the same as `QueryFetch` @@ -1025,8 +1025,8 @@ unsafe impl WorldQuery for ChangeTrackers { unsafe fn init_fetch<'w>( world: &'w World, &component_id: &ComponentId, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> ChangeTrackersFetch<'w, T> { ChangeTrackersFetch { table_added: None, @@ -1170,7 +1170,7 @@ macro_rules! impl_tuple_fetch { } #[allow(clippy::unused_unit)] - unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self::Fetch<'w> { + unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_change_tick: Tick, _change_tick: Tick) -> Self::Fetch<'w> { let ($($name,)*) = state; ($($name::init_fetch(_world, $name, _last_change_tick, _change_tick),)*) } @@ -1279,7 +1279,7 @@ macro_rules! impl_anytuple_fetch { } #[allow(clippy::unused_unit)] - unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self::Fetch<'w> { + unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_change_tick: Tick, _change_tick: Tick) -> Self::Fetch<'w> { let ($($name,)*) = state; ($(($name::init_fetch(_world, $name, _last_change_tick, _change_tick), false),)*) } @@ -1420,8 +1420,8 @@ unsafe impl WorldQuery for NopWorldQuery { unsafe fn init_fetch( _world: &World, _state: &Q::State, - _last_change_tick: u32, - _change_tick: u32, + _last_change_tick: Tick, + _change_tick: Tick, ) { } diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index a84d74f8075e3..05348882f2362 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -1,6 +1,7 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, - component::{Component, ComponentId, ComponentStorage, StorageType, Tick}, + change_detection::{SmallTick, Tick}, + component::{Component, ComponentId, ComponentStorage, StorageType}, entity::Entity, query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery}, storage::{Column, ComponentSparseSet, Table}, @@ -53,8 +54,8 @@ unsafe impl WorldQuery for With { unsafe fn init_fetch( _world: &World, _state: &ComponentId, - _last_change_tick: u32, - _change_tick: u32, + _last_change_tick: Tick, + _change_tick: Tick, ) { } @@ -155,8 +156,8 @@ unsafe impl WorldQuery for Without { unsafe fn init_fetch( _world: &World, _state: &ComponentId, - _last_change_tick: u32, - _change_tick: u32, + _last_change_tick: Tick, + _change_tick: Tick, ) { } @@ -277,7 +278,7 @@ macro_rules! impl_query_filter_tuple { const IS_ARCHETYPAL: bool = true $(&& $filter::IS_ARCHETYPAL)*; - unsafe fn init_fetch<'w>(world: &'w World, state: &Self::State, last_change_tick: u32, change_tick: u32) -> Self::Fetch<'w> { + unsafe fn init_fetch<'w>(world: &'w World, state: &Self::State, last_change_tick: Tick, change_tick: Tick) -> Self::Fetch<'w> { let ($($filter,)*) = state; ($(OrFetch { fetch: $filter::init_fetch(world, $filter, last_change_tick, change_tick), @@ -414,11 +415,11 @@ macro_rules! impl_tick_filter { #[doc(hidden)] $(#[$fetch_meta])* pub struct $fetch_name<'w, T> { - table_ticks: Option< ThinSlicePtr<'w, UnsafeCell>>, + table_ticks: Option< ThinSlicePtr<'w, UnsafeCell>>, marker: PhantomData, sparse_set: Option<&'w ComponentSparseSet>, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, } // SAFETY: `Self::ReadOnly` is the same as `Self` @@ -432,7 +433,7 @@ macro_rules! impl_tick_filter { item } - unsafe fn init_fetch<'w>(world: &'w World, &id: &ComponentId, last_change_tick: u32, change_tick: u32) -> Self::Fetch<'w> { + unsafe fn init_fetch<'w>(world: &'w World, &id: &ComponentId, last_change_tick: Tick, change_tick: Tick) -> Self::Fetch<'w> { Self::Fetch::<'w> { table_ticks: None, sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet) @@ -509,7 +510,7 @@ macro_rules! impl_tick_filter { .debug_checked_unwrap() .get(table_row) .deref() - .is_older_than(fetch.last_change_tick, fetch.change_tick) + .is_newer_than(fetch.last_change_tick, fetch.change_tick) } StorageType::SparseSet => { let sparse_set = &fetch @@ -518,7 +519,7 @@ macro_rules! impl_tick_filter { $get_sparse_set(sparse_set, entity) .debug_checked_unwrap() .deref() - .is_older_than(fetch.last_change_tick, fetch.change_tick) + .is_newer_than(fetch.last_change_tick, fetch.change_tick) } } } diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 0be5022085013..9638292a1b012 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -1,5 +1,6 @@ use crate::{ archetype::{ArchetypeEntity, ArchetypeId, Archetypes}, + change_detection::Tick, entity::{Entities, Entity}, prelude::World, query::{ArchetypeFilter, DebugCheckedUnwrap, QueryState, WorldQuery}, @@ -29,8 +30,8 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIter<'w, 's, Q, F> { pub(crate) unsafe fn new( world: &'w World, query_state: &'s QueryState, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Self { QueryIter { query_state, @@ -98,8 +99,8 @@ where world: &'w World, query_state: &'s QueryState, entity_list: EntityList, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> QueryManyIter<'w, 's, Q, F, I> { let fetch = Q::init_fetch( world, @@ -299,8 +300,8 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, const K: usize> pub(crate) unsafe fn new( world: &'w World, query_state: &'s QueryState, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Self { // Initialize array with cursors. // There is no FromIterator on arrays, so instead initialize it manually with MaybeUninit @@ -497,8 +498,8 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's, unsafe fn init_empty( world: &'w World, query_state: &'s QueryState, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Self { QueryIterationCursor { table_id_iter: [].iter(), @@ -510,8 +511,8 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's, unsafe fn init( world: &'w World, query_state: &'s QueryState, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Self { let fetch = Q::init_fetch( world, diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 245ff7dacb7b4..037ef2dfd925e 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1,5 +1,6 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId}, + change_detection::Tick, component::ComponentId, entity::Entity, prelude::FromWorld, @@ -129,7 +130,7 @@ impl QueryState { /// Checks if the query is empty for the given [`World`], where the last change and current tick are given. #[inline] - pub fn is_empty(&self, world: &World, last_change_tick: u32, change_tick: u32) -> bool { + pub fn is_empty(&self, world: &World, last_change_tick: Tick, change_tick: Tick) -> bool { // SAFETY: NopFetch does not access any members while &self ensures no one has exclusive access unsafe { self.as_nop() @@ -398,8 +399,8 @@ impl QueryState { &self, world: &'w World, entity: Entity, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Result, QueryEntityError> { let location = world .entities @@ -445,8 +446,8 @@ impl QueryState { &self, world: &'w World, entities: [Entity; N], - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Result<[ROQueryItem<'w, Q>; N], QueryEntityError> { let mut values = [(); N].map(|_| MaybeUninit::uninit()); @@ -480,8 +481,8 @@ impl QueryState { &self, world: &'w World, entities: [Entity; N], - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Result<[Q::Item<'w>; N], QueryEntityError> { // Verify that all entities are unique for i in 0..N { @@ -528,7 +529,8 @@ impl QueryState { // SAFETY: query has unique world access unsafe { self.update_archetypes(world); - self.iter_unchecked_manual(world, world.last_change_tick(), world.read_change_tick()) + let change_tick = world.change_tick(); + self.iter_unchecked_manual(world, world.last_change_tick(), change_tick) } } @@ -725,8 +727,8 @@ impl QueryState { pub(crate) unsafe fn iter_unchecked_manual<'w, 's>( &'s self, world: &'w World, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> QueryIter<'w, 's, Q, F> { QueryIter::new(world, self, last_change_tick, change_tick) } @@ -746,8 +748,8 @@ impl QueryState { &'s self, entities: EntityList, world: &'w World, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> QueryManyIter<'w, 's, Q, F, EntityList::IntoIter> where EntityList::Item: Borrow, @@ -769,8 +771,8 @@ impl QueryState { pub(crate) unsafe fn iter_combinations_unchecked_manual<'w, 's, const K: usize>( &'s self, world: &'w World, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> QueryCombinationIter<'w, 's, Q, F, K> { QueryCombinationIter::new(world, self, last_change_tick, change_tick) } @@ -929,8 +931,8 @@ impl QueryState { &self, world: &'w World, mut func: FN, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) { // NOTE: If you are changing query iteration code, remember to update the following places, where relevant: // QueryIter, QueryIterationCursor, QueryManyIter, QueryCombinationIter, QueryState::for_each_unchecked_manual, QueryState::par_for_each_unchecked_manual @@ -1003,8 +1005,8 @@ impl QueryState { world: &'w World, batch_size: usize, func: FN, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) { // NOTE: If you are changing query iteration code, remember to update the following places, where relevant: // QueryIter, QueryIterationCursor, QueryManyIter, QueryCombinationIter, QueryState::for_each_unchecked_manual, QueryState::par_for_each_unchecked_manual @@ -1237,8 +1239,8 @@ impl QueryState { pub unsafe fn get_single_unchecked_manual<'w>( &self, world: &'w World, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Result, QuerySingleError> { let mut query = self.iter_unchecked_manual(world, last_change_tick, change_tick); let first = query.next(); diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 238743965d7ef..6fad133b560f3 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -518,28 +518,7 @@ impl SystemStage { /// During each scan, any change ticks older than [`MAX_CHANGE_AGE`](crate::change_detection::MAX_CHANGE_AGE) /// are clamped to that age. This prevents false positives from appearing due to overflow. fn check_change_ticks(&mut self, world: &mut World) { - let change_tick = world.change_tick(); - let ticks_since_last_check = change_tick.wrapping_sub(self.last_tick_check); - - if ticks_since_last_check >= CHECK_TICK_THRESHOLD { - // 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 all component change ticks. - world.check_change_ticks(); - self.last_tick_check = change_tick; - } + world.check_change_ticks(); } /// Sorts run criteria and populates resolved input-criteria for piping. diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 2d852ba73e84c..c5837576af7f9 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -1,4 +1,5 @@ use crate::archetype::ArchetypeComponentId; +use crate::change_detection::{SmallTick, Tick}; use crate::component::{ComponentId, ComponentTicks, Components, TickCells}; use crate::storage::{Column, SparseSet}; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; @@ -52,7 +53,7 @@ impl ResourceData { /// /// [`World::validate_non_send_access_untyped`]: crate::world::World::validate_non_send_access_untyped #[inline] - pub(crate) unsafe fn insert(&mut self, value: OwningPtr<'_>, change_tick: u32) { + pub(crate) unsafe fn insert(&mut self, value: OwningPtr<'_>, change_tick: SmallTick) { if self.is_present() { self.column.replace(0, value, change_tick); } else { @@ -176,7 +177,7 @@ impl Resources { }) } - pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { + pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) { for info in self.resources.values_mut() { info.column.check_change_ticks(change_tick); } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 810a4fe569da2..cbed8c0cda24e 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -1,5 +1,6 @@ use crate::{ - component::{ComponentId, ComponentInfo, ComponentTicks, Tick, TickCells}, + change_detection::{SmallTick, Tick}, + component::{ComponentId, ComponentInfo, ComponentTicks, TickCells}, entity::Entity, storage::Column, }; @@ -143,7 +144,12 @@ impl ComponentSparseSet { /// # Safety /// The `value` pointer must point to a valid address that matches the [`Layout`](std::alloc::Layout) /// inside the [`ComponentInfo`] given when constructing this sparse set. - pub(crate) unsafe fn insert(&mut self, entity: Entity, value: OwningPtr<'_>, change_tick: u32) { + pub(crate) unsafe fn insert( + &mut self, + entity: Entity, + value: OwningPtr<'_>, + change_tick: SmallTick, + ) { if let Some(&dense_index) = self.sparse.get(entity.index()) { #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index as usize]); @@ -206,7 +212,7 @@ impl ComponentSparseSet { } #[inline] - pub fn get_added_ticks(&self, entity: Entity) -> Option<&UnsafeCell> { + pub fn get_added_ticks(&self, entity: Entity) -> Option<&UnsafeCell> { let dense_index = *self.sparse.get(entity.index())? as usize; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index]); @@ -215,7 +221,7 @@ impl ComponentSparseSet { } #[inline] - pub fn get_changed_ticks(&self, entity: Entity) -> Option<&UnsafeCell> { + pub fn get_changed_ticks(&self, entity: Entity) -> Option<&UnsafeCell> { let dense_index = *self.sparse.get(entity.index())? as usize; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index]); @@ -279,7 +285,7 @@ impl ComponentSparseSet { } } - pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { + pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) { self.dense.check_change_ticks(change_tick); } } @@ -502,7 +508,7 @@ impl SparseSets { } } - pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { + pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) { for set in self.sets.values_mut() { 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 9cd773af58d90..10259a5030795 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -1,5 +1,6 @@ use crate::{ - component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick, TickCells}, + change_detection::{SmallTick, Tick}, + component::{ComponentId, ComponentInfo, ComponentTicks, Components, TickCells}, entity::Entity, query::DebugCheckedUnwrap, storage::{blob_vec::BlobVec, ImmutableSparseSet, SparseSet}, @@ -35,8 +36,8 @@ impl TableId { #[derive(Debug)] pub struct Column { data: BlobVec, - added_ticks: Vec>, - changed_ticks: Vec>, + added_ticks: Vec>, + changed_ticks: Vec>, } impl Column { @@ -62,7 +63,7 @@ impl Column { /// # Safety /// Assumes data has already been allocated for the given row. #[inline] - pub(crate) unsafe fn initialize(&mut self, row: usize, data: OwningPtr<'_>, tick: Tick) { + pub(crate) unsafe fn initialize(&mut self, row: usize, data: OwningPtr<'_>, tick: SmallTick) { debug_assert!(row < self.len()); self.data.initialize_unchecked(row, data); *self.added_ticks.get_unchecked_mut(row).get_mut() = tick; @@ -75,13 +76,15 @@ impl Column { /// # Safety /// Assumes data has already been allocated for the given row. #[inline] - pub(crate) unsafe fn replace(&mut self, row: usize, data: OwningPtr<'_>, change_tick: u32) { + pub(crate) unsafe fn replace( + &mut self, + row: usize, + data: OwningPtr<'_>, + change_tick: SmallTick, + ) { debug_assert!(row < self.len()); self.data.replace_unchecked(row, data); - self.changed_ticks - .get_unchecked_mut(row) - .get_mut() - .set_changed(change_tick); + *self.changed_ticks.get_unchecked_mut(row).get_mut() = change_tick; } /// Writes component data to the column at given row. @@ -196,12 +199,12 @@ impl Column { } #[inline] - pub fn get_added_ticks_slice(&self) -> &[UnsafeCell] { + pub fn get_added_ticks_slice(&self) -> &[UnsafeCell] { &self.added_ticks } #[inline] - pub fn get_changed_ticks_slice(&self) -> &[UnsafeCell] { + pub fn get_changed_ticks_slice(&self) -> &[UnsafeCell] { &self.changed_ticks } @@ -254,12 +257,12 @@ impl Column { } #[inline] - pub fn get_added_ticks(&self, row: usize) -> Option<&UnsafeCell> { + pub fn get_added_ticks(&self, row: usize) -> Option<&UnsafeCell> { self.added_ticks.get(row) } #[inline] - pub fn get_changed_ticks(&self, row: usize) -> Option<&UnsafeCell> { + pub fn get_changed_ticks(&self, row: usize) -> Option<&UnsafeCell> { self.changed_ticks.get(row) } @@ -276,7 +279,7 @@ impl Column { /// # Safety /// index must be in-bounds #[inline] - pub unsafe fn get_added_ticks_unchecked(&self, row: usize) -> &UnsafeCell { + pub unsafe fn get_added_ticks_unchecked(&self, row: usize) -> &UnsafeCell { debug_assert!(row < self.added_ticks.len()); self.added_ticks.get_unchecked(row) } @@ -284,7 +287,7 @@ impl Column { /// # Safety /// index must be in-bounds #[inline] - pub unsafe fn get_changed_ticks_unchecked(&self, row: usize) -> &UnsafeCell { + pub unsafe fn get_changed_ticks_unchecked(&self, row: usize) -> &UnsafeCell { debug_assert!(row < self.changed_ticks.len()); self.changed_ticks.get_unchecked(row) } @@ -308,7 +311,7 @@ impl Column { } #[inline] - pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { + pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) { for component_ticks in &mut self.added_ticks { component_ticks.get_mut().check_tick(change_tick); } @@ -530,8 +533,8 @@ impl Table { self.entities.push(entity); for column in self.columns.values_mut() { column.data.set_len(self.entities.len()); - column.added_ticks.push(UnsafeCell::new(Tick::new(0))); - column.changed_ticks.push(UnsafeCell::new(Tick::new(0))); + column.added_ticks.push(UnsafeCell::new(SmallTick::new())); + column.changed_ticks.push(UnsafeCell::new(SmallTick::new())); } index } @@ -556,7 +559,7 @@ impl Table { self.entities.is_empty() } - pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { + pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) { for column in self.columns.values_mut() { column.check_change_ticks(change_tick); } @@ -658,7 +661,7 @@ impl Tables { } } - pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { + pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) { for table in &mut self.tables { table.check_change_ticks(change_tick); } @@ -684,14 +687,11 @@ impl IndexMut for Tables { #[cfg(test)] mod tests { use crate as bevy_ecs; + use crate::change_detection::SmallTick; use crate::component::Component; use crate::ptr::OwningPtr; use crate::storage::Storages; - use crate::{ - component::{Components, Tick}, - entity::Entity, - storage::TableBuilder, - }; + use crate::{component::Components, entity::Entity, storage::TableBuilder}; #[derive(Component)] struct W(T); @@ -714,7 +714,7 @@ mod tests { table.get_column_mut(component_id).unwrap().initialize( row, value_ptr, - Tick::new(0), + SmallTick::new(), ); }); }; diff --git a/crates/bevy_ecs/src/system/commands/parallel_scope.rs b/crates/bevy_ecs/src/system/commands/parallel_scope.rs index 573afa70aae51..d45e7c70c4a77 100644 --- a/crates/bevy_ecs/src/system/commands/parallel_scope.rs +++ b/crates/bevy_ecs/src/system/commands/parallel_scope.rs @@ -3,6 +3,7 @@ use std::cell::Cell; use thread_local::ThreadLocal; use crate::{ + change_detection::Tick, entity::Entities, prelude::World, system::{SystemParam, SystemParamFetch, SystemParamState}, @@ -59,7 +60,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for ParallelCommandsState { state: &'s mut Self, _: &crate::system::SystemMeta, world: &'w World, - _: u32, + _: Tick, ) -> Self::Item { ParallelCommands { state, diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs index 662d031daa2e3..696004a06b6b7 100644 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_function_system.rs @@ -1,13 +1,12 @@ use crate::{ archetype::ArchetypeComponentId, - change_detection::MAX_CHANGE_AGE, + change_detection::Tick, component::ComponentId, query::Access, schedule::{SystemLabel, SystemLabelId}, system::{ - check_system_change_tick, AsSystemLabel, ExclusiveSystemParam, ExclusiveSystemParamFetch, - ExclusiveSystemParamItem, ExclusiveSystemParamState, IntoSystem, System, SystemMeta, - SystemTypeIdLabel, + AsSystemLabel, ExclusiveSystemParam, ExclusiveSystemParamFetch, ExclusiveSystemParamItem, + ExclusiveSystemParamState, IntoSystem, System, SystemMeta, SystemTypeIdLabel, }, world::{World, WorldId}, }; @@ -101,9 +100,8 @@ where ); self.func.run(world, params); - let change_tick = world.change_tick.get_mut(); - self.system_meta.last_change_tick = *change_tick; - *change_tick += 1; + self.system_meta.last_change_tick = world.change_tick.current_mut(); + world.change_tick.next(); world.last_change_tick = saved_last_tick; } @@ -112,11 +110,11 @@ where true } - fn get_last_change_tick(&self) -> u32 { + fn get_last_change_tick(&self) -> Tick { self.system_meta.last_change_tick } - fn set_last_change_tick(&mut self, last_change_tick: u32) { + fn set_last_change_tick(&mut self, last_change_tick: Tick) { self.system_meta.last_change_tick = last_change_tick; } @@ -129,7 +127,7 @@ where #[inline] fn initialize(&mut self, world: &mut World) { self.world_id = Some(world.id()); - self.system_meta.last_change_tick = world.change_tick().wrapping_sub(MAX_CHANGE_AGE); + self.system_meta.last_change_tick = Tick::new(); self.param_state = Some(::init( world, &mut self.system_meta, @@ -138,14 +136,6 @@ where fn update_archetype_component_access(&mut self, _world: &World) {} - #[inline] - fn check_change_tick(&mut self, change_tick: u32) { - check_system_change_tick( - &mut self.system_meta.last_change_tick, - change_tick, - self.system_meta.name.as_ref(), - ); - } fn default_labels(&self) -> Vec { vec![self.func.as_system_label().as_label()] } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index c3f672e718734..cddaf259eef3e 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -1,13 +1,13 @@ use crate::{ archetype::{ArchetypeComponentId, ArchetypeGeneration, ArchetypeId}, - change_detection::MAX_CHANGE_AGE, + change_detection::Tick, component::ComponentId, prelude::FromWorld, query::{Access, FilteredAccessSet}, schedule::{SystemLabel, SystemLabelId}, system::{ - check_system_change_tick, ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch, - SystemParamItem, SystemParamState, + ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch, SystemParamItem, + SystemParamState, }, world::{World, WorldId}, }; @@ -23,7 +23,7 @@ pub struct SystemMeta { // NOTE: this must be kept private. making a SystemMeta non-send is irreversible to prevent // SystemParams from overriding each other is_send: bool, - pub(crate) last_change_tick: u32, + pub(crate) last_change_tick: Tick, } impl SystemMeta { @@ -33,7 +33,7 @@ impl SystemMeta { archetype_component_access: Access::default(), component_access_set: FilteredAccessSet::default(), is_send: true, - last_change_tick: 0, + last_change_tick: Tick::new(), } } @@ -143,7 +143,6 @@ pub struct SystemState { impl SystemState { pub fn new(world: &mut World) -> Self { let mut meta = SystemMeta::new::(); - meta.last_change_tick = world.change_tick().wrapping_sub(MAX_CHANGE_AGE); let param_state = ::init(world, &mut meta); Self { meta, @@ -411,11 +410,11 @@ where out } - fn get_last_change_tick(&self) -> u32 { + fn get_last_change_tick(&self) -> Tick { self.system_meta.last_change_tick } - fn set_last_change_tick(&mut self, last_change_tick: u32) { + fn set_last_change_tick(&mut self, last_change_tick: Tick) { self.system_meta.last_change_tick = last_change_tick; } @@ -428,7 +427,7 @@ where #[inline] fn initialize(&mut self, world: &mut World) { self.world_id = Some(world.id()); - self.system_meta.last_change_tick = world.change_tick().wrapping_sub(MAX_CHANGE_AGE); + self.system_meta.last_change_tick = Tick::new(); self.param_state = Some(::init( world, &mut self.system_meta, @@ -450,14 +449,6 @@ where } } - #[inline] - fn check_change_tick(&mut self, change_tick: u32) { - check_system_change_tick( - &mut self.system_meta.last_change_tick, - change_tick, - self.system_meta.name.as_ref(), - ); - } fn default_labels(&self) -> Vec { vec![self.func.as_system_label().as_label()] } diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 8f7e4f70bc303..fb699a18351b1 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1,4 +1,5 @@ use crate::{ + change_detection::Tick, component::Component, entity::Entity, query::{ @@ -276,8 +277,8 @@ use std::{any::TypeId, borrow::Borrow, fmt::Debug}; pub struct Query<'world, 'state, Q: WorldQuery, F: ReadOnlyWorldQuery = ()> { pub(crate) world: &'world World, pub(crate) state: &'state QueryState, - pub(crate) last_change_tick: u32, - pub(crate) change_tick: u32, + pub(crate) last_change_tick: Tick, + pub(crate) change_tick: Tick, // SAFETY: This is used to ensure that `get_component_mut::` properly fails when a Query writes C // and gets converted to a read-only query using `to_readonly`. Without checking this, `get_component_mut` relies on // QueryState's archetype_component_access, which will continue allowing write access to C after being cast to @@ -303,8 +304,8 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { pub(crate) unsafe fn new( world: &'w World, state: &'s QueryState, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Self { Self { force_read_only_component_access: false, diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index 43fcdb41dd3cb..2553171a05a11 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -1,9 +1,8 @@ -use bevy_utils::tracing::warn; use core::fmt::Debug; use crate::{ - archetype::ArchetypeComponentId, change_detection::MAX_CHANGE_AGE, component::ComponentId, - query::Access, schedule::SystemLabelId, world::World, + archetype::ArchetypeComponentId, change_detection::Tick, component::ComponentId, query::Access, + schedule::SystemLabelId, world::World, }; use std::borrow::Cow; @@ -59,44 +58,24 @@ pub trait System: Send + Sync + 'static { fn initialize(&mut self, _world: &mut World); /// Update the system's archetype component [`Access`]. fn update_archetype_component_access(&mut self, world: &World); - fn check_change_tick(&mut self, change_tick: u32); + /// The default labels for the system fn default_labels(&self) -> Vec { Vec::new() } /// Gets the system's last change tick - fn get_last_change_tick(&self) -> u32; + fn get_last_change_tick(&self) -> Tick; /// Sets the system's last change tick /// # Warning /// This is a complex and error-prone operation, that can have unexpected consequences on any system relying on this code. /// However, it can be an essential escape hatch when, for example, /// you are trying to synchronize representations using change detection and need to avoid infinite recursion. - fn set_last_change_tick(&mut self, last_change_tick: u32); + fn set_last_change_tick(&mut self, last_change_tick: Tick); } /// A convenience type alias for a boxed [`System`] trait object. pub type BoxedSystem = Box>; -pub(crate) fn check_system_change_tick( - last_change_tick: &mut u32, - change_tick: u32, - system_name: &str, -) { - let age = change_tick.wrapping_sub(*last_change_tick); - // This comparison assumes that `age` has not overflowed `u32::MAX` before, which will be true - // so long as this check always runs before that can happen. - if age > MAX_CHANGE_AGE { - warn!( - "System '{}' has not run for {} ticks. \ - Changes older than {} ticks will not be detected.", - system_name, - age, - MAX_CHANGE_AGE - 1, - ); - *last_change_tick = change_tick.wrapping_sub(MAX_CHANGE_AGE); - } -} - impl Debug for dyn System { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "System {}: {{{}}}", self.name(), { diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 4e98e422879cb..2dd53e181deb1 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -2,8 +2,8 @@ pub use crate::change_detection::{NonSendMut, ResMut}; use crate::{ archetype::{Archetype, Archetypes}, bundle::Bundles, - change_detection::Ticks, - component::{Component, ComponentId, ComponentTicks, Components, Tick}, + change_detection::{SmallTick, Tick, Ticks}, + component::{Component, ComponentId, ComponentTicks, Components}, entity::{Entities, Entity}, query::{ Access, FilteredAccess, FilteredAccessSet, QueryState, ReadOnlyWorldQuery, WorldQuery, @@ -131,7 +131,7 @@ pub trait SystemParamFetch<'world, 'state>: SystemParamState { state: &'state mut Self, system_meta: &SystemMeta, world: &'world World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item; } @@ -189,7 +189,7 @@ impl<'w, 's, Q: WorldQuery + 'static, F: ReadOnlyWorldQuery + 'static> SystemPar state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { Query::new(world, state, system_meta.last_change_tick, change_tick) } @@ -220,7 +220,7 @@ pub struct ParamSet<'w, 's, T: SystemParam> { param_states: &'s mut T::Fetch, world: &'w World, system_meta: SystemMeta, - change_tick: u32, + change_tick: Tick, } /// The [`SystemParamState`] of [`ParamSet`]. pub struct ParamSetState SystemParamFetch<'w, 's>>(T); @@ -273,10 +273,10 @@ pub trait Resource: Send + Sync + 'static {} /// Use `Option>` instead if the resource might not always exist. pub struct Res<'w, T: Resource> { value: &'w T, - added: &'w Tick, - changed: &'w Tick, - last_change_tick: u32, - change_tick: u32, + added: &'w SmallTick, + changed: &'w SmallTick, + last_change_tick: Tick, + change_tick: Tick, } // SAFETY: Res only reads a single World resource @@ -307,13 +307,13 @@ impl<'w, T: Resource> Res<'w, T> { /// Returns `true` if the resource was added after the system last ran. pub fn is_added(&self) -> bool { self.added - .is_older_than(self.last_change_tick, self.change_tick) + .is_newer_than(self.last_change_tick, self.change_tick) } /// Returns `true` if the resource was added or mutably dereferenced after the system last ran. pub fn is_changed(&self) -> bool { self.changed - .is_older_than(self.last_change_tick, self.change_tick) + .is_newer_than(self.last_change_tick, self.change_tick) } pub fn into_inner(self) -> &'w T { @@ -408,7 +408,7 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResState { state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { let (ptr, ticks) = world .get_resource_with_ticks(state.component_id) @@ -457,7 +457,7 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for OptionResState { state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { world .get_resource_with_ticks(state.0.component_id) @@ -522,7 +522,7 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResMutState { state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { let value = world .get_resource_unchecked_mut_with_id(state.component_id) @@ -570,7 +570,7 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for OptionResMutState { state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { world .get_resource_unchecked_mut_with_id(state.0.component_id) @@ -612,7 +612,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for CommandQueue { state: &'s mut Self, _system_meta: &SystemMeta, world: &'w World, - _change_tick: u32, + _change_tick: Tick, ) -> Self::Item { Commands::new(state, world) } @@ -664,7 +664,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for WorldState { _state: &'s mut Self, _system_meta: &SystemMeta, world: &'w World, - _change_tick: u32, + _change_tick: Tick, ) -> Self::Item { world } @@ -790,7 +790,7 @@ impl<'w, 's, T: FromWorld + Send + 'static> SystemParamFetch<'w, 's> for LocalSt state: &'s mut Self, _system_meta: &SystemMeta, _world: &'w World, - _change_tick: u32, + _change_tick: Tick, ) -> Self::Item { Local(state.0.get()) } @@ -885,7 +885,7 @@ impl<'w, 's, T: Component> SystemParamFetch<'w, 's> for RemovedComponentsState Self::Item { RemovedComponents { world, @@ -910,8 +910,8 @@ impl<'w, 's, T: Component> SystemParamFetch<'w, 's> for RemovedComponentsState { pub(crate) value: &'w T, ticks: ComponentTicks, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, } // SAFETY: Only reads a single World non-send resource @@ -1010,7 +1010,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendState { state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { world.validate_non_send_access::(); let (ptr, ticks) = world @@ -1060,7 +1060,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for OptionNonSendState { state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { world.validate_non_send_access::(); world @@ -1127,7 +1127,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendMutState { state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { world.validate_non_send_access::(); let (ptr, ticks) = world @@ -1171,7 +1171,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for OptionNonSendMutState { state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { world.validate_non_send_access::(); world @@ -1209,7 +1209,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for ArchetypesState { _state: &'s mut Self, _system_meta: &SystemMeta, world: &'w World, - _change_tick: u32, + _change_tick: Tick, ) -> Self::Item { world.archetypes() } @@ -1241,7 +1241,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for ComponentsState { _state: &'s mut Self, _system_meta: &SystemMeta, world: &'w World, - _change_tick: u32, + _change_tick: Tick, ) -> Self::Item { world.components() } @@ -1273,7 +1273,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for EntitiesState { _state: &'s mut Self, _system_meta: &SystemMeta, world: &'w World, - _change_tick: u32, + _change_tick: Tick, ) -> Self::Item { world.entities() } @@ -1305,7 +1305,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for BundlesState { _state: &'s mut Self, _system_meta: &SystemMeta, world: &'w World, - _change_tick: u32, + _change_tick: Tick, ) -> Self::Item { world.bundles() } @@ -1322,20 +1322,20 @@ impl<'w, 's> SystemParamFetch<'w, 's> for BundlesState { /// on a [`Mut`](crate::change_detection::Mut) or [`ResMut`](crate::change_detection::ResMut). #[derive(Debug)] pub struct SystemChangeTick { - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, } impl SystemChangeTick { /// Returns the current [`World`] change tick seen by the system. #[inline] - pub fn change_tick(&self) -> u32 { + pub fn change_tick(&self) -> Tick { self.change_tick } /// Returns the [`World`] change tick seen by the system the previous time it ran. #[inline] - pub fn last_change_tick(&self) -> u32 { + pub fn last_change_tick(&self) -> Tick { self.last_change_tick } } @@ -1365,7 +1365,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for SystemChangeTickState { _state: &'s mut Self, system_meta: &SystemMeta, _world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { SystemChangeTick { last_change_tick: system_meta.last_change_tick, @@ -1451,7 +1451,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for SystemNameState { state: &'s mut Self, _system_meta: &SystemMeta, _world: &'w World, - _change_tick: u32, + _change_tick: Tick, ) -> Self::Item { SystemName { name: state.name.as_ref(), @@ -1479,7 +1479,7 @@ macro_rules! impl_system_param_tuple { state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { let ($($param,)*) = state; @@ -1625,7 +1625,7 @@ where state: &'state mut Self, system_meta: &SystemMeta, world: &'world World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { // SAFETY: We properly delegate SystemParamState StaticSystemParam(S::get_param(&mut state.0, system_meta, world, change_tick)) diff --git a/crates/bevy_ecs/src/system/system_piping.rs b/crates/bevy_ecs/src/system/system_piping.rs index 09533a02c32b5..49a4bac6a0383 100644 --- a/crates/bevy_ecs/src/system/system_piping.rs +++ b/crates/bevy_ecs/src/system/system_piping.rs @@ -1,5 +1,6 @@ use crate::{ archetype::ArchetypeComponentId, + change_detection::Tick, component::ComponentId, query::Access, system::{IntoSystem, System}, @@ -113,16 +114,11 @@ impl> System for PipeSystem< .extend(self.system_b.archetype_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); - } - - fn get_last_change_tick(&self) -> u32 { + fn get_last_change_tick(&self) -> Tick { self.system_a.get_last_change_tick() } - fn set_last_change_tick(&mut self, last_change_tick: u32) { + fn set_last_change_tick(&mut self, last_change_tick: Tick) { self.system_a.set_last_change_tick(last_change_tick); self.system_b.set_last_change_tick(last_change_tick); } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 37bf71adad1cd..6124ff76b738d 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}, bundle::{Bundle, BundleInfo}, - change_detection::{MutUntyped, Ticks}, + change_detection::{MutUntyped, Tick, Ticks}, component::{Component, ComponentId, ComponentTicks, Components, StorageType, TickCells}, entity::{Entities, Entity, EntityLocation}, storage::{SparseSet, Storages}, @@ -95,8 +95,8 @@ impl<'w> EntityRef<'w> { #[inline] pub unsafe fn get_unchecked_mut( &self, - last_change_tick: u32, - change_tick: u32, + last_change_tick: Tick, + change_tick: Tick, ) -> Option> { get_component_and_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) .map(|(value, ticks)| Mut { @@ -241,7 +241,7 @@ impl<'w> EntityMut<'w> { /// /// This will overwrite any previous value(s) of the same component type. pub fn insert(&mut self, bundle: T) -> &mut Self { - let change_tick = self.world.change_tick(); + let change_tick = self.world.change_tick().into(); let bundle_info = self .world .bundles diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d42ae7744e127..b28e36f1368fe 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -10,7 +10,7 @@ pub use world_cell::*; use crate::{ archetype::{ArchetypeComponentId, ArchetypeId, Archetypes}, bundle::{Bundle, BundleInserter, BundleSpawner, Bundles}, - change_detection::{MutUntyped, Ticks}, + change_detection::{MutUntyped, Tick, TickCounter, Ticks}, component::{ Component, ComponentDescriptor, ComponentId, ComponentInfo, Components, TickCells, }, @@ -21,11 +21,7 @@ use crate::{ }; use bevy_ptr::{OwningPtr, Ptr}; use bevy_utils::tracing::warn; -use std::{ - any::TypeId, - fmt, - sync::atomic::{AtomicU32, Ordering}, -}; +use std::{any::TypeId, fmt}; mod identifier; pub use identifier::WorldId; @@ -60,8 +56,8 @@ 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, + pub(crate) change_tick: TickCounter, + pub(crate) last_change_tick: Tick, } impl Default for World { @@ -78,8 +74,8 @@ impl Default for World { 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, + change_tick: TickCounter::new(), + last_change_tick: Tick::new(), } } } @@ -459,7 +455,7 @@ impl World { &mut self.archetypes, &mut self.components, &mut self.storages, - *self.change_tick.get_mut(), + self.change_tick.current_mut().into(), ); // SAFETY: bundle's type matches `bundle_info`, entity is allocated but non-existent @@ -1052,7 +1048,7 @@ impl World { self.flush(); let iter = iter.into_iter(); - let change_tick = *self.change_tick.get_mut(); + let change_tick = self.change_tick.current_mut().into(); let bundle_info = self .bundles @@ -1314,7 +1310,7 @@ impl World { component_id: ComponentId, value: OwningPtr<'_>, ) { - let change_tick = self.change_tick(); + let change_tick = self.change_tick().into(); // SAFETY: component_id is valid, ensured by caller self.initialize_resource_internal(component_id) @@ -1386,32 +1382,37 @@ impl World { } #[inline] - pub fn increment_change_tick(&self) -> u32 { - self.change_tick.fetch_add(1, Ordering::AcqRel) + pub fn increment_change_tick(&self) -> Tick { + self.change_tick.next() } #[inline] - pub fn read_change_tick(&self) -> u32 { - self.change_tick.load(Ordering::Acquire) + pub fn read_change_tick(&self) -> Tick { + self.change_tick.current() } #[inline] - pub fn change_tick(&mut self) -> u32 { - *self.change_tick.get_mut() + pub fn change_tick(&mut self) -> Tick { + self.change_tick.current_mut() } #[inline] - pub fn last_change_tick(&self) -> u32 { + pub fn last_change_tick(&self) -> Tick { 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); - self.storages.resources.check_change_ticks(change_tick); + if self.change_tick.maintain() { + self.storages + .tables + .check_change_ticks(self.change_tick.current_mut()); + self.storages + .sparse_sets + .check_change_ticks(self.change_tick.current_mut()); + self.storages + .resources + .check_change_ticks(self.change_tick.current_mut()); + } } pub fn clear_entities(&mut self) { diff --git a/crates/bevy_ecs/src/world/spawn_batch.rs b/crates/bevy_ecs/src/world/spawn_batch.rs index f5e1bd2792e2b..6eeb190d8739b 100644 --- a/crates/bevy_ecs/src/world/spawn_batch.rs +++ b/crates/bevy_ecs/src/world/spawn_batch.rs @@ -37,7 +37,7 @@ where &mut world.archetypes, &mut world.components, &mut world.storages, - *world.change_tick.get_mut(), + world.change_tick.current_mut().into(), ); spawner.reserve_storage(length); diff --git a/crates/bevy_render/src/extract_param.rs b/crates/bevy_render/src/extract_param.rs index cdfb462c2324b..97f739edc334a 100644 --- a/crates/bevy_render/src/extract_param.rs +++ b/crates/bevy_render/src/extract_param.rs @@ -84,7 +84,7 @@ where state: &'s mut Self, system_meta: &SystemMeta, world: &'w World, - change_tick: u32, + change_tick: Tick, ) -> Self::Item { let main_world = ResState::::get_param( &mut state.main_world_state, diff --git a/crates/bevy_time/src/fixed_timestep.rs b/crates/bevy_time/src/fixed_timestep.rs index 8a63f67f92bb6..5e0455b0dc9bc 100644 --- a/crates/bevy_time/src/fixed_timestep.rs +++ b/crates/bevy_time/src/fixed_timestep.rs @@ -1,6 +1,7 @@ use crate::Time; use bevy_ecs::{ archetype::ArchetypeComponentId, + change_detection::Tick, component::ComponentId, query::Access, schedule::ShouldRun, @@ -225,15 +226,11 @@ impl System for FixedTimestep { .update_archetype_component_access(world); } - fn check_change_tick(&mut self, change_tick: u32) { - self.internal_system.check_change_tick(change_tick); - } - - fn get_last_change_tick(&self) -> u32 { + fn get_last_change_tick(&self) -> Tick { self.internal_system.get_last_change_tick() } - fn set_last_change_tick(&mut self, last_change_tick: u32) { + fn set_last_change_tick(&mut self, last_change_tick: Tick) { self.internal_system.set_last_change_tick(last_change_tick); } }