Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix entity does not exist message on index reuse #17264

Merged
merged 9 commits into from
Jan 12, 2025
17 changes: 13 additions & 4 deletions crates/bevy_ecs/src/entity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -979,19 +979,25 @@ impl Entities {
}

/// Returns the source code location from which this entity has last been spawned
/// or despawned. Returns `None` if this entity has never existed.
/// or despawned. Returns `None` if its index has been reused by another entity
/// or if this entity has never existed.
#[cfg(feature = "track_location")]
pub fn entity_get_spawned_or_despawned_by(
&self,
entity: Entity,
) -> Option<&'static Location<'static>> {
self.meta
.get(entity.index() as usize)
.filter(|meta|
// Generation is incremented immediately upon despawn
(meta.generation == entity.generation)
|| (meta.location.archetype_id == ArchetypeId::INVALID)
&& (meta.generation == IdentifierMask::inc_masked_high_by(entity.generation, 1)))
.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(
pub(crate) fn entity_does_not_exist_error_details(
&self,
_entity: Entity,
) -> EntityDoesNotExistDetails {
Expand All @@ -1014,9 +1020,12 @@ impl fmt::Display for EntityDoesNotExistDetails {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
#[cfg(feature = "track_location")]
if let Some(location) = self.location {
write!(f, "was despawned by {}", location)
write!(f, "was despawned by {location}")
} else {
write!(f, "was never spawned")
write!(
f,
"does not exist (index has been reused or was never spawned)"
)
}
#[cfg(not(feature = "track_location"))]
write!(
Expand Down
4 changes: 1 addition & 3 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1018,9 +1018,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
.get(entity)
.ok_or(QueryEntityError::NoSuchEntity(
entity,
world
.entities()
.entity_does_not_exist_error_details_message(entity),
world.entities().entity_does_not_exist_error_details(entity),
))?;
if !self
.matched_archetypes
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/reflect/entity_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ fn insert_reflect_with_registry_ref(
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}, which {}. See: https://bevyengine.org/learn/errors/b0003",
world.entities().entity_does_not_exist_error_details_message(entity));
world.entities().entity_does_not_exist_error_details(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
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ impl<'w, 's> Commands<'w, 's> {
fn panic_no_entity(entities: &Entities, entity: Entity) -> ! {
panic!(
"Attempting to create an EntityCommands for entity {entity}, which {}",
entities.entity_does_not_exist_error_details_message(entity)
entities.entity_does_not_exist_error_details(entity)
);
}

Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_ecs/src/world/deferred_world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ impl<'w> DeferredWorld<'w> {
Err(EntityFetchError::NoSuchEntity(..)) => {
return Err(EntityFetchError::NoSuchEntity(
entity,
self.entities()
.entity_does_not_exist_error_details_message(entity),
self.entities().entity_does_not_exist_error_details(entity),
))
}
};
Expand Down
15 changes: 5 additions & 10 deletions crates/bevy_ecs/src/world/entity_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,7 @@ unsafe impl WorldEntityFetch for Entity {
.get(self)
.ok_or(EntityFetchError::NoSuchEntity(
self,
cell.entities()
.entity_does_not_exist_error_details_message(self),
cell.entities().entity_does_not_exist_error_details(self),
))?;
// SAFETY: caller ensures that the world cell has mutable access to the entity.
let world = unsafe { cell.world_mut() };
Expand All @@ -139,8 +138,7 @@ unsafe impl WorldEntityFetch for Entity {
) -> Result<Self::DeferredMut<'_>, EntityFetchError> {
let ecell = cell.get_entity(self).ok_or(EntityFetchError::NoSuchEntity(
self,
cell.entities()
.entity_does_not_exist_error_details_message(self),
cell.entities().entity_does_not_exist_error_details(self),
))?;
// SAFETY: caller ensures that the world cell has mutable access to the entity.
Ok(unsafe { EntityMut::new(ecell) })
Expand Down Expand Up @@ -215,8 +213,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,
cell.entities()
.entity_does_not_exist_error_details_message(id),
cell.entities().entity_does_not_exist_error_details(id),
))?;
// SAFETY: caller ensures that the world cell has mutable access to the entity.
*r = MaybeUninit::new(unsafe { EntityMut::new(ecell) });
Expand Down Expand Up @@ -275,8 +272,7 @@ unsafe impl WorldEntityFetch for &'_ [Entity] {
for &id in self {
let ecell = cell.get_entity(id).ok_or(EntityFetchError::NoSuchEntity(
id,
cell.entities()
.entity_does_not_exist_error_details_message(id),
cell.entities().entity_does_not_exist_error_details(id),
))?;
// SAFETY: caller ensures that the world cell has mutable access to the entity.
refs.push(unsafe { EntityMut::new(ecell) });
Expand Down Expand Up @@ -322,8 +318,7 @@ unsafe impl WorldEntityFetch for &'_ EntityHashSet {
for &id in self {
let ecell = cell.get_entity(id).ok_or(EntityFetchError::NoSuchEntity(
id,
cell.entities()
.entity_does_not_exist_error_details_message(id),
cell.entities().entity_does_not_exist_error_details(id),
))?;
// SAFETY: caller ensures that the world cell has mutable access to the entity.
refs.insert(id, unsafe { EntityMut::new(ecell) });
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/world/entity_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,7 @@ impl<'w> EntityWorldMut<'w> {
self.entity,
self.world
.entities()
.entity_does_not_exist_error_details_message(self.entity)
.entity_does_not_exist_error_details(self.entity)
);
}

Expand Down
43 changes: 35 additions & 8 deletions crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -674,9 +674,7 @@ impl World {
fn panic_no_entity(world: &World, entity: Entity) -> ! {
panic!(
"Entity {entity} {}",
world
.entities
.entity_does_not_exist_error_details_message(entity)
world.entities.entity_does_not_exist_error_details(entity)
);
}

Expand Down Expand Up @@ -1245,8 +1243,7 @@ impl World {
Err(EntityFetchError::NoSuchEntity(..)) => {
return Err(EntityFetchError::NoSuchEntity(
entity,
self.entities()
.entity_does_not_exist_error_details_message(entity),
self.entities().entity_does_not_exist_error_details(entity),
))
}
};
Expand Down Expand Up @@ -1309,7 +1306,7 @@ impl World {
true
} else {
if log_warning {
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));
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(entity));
}
false
}
Expand Down Expand Up @@ -2468,11 +2465,11 @@ impl World {
)
};
} else {
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::<B>(), self.entities.entity_does_not_exist_error_details_message(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::<B>(), self.entities.entity_does_not_exist_error_details(entity));
}
}
} else {
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::<B>(), self.entities.entity_does_not_exist_error_details_message(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::<B>(), self.entities.entity_does_not_exist_error_details(first_entity));
}
}
}
Expand Down Expand Up @@ -4321,4 +4318,34 @@ mod tests {
.map(|_| {}),
Err(EntityFetchError::NoSuchEntity(e, ..)) if e == e1));
}

#[cfg(feature = "track_location")]
#[test]
#[track_caller]
fn entity_spawn_despawn_tracking() {
use core::panic::Location;

let mut world = World::new();
let entity = world.spawn_empty().id();
assert_eq!(
world.entities.entity_get_spawned_or_despawned_by(entity),
Some(Location::caller())
);
world.despawn(entity);
assert_eq!(
world.entities.entity_get_spawned_or_despawned_by(entity),
Some(Location::caller())
);
let new = world.spawn_empty().id();
assert_eq!(entity.index(), new.index());
assert_eq!(
world.entities.entity_get_spawned_or_despawned_by(entity),
None
);
world.despawn(new);
assert_eq!(
world.entities.entity_get_spawned_or_despawned_by(entity),
None
);
}
}
Loading