[Merged by Bors] - Split Component Ticks

crates/bevy_ecs/src/bundle.rs

@@ -9,7 +9,7 @@ use crate::{
         Archetype, ArchetypeId, Archetypes, BundleComponentStatus, ComponentStatus,
         SpawnBundleStatus,
     },
-    component::{Component, ComponentId, ComponentTicks, Components, StorageType},
+    component::{Component, ComponentId, Components, StorageType, Tick},
     entity::{Entities, Entity, EntityLocation},
     storage::{SparseSetIndex, SparseSets, Storages, Table},
 };
@@ -394,11 +394,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,
-                                ComponentTicks::new(change_tick),
-                            );
+                            column.initialize(table_row, component_ptr, Tick::new(change_tick));
                         }
                         ComponentStatus::Mutated => {
                             column.replace(table_row, component_ptr, change_tick);

crates/bevy_ecs/src/change_detection.rs

@@ -1,6 +1,11 @@
 //! Types that detect when their internal data mutate.
 
-use crate::{component::ComponentTicks, ptr::PtrMut, system::Resource};
+use crate::{
+    component::{Tick, TickCells},
+    ptr::PtrMut,
+    system::Resource,
+};
+use bevy_ptr::UnsafeCellDeref;
 use std::ops::{Deref, DerefMut};
 
 /// The (arbitrarily chosen) minimum number of world tick increments between `check_tick` scans.
@@ -95,21 +100,21 @@ macro_rules! change_detection_impl {
             #[inline]
             fn is_added(&self) -> bool {
                 self.ticks
-                    .component_ticks
-                    .is_added(self.ticks.last_change_tick, self.ticks.change_tick)
+                    .added
+                    .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick)
             }
 
             #[inline]
             fn is_changed(&self) -> bool {
                 self.ticks
-                    .component_ticks
-                    .is_changed(self.ticks.last_change_tick, self.ticks.change_tick)
+                    .changed
+                    .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick)
             }
 
             #[inline]
             fn set_changed(&mut self) {
                 self.ticks
-                    .component_ticks
+                    .changed
                     .set_changed(self.ticks.change_tick);
             }
 
@@ -224,11 +229,30 @@ macro_rules! impl_debug {
 }
 
 pub(crate) struct Ticks<'a> {
-    pub(crate) component_ticks: &'a mut ComponentTicks,
+    pub(crate) added: &'a mut Tick,
+    pub(crate) changed: &'a mut Tick,
     pub(crate) last_change_tick: u32,
     pub(crate) change_tick: u32,
 }
 
+impl<'a> Ticks<'a> {
+    /// # Safety
+    /// This should never alias the underlying ticks. All access must be unique.
+    #[inline]
+    pub(crate) unsafe fn from_tick_cells(
+        cells: TickCells<'a>,
+        last_change_tick: u32,
+        change_tick: u32,
+    ) -> Self {
+        Self {
+            added: cells.added.deref_mut(),
+            changed: cells.changed.deref_mut(),
+            last_change_tick,
+            change_tick,
+        }
+    }
+}
+
 /// Unique mutable borrow of a [`Resource`].
 ///
 /// See the [`Resource`] documentation for usage.
@@ -381,22 +405,20 @@ impl<'a> DetectChanges for MutUntyped<'a> {
     #[inline]
     fn is_added(&self) -> bool {
         self.ticks
-            .component_ticks
-            .is_added(self.ticks.last_change_tick, self.ticks.change_tick)
+            .added
+            .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick)
     }
 
     #[inline]
     fn is_changed(&self) -> bool {
         self.ticks
-            .component_ticks
-            .is_changed(self.ticks.last_change_tick, self.ticks.change_tick)
+            .changed
+            .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick)
     }
 
     #[inline]
     fn set_changed(&mut self) {
-        self.ticks
-            .component_ticks
-            .set_changed(self.ticks.change_tick);
+        self.ticks.changed.set_changed(self.ticks.change_tick);
     }
 
     #[inline]
@@ -429,10 +451,8 @@ mod tests {
 
     use crate::{
         self as bevy_ecs,
-        change_detection::{
-            ComponentTicks, Mut, NonSendMut, ResMut, Ticks, CHECK_TICK_THRESHOLD, MAX_CHANGE_AGE,
-        },
-        component::Component,
+        change_detection::{Mut, NonSendMut, ResMut, Ticks, CHECK_TICK_THRESHOLD, MAX_CHANGE_AGE},
+        component::{Component, ComponentTicks, Tick},
         query::ChangeTrackers,
         system::{IntoSystem, Query, System},
         world::World,
@@ -514,8 +534,8 @@ mod tests {
 
         let mut query = world.query::<ChangeTrackers<C>>();
         for tracker in query.iter(&world) {
-            let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added);
-            let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed);
+            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);
         }
@@ -524,8 +544,8 @@ mod tests {
         world.check_change_ticks();
 
         for tracker in query.iter(&world) {
-            let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added);
-            let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed);
+            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);
         }
@@ -534,11 +554,12 @@ mod tests {
     #[test]
     fn mut_from_res_mut() {
         let mut component_ticks = ComponentTicks {
-            added: 1,
-            changed: 2,
+            added: Tick::new(1),
+            changed: Tick::new(2),
         };
         let ticks = Ticks {
-            component_ticks: &mut component_ticks,
+            added: &mut component_ticks.added,
+            changed: &mut component_ticks.changed,
             last_change_tick: 3,
             change_tick: 4,
         };
@@ -549,20 +570,21 @@ mod tests {
         };
 
         let into_mut: Mut<R> = res_mut.into();
-        assert_eq!(1, into_mut.ticks.component_ticks.added);
-        assert_eq!(2, into_mut.ticks.component_ticks.changed);
+        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);
     }
 
     #[test]
     fn mut_from_non_send_mut() {
         let mut component_ticks = ComponentTicks {
-            added: 1,
-            changed: 2,
+            added: Tick::new(1),
+            changed: Tick::new(2),
         };
         let ticks = Ticks {
-            component_ticks: &mut component_ticks,
+            added: &mut component_ticks.added,
+            changed: &mut component_ticks.changed,
             last_change_tick: 3,
             change_tick: 4,
         };
@@ -573,8 +595,8 @@ mod tests {
         };
 
         let into_mut: Mut<R> = non_send_mut.into();
-        assert_eq!(1, into_mut.ticks.component_ticks.added);
-        assert_eq!(2, into_mut.ticks.component_ticks.changed);
+        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);
     }
@@ -584,13 +606,14 @@ mod tests {
         use super::*;
         struct Outer(i64);
 
+        let (last_change_tick, change_tick) = (2, 3);
         let mut component_ticks = ComponentTicks {
-            added: 1,
-            changed: 2,
+            added: Tick::new(1),
+            changed: Tick::new(2),
         };
-        let (last_change_tick, change_tick) = (2, 3);
         let ticks = Ticks {
-            component_ticks: &mut component_ticks,
+            added: &mut component_ticks.added,
+            changed: &mut component_ticks.changed,
             last_change_tick,
             change_tick,
         };

crates/bevy_ecs/src/component.rs

@@ -6,7 +6,8 @@ use crate::{
     system::Resource,
 };
 pub use bevy_ecs_macros::Component;
-use bevy_ptr::OwningPtr;
+use bevy_ptr::{OwningPtr, UnsafeCellDeref};
+use std::cell::UnsafeCell;
 use std::{
     alloc::Layout,
     any::{Any, TypeId},
@@ -517,58 +518,108 @@ impl Components {
     }
 }
 
-/// Records when a component was added and when it was last mutably dereferenced (or added).
+/// Used to track changes in state between system runs, e.g. components being added or accessed mutably.
 #[derive(Copy, Clone, Debug)]
-pub struct ComponentTicks {
-    pub(crate) added: u32,
-    pub(crate) changed: u32,
+pub struct Tick {
+    pub(crate) tick: u32,
 }
 
-impl ComponentTicks {
+impl Tick {
+    pub const fn new(tick: u32) -> Self {
+        Self { tick }
+    }
+
     #[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 {
+    /// 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.added`, and we scan periodically to clamp `ComponentTicks` values
+        // `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.added).min(MAX_CHANGE_AGE);
+        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<T>`](crate::change_detection::Mut), [`ResMut<T>`](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<Tick>,
+    pub changed: &'a UnsafeCell<Tick>,
+}
+
+impl<'a> TickCells<'a> {
+    /// # Safety
+    /// All cells contained within must uphold the safety invariants of [`UnsafeCellDeref::read`].
+    #[inline]
+    pub(crate) unsafe fn read(&self) -> ComponentTicks {
+        ComponentTicks {
+            added: self.added.read(),
+            changed: self.changed.read(),
+        }
+    }
+}
+
+/// 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,
+}
+
+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)
+    }
+
     #[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 {
-        // This works even with wraparound because the world tick (`change_tick`) is always "newer" than
-        // `last_change_tick` and `self.changed`, 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_change = change_tick.wrapping_sub(self.changed).min(MAX_CHANGE_AGE);
-        let ticks_since_system = change_tick
-            .wrapping_sub(last_change_tick)
-            .min(MAX_CHANGE_AGE);
-
-        ticks_since_system > ticks_since_change
+        self.changed.is_older_than(last_change_tick, change_tick)
     }
 
     pub(crate) fn new(change_tick: u32) -> Self {
         Self {
-            added: change_tick,
-            changed: change_tick,
+            added: Tick::new(change_tick),
+            changed: Tick::new(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);
-    }
-
     /// Manually sets the change tick.
     ///
     /// This is normally done automatically via the [`DerefMut`](std::ops::DerefMut) implementation
@@ -585,15 +636,6 @@ impl ComponentTicks {
     /// ```
     #[inline]
     pub fn set_changed(&mut self, change_tick: u32) {
-        self.changed = change_tick;
-    }
-}
-
-fn check_tick(last_change_tick: &mut u32, change_tick: u32) {
-    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 {
-        *last_change_tick = change_tick.wrapping_sub(MAX_CHANGE_AGE);
+        self.changed.set_changed(change_tick);
     }
 }