From 21195a75e6f10e19bff192a9f2695dc675c8a6ee Mon Sep 17 00:00:00 2001 From: SpecificProtagonist Date: Tue, 17 Dec 2024 05:46:31 +0100 Subject: [PATCH] track_change_detection: Also track spawns/despawns (#16047) # Objective Expand `track_change_detection` feature to also track entity spawns and despawns. Use this to create better error messages. # Solution Adds `Entities::entity_get_spawned_or_despawned_by` as well as `{all entity reference types}::spawned_by`. This also removes the deprecated `get_many_entities_mut` & co (and therefore can't land in 0.15) because we don't yet have no Polonius. ## Testing Added a test that checks that the locations get updated and these updates are ordered correctly vs hooks & observers. --- ## Showcase Access location: ```rust let mut world = World::new(); let entity = world.spawn_empty().id(); println!("spawned by: {}", world.entity(entity).spawned_by()); ``` ``` spawned by: src/main.rs:5:24 ``` Error message (with `track_change_detection`): ```rust world.despawn(entity); world.entity(entity); ``` ``` thread 'main' panicked at src/main.rs:11:11: Entity 0v1#4294967296 was despawned by src/main.rs:10:11 ``` and without: ``` thread 'main' panicked at src/main.rs:11:11: Entity 0v1#4294967296 does not exist (enable `track_change_detection` feature for more details) ``` Similar error messages now also exists for `Query::get`, `World::entity_mut`, `EntityCommands` creation and everything that causes `B0003`, e.g. ``` error[B0003]: Could not insert a bundle (of type `MaterialMeshBundle`) for entity Entity { index: 7, generation: 1 }, which was despawned by src/main.rs:10:11. See: https://bevyengine.org/learn/errors/#b0003 ``` --------- Co-authored-by: kurk070ff <108901106+kurk070ff@users.noreply.github.com> Co-authored-by: Freya Pines Co-authored-by: Freya Pines Co-authored-by: Matty Weatherley --- Cargo.toml | 2 +- crates/bevy_ecs/src/entity/mod.rs | 49 +++++- crates/bevy_ecs/src/query/error.rs | 33 ++-- crates/bevy_ecs/src/query/state.rs | 8 +- .../bevy_ecs/src/reflect/entity_commands.rs | 3 +- crates/bevy_ecs/src/system/commands/mod.rs | 39 +++-- crates/bevy_ecs/src/world/entity_fetch.rs | 10 +- crates/bevy_ecs/src/world/entity_ref.rs | 143 +++++++++++++++++- crates/bevy_ecs/src/world/error.rs | 60 +++++++- crates/bevy_ecs/src/world/mod.rs | 131 +++++++++++----- .../bevy_ecs/src/world/unsafe_world_cell.rs | 13 +- crates/bevy_render/src/render_phase/draw.rs | 2 +- crates/bevy_transform/src/helper.rs | 2 +- docs/cargo_features.md | 2 +- 14 files changed, 402 insertions(+), 95 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5b1e65ae50..eb7b377c5a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -451,7 +451,7 @@ 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 +# Enables source location tracking for change detection and spawning/despawning, which can assist with debugging track_change_detection = ["bevy_internal/track_change_detection"] # Enable function reflection diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 68a89f660f..6c2476169e 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -61,6 +61,8 @@ use crate::{ }, storage::{SparseSetIndex, TableId, TableRow}, }; +#[cfg(feature = "track_change_detection")] +use core::panic::Location; use core::{fmt, hash::Hash, mem, num::NonZero, sync::atomic::Ordering}; #[cfg(feature = "serialize")] use serde::{Deserialize, Serialize}; @@ -938,6 +940,46 @@ impl Entities { pub fn is_empty(&self) -> bool { self.len == 0 } + + /// Sets the source code location from which this entity has last been spawned + /// or despawned. + #[cfg(feature = "track_change_detection")] + #[inline] + pub(crate) fn set_spawned_or_despawned_by(&mut self, index: u32, caller: &'static Location) { + let meta = self + .meta + .get_mut(index as usize) + .expect("Entity index invalid"); + meta.spawned_or_despawned_by = Some(caller); + } + + /// Returns the source code location from which this entity has last been spawned + /// or despawned. Returns `None` if this entity has never existed. + #[cfg(feature = "track_change_detection")] + pub fn entity_get_spawned_or_despawned_by( + &self, + entity: Entity, + ) -> Option<&'static Location<'static>> { + self.meta + .get(entity.index() as usize) + .and_then(|meta| meta.spawned_or_despawned_by) + } + + /// Constructs a message explaining why an entity does not exists, if known. + pub(crate) fn entity_does_not_exist_error_details_message(&self, _entity: Entity) -> String { + #[cfg(feature = "track_change_detection")] + { + if let Some(location) = self.entity_get_spawned_or_despawned_by(_entity) { + format!("was despawned by {location}",) + } else { + "was never spawned".to_owned() + } + } + #[cfg(not(feature = "track_change_detection"))] + { + "does not exist (enable `track_change_detection` feature for more details)".to_owned() + } + } } #[derive(Copy, Clone, Debug)] @@ -946,6 +988,9 @@ struct EntityMeta { pub generation: NonZero, /// The current location of the [`Entity`] pub location: EntityLocation, + /// Location of the last spawn or despawn of this entity + #[cfg(feature = "track_change_detection")] + spawned_or_despawned_by: Option<&'static Location<'static>>, } impl EntityMeta { @@ -953,10 +998,12 @@ impl EntityMeta { const EMPTY: EntityMeta = EntityMeta { generation: NonZero::::MIN, location: EntityLocation::INVALID, + #[cfg(feature = "track_change_detection")] + spawned_or_despawned_by: None, }; } -/// Records where an entity's data is stored. +/// A location of an entity in an archetype. #[derive(Copy, Clone, Debug, PartialEq)] pub struct EntityLocation { /// The ID of the [`Archetype`] the [`Entity`] belongs to. diff --git a/crates/bevy_ecs/src/query/error.rs b/crates/bevy_ecs/src/query/error.rs index 97cc011d39..9746471c66 100644 --- a/crates/bevy_ecs/src/query/error.rs +++ b/crates/bevy_ecs/src/query/error.rs @@ -11,7 +11,7 @@ pub enum QueryEntityError<'w> { /// Either it does not have a requested component, or it has a component which the query filters out. QueryDoesNotMatch(Entity, UnsafeWorldCell<'w>), /// The given [`Entity`] does not exist. - NoSuchEntity(Entity), + NoSuchEntity(Entity, UnsafeWorldCell<'w>), /// The [`Entity`] was requested mutably more than once. /// /// See [`QueryState::get_many_mut`](crate::query::QueryState::get_many_mut) for an example. @@ -26,15 +26,22 @@ impl<'w> core::fmt::Display for QueryEntityError<'w> { Self::QueryDoesNotMatch(entity, world) => { write!( f, - "The query does not match the entity {entity}, which has components " + "The query does not match entity {entity}, which has components " )?; format_archetype(f, world, entity) } - Self::NoSuchEntity(entity) => write!(f, "The entity {entity} does not exist"), - Self::AliasedMutability(entity) => write!( - f, - "The entity {entity} was requested mutably more than once" - ), + Self::NoSuchEntity(entity, world) => { + write!( + f, + "Entity {entity} {}", + world + .entities() + .entity_does_not_exist_error_details_message(entity) + ) + } + Self::AliasedMutability(entity) => { + write!(f, "Entity {entity} was requested mutably more than once") + } } } } @@ -47,7 +54,15 @@ impl<'w> core::fmt::Debug for QueryEntityError<'w> { format_archetype(f, world, entity)?; write!(f, ")") } - Self::NoSuchEntity(entity) => write!(f, "NoSuchEntity({entity})"), + Self::NoSuchEntity(entity, world) => { + write!( + f, + "NoSuchEntity({entity} {})", + world + .entities() + .entity_does_not_exist_error_details_message(entity) + ) + } Self::AliasedMutability(entity) => write!(f, "AliasedMutability({entity})"), } } @@ -79,7 +94,7 @@ impl<'w> PartialEq for QueryEntityError<'w> { fn eq(&self, other: &Self) -> bool { match (self, other) { (Self::QueryDoesNotMatch(e1, _), Self::QueryDoesNotMatch(e2, _)) if e1 == e2 => true, - (Self::NoSuchEntity(e1), Self::NoSuchEntity(e2)) if e1 == e2 => true, + (Self::NoSuchEntity(e1, _), Self::NoSuchEntity(e2, _)) if e1 == e2 => true, (Self::AliasedMutability(e1), Self::AliasedMutability(e2)) if e1 == e2 => true, _ => false, } diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 52cae9d5b0..997c5133ec 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -841,7 +841,7 @@ impl QueryState { /// /// let wrong_entity = Entity::from_raw(365); /// - /// assert_eq!(query_state.get_many(&world, [wrong_entity]), Err(QueryEntityError::NoSuchEntity(wrong_entity))); + /// assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::NoSuchEntity(entity, _) => entity, _ => panic!()}, wrong_entity); /// ``` #[inline] pub fn get_many<'w, const N: usize>( @@ -921,7 +921,7 @@ impl QueryState { /// let wrong_entity = Entity::from_raw(57); /// let invalid_entity = world.spawn_empty().id(); /// - /// assert_eq!(query_state.get_many_mut(&mut world, [wrong_entity]).unwrap_err(), QueryEntityError::NoSuchEntity(wrong_entity)); + /// assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::NoSuchEntity(entity, _) => entity, _ => panic!()}, wrong_entity); /// assert_eq!(match query_state.get_many_mut(&mut world, [invalid_entity]).unwrap_err() {QueryEntityError::QueryDoesNotMatch(entity, _) => entity, _ => panic!()}, invalid_entity); /// assert_eq!(query_state.get_many_mut(&mut world, [entities[0], entities[0]]).unwrap_err(), QueryEntityError::AliasedMutability(entities[0])); /// ``` @@ -1018,7 +1018,7 @@ impl QueryState { let location = world .entities() .get(entity) - .ok_or(QueryEntityError::NoSuchEntity(entity))?; + .ok_or(QueryEntityError::NoSuchEntity(entity, world))?; if !self .matched_archetypes .contains(location.archetype_id.index()) @@ -1495,7 +1495,7 @@ impl QueryState { /// # let wrong_entity = Entity::from_raw(57); /// # let invalid_entity = world.spawn_empty().id(); /// - /// # assert_eq!(query_state.get_many_mut(&mut world, [wrong_entity]).unwrap_err(), QueryEntityError::NoSuchEntity(wrong_entity)); + /// # assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::NoSuchEntity(entity, _) => entity, _ => panic!()}, wrong_entity); /// assert_eq!(match query_state.get_many_mut(&mut world, [invalid_entity]).unwrap_err() {QueryEntityError::QueryDoesNotMatch(entity, _) => entity, _ => panic!()}, invalid_entity); /// # assert_eq!(query_state.get_many_mut(&mut world, [entities[0], entities[0]]).unwrap_err(), QueryEntityError::AliasedMutability(entities[0])); /// ``` diff --git a/crates/bevy_ecs/src/reflect/entity_commands.rs b/crates/bevy_ecs/src/reflect/entity_commands.rs index 60c89c7f7f..2f24ea4542 100644 --- a/crates/bevy_ecs/src/reflect/entity_commands.rs +++ b/crates/bevy_ecs/src/reflect/entity_commands.rs @@ -222,7 +222,8 @@ fn insert_reflect( .expect("component should represent a type."); let type_path = type_info.type_path(); let Ok(mut entity) = world.get_entity_mut(entity) else { - panic!("error[B0003]: Could not insert a reflected component (of type {type_path}) for entity {entity:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003"); + panic!("error[B0003]: Could not insert a reflected component (of type {type_path}) for entity {entity}, which {}. See: https://bevyengine.org/learn/errors/b0003", + world.entities().entity_does_not_exist_error_details_message(entity)); }; let Some(type_registration) = type_registry.get(type_info.type_id()) else { panic!("`{type_path}` should be registered in type registry via `App::register_type<{type_path}>`"); diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index f2d578bef3..03f69acd61 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -329,10 +329,16 @@ impl<'w, 's> Commands<'w, 's> { /// apps, and only when they have a scheme worked out to share an ID space (which doesn't happen /// by default). #[deprecated(since = "0.15.0", note = "use Commands::spawn instead")] + #[track_caller] pub fn get_or_spawn(&mut self, entity: Entity) -> EntityCommands { + #[cfg(feature = "track_change_detection")] + let caller = Location::caller(); self.queue(move |world: &mut World| { - #[allow(deprecated)] - world.get_or_spawn(entity); + world.get_or_spawn_with_caller( + entity, + #[cfg(feature = "track_change_detection")] + caller, + ); }); EntityCommands { entity, @@ -440,15 +446,20 @@ impl<'w, 's> Commands<'w, 's> { #[inline(never)] #[cold] #[track_caller] - fn panic_no_entity(entity: Entity) -> ! { + fn panic_no_entity(entities: &Entities, entity: Entity) -> ! { panic!( - "Attempting to create an EntityCommands for entity {entity:?}, which doesn't exist.", + "Attempting to create an EntityCommands for entity {entity:?}, which {}", + entities.entity_does_not_exist_error_details_message(entity) ); } - match self.get_entity(entity) { - Some(entity) => entity, - None => panic_no_entity(entity), + if self.get_entity(entity).is_some() { + EntityCommands { + entity, + commands: self.reborrow(), + } + } else { + panic_no_entity(self.entities, entity) } } @@ -1345,8 +1356,8 @@ impl<'a> EntityCommands<'a> { ) -> &mut Self { let caller = Location::caller(); // SAFETY: same invariants as parent call - self.queue(unsafe {insert_by_id(component_id, value, move |entity| { - panic!("error[B0003]: {caller}: Could not insert a component {component_id:?} (with type {}) for entity {entity:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::()); + self.queue(unsafe {insert_by_id(component_id, value, move |world, entity| { + panic!("error[B0003]: {caller}: Could not insert a component {component_id:?} (with type {}) for entity {entity:?}, which {}. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), world.entities().entity_does_not_exist_error_details_message(entity)); })}) } @@ -1364,7 +1375,7 @@ impl<'a> EntityCommands<'a> { value: T, ) -> &mut Self { // SAFETY: same invariants as parent call - self.queue(unsafe { insert_by_id(component_id, value, |_| {}) }) + self.queue(unsafe { insert_by_id(component_id, value, |_, _| {}) }) } /// Tries to add a [`Bundle`] of components to the entity. @@ -2203,7 +2214,7 @@ fn insert(bundle: T, mode: InsertMode) -> impl EntityCommand { caller, ); } else { - panic!("error[B0003]: {caller}: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), entity); + panic!("error[B0003]: {caller}: Could not insert a bundle (of type `{}`) for entity {entity:?}, which {}. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), world.entities().entity_does_not_exist_error_details_message(entity)); } } } @@ -2222,7 +2233,7 @@ fn insert_from_world(mode: InsertMode) -> impl EntityC caller, ); } else { - panic!("error[B0003]: {caller}: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), entity); + panic!("error[B0003]: {caller}: Could not insert a bundle (of type `{}`) for {entity:?}, which {}. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), world.entities().entity_does_not_exist_error_details_message(entity) ); } } } @@ -2254,7 +2265,7 @@ fn try_insert(bundle: impl Bundle, mode: InsertMode) -> impl EntityCommand { unsafe fn insert_by_id( component_id: ComponentId, value: T, - on_none_entity: impl FnOnce(Entity) + Send + 'static, + on_none_entity: impl FnOnce(&mut World, Entity) + Send + 'static, ) -> impl EntityCommand { move |entity: Entity, world: &mut World| { if let Ok(mut entity) = world.get_entity_mut(entity) { @@ -2265,7 +2276,7 @@ unsafe fn insert_by_id( entity.insert_by_id(component_id, ptr); }); } else { - on_none_entity(entity); + on_none_entity(world, entity); } } } diff --git a/crates/bevy_ecs/src/world/entity_fetch.rs b/crates/bevy_ecs/src/world/entity_fetch.rs index 62d63ced54..8464420a90 100644 --- a/crates/bevy_ecs/src/world/entity_fetch.rs +++ b/crates/bevy_ecs/src/world/entity_fetch.rs @@ -123,7 +123,7 @@ unsafe impl WorldEntityFetch for Entity { let location = cell .entities() .get(self) - .ok_or(EntityFetchError::NoSuchEntity(self))?; + .ok_or(EntityFetchError::NoSuchEntity(self, cell))?; // SAFETY: caller ensures that the world cell has mutable access to the entity. let world = unsafe { cell.world_mut() }; // SAFETY: location was fetched from the same world's `Entities`. @@ -136,7 +136,7 @@ unsafe impl WorldEntityFetch for Entity { ) -> Result, EntityFetchError> { let ecell = cell .get_entity(self) - .ok_or(EntityFetchError::NoSuchEntity(self))?; + .ok_or(EntityFetchError::NoSuchEntity(self, cell))?; // SAFETY: caller ensures that the world cell has mutable access to the entity. Ok(unsafe { EntityMut::new(ecell) }) } @@ -210,7 +210,7 @@ unsafe impl WorldEntityFetch for &'_ [Entity; N] { for (r, &id) in core::iter::zip(&mut refs, self) { let ecell = cell .get_entity(id) - .ok_or(EntityFetchError::NoSuchEntity(id))?; + .ok_or(EntityFetchError::NoSuchEntity(id, cell))?; // SAFETY: caller ensures that the world cell has mutable access to the entity. *r = MaybeUninit::new(unsafe { EntityMut::new(ecell) }); } @@ -268,7 +268,7 @@ unsafe impl WorldEntityFetch for &'_ [Entity] { for &id in self { let ecell = cell .get_entity(id) - .ok_or(EntityFetchError::NoSuchEntity(id))?; + .ok_or(EntityFetchError::NoSuchEntity(id, cell))?; // SAFETY: caller ensures that the world cell has mutable access to the entity. refs.push(unsafe { EntityMut::new(ecell) }); } @@ -313,7 +313,7 @@ unsafe impl WorldEntityFetch for &'_ EntityHashSet { for &id in self { let ecell = cell .get_entity(id) - .ok_or(EntityFetchError::NoSuchEntity(id))?; + .ok_or(EntityFetchError::NoSuchEntity(id, cell))?; // SAFETY: caller ensures that the world cell has mutable access to the entity. refs.insert(id, unsafe { EntityMut::new(ecell) }); } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 697dd34554..c02bbd0b34 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -14,6 +14,8 @@ use crate::{ }; use bevy_ptr::{OwningPtr, Ptr}; use bevy_utils::{HashMap, HashSet}; +#[cfg(feature = "track_change_detection")] +use core::panic::Location; use core::{any::TypeId, marker::PhantomData, mem::MaybeUninit}; use thiserror::Error; @@ -272,6 +274,12 @@ impl<'w> EntityRef<'w> { // SAFETY: We have read-only access to all components of this entity. unsafe { self.0.get_components::() } } + + /// Returns the source code location from which this entity has been spawned. + #[cfg(feature = "track_change_detection")] + pub fn spawned_by(&self) -> &'static Location<'static> { + self.0.spawned_by() + } } impl<'w> From> for EntityRef<'w> { @@ -802,6 +810,12 @@ impl<'w> EntityMut<'w> { // - We have exclusive access to all components of this entity. unsafe { component_ids.fetch_mut(self.0) } } + + /// Returns the source code location from which this entity has been spawned. + #[cfg(feature = "track_change_detection")] + pub fn spawned_by(&self) -> &'static Location<'static> { + self.0.spawned_by() + } } impl<'w> From<&'w mut EntityMut<'_>> for EntityMut<'w> { @@ -876,7 +890,13 @@ impl<'w> EntityWorldMut<'w> { #[inline(never)] #[cold] fn panic_despawned(&self) -> ! { - panic!("Entity {} has been despawned, possibly by hooks or observers, so must not be accessed through EntityWorldMut after despawn.", self.entity); + panic!( + "Entity {} {}", + self.entity, + self.world + .entities() + .entity_does_not_exist_error_details_message(self.entity) + ); } #[inline(always)] @@ -1304,7 +1324,7 @@ impl<'w> EntityWorldMut<'w> { bundle, InsertMode::Replace, #[cfg(feature = "track_change_detection")] - core::panic::Location::caller(), + Location::caller(), ) } @@ -1322,7 +1342,7 @@ impl<'w> EntityWorldMut<'w> { bundle, InsertMode::Keep, #[cfg(feature = "track_change_detection")] - core::panic::Location::caller(), + Location::caller(), ) } @@ -1333,7 +1353,7 @@ impl<'w> EntityWorldMut<'w> { &mut self, bundle: T, mode: InsertMode, - #[cfg(feature = "track_change_detection")] caller: &'static core::panic::Location, + #[cfg(feature = "track_change_detection")] caller: &'static Location, ) -> &mut Self { self.assert_not_despawned(); let change_tick = self.world.change_tick(); @@ -1873,7 +1893,18 @@ impl<'w> EntityWorldMut<'w> { /// # Panics /// /// If the entity has been despawned while this `EntityWorldMut` is still alive. + #[track_caller] pub fn despawn(self) { + self.despawn_with_caller( + #[cfg(feature = "track_change_detection")] + Location::caller(), + ); + } + + pub(crate) fn despawn_with_caller( + self, + #[cfg(feature = "track_change_detection")] caller: &'static Location, + ) { self.assert_not_despawned(); let world = self.world; let archetype = &world.archetypes[self.location.archetype_id]; @@ -1962,6 +1993,16 @@ impl<'w> EntityWorldMut<'w> { .set_entity_table_row(moved_location.archetype_row, table_row); } world.flush(); + + #[cfg(feature = "track_change_detection")] + { + // SAFETY: No structural changes + unsafe { + world + .entities_mut() + .set_spawned_or_despawned_by(self.entity.index(), caller); + } + } } /// Ensures any commands triggered by the actions of Self are applied, equivalent to [`World::flush`] @@ -2287,6 +2328,15 @@ impl<'w> EntityWorldMut<'w> { self.update_location(); self } + + /// Returns the source code location from which this entity has last been spawned. + #[cfg(feature = "track_change_detection")] + pub fn spawned_by(&self) -> &'static Location<'static> { + self.world() + .entities() + .entity_get_spawned_or_despawned_by(self.entity) + .unwrap() + } } /// # Safety @@ -2820,6 +2870,12 @@ impl<'w> FilteredEntityRef<'w> { .then(|| unsafe { self.entity.get_by_id(component_id) }) .flatten() } + + /// Returns the source code location from which this entity has been spawned. + #[cfg(feature = "track_change_detection")] + pub fn spawned_by(&self) -> &'static Location<'static> { + self.entity.spawned_by() + } } impl<'w> From> for FilteredEntityRef<'w> { @@ -3141,6 +3197,12 @@ impl<'w> FilteredEntityMut<'w> { .then(|| unsafe { self.entity.get_mut_by_id(component_id).ok() }) .flatten() } + + /// Returns the source code location from which this entity has last been spawned. + #[cfg(feature = "track_change_detection")] + pub fn spawned_by(&self) -> &'static Location<'static> { + self.entity.spawned_by() + } } impl<'a> From> for FilteredEntityMut<'a> { @@ -3279,6 +3341,12 @@ where unsafe { self.entity.get_ref() } } } + + /// Returns the source code location from which this entity has been spawned. + #[cfg(feature = "track_change_detection")] + pub fn spawned_by(&self) -> &'static Location<'static> { + self.entity.spawned_by() + } } impl<'a, B> From<&'a EntityMutExcept<'_, B>> for EntityRefExcept<'a, B> @@ -3387,6 +3455,12 @@ where unsafe { self.entity.get_mut() } } } + + /// Returns the source code location from which this entity has been spawned. + #[cfg(feature = "track_change_detection")] + pub fn spawned_by(&self) -> &'static Location<'static> { + self.entity.spawned_by() + } } fn bundle_contains_component(components: &Components, query_id: ComponentId) -> bool @@ -3445,7 +3519,7 @@ unsafe fn insert_dynamic_bundle< bundle, InsertMode::Replace, #[cfg(feature = "track_change_detection")] - core::panic::Location::caller(), + Location::caller(), ) } } @@ -3747,6 +3821,10 @@ unsafe impl DynamicComponentFetch for &'_ HashSet { mod tests { use bevy_ptr::{OwningPtr, Ptr}; use core::panic::AssertUnwindSafe; + #[cfg(feature = "track_change_detection")] + use core::panic::Location; + #[cfg(feature = "track_change_detection")] + use std::sync::OnceLock; use crate::{ self as bevy_ecs, @@ -4756,9 +4834,7 @@ mod tests { } #[test] - #[should_panic( - expected = "Entity 1v1 has been despawned, possibly by hooks or observers, so must not be accessed through EntityWorldMut after despawn." - )] + #[should_panic] fn location_on_despawned_entity_panics() { let mut world = World::new(); world.add_observer( @@ -5027,4 +5103,55 @@ mod tests { assert_eq!(world.entity(entity_a).get::(), None); assert_eq!(world.entity(entity_b).get::(), Some(&D)); } + + #[test] + #[cfg(feature = "track_change_detection")] + fn update_despawned_by_after_observers() { + let mut world = World::new(); + + #[derive(Component)] + #[component(on_remove = get_tracked)] + struct C; + + static TRACKED: OnceLock<&'static Location<'static>> = OnceLock::new(); + fn get_tracked(world: DeferredWorld, entity: Entity, _: ComponentId) { + TRACKED.get_or_init(|| { + world + .entities + .entity_get_spawned_or_despawned_by(entity) + .unwrap() + }); + } + + #[track_caller] + fn caller_spawn(world: &mut World) -> (Entity, &'static Location<'static>) { + let caller = Location::caller(); + (world.spawn(C).id(), caller) + } + let (entity, spawner) = caller_spawn(&mut world); + + assert_eq!( + spawner, + world + .entities() + .entity_get_spawned_or_despawned_by(entity) + .unwrap() + ); + + #[track_caller] + fn caller_despawn(world: &mut World, entity: Entity) -> &'static Location<'static> { + world.despawn(entity); + Location::caller() + } + let despawner = caller_despawn(&mut world, entity); + + assert_eq!(spawner, *TRACKED.get().unwrap()); + assert_eq!( + despawner, + world + .entities() + .entity_get_spawned_or_despawned_by(entity) + .unwrap() + ); + } } diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index 91cd2fa034..7f137fa012 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -4,6 +4,8 @@ use thiserror::Error; use crate::{component::ComponentId, entity::Entity, schedule::InternedScheduleLabel}; +use super::unsafe_world_cell::UnsafeWorldCell; + /// The error type returned by [`World::try_run_schedule`] if the provided schedule does not exist. /// /// [`World::try_run_schedule`]: crate::world::World::try_run_schedule @@ -23,12 +25,60 @@ pub enum EntityComponentError { } /// An error that occurs when fetching entities mutably from a world. -#[derive(Error, Debug, Clone, Copy, PartialEq, Eq)] -pub enum EntityFetchError { +#[derive(Clone, Copy)] +pub enum EntityFetchError<'w> { /// The entity with the given ID does not exist. - #[error("The entity with ID {0:?} does not exist.")] - NoSuchEntity(Entity), + NoSuchEntity(Entity, UnsafeWorldCell<'w>), /// The entity with the given ID was requested mutably more than once. - #[error("The entity with ID {0:?} was requested mutably more than once.")] AliasedMutability(Entity), } + +impl<'w> core::error::Error for EntityFetchError<'w> {} + +impl<'w> core::fmt::Display for EntityFetchError<'w> { + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + match *self { + Self::NoSuchEntity(entity, world) => { + write!( + f, + "Entity {entity} {}", + world + .entities() + .entity_does_not_exist_error_details_message(entity) + ) + } + Self::AliasedMutability(entity) => { + write!(f, "Entity {entity} was requested mutably more than once") + } + } + } +} + +impl<'w> core::fmt::Debug for EntityFetchError<'w> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match *self { + Self::NoSuchEntity(entity, world) => { + write!( + f, + "NoSuchEntity({entity} {})", + world + .entities() + .entity_does_not_exist_error_details_message(entity) + ) + } + Self::AliasedMutability(entity) => write!(f, "AliasedMutability({entity})"), + } + } +} + +impl<'w> PartialEq for EntityFetchError<'w> { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Self::NoSuchEntity(e1, _), Self::NoSuchEntity(e2, _)) if e1 == e2 => true, + (Self::AliasedMutability(e1), Self::AliasedMutability(e2)) if e1 == e2 => true, + _ => false, + } + } +} + +impl<'w> Eq for EntityFetchError<'w> {} diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index c2a5199f86..63d6ea725a 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -677,7 +677,7 @@ impl World { /// } /// ``` /// - /// ## [`EntityHashSet`] + /// ## [`EntityHashSet`](crate::entity::EntityHashMap) /// /// ``` /// # use bevy_ecs::{prelude::*, entity::EntityHashSet}; @@ -703,13 +703,18 @@ impl World { #[inline(never)] #[cold] #[track_caller] - fn panic_no_entity(entity: Entity) -> ! { - panic!("Entity {entity:?} does not exist"); + fn panic_no_entity(world: &World, entity: Entity) -> ! { + panic!( + "Entity {entity:?} {}", + world + .entities + .entity_does_not_exist_error_details_message(entity) + ); } match self.get_entity(entities) { Ok(fetched) => fetched, - Err(entity) => panic_no_entity(entity), + Err(entity) => panic_no_entity(self, entity), } } @@ -724,7 +729,7 @@ impl World { /// such as adding or removing components, or despawning the entity. /// - Pass a slice of [`Entity`]s to receive a [`Vec`]. /// - Pass an array of [`Entity`]s to receive an equally-sized array of [`EntityMut`]s. - /// - Pass a reference to a [`EntityHashSet`] to receive an + /// - Pass a reference to a [`EntityHashSet`](crate::entity::EntityHashMap) to receive an /// [`EntityHashMap`](crate::entity::EntityHashMap). /// /// In order to perform structural changes on the returned entity reference, @@ -806,7 +811,7 @@ impl World { /// } /// ``` /// - /// ## [`EntityHashSet`] + /// ## [`EntityHashSet`](crate::entity::EntityHashMap) /// /// ``` /// # use bevy_ecs::{prelude::*, entity::EntityHashSet}; @@ -947,6 +952,19 @@ impl World { #[inline] #[deprecated(since = "0.15.0", note = "use `World::spawn` instead")] pub fn get_or_spawn(&mut self, entity: Entity) -> Option { + self.get_or_spawn_with_caller( + entity, + #[cfg(feature = "track_change_detection")] + Location::caller(), + ) + } + + #[inline] + pub(crate) fn get_or_spawn_with_caller( + &mut self, + entity: Entity, + #[cfg(feature = "track_change_detection")] caller: &'static Location, + ) -> Option { self.flush(); match self.entities.alloc_at_without_replacement(entity) { AllocAtWithoutReplacement::Exists(location) => { @@ -955,7 +973,13 @@ impl World { } AllocAtWithoutReplacement::DidNotExist => { // SAFETY: entity was just allocated - Some(unsafe { self.spawn_at_empty_internal(entity) }) + Some(unsafe { + self.spawn_at_empty_internal( + entity, + #[cfg(feature = "track_change_detection")] + caller, + ) + }) } AllocAtWithoutReplacement::ExistsWithWrongGeneration => None, } @@ -970,7 +994,7 @@ impl World { /// - Pass an [`Entity`] to receive a single [`EntityRef`]. /// - Pass a slice of [`Entity`]s to receive a [`Vec`]. /// - Pass an array of [`Entity`]s to receive an equally-sized array of [`EntityRef`]s. - /// - Pass a reference to a [`EntityHashSet`] to receive an + /// - Pass a reference to a [`EntityHashSet`](crate::entity::EntityHashMap) to receive an /// [`EntityHashMap`](crate::entity::EntityHashMap). /// /// # Errors @@ -1063,7 +1087,7 @@ impl World { /// such as adding or removing components, or despawning the entity. /// - Pass a slice of [`Entity`]s to receive a [`Vec`]. /// - Pass an array of [`Entity`]s to receive an equally-sized array of [`EntityMut`]s. - /// - Pass a reference to a [`EntityHashSet`] to receive an + /// - Pass a reference to a [`EntityHashSet`](crate::entity::EntityHashMap) to receive an /// [`EntityHashMap`](crate::entity::EntityHashMap). /// /// In order to perform structural changes on the returned entity reference, @@ -1178,7 +1202,9 @@ impl World { entities: [Entity; N], ) -> Result<[EntityMut<'_>; N], QueryEntityError<'_>> { self.get_entity_mut(entities).map_err(|e| match e { - EntityFetchError::NoSuchEntity(entity) => QueryEntityError::NoSuchEntity(entity), + EntityFetchError::NoSuchEntity(entity, world) => { + QueryEntityError::NoSuchEntity(entity, world) + } EntityFetchError::AliasedMutability(entity) => { QueryEntityError::AliasedMutability(entity) } @@ -1215,7 +1241,9 @@ impl World { entities: &[Entity], ) -> Result>, QueryEntityError<'w>> { self.get_entity_mut(entities).map_err(|e| match e { - EntityFetchError::NoSuchEntity(entity) => QueryEntityError::NoSuchEntity(entity), + EntityFetchError::NoSuchEntity(entity, world) => { + QueryEntityError::NoSuchEntity(entity, world) + } EntityFetchError::AliasedMutability(entity) => { QueryEntityError::AliasedMutability(entity) } @@ -1259,7 +1287,9 @@ impl World { self.get_entity_mut(entities) .map(|fetched| fetched.into_values().collect()) .map_err(|e| match e { - EntityFetchError::NoSuchEntity(entity) => QueryEntityError::NoSuchEntity(entity), + EntityFetchError::NoSuchEntity(entity, world) => { + QueryEntityError::NoSuchEntity(entity, world) + } EntityFetchError::AliasedMutability(entity) => { QueryEntityError::AliasedMutability(entity) } @@ -1291,11 +1321,18 @@ impl World { /// let position = world.entity(entity).get::().unwrap(); /// assert_eq!(position.x, 0.0); /// ``` + #[track_caller] pub fn spawn_empty(&mut self) -> EntityWorldMut { self.flush(); let entity = self.entities.alloc(); // SAFETY: entity was just allocated - unsafe { self.spawn_at_empty_internal(entity) } + unsafe { + self.spawn_at_empty_internal( + entity, + #[cfg(feature = "track_change_detection")] + Location::caller(), + ) + } } /// Spawns a new [`Entity`] with a given [`Bundle`] of [components](`Component`) and returns @@ -1376,23 +1413,33 @@ impl World { } }; + #[cfg(feature = "track_change_detection")] + self.entities + .set_spawned_or_despawned_by(entity.index(), Location::caller()); + // SAFETY: entity and location are valid, as they were just created above unsafe { EntityWorldMut::new(self, entity, entity_location) } } /// # Safety /// must be called on an entity that was just allocated - unsafe fn spawn_at_empty_internal(&mut self, entity: Entity) -> EntityWorldMut { + unsafe fn spawn_at_empty_internal( + &mut self, + entity: Entity, + #[cfg(feature = "track_change_detection")] caller: &'static Location, + ) -> EntityWorldMut { let archetype = self.archetypes.empty_mut(); // PERF: consider avoiding allocating entities in the empty archetype unless needed let table_row = self.storages.tables[archetype.table_id()].allocate(entity); // SAFETY: no components are allocated by archetype.allocate() because the archetype is // empty let location = unsafe { archetype.allocate(entity, table_row) }; - // SAFETY: entity index was just allocated - unsafe { - self.entities.set(entity.index(), location); - } + self.entities.set(entity.index(), location); + + #[cfg(feature = "track_change_detection")] + self.entities + .set_spawned_or_despawned_by(entity.index(), caller); + EntityWorldMut::new(self, entity, location) } @@ -1528,11 +1575,14 @@ impl World { ) -> bool { self.flush(); if let Ok(entity) = self.get_entity_mut(entity) { - entity.despawn(); + entity.despawn_with_caller( + #[cfg(feature = "track_change_detection")] + caller, + ); true } else { if log_warning { - warn!("error[B0003]: {caller}: Could not despawn entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", entity); + warn!("error[B0003]: {caller}: Could not despawn entity {entity:?}, which {}. See: https://bevyengine.org/learn/errors/b0003", self.entities.entity_does_not_exist_error_details_message(entity)); } false } @@ -2694,11 +2744,11 @@ impl World { ) }; } 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", core::any::type_name::(), entity); + panic!("error[B0003]: Could not insert a bundle (of type `{}`) for entity {entity:?}, which {}. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), self.entities.entity_does_not_exist_error_details_message(entity)); } } } 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", core::any::type_name::(), first_entity); + panic!("error[B0003]: Could not insert a bundle (of type `{}`) for entity {first_entity:?}, which {}. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), self.entities.entity_does_not_exist_error_details_message(first_entity)); } } } @@ -4531,29 +4581,26 @@ mod tests { world.entity_mut(e1).despawn(); - assert_eq!( - Err(EntityFetchError::NoSuchEntity(e1)), - world.get_entity_mut(e1).map(|_| {}) - ); - assert_eq!( - Err(EntityFetchError::NoSuchEntity(e1)), - world.get_entity_mut([e1, e2]).map(|_| {}) - ); - assert_eq!( - Err(EntityFetchError::NoSuchEntity(e1)), + assert!(matches!( + world.get_entity_mut(e1).map(|_| {}), + Err(EntityFetchError::NoSuchEntity(e, ..)) if e == e1 + )); + assert!(matches!( + world.get_entity_mut([e1, e2]).map(|_| {}), + Err(EntityFetchError::NoSuchEntity(e,..)) if e == e1)); + assert!(matches!( world .get_entity_mut(&[e1, e2] /* this is an array not a slice */) - .map(|_| {}) - ); - assert_eq!( - Err(EntityFetchError::NoSuchEntity(e1)), - world.get_entity_mut(&vec![e1, e2][..]).map(|_| {}) - ); - assert_eq!( - Err(EntityFetchError::NoSuchEntity(e1)), + .map(|_| {}), + Err(EntityFetchError::NoSuchEntity(e, ..)) if e == e1)); + assert!(matches!( + world.get_entity_mut(&vec![e1, e2][..]).map(|_| {}), + Err(EntityFetchError::NoSuchEntity(e, ..)) if e == e1, + )); + assert!(matches!( world .get_entity_mut(&EntityHashSet::from_iter([e1, e2])) - .map(|_| {}) - ); + .map(|_| {}), + Err(EntityFetchError::NoSuchEntity(e, ..)) if e == e1)); } } diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index aed797c555..2327c23f56 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -20,6 +20,8 @@ use crate::{ use bevy_ptr::Ptr; #[cfg(feature = "track_change_detection")] use bevy_ptr::UnsafeCellDeref; +#[cfg(feature = "track_change_detection")] +use core::panic::Location; use core::{any::TypeId, cell::UnsafeCell, fmt::Debug, marker::PhantomData, ptr}; use thiserror::Error; @@ -950,9 +952,7 @@ impl<'w> UnsafeEntityCell<'w> { None } } -} -impl<'w> UnsafeEntityCell<'w> { /// Gets the component of the given [`ComponentId`] from the entity. /// /// **You should prefer to use the typed API where possible and only @@ -1030,6 +1030,15 @@ impl<'w> UnsafeEntityCell<'w> { .ok_or(GetEntityMutByIdError::ComponentNotFound) } } + + /// Returns the source code location from which this entity has been spawned. + #[cfg(feature = "track_change_detection")] + pub fn spawned_by(self) -> &'static Location<'static> { + self.world() + .entities() + .entity_get_spawned_or_despawned_by(self.entity) + .unwrap() + } } /// Error that may be returned when calling [`UnsafeEntityCell::get_mut_by_id`]. diff --git a/crates/bevy_render/src/render_phase/draw.rs b/crates/bevy_render/src/render_phase/draw.rs index 8b197025e0..6f45e1b397 100644 --- a/crates/bevy_render/src/render_phase/draw.rs +++ b/crates/bevy_render/src/render_phase/draw.rs @@ -321,7 +321,7 @@ where let view = match self.view.get_manual(world, view) { Ok(view) => view, Err(err) => match err { - QueryEntityError::NoSuchEntity(_) => return Err(DrawError::ViewEntityNotFound), + QueryEntityError::NoSuchEntity(_, _) => return Err(DrawError::ViewEntityNotFound), QueryEntityError::QueryDoesNotMatch(_, _) | QueryEntityError::AliasedMutability(_) => { return Err(DrawError::InvalidViewQuery) diff --git a/crates/bevy_transform/src/helper.rs b/crates/bevy_transform/src/helper.rs index 1ae3ca9616..2911c1620a 100644 --- a/crates/bevy_transform/src/helper.rs +++ b/crates/bevy_transform/src/helper.rs @@ -52,7 +52,7 @@ fn map_error(err: QueryEntityError, ancestor: bool) -> ComputeGlobalTransformErr use ComputeGlobalTransformError::*; match err { QueryEntityError::QueryDoesNotMatch(entity, _) => MissingTransform(entity), - QueryEntityError::NoSuchEntity(entity) => { + QueryEntityError::NoSuchEntity(entity, _) => { if ancestor { MalformedHierarchy(entity) } else { diff --git a/docs/cargo_features.md b/docs/cargo_features.md index c07158ef2e..f59c4e87a4 100644 --- a/docs/cargo_features.md +++ b/docs/cargo_features.md @@ -106,7 +106,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| +|track_change_detection|Enables source location tracking for change detection and spawning/despawning, 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.|