From 9575b20d31cafdefda1a5913960bcaf758b03245 Mon Sep 17 00:00:00 2001 From: Aevyrie Date: Tue, 30 Jul 2024 05:02:38 -0700 Subject: [PATCH] Track source location in change detection (#14034) # Objective - Make it possible to know *what* changed your component or resource. - Common need when debugging, when you want to know the last code location that mutated a value in the ECS. - This feature would be very useful for the editor alongside system stepping. ## Solution - Adds the caller location to column data. - Mutations now `track_caller` all the way up to the public API. - Commands that invoke these functions immediately call `Location::caller`, and pass this into the functions, instead of the functions themselves attempting to get the caller. This would not work for commands which are deferred, as the commands are executed by the scheduler, not the user's code. ## Testing - The `component_change_detection` example now shows where the component was mutated: ``` 2024-07-28T06:57:48.946022Z INFO component_change_detection: Entity { index: 1, generation: 1 }: New value: MyComponent(0.0) 2024-07-28T06:57:49.004371Z INFO component_change_detection: Entity { index: 1, generation: 1 }: New value: MyComponent(1.0) 2024-07-28T06:57:49.012738Z WARN component_change_detection: Change detected! -> value: Ref(MyComponent(1.0)) -> added: false -> changed: true -> changed by: examples/ecs/component_change_detection.rs:36:23 ``` - It's also possible to inspect change location from a debugger: image --- ## Changelog - Added source locations to ECS change detection behind the `track_change_detection` flag. ## Migration Guide - Added `changed_by` field to many internal ECS functions used with change detection when the `track_change_detection` feature flag is enabled. Use Location::caller() to provide the source of the function call. --------- Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com> Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- Cargo.toml | 14 +- crates/bevy_ecs/Cargo.toml | 3 +- crates/bevy_ecs/src/bundle.rs | 50 ++++- crates/bevy_ecs/src/change_detection.rs | 174 +++++++++++++++- crates/bevy_ecs/src/query/fetch.rs | 34 ++- crates/bevy_ecs/src/reflect/component.rs | 4 + crates/bevy_ecs/src/reflect/resource.rs | 2 + crates/bevy_ecs/src/storage/resource.rs | 47 ++++- crates/bevy_ecs/src/storage/sparse_set.rs | 46 +++- crates/bevy_ecs/src/storage/table.rs | 118 ++++++++++- crates/bevy_ecs/src/system/commands/mod.rs | 64 +++++- crates/bevy_ecs/src/system/system_param.rs | 91 +++++--- crates/bevy_ecs/src/world/entity_ref.rs | 36 +++- crates/bevy_ecs/src/world/mod.rs | 196 ++++++++++++++++-- crates/bevy_ecs/src/world/spawn_batch.rs | 21 +- .../bevy_ecs/src/world/unsafe_world_cell.rs | 51 ++++- crates/bevy_internal/Cargo.toml | 3 + docs/cargo_features.md | 1 + examples/README.md | 2 +- examples/ecs/change_detection.rs | 106 ++++++++++ examples/ecs/component_change_detection.rs | 58 ------ 21 files changed, 957 insertions(+), 164 deletions(-) create mode 100644 examples/ecs/change_detection.rs delete mode 100644 examples/ecs/component_change_detection.rs diff --git a/Cargo.toml b/Cargo.toml index 77c5685f8c4d2..f032cdd526a70 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"] @@ -1611,13 +1614,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 28912d99a8fd5..cc83f5de0a9a1 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 b761661bb6df3..e3d51405771bc 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 8af6a89d52203..956eb752cb945 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