Skip to content

Commit

Permalink
track_change_detection: Also track spawns/despawns (#16047)
Browse files Browse the repository at this point in the history
# 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<StandardMaterial>`) 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 <[email protected]>
Co-authored-by: Freya Pines <[email protected]>
Co-authored-by: Freya Pines <[email protected]>
Co-authored-by: Matty Weatherley <[email protected]>
  • Loading branch information
5 people authored Dec 17, 2024
1 parent 7be844b commit 21195a7
Show file tree
Hide file tree
Showing 14 changed files with 402 additions and 95 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 48 additions & 1 deletion crates/bevy_ecs/src/entity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)]
Expand All @@ -946,17 +988,22 @@ struct EntityMeta {
pub generation: NonZero<u32>,
/// 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 {
/// meta for **pending entity**
const EMPTY: EntityMeta = EntityMeta {
generation: NonZero::<u32>::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.
Expand Down
33 changes: 24 additions & 9 deletions crates/bevy_ecs/src/query/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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")
}
}
}
}
Expand All @@ -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})"),
}
}
Expand Down Expand Up @@ -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,
}
Expand Down
8 changes: 4 additions & 4 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
///
/// 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>(
Expand Down Expand Up @@ -921,7 +921,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
/// 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]));
/// ```
Expand Down Expand Up @@ -1018,7 +1018,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
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())
Expand Down Expand Up @@ -1495,7 +1495,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
/// # 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]));
/// ```
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_ecs/src/reflect/entity_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}>`");
Expand Down
39 changes: 25 additions & 14 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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::<T>());
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::<T>(), world.entities().entity_does_not_exist_error_details_message(entity));
})})
}

Expand All @@ -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.
Expand Down Expand Up @@ -2203,7 +2214,7 @@ fn insert<T: Bundle>(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::<T>(), 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::<T>(), world.entities().entity_does_not_exist_error_details_message(entity));
}
}
}
Expand All @@ -2222,7 +2233,7 @@ fn insert_from_world<T: Component + FromWorld>(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::<T>(), 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::<T>(), world.entities().entity_does_not_exist_error_details_message(entity) );
}
}
}
Expand Down Expand Up @@ -2254,7 +2265,7 @@ fn try_insert(bundle: impl Bundle, mode: InsertMode) -> impl EntityCommand {
unsafe fn insert_by_id<T: Send + 'static>(
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) {
Expand All @@ -2265,7 +2276,7 @@ unsafe fn insert_by_id<T: Send + 'static>(
entity.insert_by_id(component_id, ptr);
});
} else {
on_none_entity(entity);
on_none_entity(world, entity);
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_ecs/src/world/entity_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -136,7 +136,7 @@ unsafe impl WorldEntityFetch for Entity {
) -> Result<Self::DeferredMut<'_>, 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) })
}
Expand Down Expand Up @@ -210,7 +210,7 @@ unsafe impl<const N: usize> 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) });
}
Expand Down Expand Up @@ -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) });
}
Expand Down Expand Up @@ -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) });
}
Expand Down
Loading

0 comments on commit 21195a7

Please sign in to comment.