diff --git a/Cargo.toml b/Cargo.toml index 5f162b21a372f..7d2edc47fd808 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -348,6 +348,9 @@ ios_simulator = ["bevy_internal/ios_simulator"] # Enable built in global state machines bevy_state = ["bevy_internal/bevy_state"] +# Enables source location tracking for change detection, which can assist with debugging +track_change_detection = ["bevy_internal/track_change_detection"] + # Enable function reflection reflect_functions = ["bevy_internal/reflect_functions"] @@ -1610,13 +1613,14 @@ category = "ECS (Entity Component System)" wasm = false [[example]] -name = "component_change_detection" -path = "examples/ecs/component_change_detection.rs" +name = "change_detection" +path = "examples/ecs/change_detection.rs" doc-scrape-examples = true +required-features = ["track_change_detection"] -[package.metadata.example.component_change_detection] -name = "Component Change Detection" -description = "Change detection on components" +[package.metadata.example.change_detection] +name = "Change Detection" +description = "Change detection on components and resources" category = "ECS (Entity Component System)" wasm = false diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index b932ac3b980e1..9fff70418c2e5 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -11,11 +11,12 @@ categories = ["game-engines", "data-structures"] rust-version = "1.77.0" [features] +default = ["bevy_reflect"] trace = [] multi_threaded = ["bevy_tasks/multi_threaded", "arrayvec"] bevy_debug_stepping = [] -default = ["bevy_reflect"] serialize = ["dep:serde"] +track_change_detection = [] [dependencies] bevy_ptr = { path = "../bevy_ptr", version = "0.15.0-dev" } diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index efb850db54b84..563cb0bf7962e 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -22,6 +22,8 @@ use crate::{ use bevy_ptr::{ConstNonNull, OwningPtr}; use bevy_utils::{all_tuples, HashMap, HashSet, TypeIdMap}; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; use std::ptr::NonNull; /// The `Bundle` trait enables insertion and removal of [`Component`]s from an entity. @@ -401,6 +403,7 @@ impl BundleInfo { table_row: TableRow, change_tick: Tick, bundle: T, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) { // NOTE: get_components calls this closure on each component in "bundle order". // bundle_info.component_ids are also in "bundle order" @@ -417,10 +420,22 @@ impl BundleInfo { let status = unsafe { bundle_component_status.get_status(bundle_component) }; match status { ComponentStatus::Added => { - column.initialize(table_row, component_ptr, change_tick); + column.initialize( + table_row, + component_ptr, + change_tick, + #[cfg(feature = "track_change_detection")] + caller, + ); } ComponentStatus::Mutated => { - column.replace(table_row, component_ptr, change_tick); + column.replace( + table_row, + component_ptr, + change_tick, + #[cfg(feature = "track_change_detection")] + caller, + ); } } } @@ -429,7 +444,13 @@ impl BundleInfo { // SAFETY: If component_id is in self.component_ids, BundleInfo::new requires that // a sparse set exists for the component. unsafe { sparse_sets.get_mut(component_id).debug_checked_unwrap() }; - sparse_set.insert(entity, component_ptr, change_tick); + sparse_set.insert( + entity, + component_ptr, + change_tick, + #[cfg(feature = "track_change_detection")] + caller, + ); } } bundle_component += 1; @@ -664,6 +685,7 @@ impl<'w> BundleInserter<'w> { entity: Entity, location: EntityLocation, bundle: T, + #[cfg(feature = "track_change_detection")] caller: &'static core::panic::Location<'static>, ) -> EntityLocation { let bundle_info = self.bundle_info.as_ref(); let add_bundle = self.add_bundle.as_ref(); @@ -706,6 +728,8 @@ impl<'w> BundleInserter<'w> { location.table_row, self.change_tick, bundle, + #[cfg(feature = "track_change_detection")] + caller, ); (archetype, location) @@ -744,6 +768,8 @@ impl<'w> BundleInserter<'w> { result.table_row, self.change_tick, bundle, + #[cfg(feature = "track_change_detection")] + caller, ); (new_archetype, new_location) @@ -823,6 +849,8 @@ impl<'w> BundleInserter<'w> { move_result.new_row, self.change_tick, bundle, + #[cfg(feature = "track_change_detection")] + caller, ); (new_archetype, new_location) @@ -919,6 +947,7 @@ impl<'w> BundleSpawner<'w> { &mut self, entity: Entity, bundle: T, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) -> EntityLocation { // SAFETY: We do not make any structural changes to the archetype graph through self.world so these pointers always remain valid let bundle_info = self.bundle_info.as_ref(); @@ -941,6 +970,8 @@ impl<'w> BundleSpawner<'w> { table_row, self.change_tick, bundle, + #[cfg(feature = "track_change_detection")] + caller, ); entities.set(entity.index(), location); location @@ -969,11 +1000,20 @@ impl<'w> BundleSpawner<'w> { /// # Safety /// `T` must match this [`BundleInfo`]'s type #[inline] - pub unsafe fn spawn(&mut self, bundle: T) -> Entity { + pub unsafe fn spawn( + &mut self, + bundle: T, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, + ) -> Entity { let entity = self.entities().alloc(); // SAFETY: entity is allocated (but non-existent), `T` matches this BundleInfo's type unsafe { - self.spawn_non_existent(entity, bundle); + self.spawn_non_existent( + entity, + bundle, + #[cfg(feature = "track_change_detection")] + caller, + ); } entity } diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index fc39a0e92274f..cb6f3a72d7cca 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -5,9 +5,13 @@ use crate::{ ptr::PtrMut, system::Resource, }; +#[cfg(feature = "track_change_detection")] +use bevy_ptr::ThinSlicePtr; use bevy_ptr::{Ptr, UnsafeCellDeref}; use std::mem; use std::ops::{Deref, DerefMut}; +#[cfg(feature = "track_change_detection")] +use std::{cell::UnsafeCell, panic::Location}; /// The (arbitrarily chosen) minimum number of world tick increments between `check_tick` scans. /// @@ -63,6 +67,10 @@ pub trait DetectChanges { /// [`SystemChangeTick`](crate::system::SystemChangeTick) /// [`SystemParam`](crate::system::SystemParam). fn last_changed(&self) -> Tick; + + /// The location that last caused this to change. + #[cfg(feature = "track_change_detection")] + fn changed_by(&self) -> &'static Location<'static>; } /// Types that implement reliable change detection. @@ -167,6 +175,7 @@ pub trait DetectChangesMut: DetectChanges { /// # assert!(!score_changed.run((), &mut world)); /// ``` #[inline] + #[track_caller] fn set_if_neq(&mut self, value: Self::Inner) -> bool where Self::Inner: Sized + PartialEq, @@ -280,6 +289,12 @@ macro_rules! change_detection_impl { fn last_changed(&self) -> Tick { *self.ticks.changed } + + #[inline] + #[cfg(feature = "track_change_detection")] + fn changed_by(&self) -> &'static Location<'static> { + self.changed_by + } } impl<$($generics),*: ?Sized $(+ $traits)?> Deref for $name<$($generics),*> { @@ -306,13 +321,23 @@ macro_rules! change_detection_mut_impl { type Inner = $target; #[inline] + #[track_caller] fn set_changed(&mut self) { *self.ticks.changed = self.ticks.this_run; + #[cfg(feature = "track_change_detection")] + { + *self.changed_by = Location::caller(); + } } #[inline] + #[track_caller] fn set_last_changed(&mut self, last_changed: Tick) { *self.ticks.changed = last_changed; + #[cfg(feature = "track_change_detection")] + { + *self.changed_by = Location::caller(); + } } #[inline] @@ -323,8 +348,13 @@ macro_rules! change_detection_mut_impl { impl<$($generics),* : ?Sized $(+ $traits)?> DerefMut for $name<$($generics),*> { #[inline] + #[track_caller] fn deref_mut(&mut self) -> &mut Self::Target { self.set_changed(); + #[cfg(feature = "track_change_detection")] + { + *self.changed_by = Location::caller(); + } self.value } } @@ -361,7 +391,9 @@ macro_rules! impl_methods { changed: self.ticks.changed, last_run: self.ticks.last_run, this_run: self.ticks.this_run, - } + }, + #[cfg(feature = "track_change_detection")] + changed_by: self.changed_by, } } @@ -391,6 +423,8 @@ macro_rules! impl_methods { Mut { value: f(self.value), ticks: self.ticks, + #[cfg(feature = "track_change_detection")] + changed_by: self.changed_by, } } @@ -501,6 +535,8 @@ impl<'w> From> for Ticks<'w> { pub struct Res<'w, T: ?Sized + Resource> { pub(crate) value: &'w T, pub(crate) ticks: Ticks<'w>, + #[cfg(feature = "track_change_detection")] + pub(crate) changed_by: &'static Location<'static>, } impl<'w, T: Resource> Res<'w, T> { @@ -513,6 +549,8 @@ impl<'w, T: Resource> Res<'w, T> { Self { value: this.value, ticks: this.ticks.clone(), + #[cfg(feature = "track_change_detection")] + changed_by: this.changed_by, } } @@ -529,6 +567,8 @@ impl<'w, T: Resource> From> for Res<'w, T> { Self { value: res.value, ticks: res.ticks.into(), + #[cfg(feature = "track_change_detection")] + changed_by: res.changed_by, } } } @@ -561,6 +601,8 @@ impl_debug!(Res<'w, T>, Resource); pub struct ResMut<'w, T: ?Sized + Resource> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, + #[cfg(feature = "track_change_detection")] + pub(crate) changed_by: &'w mut &'static Location<'static>, } impl<'w, 'a, T: Resource> IntoIterator for &'a ResMut<'w, T> @@ -600,6 +642,8 @@ impl<'w, T: Resource> From> for Mut<'w, T> { Mut { value: other.value, ticks: other.ticks, + #[cfg(feature = "track_change_detection")] + changed_by: other.changed_by, } } } @@ -619,6 +663,8 @@ impl<'w, T: Resource> From> for Mut<'w, T> { pub struct NonSendMut<'w, T: ?Sized + 'static> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, + #[cfg(feature = "track_change_detection")] + pub(crate) changed_by: &'w mut &'static Location<'static>, } change_detection_impl!(NonSendMut<'w, T>, T,); @@ -633,6 +679,8 @@ impl<'w, T: 'static> From> for Mut<'w, T> { Mut { value: other.value, ticks: other.ticks, + #[cfg(feature = "track_change_detection")] + changed_by: other.changed_by, } } } @@ -664,6 +712,8 @@ impl<'w, T: 'static> From> for Mut<'w, T> { pub struct Ref<'w, T: ?Sized> { pub(crate) value: &'w T, pub(crate) ticks: Ticks<'w>, + #[cfg(feature = "track_change_detection")] + pub(crate) changed_by: &'static Location<'static>, } impl<'w, T: ?Sized> Ref<'w, T> { @@ -680,6 +730,8 @@ impl<'w, T: ?Sized> Ref<'w, T> { Ref { value: f(self.value), ticks: self.ticks, + #[cfg(feature = "track_change_detection")] + changed_by: self.changed_by, } } @@ -700,6 +752,7 @@ impl<'w, T: ?Sized> Ref<'w, T> { changed: &'w Tick, last_run: Tick, this_run: Tick, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) -> Ref<'w, T> { Ref { value, @@ -709,6 +762,8 @@ impl<'w, T: ?Sized> Ref<'w, T> { last_run, this_run, }, + #[cfg(feature = "track_change_detection")] + changed_by: caller, } } } @@ -790,6 +845,8 @@ impl_debug!(Ref<'w, T>,); pub struct Mut<'w, T: ?Sized> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, + #[cfg(feature = "track_change_detection")] + pub(crate) changed_by: &'w mut &'static Location<'static>, } impl<'w, T: ?Sized> Mut<'w, T> { @@ -814,6 +871,7 @@ impl<'w, T: ?Sized> Mut<'w, T> { last_changed: &'w mut Tick, last_run: Tick, this_run: Tick, + #[cfg(feature = "track_change_detection")] caller: &'w mut &'static Location<'static>, ) -> Self { Self { value, @@ -823,6 +881,8 @@ impl<'w, T: ?Sized> Mut<'w, T> { last_run, this_run, }, + #[cfg(feature = "track_change_detection")] + changed_by: caller, } } } @@ -832,6 +892,8 @@ impl<'w, T: ?Sized> From> for Ref<'w, T> { Self { value: mut_ref.value, ticks: mut_ref.ticks.into(), + #[cfg(feature = "track_change_detection")] + changed_by: mut_ref.changed_by, } } } @@ -877,6 +939,8 @@ impl_debug!(Mut<'w, T>,); pub struct MutUntyped<'w> { pub(crate) value: PtrMut<'w>, pub(crate) ticks: TicksMut<'w>, + #[cfg(feature = "track_change_detection")] + pub(crate) changed_by: &'w mut &'static core::panic::Location<'static>, } impl<'w> MutUntyped<'w> { @@ -901,6 +965,8 @@ impl<'w> MutUntyped<'w> { last_run: self.ticks.last_run, this_run: self.ticks.this_run, }, + #[cfg(feature = "track_change_detection")] + changed_by: self.changed_by, } } @@ -951,6 +1017,8 @@ impl<'w> MutUntyped<'w> { Mut { value: f(self.value), ticks: self.ticks, + #[cfg(feature = "track_change_detection")] + changed_by: self.changed_by, } } @@ -963,6 +1031,9 @@ impl<'w> MutUntyped<'w> { // SAFETY: `value` is `Aligned` and caller ensures the pointee type is `T`. value: unsafe { self.value.deref_mut() }, ticks: self.ticks, + // SAFETY: `caller` is `Aligned`. + #[cfg(feature = "track_change_detection")] + changed_by: self.changed_by, } } } @@ -986,22 +1057,39 @@ impl<'w> DetectChanges for MutUntyped<'w> { fn last_changed(&self) -> Tick { *self.ticks.changed } + + #[inline] + #[cfg(feature = "track_change_detection")] + fn changed_by(&self) -> &'static Location<'static> { + self.changed_by + } } impl<'w> DetectChangesMut for MutUntyped<'w> { type Inner = PtrMut<'w>; #[inline] + #[track_caller] fn set_changed(&mut self) { *self.ticks.changed = self.ticks.this_run; + #[cfg(feature = "track_change_detection")] + { + *self.changed_by = Location::caller(); + } } #[inline] + #[track_caller] fn set_last_changed(&mut self, last_changed: Tick) { *self.ticks.changed = last_changed; + #[cfg(feature = "track_change_detection")] + { + *self.changed_by = Location::caller(); + } } #[inline] + #[track_caller] fn bypass_change_detection(&mut self) -> &mut Self::Inner { &mut self.value } @@ -1020,16 +1108,71 @@ impl<'w, T> From> for MutUntyped<'w> { MutUntyped { value: value.value.into(), ticks: value.ticks, + #[cfg(feature = "track_change_detection")] + changed_by: value.changed_by, } } } +/// A type alias to [`&'static Location<'static>`](std::panic::Location) when the `track_change_detection` feature is +/// enabled, and the unit type `()` when it is not. +/// +/// This is primarily used in places where `#[cfg(...)]` attributes are not allowed, such as +/// function return types. Because unit is a zero-sized type, it is the equivalent of not using a +/// `Location` at all. +/// +/// Please use this type sparingly: prefer normal `#[cfg(...)]` attributes when possible. +#[cfg(feature = "track_change_detection")] +pub(crate) type MaybeLocation = &'static Location<'static>; + +/// A type alias to [`&'static Location<'static>`](std::panic::Location) when the `track_change_detection` feature is +/// enabled, and the unit type `()` when it is not. +/// +/// This is primarily used in places where `#[cfg(...)]` attributes are not allowed, such as +/// function return types. Because unit is a zero-sized type, it is the equivalent of not using a +/// `Location` at all. +/// +/// Please use this type sparingly: prefer normal `#[cfg(...)]` attributes when possible. +#[cfg(not(feature = "track_change_detection"))] +pub(crate) type MaybeLocation = (); + +/// A type alias to `&UnsafeCell<&'static Location<'static>>` when the `track_change_detection` +/// feature is enabled, and the unit type `()` when it is not. +/// +/// See [`MaybeLocation`] for further information. +#[cfg(feature = "track_change_detection")] +pub(crate) type MaybeUnsafeCellLocation<'a> = &'a UnsafeCell<&'static Location<'static>>; + +/// A type alias to `&UnsafeCell<&'static Location<'static>>` when the `track_change_detection` +/// feature is enabled, and the unit type `()` when it is not. +/// +/// See [`MaybeLocation`] for further information. +#[cfg(not(feature = "track_change_detection"))] +pub(crate) type MaybeUnsafeCellLocation<'a> = (); + +/// A type alias to `ThinSlicePtr<'w, UnsafeCell<&'static Location<'static>>>` when the +/// `track_change_detection` feature is enabled, and the unit type `()` when it is not. +/// +/// See [`MaybeLocation`] for further information. +#[cfg(feature = "track_change_detection")] +pub(crate) type MaybeThinSlicePtrLocation<'w> = + ThinSlicePtr<'w, UnsafeCell<&'static Location<'static>>>; + +/// A type alias to `ThinSlicePtr<'w, UnsafeCell<&'static Location<'static>>>` when the +/// `track_change_detection` feature is enabled, and the unit type `()` when it is not. +/// +/// See [`MaybeLocation`] for further information. +#[cfg(not(feature = "track_change_detection"))] +pub(crate) type MaybeThinSlicePtrLocation<'w> = (); + #[cfg(test)] mod tests { use bevy_ecs_macros::Resource; use bevy_ptr::PtrMut; use bevy_reflect::{FromType, ReflectFromPtr}; use std::ops::{Deref, DerefMut}; + #[cfg(feature = "track_change_detection")] + use std::panic::Location; use crate::{ self as bevy_ecs, @@ -1160,9 +1303,14 @@ mod tests { this_run: Tick::new(4), }; let mut res = R {}; + #[cfg(feature = "track_change_detection")] + let mut caller = Location::caller(); + let res_mut = ResMut { value: &mut res, ticks, + #[cfg(feature = "track_change_detection")] + changed_by: &mut caller, }; let into_mut: Mut = res_mut.into(); @@ -1179,6 +1327,8 @@ mod tests { changed: Tick::new(3), }; let mut res = R {}; + #[cfg(feature = "track_change_detection")] + let mut caller = Location::caller(); let val = Mut::new( &mut res, @@ -1186,6 +1336,8 @@ mod tests { &mut component_ticks.changed, Tick::new(2), // last_run Tick::new(4), // this_run + #[cfg(feature = "track_change_detection")] + &mut caller, ); assert!(!val.is_added()); @@ -1205,9 +1357,14 @@ mod tests { this_run: Tick::new(4), }; let mut res = R {}; + #[cfg(feature = "track_change_detection")] + let mut caller = Location::caller(); + let non_send_mut = NonSendMut { value: &mut res, ticks, + #[cfg(feature = "track_change_detection")] + changed_by: &mut caller, }; let into_mut: Mut = non_send_mut.into(); @@ -1236,9 +1393,14 @@ mod tests { }; let mut outer = Outer(0); + #[cfg(feature = "track_change_detection")] + let mut caller = Location::caller(); + let ptr = Mut { value: &mut outer, ticks, + #[cfg(feature = "track_change_detection")] + changed_by: &mut caller, }; assert!(!ptr.is_changed()); @@ -1321,9 +1483,14 @@ mod tests { }; let mut value: i32 = 5; + #[cfg(feature = "track_change_detection")] + let mut caller = Location::caller(); + let value = MutUntyped { value: PtrMut::from(&mut value), ticks, + #[cfg(feature = "track_change_detection")] + changed_by: &mut caller, }; let reflect_from_ptr = >::from_type(); @@ -1354,9 +1521,14 @@ mod tests { this_run: Tick::new(4), }; let mut c = C {}; + #[cfg(feature = "track_change_detection")] + let mut caller = Location::caller(); + let mut_typed = Mut { value: &mut c, ticks, + #[cfg(feature = "track_change_detection")] + changed_by: &mut caller, }; let into_mut: MutUntyped = mut_typed.into(); diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 08bcbbe27bf25..2861736f146ee 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1,6 +1,6 @@ use crate::{ archetype::{Archetype, Archetypes}, - change_detection::{Ticks, TicksMut}, + change_detection::{MaybeThinSlicePtrLocation, Ticks, TicksMut}, component::{Component, ComponentId, Components, StorageType, Tick}, entity::{Entities, Entity, EntityLocation}, query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery}, @@ -1026,6 +1026,7 @@ pub struct RefFetch<'w, T> { ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, + MaybeThinSlicePtrLocation<'w>, )>, // T::STORAGE_TYPE = StorageType::SparseSet sparse_set: Option<&'w ComponentSparseSet>, @@ -1115,6 +1116,10 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { column.get_data_slice().into(), column.get_added_ticks_slice().into(), column.get_changed_ticks_slice().into(), + #[cfg(feature = "track_change_detection")] + column.get_changed_by_slice().into(), + #[cfg(not(feature = "track_change_detection"))] + (), )); } @@ -1127,7 +1132,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { match T::STORAGE_TYPE { StorageType::Table => { // SAFETY: STORAGE_TYPE = Table - let (table_components, added_ticks, changed_ticks) = + let (table_components, added_ticks, changed_ticks, _callers) = unsafe { fetch.table_data.debug_checked_unwrap() }; // SAFETY: The caller ensures `table_row` is in range. @@ -1136,6 +1141,9 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { let added = unsafe { added_ticks.get(table_row.as_usize()) }; // SAFETY: The caller ensures `table_row` is in range. let changed = unsafe { changed_ticks.get(table_row.as_usize()) }; + // SAFETY: The caller ensures `table_row` is in range. + #[cfg(feature = "track_change_detection")] + let caller = unsafe { _callers.get(table_row.as_usize()) }; Ref { value: component.deref(), @@ -1145,6 +1153,8 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { this_run: fetch.this_run, last_run: fetch.last_run, }, + #[cfg(feature = "track_change_detection")] + changed_by: caller.deref(), } } StorageType::SparseSet => { @@ -1152,7 +1162,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { let component_sparse_set = unsafe { fetch.sparse_set.debug_checked_unwrap() }; // SAFETY: The caller ensures `entity` is in range. - let (component, ticks) = unsafe { + let (component, ticks, _caller) = unsafe { component_sparse_set .get_with_ticks(entity) .debug_checked_unwrap() @@ -1161,6 +1171,8 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { Ref { value: component.deref(), ticks: Ticks::from_tick_cells(ticks, fetch.last_run, fetch.this_run), + #[cfg(feature = "track_change_detection")] + changed_by: _caller.deref(), } } } @@ -1209,6 +1221,7 @@ pub struct WriteFetch<'w, T> { ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, + MaybeThinSlicePtrLocation<'w>, )>, // T::STORAGE_TYPE = StorageType::SparseSet sparse_set: Option<&'w ComponentSparseSet>, @@ -1298,6 +1311,10 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { column.get_data_slice().into(), column.get_added_ticks_slice().into(), column.get_changed_ticks_slice().into(), + #[cfg(feature = "track_change_detection")] + column.get_changed_by_slice().into(), + #[cfg(not(feature = "track_change_detection"))] + (), )); } @@ -1310,7 +1327,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { match T::STORAGE_TYPE { StorageType::Table => { // SAFETY: STORAGE_TYPE = Table - let (table_components, added_ticks, changed_ticks) = + let (table_components, added_ticks, changed_ticks, _callers) = unsafe { fetch.table_data.debug_checked_unwrap() }; // SAFETY: The caller ensures `table_row` is in range. @@ -1319,6 +1336,9 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { let added = unsafe { added_ticks.get(table_row.as_usize()) }; // SAFETY: The caller ensures `table_row` is in range. let changed = unsafe { changed_ticks.get(table_row.as_usize()) }; + // SAFETY: The caller ensures `table_row` is in range. + #[cfg(feature = "track_change_detection")] + let caller = unsafe { _callers.get(table_row.as_usize()) }; Mut { value: component.deref_mut(), @@ -1328,6 +1348,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { this_run: fetch.this_run, last_run: fetch.last_run, }, + #[cfg(feature = "track_change_detection")] + changed_by: caller.deref_mut(), } } StorageType::SparseSet => { @@ -1335,7 +1357,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { let component_sparse_set = unsafe { fetch.sparse_set.debug_checked_unwrap() }; // SAFETY: The caller ensures `entity` is in range. - let (component, ticks) = unsafe { + let (component, ticks, _caller) = unsafe { component_sparse_set .get_with_ticks(entity) .debug_checked_unwrap() @@ -1344,6 +1366,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { Mut { value: component.assert_unique().deref_mut(), ticks: TicksMut::from_tick_cells(ticks, fetch.last_run, fetch.this_run), + #[cfg(feature = "track_change_detection")] + changed_by: _caller.deref_mut(), } } } diff --git a/crates/bevy_ecs/src/reflect/component.rs b/crates/bevy_ecs/src/reflect/component.rs index f211597e41ec1..b41d206dd8884 100644 --- a/crates/bevy_ecs/src/reflect/component.rs +++ b/crates/bevy_ecs/src/reflect/component.rs @@ -296,6 +296,8 @@ impl FromType for ReflectComponent { entity.into_mut::().map(|c| Mut { value: c.value as &mut dyn Reflect, ticks: c.ticks, + #[cfg(feature = "track_change_detection")] + changed_by: c.changed_by, }) }, reflect_unchecked_mut: |entity| { @@ -305,6 +307,8 @@ impl FromType for ReflectComponent { entity.get_mut::().map(|c| Mut { value: c.value as &mut dyn Reflect, ticks: c.ticks, + #[cfg(feature = "track_change_detection")] + changed_by: c.changed_by, }) } }, diff --git a/crates/bevy_ecs/src/reflect/resource.rs b/crates/bevy_ecs/src/reflect/resource.rs index a537e3b55ded0..4fa254a8aecb6 100644 --- a/crates/bevy_ecs/src/reflect/resource.rs +++ b/crates/bevy_ecs/src/reflect/resource.rs @@ -207,6 +207,8 @@ impl FromType for ReflectResource { world.get_resource_mut::().map(|res| Mut { value: res.value as &mut dyn Reflect, ticks: res.ticks, + #[cfg(feature = "track_change_detection")] + changed_by: res.changed_by, }) } }, diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 83e533fe609c4..b5ab39ed7a192 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -1,8 +1,10 @@ use crate::archetype::ArchetypeComponentId; -use crate::change_detection::{MutUntyped, TicksMut}; +use crate::change_detection::{MaybeLocation, MaybeUnsafeCellLocation, MutUntyped, TicksMut}; use crate::component::{ComponentId, ComponentTicks, Components, Tick, TickCells}; use crate::storage::{blob_vec::BlobVec, SparseSet}; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; use std::{cell::UnsafeCell, mem::ManuallyDrop, thread::ThreadId}; /// The type-erased backing storage and metadata for a single resource within a [`World`]. @@ -17,6 +19,8 @@ pub struct ResourceData { type_name: String, id: ArchetypeComponentId, origin_thread_id: Option, + #[cfg(feature = "track_change_detection")] + changed_by: UnsafeCell<&'static Location<'static>>, } impl Drop for ResourceData { @@ -109,7 +113,9 @@ impl ResourceData { /// If `SEND` is false, this will panic if a value is present and is not accessed from the /// original thread it was inserted in. #[inline] - pub(crate) fn get_with_ticks(&self) -> Option<(Ptr<'_>, TickCells<'_>)> { + pub(crate) fn get_with_ticks( + &self, + ) -> Option<(Ptr<'_>, TickCells<'_>, MaybeUnsafeCellLocation<'_>)> { self.is_present().then(|| { self.validate_access(); ( @@ -119,6 +125,10 @@ impl ResourceData { added: &self.added_ticks, changed: &self.changed_ticks, }, + #[cfg(feature = "track_change_detection")] + &self.changed_by, + #[cfg(not(feature = "track_change_detection"))] + (), ) }) } @@ -129,12 +139,15 @@ impl ResourceData { /// If `SEND` is false, this will panic if a value is present and is not accessed from the /// original thread it was inserted in. pub(crate) fn get_mut(&mut self, last_run: Tick, this_run: Tick) -> Option> { - let (ptr, ticks) = self.get_with_ticks()?; + let (ptr, ticks, _caller) = self.get_with_ticks()?; Some(MutUntyped { // SAFETY: We have exclusive access to the underlying storage. value: unsafe { ptr.assert_unique() }, // SAFETY: We have exclusive access to the underlying storage. ticks: unsafe { TicksMut::from_tick_cells(ticks, last_run, this_run) }, + #[cfg(feature = "track_change_detection")] + // SAFETY: We have exclusive access to the underlying storage. + changed_by: unsafe { _caller.deref_mut() }, }) } @@ -148,7 +161,12 @@ impl ResourceData { /// # Safety /// - `value` must be valid for the underlying type for the resource. #[inline] - pub(crate) unsafe fn insert(&mut self, value: OwningPtr<'_>, change_tick: Tick) { + pub(crate) unsafe fn insert( + &mut self, + value: OwningPtr<'_>, + change_tick: Tick, + #[cfg(feature = "track_change_detection")] caller: &'static core::panic::Location, + ) { if self.is_present() { self.validate_access(); // SAFETY: The caller ensures that the provided value is valid for the underlying type and @@ -165,6 +183,10 @@ impl ResourceData { *self.added_ticks.deref_mut() = change_tick; } *self.changed_ticks.deref_mut() = change_tick; + #[cfg(feature = "track_change_detection")] + { + *self.changed_by.deref_mut() = caller; + } } /// Inserts a value into the resource with a pre-existing change tick. If a @@ -181,6 +203,7 @@ impl ResourceData { &mut self, value: OwningPtr<'_>, change_ticks: ComponentTicks, + #[cfg(feature = "track_change_detection")] caller: &'static core::panic::Location, ) { if self.is_present() { self.validate_access(); @@ -198,6 +221,10 @@ impl ResourceData { } *self.added_ticks.deref_mut() = change_ticks.added; *self.changed_ticks.deref_mut() = change_ticks.changed; + #[cfg(feature = "track_change_detection")] + { + *self.changed_by.deref_mut() = caller; + } } /// Removes a value from the resource, if present. @@ -207,7 +234,7 @@ impl ResourceData { /// original thread it was inserted from. #[inline] #[must_use = "The returned pointer to the removed component should be used or dropped"] - pub(crate) fn remove(&mut self) -> Option<(OwningPtr<'_>, ComponentTicks)> { + pub(crate) fn remove(&mut self) -> Option<(OwningPtr<'_>, ComponentTicks, MaybeLocation)> { if !self.is_present() { return None; } @@ -216,6 +243,13 @@ impl ResourceData { } // SAFETY: We've already validated that the row is present. let res = unsafe { self.data.swap_remove_and_forget_unchecked(Self::ROW) }; + + // SAFETY: This function is being called through an exclusive mutable reference to Self + #[cfg(feature = "track_change_detection")] + let caller = unsafe { *self.changed_by.deref_mut() }; + #[cfg(not(feature = "track_change_detection"))] + let caller = (); + // SAFETY: This function is being called through an exclusive mutable reference to Self, which // makes it sound to read these ticks. unsafe { @@ -225,6 +259,7 @@ impl ResourceData { added: self.added_ticks.read(), changed: self.changed_ticks.read(), }, + caller, )) } } @@ -333,6 +368,8 @@ impl Resources { type_name: String::from(component_info.name()), id: f(), origin_thread_id: None, + #[cfg(feature = "track_change_detection")] + changed_by: UnsafeCell::new(Location::caller()) } }) } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 3ef1242c62bea..3a85358c217aa 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -1,10 +1,13 @@ use crate::{ + change_detection::MaybeUnsafeCellLocation, component::{ComponentId, ComponentInfo, ComponentTicks, Tick, TickCells}, entity::Entity, storage::{Column, TableRow}, }; use bevy_ptr::{OwningPtr, Ptr}; use nonmax::NonMaxUsize; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData}; type EntityIndex = u32; @@ -169,14 +172,26 @@ impl ComponentSparseSet { entity: Entity, value: OwningPtr<'_>, change_tick: Tick, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) { if let Some(&dense_index) = self.sparse.get(entity.index()) { #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); - self.dense.replace(dense_index, value, change_tick); + self.dense.replace( + dense_index, + value, + change_tick, + #[cfg(feature = "track_change_detection")] + caller, + ); } else { let dense_index = self.dense.len(); - self.dense.push(value, ComponentTicks::new(change_tick)); + self.dense.push( + value, + ComponentTicks::new(change_tick), + #[cfg(feature = "track_change_detection")] + caller, + ); self.sparse .insert(entity.index(), TableRow::from_usize(dense_index)); #[cfg(debug_assertions)] @@ -222,7 +237,10 @@ impl ComponentSparseSet { /// /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] - pub fn get_with_ticks(&self, entity: Entity) -> Option<(Ptr<'_>, TickCells<'_>)> { + pub fn get_with_ticks( + &self, + entity: Entity, + ) -> Option<(Ptr<'_>, TickCells<'_>, MaybeUnsafeCellLocation<'_>)> { let dense_index = *self.sparse.get(entity.index())?; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); @@ -234,6 +252,10 @@ impl ComponentSparseSet { added: self.dense.get_added_tick_unchecked(dense_index), changed: self.dense.get_changed_tick_unchecked(dense_index), }, + #[cfg(feature = "track_change_detection")] + self.dense.get_changed_by_unchecked(dense_index), + #[cfg(not(feature = "track_change_detection"))] + (), )) } } @@ -274,6 +296,22 @@ impl ComponentSparseSet { unsafe { Some(self.dense.get_ticks_unchecked(dense_index)) } } + /// Returns a reference to the calling location that last changed the entity's component value. + /// + /// Returns `None` if `entity` does not have a component in the sparse set. + #[inline] + #[cfg(feature = "track_change_detection")] + pub fn get_changed_by( + &self, + entity: Entity, + ) -> Option<&UnsafeCell<&'static core::panic::Location<'static>>> { + let dense_index = *self.sparse.get(entity.index())?; + #[cfg(debug_assertions)] + assert_eq!(entity, self.entities[dense_index.as_usize()]); + // SAFETY: if the sparse index points to something in the dense vec, it exists + unsafe { Some(self.dense.get_changed_by_unchecked(dense_index)) } + } + /// Removes the `entity` from this sparse set and returns a pointer to the associated value (if /// it exists). #[must_use = "The returned pointer must be used to drop the removed component."] @@ -284,7 +322,7 @@ impl ComponentSparseSet { self.entities.swap_remove(dense_index.as_usize()); let is_last = dense_index.as_usize() == self.dense.len() - 1; // SAFETY: dense_index was just removed from `sparse`, which ensures that it is valid - let (value, _) = unsafe { self.dense.swap_remove_and_forget_unchecked(dense_index) }; + let (value, _, _) = unsafe { self.dense.swap_remove_and_forget_unchecked(dense_index) }; if !is_last { let swapped_entity = self.entities[dense_index.as_usize()]; #[cfg(not(debug_assertions))] diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 8fc73bb873abb..57cfe6170ea69 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -1,4 +1,5 @@ use crate::{ + change_detection::{MaybeLocation, MaybeUnsafeCellLocation}, component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick, TickCells}, entity::Entity, query::DebugCheckedUnwrap, @@ -7,6 +8,8 @@ use crate::{ use bevy_ptr::{OwningPtr, Ptr, PtrMut, UnsafeCellDeref}; use bevy_utils::HashMap; use std::alloc::Layout; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; use std::{ cell::UnsafeCell, ops::{Index, IndexMut}, @@ -152,6 +155,8 @@ pub struct Column { data: BlobVec, added_ticks: Vec>, changed_ticks: Vec>, + #[cfg(feature = "track_change_detection")] + changed_by: Vec>>, } impl Column { @@ -163,6 +168,8 @@ impl Column { data: unsafe { BlobVec::new(component_info.layout(), component_info.drop(), capacity) }, added_ticks: Vec::with_capacity(capacity), changed_ticks: Vec::with_capacity(capacity), + #[cfg(feature = "track_change_detection")] + changed_by: Vec::with_capacity(capacity), } } @@ -179,7 +186,13 @@ impl Column { /// # Safety /// Assumes data has already been allocated for the given row. #[inline] - pub(crate) unsafe fn initialize(&mut self, row: TableRow, data: OwningPtr<'_>, tick: Tick) { + pub(crate) unsafe fn initialize( + &mut self, + row: TableRow, + data: OwningPtr<'_>, + tick: Tick, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, + ) { debug_assert!(row.as_usize() < self.len()); self.data.initialize_unchecked(row.as_usize(), data); *self.added_ticks.get_unchecked_mut(row.as_usize()).get_mut() = tick; @@ -187,6 +200,10 @@ impl Column { .changed_ticks .get_unchecked_mut(row.as_usize()) .get_mut() = tick; + #[cfg(feature = "track_change_detection")] + { + *self.changed_by.get_unchecked_mut(row.as_usize()).get_mut() = caller; + } } /// Writes component data to the column at given row. @@ -195,13 +212,24 @@ impl Column { /// # Safety /// Assumes data has already been allocated for the given row. #[inline] - pub(crate) unsafe fn replace(&mut self, row: TableRow, data: OwningPtr<'_>, change_tick: Tick) { + pub(crate) unsafe fn replace( + &mut self, + row: TableRow, + data: OwningPtr<'_>, + change_tick: Tick, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, + ) { debug_assert!(row.as_usize() < self.len()); self.data.replace_unchecked(row.as_usize(), data); *self .changed_ticks .get_unchecked_mut(row.as_usize()) .get_mut() = change_tick; + + #[cfg(feature = "track_change_detection")] + { + *self.changed_by.get_unchecked_mut(row.as_usize()).get_mut() = caller; + } } /// Gets the current number of elements stored in the column. @@ -231,6 +259,8 @@ impl Column { self.data.swap_remove_and_drop_unchecked(row.as_usize()); self.added_ticks.swap_remove(row.as_usize()); self.changed_ticks.swap_remove(row.as_usize()); + #[cfg(feature = "track_change_detection")] + self.changed_by.swap_remove(row.as_usize()); } /// Removes an element from the [`Column`] and returns it and its change detection ticks. @@ -249,11 +279,15 @@ impl Column { pub(crate) unsafe fn swap_remove_and_forget_unchecked( &mut self, row: TableRow, - ) -> (OwningPtr<'_>, ComponentTicks) { + ) -> (OwningPtr<'_>, ComponentTicks, MaybeLocation) { let data = self.data.swap_remove_and_forget_unchecked(row.as_usize()); let added = self.added_ticks.swap_remove(row.as_usize()).into_inner(); let changed = self.changed_ticks.swap_remove(row.as_usize()).into_inner(); - (data, ComponentTicks { added, changed }) + #[cfg(feature = "track_change_detection")] + let caller = self.changed_by.swap_remove(row.as_usize()).into_inner(); + #[cfg(not(feature = "track_change_detection"))] + let caller = (); + (data, ComponentTicks { added, changed }, caller) } /// Removes the element from `other` at `src_row` and inserts it @@ -281,16 +315,28 @@ impl Column { other.added_ticks.swap_remove(src_row.as_usize()); *self.changed_ticks.get_unchecked_mut(dst_row.as_usize()) = other.changed_ticks.swap_remove(src_row.as_usize()); + #[cfg(feature = "track_change_detection")] + { + *self.changed_by.get_unchecked_mut(dst_row.as_usize()) = + other.changed_by.swap_remove(src_row.as_usize()); + } } /// Pushes a new value onto the end of the [`Column`]. /// /// # Safety /// `ptr` must point to valid data of this column's component type - pub(crate) unsafe fn push(&mut self, ptr: OwningPtr<'_>, ticks: ComponentTicks) { + pub(crate) unsafe fn push( + &mut self, + ptr: OwningPtr<'_>, + ticks: ComponentTicks, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, + ) { self.data.push(ptr); self.added_ticks.push(UnsafeCell::new(ticks.added)); self.changed_ticks.push(UnsafeCell::new(ticks.changed)); + #[cfg(feature = "track_change_detection")] + self.changed_by.push(UnsafeCell::new(caller)); } #[inline] @@ -298,6 +344,8 @@ impl Column { self.data.reserve_exact(additional); self.added_ticks.reserve_exact(additional); self.changed_ticks.reserve_exact(additional); + #[cfg(feature = "track_change_detection")] + self.changed_by.reserve_exact(additional); } /// Fetches the data pointer to the first element of the [`Column`]. @@ -342,11 +390,25 @@ impl Column { &self.changed_ticks } + /// Fetches the slice to the [`Column`]'s caller locations. + /// + /// Note: The values stored within are [`UnsafeCell`]. + /// Users of this API must ensure that accesses to each individual element + /// adhere to the safety invariants of [`UnsafeCell`]. + #[inline] + #[cfg(feature = "track_change_detection")] + pub fn get_changed_by_slice(&self) -> &[UnsafeCell<&'static Location<'static>>] { + &self.changed_by + } + /// Fetches a reference to the data and change detection ticks at `row`. /// /// Returns `None` if `row` is out of bounds. #[inline] - pub fn get(&self, row: TableRow) -> Option<(Ptr<'_>, TickCells<'_>)> { + pub fn get( + &self, + row: TableRow, + ) -> Option<(Ptr<'_>, TickCells<'_>, MaybeUnsafeCellLocation<'_>)> { (row.as_usize() < self.data.len()) // SAFETY: The row is length checked before fetching the pointer. This is being // accessed through a read-only reference to the column. @@ -357,6 +419,10 @@ impl Column { added: self.added_ticks.get_unchecked(row.as_usize()), changed: self.changed_ticks.get_unchecked(row.as_usize()), }, + #[cfg(feature = "track_change_detection")] + self.changed_by.get_unchecked(row.as_usize()), + #[cfg(not(feature = "track_change_detection"))] + (), ) }) } @@ -483,6 +549,35 @@ impl Column { } } + /// Fetches the calling location that last changed the value at `row`. + /// + /// Returns `None` if `row` is out of bounds. + /// + /// Note: The values stored within are [`UnsafeCell`]. + /// Users of this API must ensure that accesses to each individual element + /// adhere to the safety invariants of [`UnsafeCell`]. + #[inline] + #[cfg(feature = "track_change_detection")] + pub fn get_changed_by(&self, row: TableRow) -> Option<&UnsafeCell<&'static Location<'static>>> { + self.changed_by.get(row.as_usize()) + } + + /// Fetches the calling location that last changed the value at `row`. + /// + /// Unlike [`Column::get_changed_by`] this function does not do any bounds checking. + /// + /// # Safety + /// `row` must be within the range `[0, self.len())`. + #[inline] + #[cfg(feature = "track_change_detection")] + pub unsafe fn get_changed_by_unchecked( + &self, + row: TableRow, + ) -> &UnsafeCell<&'static Location<'static>> { + debug_assert!(row.as_usize() < self.changed_by.len()); + self.changed_by.get_unchecked(row.as_usize()) + } + /// Clears the column, removing all values. /// /// Note that this function has no effect on the allocated capacity of the [`Column`]> @@ -490,6 +585,8 @@ impl Column { self.data.clear(); self.added_ticks.clear(); self.changed_ticks.clear(); + #[cfg(feature = "track_change_detection")] + self.changed_by.clear(); } #[inline] @@ -608,7 +705,7 @@ impl Table { new_column.initialize_from_unchecked(column, row, new_row); } else { // It's the caller's responsibility to drop these cases. - let (_, _) = column.swap_remove_and_forget_unchecked(row); + let (_, _, _) = column.swap_remove_and_forget_unchecked(row); } } TableMoveResult { @@ -740,6 +837,8 @@ impl Table { 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))); + #[cfg(feature = "track_change_detection")] + column.changed_by.push(UnsafeCell::new(Location::caller())); } TableRow::from_usize(index) } @@ -924,6 +1023,9 @@ mod tests { entity::Entity, storage::{TableBuilder, TableRow}, }; + #[cfg(feature = "track_change_detection")] + use std::panic::Location; + #[derive(Component)] struct W(T); @@ -947,6 +1049,8 @@ mod tests { row, value_ptr, Tick::new(0), + #[cfg(feature = "track_change_detection")] + Location::caller(), ); }); }; diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index d9db569d518b5..24de8ad572a27 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1,5 +1,8 @@ mod parallel_scope; +#[cfg(feature = "track_change_detection")] +use core::panic::Location; + use super::{Deferred, IntoObserverSystem, IntoSystem, RegisterSystem, Resource}; use crate::{ self as bevy_ecs, @@ -9,8 +12,10 @@ use crate::{ event::Event, observer::{Observer, TriggerEvent, TriggerTargets}, system::{RunSystemWithInput, SystemId}, - world::command_queue::RawCommandQueue, - world::{Command, CommandQueue, EntityWorldMut, FromWorld, World}, + world::{ + command_queue::RawCommandQueue, Command, CommandQueue, EntityWorldMut, FromWorld, + SpawnBatchIter, World, + }, }; use bevy_ptr::OwningPtr; use bevy_utils::tracing::{error, info}; @@ -352,6 +357,7 @@ impl<'w, 's> Commands<'w, 's> { /// /// - [`spawn_empty`](Self::spawn_empty) to spawn an entity without any components. /// - [`spawn_batch`](Self::spawn_batch) to spawn entities with a bundle each. + #[track_caller] pub fn spawn(&mut self, bundle: T) -> EntityCommands { let mut e = self.spawn_empty(); e.insert(bundle); @@ -488,6 +494,7 @@ impl<'w, 's> Commands<'w, 's> { /// /// - [`spawn`](Self::spawn) to spawn an entity with a bundle. /// - [`spawn_empty`](Self::spawn_empty) to spawn an entity without any components. + #[track_caller] pub fn spawn_batch(&mut self, bundles_iter: I) where I: IntoIterator + Send + Sync + 'static, @@ -533,6 +540,7 @@ impl<'w, 's> Commands<'w, 's> { /// Spawning a specific `entity` value is rarely the right choice. Most apps should use [`Commands::spawn_batch`]. /// This method should generally only be used for sharing entities across apps, and only when they have a scheme /// worked out to share an ID space (which doesn't happen by default). + #[track_caller] pub fn insert_or_spawn_batch(&mut self, bundles_iter: I) where I: IntoIterator + Send + Sync + 'static, @@ -565,6 +573,7 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(initialise_scoreboard); /// ``` + #[track_caller] pub fn init_resource(&mut self) { self.push(init_resource::); } @@ -594,6 +603,7 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(system); /// ``` + #[track_caller] pub fn insert_resource(&mut self, resource: R) { self.push(insert_resource(resource)); } @@ -934,6 +944,7 @@ impl EntityCommands<'_> { /// } /// # bevy_ecs::system::assert_is_system(add_combat_stats_system); /// ``` + #[track_caller] pub fn insert(&mut self, bundle: impl Bundle) -> &mut Self { self.add(insert(bundle)) } @@ -1030,6 +1041,7 @@ impl EntityCommands<'_> { /// } /// # bevy_ecs::system::assert_is_system(add_combat_stats_system); /// ``` + #[track_caller] pub fn try_insert(&mut self, bundle: impl Bundle) -> &mut Self { self.add(try_insert(bundle)) } @@ -1236,13 +1248,21 @@ where /// A [`Command`] that consumes an iterator of [`Bundle`]s to spawn a series of entities. /// /// This is more efficient than spawning the entities individually. +#[track_caller] fn spawn_batch(bundles_iter: I) -> impl Command where I: IntoIterator + Send + Sync + 'static, B: Bundle, { + #[cfg(feature = "track_change_detection")] + let caller = Location::caller(); move |world: &mut World| { - world.spawn_batch(bundles_iter); + SpawnBatchIter::new( + world, + bundles_iter.into_iter(), + #[cfg(feature = "track_change_detection")] + caller, + ); } } @@ -1250,13 +1270,20 @@ where /// If any entities do not already exist in the world, they will be spawned. /// /// This is more efficient than inserting the bundles individually. +#[track_caller] fn insert_or_spawn_batch(bundles_iter: I) -> impl Command where I: IntoIterator + Send + Sync + 'static, B: Bundle, { + #[cfg(feature = "track_change_detection")] + let caller = core::panic::Location::caller(); move |world: &mut World| { - if let Err(invalid_entities) = world.insert_or_spawn_batch(bundles_iter) { + if let Err(invalid_entities) = world.insert_or_spawn_batch_with_caller( + bundles_iter, + #[cfg(feature = "track_change_detection")] + caller, + ) { error!( "Failed to 'insert or spawn' bundle of type {} into the following invalid entities: {:?}", std::any::type_name::(), @@ -1278,10 +1305,17 @@ fn despawn(entity: Entity, world: &mut World) { } /// An [`EntityCommand`] that adds the components in a [`Bundle`] to an entity. +#[track_caller] fn insert(bundle: T) -> impl EntityCommand { + #[cfg(feature = "track_change_detection")] + let caller = core::panic::Location::caller(); move |entity: Entity, world: &mut World| { if let Some(mut entity) = world.get_entity_mut(entity) { - entity.insert(bundle); + entity.insert_with_caller( + bundle, + #[cfg(feature = "track_change_detection")] + caller, + ); } else { panic!("error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", std::any::type_name::(), entity); } @@ -1289,10 +1323,17 @@ fn insert(bundle: T) -> impl EntityCommand { } /// An [`EntityCommand`] that attempts to add the components in a [`Bundle`] to an entity. +#[track_caller] fn try_insert(bundle: impl Bundle) -> impl EntityCommand { + #[cfg(feature = "track_change_detection")] + let caller = core::panic::Location::caller(); move |entity, world: &mut World| { if let Some(mut entity) = world.get_entity_mut(entity) { - entity.insert(bundle); + entity.insert_with_caller( + bundle, + #[cfg(feature = "track_change_detection")] + caller, + ); } } } @@ -1363,19 +1404,28 @@ fn retain(entity: Entity, world: &mut World) { /// A [`Command`] that inserts a [`Resource`] into the world using a value /// created with the [`FromWorld`] trait. +#[track_caller] fn init_resource(world: &mut World) { world.init_resource::(); } /// A [`Command`] that removes the [resource](Resource) `R` from the world. +#[track_caller] fn remove_resource(world: &mut World) { world.remove_resource::(); } /// A [`Command`] that inserts a [`Resource`] into the world. +#[track_caller] fn insert_resource(resource: R) -> impl Command { + #[cfg(feature = "track_change_detection")] + let caller = Location::caller(); move |world: &mut World| { - world.insert_resource(resource); + world.insert_resource_with_caller( + resource, + #[cfg(feature = "track_change_detection")] + caller, + ); } } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 137bbfa696204..8898e34d092e1 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -18,6 +18,8 @@ pub use bevy_ecs_macros::Resource; pub use bevy_ecs_macros::SystemParam; use bevy_ptr::UnsafeCellDeref; use bevy_utils::{all_tuples, synccell::SyncCell}; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; use std::{ fmt::Debug, marker::PhantomData, @@ -528,15 +530,16 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Self::Item<'w, 's> { - let (ptr, ticks) = world - .get_resource_with_ticks(component_id) - .unwrap_or_else(|| { - panic!( - "Resource requested by {} does not exist: {}", - system_meta.name, - std::any::type_name::() - ) - }); + let (ptr, ticks, _caller) = + world + .get_resource_with_ticks(component_id) + .unwrap_or_else(|| { + panic!( + "Resource requested by {} does not exist: {}", + system_meta.name, + std::any::type_name::() + ) + }); Res { value: ptr.deref(), ticks: Ticks { @@ -545,6 +548,8 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { last_run: system_meta.last_run, this_run: change_tick, }, + #[cfg(feature = "track_change_detection")] + changed_by: _caller.deref(), } } } @@ -570,7 +575,7 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { ) -> Self::Item<'w, 's> { world .get_resource_with_ticks(component_id) - .map(|(ptr, ticks)| Res { + .map(|(ptr, ticks, _caller)| Res { value: ptr.deref(), ticks: Ticks { added: ticks.added.deref(), @@ -578,6 +583,8 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { last_run: system_meta.last_run, this_run: change_tick, }, + #[cfg(feature = "track_change_detection")] + changed_by: _caller.deref(), }) } } @@ -640,6 +647,8 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { last_run: system_meta.last_run, this_run: change_tick, }, + #[cfg(feature = "track_change_detection")] + changed_by: value.changed_by, } } } @@ -670,6 +679,8 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { last_run: system_meta.last_run, this_run: change_tick, }, + #[cfg(feature = "track_change_detection")] + changed_by: value.changed_by, }) } } @@ -1071,6 +1082,8 @@ pub struct NonSend<'w, T: 'static> { ticks: ComponentTicks, last_run: Tick, this_run: Tick, + #[cfg(feature = "track_change_detection")] + changed_by: &'static Location<'static>, } // SAFETY: Only reads a single World non-send resource @@ -1095,6 +1108,12 @@ impl<'w, T: 'static> NonSend<'w, T> { pub fn is_changed(&self) -> bool { self.ticks.is_changed(self.last_run, self.this_run) } + + /// The location that last caused this to change. + #[cfg(feature = "track_change_detection")] + pub fn changed_by(&self) -> &'static Location<'static> { + self.changed_by + } } impl<'w, T> Deref for NonSend<'w, T> { @@ -1114,6 +1133,8 @@ impl<'a, T> From> for NonSend<'a, T> { }, this_run: nsm.ticks.this_run, last_run: nsm.ticks.last_run, + #[cfg(feature = "track_change_detection")] + changed_by: nsm.changed_by, } } } @@ -1158,21 +1179,24 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Self::Item<'w, 's> { - let (ptr, ticks) = world - .get_non_send_with_ticks(component_id) - .unwrap_or_else(|| { - panic!( - "Non-send resource requested by {} does not exist: {}", - system_meta.name, - std::any::type_name::() - ) - }); + let (ptr, ticks, _caller) = + world + .get_non_send_with_ticks(component_id) + .unwrap_or_else(|| { + panic!( + "Non-send resource requested by {} does not exist: {}", + system_meta.name, + std::any::type_name::() + ) + }); NonSend { value: ptr.deref(), ticks: ticks.read(), last_run: system_meta.last_run, this_run: change_tick, + #[cfg(feature = "track_change_detection")] + changed_by: _caller.deref(), } } } @@ -1198,11 +1222,13 @@ unsafe impl SystemParam for Option> { ) -> Self::Item<'w, 's> { world .get_non_send_with_ticks(component_id) - .map(|(ptr, ticks)| NonSend { + .map(|(ptr, ticks, _caller)| NonSend { value: ptr.deref(), ticks: ticks.read(), last_run: system_meta.last_run, this_run: change_tick, + #[cfg(feature = "track_change_detection")] + changed_by: _caller.deref(), }) } } @@ -1250,18 +1276,21 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Self::Item<'w, 's> { - let (ptr, ticks) = world - .get_non_send_with_ticks(component_id) - .unwrap_or_else(|| { - panic!( - "Non-send resource requested by {} does not exist: {}", - system_meta.name, - std::any::type_name::() - ) - }); + let (ptr, ticks, _caller) = + world + .get_non_send_with_ticks(component_id) + .unwrap_or_else(|| { + panic!( + "Non-send resource requested by {} does not exist: {}", + system_meta.name, + std::any::type_name::() + ) + }); NonSendMut { value: ptr.assert_unique().deref_mut(), ticks: TicksMut::from_tick_cells(ticks, system_meta.last_run, change_tick), + #[cfg(feature = "track_change_detection")] + changed_by: _caller.deref_mut(), } } } @@ -1284,9 +1313,11 @@ unsafe impl<'a, T: 'static> SystemParam for Option> { ) -> Self::Item<'w, 's> { world .get_non_send_with_ticks(component_id) - .map(|(ptr, ticks)| NonSendMut { + .map(|(ptr, ticks, _caller)| NonSendMut { value: ptr.assert_unique().deref_mut(), ticks: TicksMut::from_tick_cells(ticks, system_meta.last_run, change_tick), + #[cfg(feature = "track_change_detection")] + changed_by: _caller.deref_mut(), }) } } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index a039cb4a90f7c..1dd522c17c802 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -768,12 +768,31 @@ impl<'w> EntityWorldMut<'w> { /// Adds a [`Bundle`] of components to the entity. /// /// This will overwrite any previous value(s) of the same component type. + #[track_caller] pub fn insert(&mut self, bundle: T) -> &mut Self { + self.insert_with_caller( + bundle, + #[cfg(feature = "track_change_detection")] + core::panic::Location::caller(), + ) + } + + /// Split into a new function so we can pass the calling location into the function when using + /// as a command. + #[inline] + pub(crate) fn insert_with_caller( + &mut self, + bundle: T, + #[cfg(feature = "track_change_detection")] caller: &'static core::panic::Location, + ) -> &mut Self { let change_tick = self.world.change_tick(); let mut bundle_inserter = BundleInserter::new::(self.world, self.location.archetype_id, change_tick); - // SAFETY: location matches current entity. `T` matches `bundle_info` - self.location = unsafe { bundle_inserter.insert(self.entity, self.location, bundle) }; + self.location = + // SAFETY: location matches current entity. `T` matches `bundle_info` + unsafe { + bundle_inserter.insert(self.entity, self.location, bundle, #[cfg(feature = "track_change_detection")] caller) + }; self } @@ -787,6 +806,7 @@ impl<'w> EntityWorldMut<'w> { /// /// - [`ComponentId`] must be from the same world as [`EntityWorldMut`] /// - [`OwningPtr`] must be a valid reference to the type represented by [`ComponentId`] + #[track_caller] pub unsafe fn insert_by_id( &mut self, component_id: ComponentId, @@ -828,6 +848,7 @@ impl<'w> EntityWorldMut<'w> { /// # Safety /// - Each [`ComponentId`] must be from the same world as [`EntityWorldMut`] /// - Each [`OwningPtr`] must be a valid reference to the type represented by [`ComponentId`] + #[track_caller] pub unsafe fn insert_by_ids<'a, I: Iterator>>( &mut self, component_ids: &[ComponentId], @@ -2300,6 +2321,7 @@ pub enum TryFromFilteredError { /// - [`OwningPtr`] and [`StorageType`] iterators must correspond to the /// [`BundleInfo`] used to construct [`BundleInserter`] /// - [`Entity`] must correspond to [`EntityLocation`] +#[track_caller] unsafe fn insert_dynamic_bundle< 'a, I: Iterator>, @@ -2328,7 +2350,15 @@ unsafe fn insert_dynamic_bundle< }; // SAFETY: location matches current entity. - unsafe { bundle_inserter.insert(entity, location, bundle) } + unsafe { + bundle_inserter.insert( + entity, + location, + bundle, + #[cfg(feature = "track_change_detection")] + core::panic::Location::caller(), + ) + } } /// Removes a bundle from the given archetype and returns the resulting archetype (or None if the diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index c7a66c86a72e3..06ecb8b98db4f 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -52,6 +52,12 @@ use std::{ mem::MaybeUninit, sync::atomic::{AtomicU32, Ordering}, }; + +#[cfg(feature = "track_change_detection")] +use bevy_ptr::UnsafeCellDeref; +#[cfg(feature = "track_change_detection")] +use core::panic::Location; + use unsafe_world_cell::{UnsafeEntityCell, UnsafeWorldCell}; /// A [`World`] mutation. @@ -971,6 +977,7 @@ impl World { /// let position = world.entity(entity).get::().unwrap(); /// assert_eq!(position.x, 2.0); /// ``` + #[track_caller] pub fn spawn(&mut self, bundle: B) -> EntityWorldMut { self.flush(); let change_tick = self.change_tick(); @@ -978,7 +985,14 @@ impl World { let entity_location = { let mut bundle_spawner = BundleSpawner::new::(self, change_tick); // SAFETY: bundle's type matches `bundle_info`, entity is allocated but non-existent - unsafe { bundle_spawner.spawn_non_existent(entity, bundle) } + unsafe { + bundle_spawner.spawn_non_existent( + entity, + bundle, + #[cfg(feature = "track_change_detection")] + Location::caller(), + ) + } }; // SAFETY: entity and location are valid, as they were just created above @@ -1023,12 +1037,18 @@ impl World { /// /// assert_eq!(entities.len(), 2); /// ``` + #[track_caller] pub fn spawn_batch(&mut self, iter: I) -> SpawnBatchIter<'_, I::IntoIter> where I: IntoIterator, I::Item: Bundle, { - SpawnBatchIter::new(self, iter.into_iter()) + SpawnBatchIter::new( + self, + iter.into_iter(), + #[cfg(feature = "track_change_detection")] + Location::caller(), + ) } /// Retrieves a reference to the given `entity`'s [`Component`] of the given type. @@ -1277,7 +1297,10 @@ impl World { /// Note that any resource with the [`Default`] trait automatically implements [`FromWorld`], /// and those default values will be here instead. #[inline] + #[track_caller] pub fn init_resource(&mut self) -> ComponentId { + #[cfg(feature = "track_change_detection")] + let caller = Location::caller(); let component_id = self.components.init_resource::(); if self .storages @@ -1289,7 +1312,12 @@ impl World { OwningPtr::make(value, |ptr| { // SAFETY: component_id was just initialized and corresponds to resource of type R. unsafe { - self.insert_resource_by_id(component_id, ptr); + self.insert_resource_by_id( + component_id, + ptr, + #[cfg(feature = "track_change_detection")] + caller, + ); } }); } @@ -1302,12 +1330,33 @@ impl World { /// If you insert a resource of a type that already exists, /// you will overwrite any existing data. #[inline] + #[track_caller] pub fn insert_resource(&mut self, value: R) { + self.insert_resource_with_caller( + value, + #[cfg(feature = "track_change_detection")] + Location::caller(), + ); + } + + /// Split into a new function so we can pass the calling location into the function when using + /// as a command. + #[inline] + pub(crate) fn insert_resource_with_caller( + &mut self, + value: R, + #[cfg(feature = "track_change_detection")] caller: &'static Location, + ) { let component_id = self.components.init_resource::(); OwningPtr::make(value, |ptr| { // SAFETY: component_id was just initialized and corresponds to resource of type R. unsafe { - self.insert_resource_by_id(component_id, ptr); + self.insert_resource_by_id( + component_id, + ptr, + #[cfg(feature = "track_change_detection")] + caller, + ); } }); } @@ -1324,7 +1373,10 @@ impl World { /// /// Panics if called from a thread other than the main thread. #[inline] + #[track_caller] pub fn init_non_send_resource(&mut self) -> ComponentId { + #[cfg(feature = "track_change_detection")] + let caller = Location::caller(); let component_id = self.components.init_non_send::(); if self .storages @@ -1336,7 +1388,12 @@ impl World { OwningPtr::make(value, |ptr| { // SAFETY: component_id was just initialized and corresponds to resource of type R. unsafe { - self.insert_non_send_by_id(component_id, ptr); + self.insert_non_send_by_id( + component_id, + ptr, + #[cfg(feature = "track_change_detection")] + caller, + ); } }); } @@ -1353,12 +1410,20 @@ impl World { /// If a value is already present, this function will panic if called /// from a different thread than where the original value was inserted from. #[inline] + #[track_caller] pub fn insert_non_send_resource(&mut self, value: R) { + #[cfg(feature = "track_change_detection")] + let caller = Location::caller(); let component_id = self.components.init_non_send::(); OwningPtr::make(value, |ptr| { // SAFETY: component_id was just initialized and corresponds to resource of type R. unsafe { - self.insert_non_send_by_id(component_id, ptr); + self.insert_non_send_by_id( + component_id, + ptr, + #[cfg(feature = "track_change_detection")] + caller, + ); } }); } @@ -1367,7 +1432,7 @@ impl World { #[inline] pub fn remove_resource(&mut self) -> Option { let component_id = self.components.get_resource_id(TypeId::of::())?; - let (ptr, _) = self.storages.resources.get_mut(component_id)?.remove()?; + let (ptr, _, _) = self.storages.resources.get_mut(component_id)?.remove()?; // SAFETY: `component_id` was gotten via looking up the `R` type unsafe { Some(ptr.read::()) } } @@ -1386,7 +1451,7 @@ impl World { #[inline] pub fn remove_non_send_resource(&mut self) -> Option { let component_id = self.components.get_resource_id(TypeId::of::())?; - let (ptr, _) = self + let (ptr, _, _) = self .storages .non_send_resources .get_mut(component_id)? @@ -1603,10 +1668,13 @@ impl World { /// Gets a mutable reference to the resource of type `T` if it exists, /// otherwise inserts the resource using the result of calling `func`. #[inline] + #[track_caller] pub fn get_resource_or_insert_with( &mut self, func: impl FnOnce() -> R, ) -> Mut<'_, R> { + #[cfg(feature = "track_change_detection")] + let caller = Location::caller(); let change_tick = self.change_tick(); let last_change_tick = self.last_change_tick(); @@ -1616,7 +1684,12 @@ impl World { OwningPtr::make(func(), |ptr| { // SAFETY: component_id was just initialized and corresponds to resource of type R. unsafe { - data.insert(ptr, change_tick); + data.insert( + ptr, + change_tick, + #[cfg(feature = "track_change_detection")] + caller, + ); } }); } @@ -1749,7 +1822,28 @@ impl World { /// /// assert_eq!(world.get::(e0), Some(&B(0.0))); /// ``` + #[track_caller] pub fn insert_or_spawn_batch(&mut self, iter: I) -> Result<(), Vec> + where + I: IntoIterator, + I::IntoIter: Iterator, + B: Bundle, + { + self.insert_or_spawn_batch_with_caller( + iter, + #[cfg(feature = "track_change_detection")] + Location::caller(), + ) + } + + /// Split into a new function so we can pass the calling location into the function when using + /// as a command. + #[inline] + pub(crate) fn insert_or_spawn_batch_with_caller( + &mut self, + iter: I, + #[cfg(feature = "track_change_detection")] caller: &'static Location, + ) -> Result<(), Vec> where I: IntoIterator, I::IntoIter: Iterator, @@ -1792,7 +1886,15 @@ impl World { if location.archetype_id == archetype => { // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter - unsafe { inserter.insert(entity, location, bundle) }; + unsafe { + inserter.insert( + entity, + location, + bundle, + #[cfg(feature = "track_change_detection")] + caller, + ) + }; } _ => { // SAFETY: we initialized this bundle_id in `init_info` @@ -1805,7 +1907,15 @@ impl World { ) }; // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter - unsafe { inserter.insert(entity, location, bundle) }; + unsafe { + inserter.insert( + entity, + location, + bundle, + #[cfg(feature = "track_change_detection")] + caller, + ) + }; spawn_or_insert = SpawnOrInsert::Insert(inserter, location.archetype_id); } @@ -1814,13 +1924,27 @@ impl World { AllocAtWithoutReplacement::DidNotExist => { if let SpawnOrInsert::Spawn(ref mut spawner) = spawn_or_insert { // SAFETY: `entity` is allocated (but non existent), bundle matches inserter - unsafe { spawner.spawn_non_existent(entity, bundle) }; + unsafe { + spawner.spawn_non_existent( + entity, + bundle, + #[cfg(feature = "track_change_detection")] + caller, + ) + }; } else { // SAFETY: we initialized this bundle_id in `init_info` let mut spawner = unsafe { BundleSpawner::new_with_id(self, bundle_id, change_tick) }; // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter - unsafe { spawner.spawn_non_existent(entity, bundle) }; + unsafe { + spawner.spawn_non_existent( + entity, + bundle, + #[cfg(feature = "track_change_detection")] + caller, + ) + }; spawn_or_insert = SpawnOrInsert::Spawn(spawner); } } @@ -1860,6 +1984,7 @@ impl World { /// }); /// assert_eq!(world.get_resource::().unwrap().0, 2); /// ``` + #[track_caller] pub fn resource_scope(&mut self, f: impl FnOnce(&mut World, Mut) -> U) -> U { let last_change_tick = self.last_change_tick(); let change_tick = self.change_tick(); @@ -1868,7 +1993,7 @@ impl World { .components .get_resource_id(TypeId::of::()) .unwrap_or_else(|| panic!("resource does not exist: {}", std::any::type_name::())); - let (ptr, mut ticks) = self + let (ptr, mut ticks, mut _caller) = self .storages .resources .get_mut(component_id) @@ -1885,6 +2010,8 @@ impl World { last_run: last_change_tick, this_run: change_tick, }, + #[cfg(feature = "track_change_detection")] + changed_by: &mut _caller, }; let result = f(self, value_mut); assert!(!self.contains_resource::(), @@ -1898,7 +2025,14 @@ impl World { self.storages .resources .get_mut(component_id) - .map(|info| info.insert_with_ticks(ptr, ticks)) + .map(|info| { + info.insert_with_ticks( + ptr, + ticks, + #[cfg(feature = "track_change_detection")] + _caller, + ); + }) .unwrap_or_else(|| { panic!( "No resource of type {} exists in the World.", @@ -1953,17 +2087,24 @@ impl World { /// # Safety /// The value referenced by `value` must be valid for the given [`ComponentId`] of this world. #[inline] + #[track_caller] pub unsafe fn insert_resource_by_id( &mut self, component_id: ComponentId, value: OwningPtr<'_>, + #[cfg(feature = "track_change_detection")] caller: &'static Location, ) { let change_tick = self.change_tick(); let resource = self.initialize_resource_internal(component_id); // SAFETY: `value` is valid for `component_id`, ensured by caller unsafe { - resource.insert(value, change_tick); + resource.insert( + value, + change_tick, + #[cfg(feature = "track_change_detection")] + caller, + ); } } @@ -1980,17 +2121,24 @@ impl World { /// # Safety /// The value referenced by `value` must be valid for the given [`ComponentId`] of this world. #[inline] + #[track_caller] pub unsafe fn insert_non_send_by_id( &mut self, component_id: ComponentId, value: OwningPtr<'_>, + #[cfg(feature = "track_change_detection")] caller: &'static Location, ) { let change_tick = self.change_tick(); let resource = self.initialize_non_send_internal(component_id); // SAFETY: `value` is valid for `component_id`, ensured by caller unsafe { - resource.insert(value, change_tick); + resource.insert( + value, + change_tick, + #[cfg(feature = "track_change_detection")] + caller, + ); } } @@ -2513,7 +2661,7 @@ impl World { .get_info(component_id) .debug_checked_unwrap() }; - let (ptr, ticks) = data.get_with_ticks()?; + let (ptr, ticks, _caller) = data.get_with_ticks()?; // SAFETY: // - We have exclusive access to the world, so no other code can be aliasing the `TickCells` @@ -2532,6 +2680,11 @@ impl World { // - We iterate one resource at a time, and we let go of each `PtrMut` before getting the next one value: unsafe { ptr.assert_unique() }, ticks, + #[cfg(feature = "track_change_detection")] + // SAFETY: + // - We have exclusive access to the world, so no other code can be aliasing the `Ptr` + // - We iterate one resource at a time, and we let go of each `PtrMut` before getting the next one + changed_by: unsafe { _caller.deref_mut() }, }; Some((component_info, mut_untyped)) @@ -3088,7 +3241,12 @@ mod tests { OwningPtr::make(value, |ptr| { // SAFETY: value is valid for the component layout unsafe { - world.insert_resource_by_id(component_id, ptr); + world.insert_resource_by_id( + component_id, + ptr, + #[cfg(feature = "track_change_detection")] + core::panic::Location::caller(), + ); } }); diff --git a/crates/bevy_ecs/src/world/spawn_batch.rs b/crates/bevy_ecs/src/world/spawn_batch.rs index f6cc9c9a2e6eb..ea6955a96d9ef 100644 --- a/crates/bevy_ecs/src/world/spawn_batch.rs +++ b/crates/bevy_ecs/src/world/spawn_batch.rs @@ -4,6 +4,8 @@ use crate::{ world::World, }; use std::iter::FusedIterator; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; /// An iterator that spawns a series of entities and returns the [ID](Entity) of /// each spawned entity. @@ -16,6 +18,8 @@ where { inner: I, spawner: BundleSpawner<'w>, + #[cfg(feature = "track_change_detection")] + caller: &'static Location<'static>, } impl<'w, I> SpawnBatchIter<'w, I> @@ -24,7 +28,12 @@ where I::Item: Bundle, { #[inline] - pub(crate) fn new(world: &'w mut World, iter: I) -> Self { + #[track_caller] + pub(crate) fn new( + world: &'w mut World, + iter: I, + #[cfg(feature = "track_change_detection")] caller: &'static Location, + ) -> Self { // Ensure all entity allocations are accounted for so `self.entities` can realloc if // necessary world.flush(); @@ -41,6 +50,8 @@ where Self { inner: iter, spawner, + #[cfg(feature = "track_change_detection")] + caller, } } } @@ -69,7 +80,13 @@ where fn next(&mut self) -> Option { let bundle = self.inner.next()?; // SAFETY: bundle matches spawner type - unsafe { Some(self.spawner.spawn(bundle)) } + unsafe { + Some(self.spawner.spawn( + bundle, + #[cfg(feature = "track_change_detection")] + self.caller, + )) + } } fn size_hint(&self) -> (usize, Option) { diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index bbcc4b58ba473..61beeff8442ef 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -6,7 +6,7 @@ use super::{Mut, Ref, World, WorldId}; use crate::{ archetype::{Archetype, Archetypes}, bundle::Bundles, - change_detection::{MutUntyped, Ticks, TicksMut}, + change_detection::{MaybeUnsafeCellLocation, MutUntyped, Ticks, TicksMut}, component::{ComponentId, ComponentTicks, Components, StorageType, Tick, TickCells}, entity::{Entities, Entity, EntityLocation}, observer::Observers, @@ -17,6 +17,8 @@ use crate::{ world::RawCommandQueue, }; use bevy_ptr::Ptr; +#[cfg(feature = "track_change_detection")] +use bevy_ptr::UnsafeCellDeref; use std::{any::TypeId, cell::UnsafeCell, fmt::Debug, marker::PhantomData, ptr}; /// Variant of the [`World`] where resource and component accesses take `&self`, and the responsibility to avoid @@ -334,7 +336,7 @@ impl<'w> UnsafeWorldCell<'w> { // SAFETY: caller ensures `self` has permission to access the resource // caller also ensure that no mutable reference to the resource exists - let (ptr, ticks) = unsafe { self.get_resource_with_ticks(component_id)? }; + let (ptr, ticks, _caller) = unsafe { self.get_resource_with_ticks(component_id)? }; // SAFETY: `component_id` was obtained from the type ID of `R` let value = unsafe { ptr.deref::() }; @@ -343,7 +345,16 @@ impl<'w> UnsafeWorldCell<'w> { let ticks = unsafe { Ticks::from_tick_cells(ticks, self.last_change_tick(), self.change_tick()) }; - Some(Res { value, ticks }) + // SAFETY: caller ensures that no mutable reference to the resource exists + #[cfg(feature = "track_change_detection")] + let caller = unsafe { _caller.deref() }; + + Some(Res { + value, + ticks, + #[cfg(feature = "track_change_detection")] + changed_by: caller, + }) } /// Gets a pointer to the resource with the id [`ComponentId`] if it exists. @@ -446,7 +457,7 @@ impl<'w> UnsafeWorldCell<'w> { ) -> Option> { // SAFETY: we only access data that the caller has ensured is unaliased and `self` // has permission to access. - let (ptr, ticks) = unsafe { self.storages() } + let (ptr, ticks, _caller) = unsafe { self.storages() } .resources .get(component_id)? .get_with_ticks()?; @@ -464,6 +475,11 @@ impl<'w> UnsafeWorldCell<'w> { // - caller ensures that the resource is unaliased value: unsafe { ptr.assert_unique() }, ticks, + #[cfg(feature = "track_change_detection")] + // SAFETY: + // - caller ensures that `self` has permission to access the resource + // - caller ensures that the resource is unaliased + changed_by: unsafe { _caller.deref_mut() }, }) } @@ -508,7 +524,7 @@ impl<'w> UnsafeWorldCell<'w> { let change_tick = self.change_tick(); // SAFETY: we only access data that the caller has ensured is unaliased and `self` // has permission to access. - let (ptr, ticks) = unsafe { self.storages() } + let (ptr, ticks, _caller) = unsafe { self.storages() } .non_send_resources .get(component_id)? .get_with_ticks()?; @@ -523,6 +539,9 @@ impl<'w> UnsafeWorldCell<'w> { // SAFETY: This function has exclusive access to the world so nothing aliases `ptr`. value: unsafe { ptr.assert_unique() }, ticks, + #[cfg(feature = "track_change_detection")] + // SAFETY: This function has exclusive access to the world + changed_by: unsafe { _caller.deref_mut() }, }) } @@ -536,7 +555,7 @@ impl<'w> UnsafeWorldCell<'w> { pub(crate) unsafe fn get_resource_with_ticks( self, component_id: ComponentId, - ) -> Option<(Ptr<'w>, TickCells<'w>)> { + ) -> Option<(Ptr<'w>, TickCells<'w>, MaybeUnsafeCellLocation<'w>)> { // SAFETY: // - caller ensures there is no `&mut World` // - caller ensures there are no mutable borrows of this resource @@ -560,7 +579,7 @@ impl<'w> UnsafeWorldCell<'w> { pub(crate) unsafe fn get_non_send_with_ticks( self, component_id: ComponentId, - ) -> Option<(Ptr<'w>, TickCells<'w>)> { + ) -> Option<(Ptr<'w>, TickCells<'w>, MaybeUnsafeCellLocation<'w>)> { // SAFETY: // - caller ensures there is no `&mut World` // - caller ensures there are no mutable borrows of this resource @@ -732,10 +751,12 @@ impl<'w> UnsafeEntityCell<'w> { self.entity, self.location, ) - .map(|(value, cells)| Ref { + .map(|(value, cells, _caller)| Ref { // SAFETY: returned component is of type T value: value.deref::(), ticks: Ticks::from_tick_cells(cells, last_change_tick, change_tick), + #[cfg(feature = "track_change_detection")] + changed_by: _caller.deref(), }) } } @@ -831,10 +852,12 @@ impl<'w> UnsafeEntityCell<'w> { self.entity, self.location, ) - .map(|(value, cells)| Mut { + .map(|(value, cells, _caller)| Mut { // SAFETY: returned component is of type T value: value.assert_unique().deref_mut::(), ticks: TicksMut::from_tick_cells(cells, last_change_tick, change_tick), + #[cfg(feature = "track_change_detection")] + changed_by: _caller.deref_mut(), }) } } @@ -891,7 +914,7 @@ impl<'w> UnsafeEntityCell<'w> { self.entity, self.location, ) - .map(|(value, cells)| MutUntyped { + .map(|(value, cells, _caller)| MutUntyped { // SAFETY: world access validated by caller and ties world lifetime to `MutUntyped` lifetime value: value.assert_unique(), ticks: TicksMut::from_tick_cells( @@ -899,6 +922,8 @@ impl<'w> UnsafeEntityCell<'w> { self.world.last_change_tick(), self.world.change_tick(), ), + #[cfg(feature = "track_change_detection")] + changed_by: _caller.deref_mut(), }) } } @@ -975,7 +1000,7 @@ unsafe fn get_component_and_ticks( storage_type: StorageType, entity: Entity, location: EntityLocation, -) -> Option<(Ptr<'_>, TickCells<'_>)> { +) -> Option<(Ptr<'_>, TickCells<'_>, MaybeUnsafeCellLocation<'_>)> { match storage_type { StorageType::Table => { let components = world.fetch_table(location, component_id)?; @@ -987,6 +1012,10 @@ unsafe fn get_component_and_ticks( added: components.get_added_tick_unchecked(location.table_row), changed: components.get_changed_tick_unchecked(location.table_row), }, + #[cfg(feature = "track_change_detection")] + components.get_changed_by_unchecked(location.table_row), + #[cfg(not(feature = "track_change_detection"))] + (), )) } StorageType::SparseSet => world.fetch_sparse_set(component_id)?.get_with_ticks(entity), diff --git a/crates/bevy_internal/Cargo.toml b/crates/bevy_internal/Cargo.toml index a07b357f2e2fe..bc15b6490815b 100644 --- a/crates/bevy_internal/Cargo.toml +++ b/crates/bevy_internal/Cargo.toml @@ -195,6 +195,9 @@ ios_simulator = ["bevy_pbr?/ios_simulator", "bevy_render?/ios_simulator"] # Enable built in global state machines bevy_state = ["dep:bevy_state"] +# Enables source location tracking for change detection, which can assist with debugging +track_change_detection = ["bevy_ecs/track_change_detection"] + # Enable function reflection reflect_functions = ["bevy_reflect/functions"] diff --git a/docs/cargo_features.md b/docs/cargo_features.md index 568048759b3c9..1c538a821bbf2 100644 --- a/docs/cargo_features.md +++ b/docs/cargo_features.md @@ -90,6 +90,7 @@ The default feature set enables most of the expected features of a game engine, |trace_chrome|Tracing support, saving a file in Chrome Tracing format| |trace_tracy|Tracing support, exposing a port for Tracy| |trace_tracy_memory|Tracing support, with memory profiling, exposing a port for Tracy| +|track_change_detection|Enables source location tracking for change detection, which can assist with debugging| |wav|WAV audio format support| |wayland|Wayland display server support| |webgpu|Enable support for WebGPU in Wasm. When enabled, this feature will override the `webgl2` feature and you won't be able to run Wasm builds with WebGL2, only with WebGPU.| diff --git a/examples/README.md b/examples/README.md index 56678ffd5ac25..5f77b5d47e5ec 100644 --- a/examples/README.md +++ b/examples/README.md @@ -269,7 +269,7 @@ Example | Description Example | Description --- | --- -[Component Change Detection](../examples/ecs/component_change_detection.rs) | Change detection on components +[Change Detection](../examples/ecs/change_detection.rs) | Change detection on components and resources [Component Hooks](../examples/ecs/component_hooks.rs) | Define component hooks to manage component lifecycle events [Custom Query Parameters](../examples/ecs/custom_query_param.rs) | Groups commonly used compound queries and query filters into a single type [Custom Schedule](../examples/ecs/custom_schedule.rs) | Demonstrates how to add custom schedules diff --git a/examples/ecs/change_detection.rs b/examples/ecs/change_detection.rs new file mode 100644 index 0000000000000..c605a4c6e8932 --- /dev/null +++ b/examples/ecs/change_detection.rs @@ -0,0 +1,106 @@ +//! This example illustrates how to react to component and resource changes. + +use bevy::prelude::*; +use rand::Rng; + +fn main() { + App::new() + .add_plugins(DefaultPlugins) + .add_systems(Startup, setup) + .add_systems( + Update, + ( + change_component, + change_component_2, + change_resource, + change_detection, + ), + ) + .run(); +} + +#[derive(Component, PartialEq, Debug)] +struct MyComponent(f32); + +#[derive(Resource, PartialEq, Debug)] +struct MyResource(f32); + +fn setup(mut commands: Commands) { + // Note the first change detection log correctly points to this line because the component is + // added. Although commands are deferred, they are able to track the original calling location. + commands.spawn(MyComponent(0.0)); + commands.insert_resource(MyResource(0.0)); +} + +fn change_component(time: Res