diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 74f68870a3..853cb7e481 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -1013,13 +1013,32 @@ impl Entities { .map(Option::flatten) } - /// Constructs a message explaining why an entity does not exists, if known. + /// Constructs a message explaining why an entity does not exist, if known. pub(crate) fn entity_does_not_exist_error_details( &self, - _entity: Entity, + entity: Entity, ) -> EntityDoesNotExistDetails { EntityDoesNotExistDetails { - location: self.entity_get_spawned_or_despawned_by(_entity), + location: self.entity_get_spawned_or_despawned_by(entity), + } + } +} + +/// An error that occurs when a specified [`Entity`] does not exist. +#[derive(thiserror::Error, Debug, Clone, Copy, PartialEq, Eq)] +#[error("The entity with ID {entity} {details}")] +pub struct EntityDoesNotExistError { + /// The entity's ID. + pub entity: Entity, + /// Details on why the entity does not exist, if available. + pub details: EntityDoesNotExistDetails, +} + +impl EntityDoesNotExistError { + pub(crate) fn new(entity: Entity, entities: &Entities) -> Self { + Self { + entity, + details: entities.entity_does_not_exist_error_details(entity), } } } diff --git a/crates/bevy_ecs/src/query/error.rs b/crates/bevy_ecs/src/query/error.rs index 464d0b2fb1..f1f9ddfe21 100644 --- a/crates/bevy_ecs/src/query/error.rs +++ b/crates/bevy_ecs/src/query/error.rs @@ -1,7 +1,7 @@ use thiserror::Error; use crate::{ - entity::{Entity, EntityDoesNotExistDetails}, + entity::{Entity, EntityDoesNotExistError}, world::unsafe_world_cell::UnsafeWorldCell, }; @@ -14,13 +14,19 @@ 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, EntityDoesNotExistDetails), + EntityDoesNotExist(EntityDoesNotExistError), /// The [`Entity`] was requested mutably more than once. /// /// See [`Query::get_many_mut`](crate::system::Query::get_many_mut) for an example. AliasedMutability(Entity), } +impl<'w> From for QueryEntityError<'w> { + fn from(error: EntityDoesNotExistError) -> Self { + QueryEntityError::EntityDoesNotExist(error) + } +} + impl<'w> core::error::Error for QueryEntityError<'w> {} impl<'w> core::fmt::Display for QueryEntityError<'w> { @@ -33,8 +39,8 @@ impl<'w> core::fmt::Display for QueryEntityError<'w> { )?; format_archetype(f, world, entity) } - Self::NoSuchEntity(entity, details) => { - write!(f, "The entity with ID {entity} {details}") + Self::EntityDoesNotExist(error) => { + write!(f, "{error}") } Self::AliasedMutability(entity) => { write!( @@ -54,8 +60,8 @@ impl<'w> core::fmt::Debug for QueryEntityError<'w> { format_archetype(f, world, entity)?; write!(f, ")") } - Self::NoSuchEntity(entity, details) => { - write!(f, "NoSuchEntity({entity} {details})") + Self::EntityDoesNotExist(error) => { + write!(f, "EntityDoesNotExist({error})") } Self::AliasedMutability(entity) => write!(f, "AliasedMutability({entity})"), } @@ -88,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::EntityDoesNotExist(e1), Self::EntityDoesNotExist(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 b568897ee3..1e851285dc 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -949,7 +949,7 @@ impl QueryState { /// /// let wrong_entity = Entity::from_raw(365); /// - /// 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 world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); /// ``` #[inline] pub fn get_many<'w, const N: usize>( @@ -1006,7 +1006,7 @@ impl QueryState { /// let wrong_entity = Entity::from_raw(57); /// let invalid_entity = world.spawn_empty().id(); /// - /// 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 world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.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])); /// ``` @@ -1335,7 +1335,7 @@ impl QueryState { /// # let wrong_entity = Entity::from_raw(57); /// # let invalid_entity = world.spawn_empty().id(); /// - /// # 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 world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.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/relationship/mod.rs b/crates/bevy_ecs/src/relationship/mod.rs index b11e2f5bdd..540b049541 100644 --- a/crates/bevy_ecs/src/relationship/mod.rs +++ b/crates/bevy_ecs/src/relationship/mod.rs @@ -186,7 +186,7 @@ pub trait RelationshipTarget: Component + Sized { let relationship_target = world.get_entity(entity).unwrap().get::().unwrap(); let mut commands = world.get_raw_command_queue(); for source_entity in relationship_target.iter() { - if world.get_entity(source_entity).is_some() { + if world.get_entity(source_entity).is_ok() { commands.push( entity_command::remove::() .with_entity(source_entity) @@ -217,7 +217,7 @@ pub trait RelationshipTarget: Component + Sized { let relationship_target = world.get_entity(entity).unwrap().get::().unwrap(); let mut commands = world.get_raw_command_queue(); for source_entity in relationship_target.iter() { - if world.get_entity(source_entity).is_some() { + if world.get_entity(source_entity).is_ok() { commands.push( entity_command::despawn() .with_entity(source_entity) diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index bbc8cb88ba..20dd9e0289 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -15,7 +15,7 @@ use crate::{ event::Event, result::Result, system::{command::HandleError, Command, IntoObserverSystem}, - world::{error::EntityFetchError, EntityWorldMut, FromWorld, World}, + world::{error::EntityMutableFetchError, EntityWorldMut, FromWorld, World}, }; use bevy_ptr::OwningPtr; @@ -96,13 +96,13 @@ pub trait CommandWithEntity { fn with_entity(self, entity: Entity) -> impl Command + HandleError; } -impl CommandWithEntity> for C { +impl CommandWithEntity> for C { fn with_entity( self, entity: Entity, - ) -> impl Command> + HandleError> - { - move |world: &mut World| -> Result<(), EntityFetchError> { + ) -> impl Command> + + HandleError> { + move |world: &mut World| -> Result<(), EntityMutableFetchError> { let entity = world.get_entity_mut(entity)?; self.apply(entity); Ok(()) @@ -134,7 +134,7 @@ impl< pub enum EntityCommandError { /// The entity this [`EntityCommand`] tried to run on could not be fetched. #[error(transparent)] - EntityFetchError(#[from] EntityFetchError), + EntityFetchError(#[from] EntityMutableFetchError), /// An error that occurred while running the [`EntityCommand`]. #[error("{0}")] CommandFailed(E), @@ -300,6 +300,7 @@ pub fn log_components() -> impl EntityCommand { let debug_infos: Vec<_> = entity .world() .inspect_entity(entity.id()) + .expect("Entity existence is verified before an EntityCommand is executed") .map(ComponentInfo::name) .collect(); info!("Entity {}: {debug_infos:?}", entity.id()); diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 34873ce34a..a0457dc667 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1,7 +1,7 @@ use crate::{ batching::BatchingStrategy, component::Tick, - entity::{Entity, EntityBorrow, EntitySet}, + entity::{Entity, EntityBorrow, EntityDoesNotExistError, EntitySet}, query::{ DebugCheckedUnwrap, NopWorldQuery, QueryCombinationIter, QueryData, QueryEntityError, QueryFilter, QueryIter, QueryManyIter, QueryManyUniqueIter, QueryParIter, QueryParManyIter, @@ -1310,7 +1310,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// assert_eq!( /// match query.get_many([wrong_entity]).unwrap_err() { - /// QueryEntityError::NoSuchEntity(entity, _) => entity, + /// QueryEntityError::EntityDoesNotExist(error) => error.entity, /// _ => panic!(), /// }, /// wrong_entity @@ -1430,16 +1430,11 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { - let location = - self.world - .entities() - .get(entity) - .ok_or(QueryEntityError::NoSuchEntity( - entity, - self.world - .entities() - .entity_does_not_exist_error_details(entity), - ))?; + let location = self + .world + .entities() + .get(entity) + .ok_or(EntityDoesNotExistError::new(entity, self.world.entities()))?; if !self .state .matched_archetypes @@ -1524,7 +1519,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// .get_many_mut([wrong_entity]) /// .unwrap_err() /// { - /// QueryEntityError::NoSuchEntity(entity, _) => entity, + /// QueryEntityError::EntityDoesNotExist(error) => error.entity, /// _ => panic!(), /// }, /// wrong_entity diff --git a/crates/bevy_ecs/src/world/deferred_world.rs b/crates/bevy_ecs/src/world/deferred_world.rs index b149761bd5..654d35266e 100644 --- a/crates/bevy_ecs/src/world/deferred_world.rs +++ b/crates/bevy_ecs/src/world/deferred_world.rs @@ -12,7 +12,7 @@ use crate::{ resource::Resource, system::{Commands, Query}, traversal::Traversal, - world::{error::EntityFetchError, WorldEntityFetch}, + world::{error::EntityMutableFetchError, WorldEntityFetch}, }; use super::{unsafe_world_cell::UnsafeWorldCell, Mut, World, ON_INSERT, ON_REPLACE}; @@ -94,24 +94,13 @@ impl<'w> DeferredWorld<'w> { &mut self, entity: Entity, f: impl FnOnce(&mut T) -> R, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { // If the component is not registered, then it doesn't exist on this entity, so no action required. let Some(component_id) = self.component_id::() else { return Ok(None); }; - let entity_cell = match self.get_entity_mut(entity) { - Ok(cell) => cell, - Err(EntityFetchError::AliasedMutability(..)) => { - return Err(EntityFetchError::AliasedMutability(entity)) - } - Err(EntityFetchError::NoSuchEntity(..)) => { - return Err(EntityFetchError::NoSuchEntity( - entity, - self.entities().entity_does_not_exist_error_details(entity), - )) - } - }; + let entity_cell = self.get_entity_mut(entity)?; if !entity_cell.contains::() { return Ok(None); @@ -201,9 +190,9 @@ impl<'w> DeferredWorld<'w> { /// /// # Errors /// - /// - Returns [`EntityFetchError::NoSuchEntity`] if any of the given `entities` do not exist in the world. + /// - Returns [`EntityMutableFetchError::EntityDoesNotExist`] if any of the given `entities` do not exist in the world. /// - Only the first entity found to be missing will be returned. - /// - Returns [`EntityFetchError::AliasedMutability`] if the same entity is requested multiple times. + /// - Returns [`EntityMutableFetchError::AliasedMutability`] if the same entity is requested multiple times. /// /// # Examples /// @@ -217,7 +206,7 @@ impl<'w> DeferredWorld<'w> { pub fn get_entity_mut( &mut self, entities: F, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { let cell = self.as_unsafe_world_cell(); // SAFETY: `&mut self` gives mutable access to the entire world, // and prevents any other access to the world. diff --git a/crates/bevy_ecs/src/world/entity_fetch.rs b/crates/bevy_ecs/src/world/entity_fetch.rs index 466dcc31d0..e731582f48 100644 --- a/crates/bevy_ecs/src/world/entity_fetch.rs +++ b/crates/bevy_ecs/src/world/entity_fetch.rs @@ -2,9 +2,9 @@ use alloc::vec::Vec; use core::mem::MaybeUninit; use crate::{ - entity::{hash_map::EntityHashMap, hash_set::EntityHashSet, Entity}, + entity::{hash_map::EntityHashMap, hash_set::EntityHashSet, Entity, EntityDoesNotExistError}, world::{ - error::EntityFetchError, unsafe_world_cell::UnsafeWorldCell, EntityMut, EntityRef, + error::EntityMutableFetchError, unsafe_world_cell::UnsafeWorldCell, EntityMut, EntityRef, EntityWorldMut, }, }; @@ -56,8 +56,11 @@ pub unsafe trait WorldEntityFetch { /// /// # Errors /// - /// - Returns [`Entity`] if the entity does not exist. - unsafe fn fetch_ref(self, cell: UnsafeWorldCell<'_>) -> Result, Entity>; + /// - Returns [`EntityDoesNotExistError`] if the entity does not exist. + unsafe fn fetch_ref( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityDoesNotExistError>; /// Returns mutable reference(s) to the entities with the given [`Entity`] /// IDs, as determined by `self`. @@ -70,11 +73,13 @@ pub unsafe trait WorldEntityFetch { /// /// # Errors /// - /// - Returns [`EntityFetchError::NoSuchEntity`] if the entity does not exist. - /// - Returns [`EntityFetchError::AliasedMutability`] if the entity was + /// - Returns [`EntityMutableFetchError::EntityDoesNotExist`] if the entity does not exist. + /// - Returns [`EntityMutableFetchError::AliasedMutability`] if the entity was /// requested mutably more than once. - unsafe fn fetch_mut(self, cell: UnsafeWorldCell<'_>) - -> Result, EntityFetchError>; + unsafe fn fetch_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityMutableFetchError>; /// Returns mutable reference(s) to the entities with the given [`Entity`] /// IDs, as determined by `self`, but without structural mutability. @@ -91,13 +96,13 @@ pub unsafe trait WorldEntityFetch { /// /// # Errors /// - /// - Returns [`EntityFetchError::NoSuchEntity`] if the entity does not exist. - /// - Returns [`EntityFetchError::AliasedMutability`] if the entity was + /// - Returns [`EntityMutableFetchError::EntityDoesNotExist`] if the entity does not exist. + /// - Returns [`EntityMutableFetchError::AliasedMutability`] if the entity was /// requested mutably more than once. unsafe fn fetch_deferred_mut( self, cell: UnsafeWorldCell<'_>, - ) -> Result, EntityFetchError>; + ) -> Result, EntityMutableFetchError>; } // SAFETY: @@ -109,8 +114,11 @@ unsafe impl WorldEntityFetch for Entity { type Mut<'w> = EntityWorldMut<'w>; type DeferredMut<'w> = EntityMut<'w>; - unsafe fn fetch_ref(self, cell: UnsafeWorldCell<'_>) -> Result, Entity> { - let ecell = cell.get_entity(self).ok_or(self)?; + unsafe fn fetch_ref( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityDoesNotExistError> { + let ecell = cell.get_entity(self)?; // SAFETY: caller ensures that the world cell has read-only access to the entity. Ok(unsafe { EntityRef::new(ecell) }) } @@ -118,14 +126,11 @@ unsafe impl WorldEntityFetch for Entity { unsafe fn fetch_mut( self, cell: UnsafeWorldCell<'_>, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { let location = cell .entities() .get(self) - .ok_or(EntityFetchError::NoSuchEntity( - self, - cell.entities().entity_does_not_exist_error_details(self), - ))?; + .ok_or(EntityDoesNotExistError::new(self, cell.entities()))?; // 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`. @@ -135,11 +140,8 @@ unsafe impl WorldEntityFetch for Entity { unsafe fn fetch_deferred_mut( self, cell: UnsafeWorldCell<'_>, - ) -> Result, EntityFetchError> { - let ecell = cell.get_entity(self).ok_or(EntityFetchError::NoSuchEntity( - self, - cell.entities().entity_does_not_exist_error_details(self), - ))?; + ) -> Result, EntityMutableFetchError> { + let ecell = cell.get_entity(self)?; // SAFETY: caller ensures that the world cell has mutable access to the entity. Ok(unsafe { EntityMut::new(ecell) }) } @@ -154,21 +156,24 @@ unsafe impl WorldEntityFetch for [Entity; N] { type Mut<'w> = [EntityMut<'w>; N]; type DeferredMut<'w> = [EntityMut<'w>; N]; - unsafe fn fetch_ref(self, cell: UnsafeWorldCell<'_>) -> Result, Entity> { + unsafe fn fetch_ref( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityDoesNotExistError> { <&Self>::fetch_ref(&self, cell) } unsafe fn fetch_mut( self, cell: UnsafeWorldCell<'_>, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { <&Self>::fetch_mut(&self, cell) } unsafe fn fetch_deferred_mut( self, cell: UnsafeWorldCell<'_>, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { <&Self>::fetch_deferred_mut(&self, cell) } } @@ -182,10 +187,13 @@ unsafe impl WorldEntityFetch for &'_ [Entity; N] { type Mut<'w> = [EntityMut<'w>; N]; type DeferredMut<'w> = [EntityMut<'w>; N]; - unsafe fn fetch_ref(self, cell: UnsafeWorldCell<'_>) -> Result, Entity> { + unsafe fn fetch_ref( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityDoesNotExistError> { let mut refs = [MaybeUninit::uninit(); N]; for (r, &id) in core::iter::zip(&mut refs, self) { - let ecell = cell.get_entity(id).ok_or(id)?; + let ecell = cell.get_entity(id)?; // SAFETY: caller ensures that the world cell has read-only access to the entity. *r = MaybeUninit::new(unsafe { EntityRef::new(ecell) }); } @@ -199,22 +207,19 @@ unsafe impl WorldEntityFetch for &'_ [Entity; N] { unsafe fn fetch_mut( self, cell: UnsafeWorldCell<'_>, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { // Check for duplicate entities. for i in 0..self.len() { for j in 0..i { if self[i] == self[j] { - return Err(EntityFetchError::AliasedMutability(self[i])); + return Err(EntityMutableFetchError::AliasedMutability(self[i])); } } } let mut refs = [const { MaybeUninit::uninit() }; 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(id), - ))?; + let ecell = cell.get_entity(id)?; // SAFETY: caller ensures that the world cell has mutable access to the entity. *r = MaybeUninit::new(unsafe { EntityMut::new(ecell) }); } @@ -228,7 +233,7 @@ unsafe impl WorldEntityFetch for &'_ [Entity; N] { unsafe fn fetch_deferred_mut( self, cell: UnsafeWorldCell<'_>, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { // SAFETY: caller ensures that the world cell has mutable access to the entity, // and `fetch_mut` does not return structurally-mutable references. unsafe { self.fetch_mut(cell) } @@ -244,10 +249,13 @@ unsafe impl WorldEntityFetch for &'_ [Entity] { type Mut<'w> = Vec>; type DeferredMut<'w> = Vec>; - unsafe fn fetch_ref(self, cell: UnsafeWorldCell<'_>) -> Result, Entity> { + unsafe fn fetch_ref( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityDoesNotExistError> { let mut refs = Vec::with_capacity(self.len()); for &id in self { - let ecell = cell.get_entity(id).ok_or(id)?; + let ecell = cell.get_entity(id)?; // SAFETY: caller ensures that the world cell has read-only access to the entity. refs.push(unsafe { EntityRef::new(ecell) }); } @@ -258,22 +266,19 @@ unsafe impl WorldEntityFetch for &'_ [Entity] { unsafe fn fetch_mut( self, cell: UnsafeWorldCell<'_>, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { // Check for duplicate entities. for i in 0..self.len() { for j in 0..i { if self[i] == self[j] { - return Err(EntityFetchError::AliasedMutability(self[i])); + return Err(EntityMutableFetchError::AliasedMutability(self[i])); } } } let mut refs = Vec::with_capacity(self.len()); for &id in self { - let ecell = cell.get_entity(id).ok_or(EntityFetchError::NoSuchEntity( - id, - cell.entities().entity_does_not_exist_error_details(id), - ))?; + let ecell = cell.get_entity(id)?; // SAFETY: caller ensures that the world cell has mutable access to the entity. refs.push(unsafe { EntityMut::new(ecell) }); } @@ -284,7 +289,7 @@ unsafe impl WorldEntityFetch for &'_ [Entity] { unsafe fn fetch_deferred_mut( self, cell: UnsafeWorldCell<'_>, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { // SAFETY: caller ensures that the world cell has mutable access to the entity, // and `fetch_mut` does not return structurally-mutable references. unsafe { self.fetch_mut(cell) } @@ -300,10 +305,13 @@ unsafe impl WorldEntityFetch for &'_ EntityHashSet { type Mut<'w> = EntityHashMap>; type DeferredMut<'w> = EntityHashMap>; - unsafe fn fetch_ref(self, cell: UnsafeWorldCell<'_>) -> Result, Entity> { + unsafe fn fetch_ref( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityDoesNotExistError> { let mut refs = EntityHashMap::with_capacity(self.len()); for &id in self { - let ecell = cell.get_entity(id).ok_or(id)?; + let ecell = cell.get_entity(id)?; // SAFETY: caller ensures that the world cell has read-only access to the entity. refs.insert(id, unsafe { EntityRef::new(ecell) }); } @@ -313,13 +321,10 @@ unsafe impl WorldEntityFetch for &'_ EntityHashSet { unsafe fn fetch_mut( self, cell: UnsafeWorldCell<'_>, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { let mut refs = EntityHashMap::with_capacity(self.len()); for &id in self { - let ecell = cell.get_entity(id).ok_or(EntityFetchError::NoSuchEntity( - id, - cell.entities().entity_does_not_exist_error_details(id), - ))?; + let ecell = cell.get_entity(id)?; // SAFETY: caller ensures that the world cell has mutable access to the entity. refs.insert(id, unsafe { EntityMut::new(ecell) }); } @@ -329,7 +334,7 @@ unsafe impl WorldEntityFetch for &'_ EntityHashSet { unsafe fn fetch_deferred_mut( self, cell: UnsafeWorldCell<'_>, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { // SAFETY: caller ensures that the world cell has mutable access to the entity, // and `fetch_mut` does not return structurally-mutable references. unsafe { self.fetch_mut(cell) } diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index 7609b15e01..1542a12851 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -1,39 +1,26 @@ //! Contains error types returned by bevy's schedule. use alloc::vec::Vec; -use thiserror::Error; use crate::{ component::ComponentId, - entity::{Entity, EntityDoesNotExistDetails}, + entity::{Entity, EntityDoesNotExistError}, schedule::InternedScheduleLabel, }; /// 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 -#[derive(Error, Debug)] +#[derive(thiserror::Error, Debug)] #[error("The schedule with the label {0:?} was not found.")] pub struct TryRunScheduleError(pub InternedScheduleLabel); -/// The error type returned by [`World::try_despawn`] if the provided entity does not exist. -/// -/// [`World::try_despawn`]: crate::world::World::try_despawn -#[derive(Error, Debug, Clone, Copy)] -#[error("Could not despawn the entity with ID {entity} because it {details}")] -pub struct TryDespawnError { - /// The entity's ID. - pub entity: Entity, - /// Details on why the entity does not exist, if available. - pub details: EntityDoesNotExistDetails, -} - /// The error type returned by [`World::try_insert_batch`] and [`World::try_insert_batch_if_new`] /// if any of the provided entities do not exist. /// /// [`World::try_insert_batch`]: crate::world::World::try_insert_batch /// [`World::try_insert_batch_if_new`]: crate::world::World::try_insert_batch_if_new -#[derive(Error, Debug, Clone)] +#[derive(thiserror::Error, Debug, Clone)] #[error("Could not insert bundles of type {bundle_type} into the entities with the following IDs because they do not exist: {entities:?}")] pub struct TryInsertBatchError { /// The bundles' type name. @@ -42,8 +29,13 @@ pub struct TryInsertBatchError { pub entities: Vec, } +/// An error that occurs when a specified [`Entity`] could not be despawned. +#[derive(thiserror::Error, Debug, Clone, Copy)] +#[error("Could not despawn entity: {0}")] +pub struct EntityDespawnError(#[from] pub EntityMutableFetchError); + /// An error that occurs when dynamically retrieving components from an entity. -#[derive(Error, Debug, Clone, Copy, PartialEq, Eq)] +#[derive(thiserror::Error, Debug, Clone, Copy, PartialEq, Eq)] pub enum EntityComponentError { /// The component with the given [`ComponentId`] does not exist on the entity. #[error("The component with ID {0:?} does not exist on the entity.")] @@ -54,24 +46,12 @@ pub enum EntityComponentError { } /// An error that occurs when fetching entities mutably from a world. -#[derive(Error, Debug, Clone, Copy)] -pub enum EntityFetchError { +#[derive(thiserror::Error, Debug, Clone, Copy, PartialEq, Eq)] +pub enum EntityMutableFetchError { /// The entity with the given ID does not exist. - #[error("The entity with ID {0} {1}")] - NoSuchEntity(Entity, EntityDoesNotExistDetails), + #[error(transparent)] + EntityDoesNotExist(#[from] EntityDoesNotExistError), /// 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 PartialEq for EntityFetchError { - 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 Eq for EntityFetchError {} diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 12dcb73134..db812ff165 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -41,7 +41,9 @@ use crate::{ Component, ComponentDescriptor, ComponentHooks, ComponentId, ComponentInfo, ComponentTicks, Components, Mutable, RequiredComponents, RequiredComponentsError, Tick, }, - entity::{AllocAtWithoutReplacement, Entities, Entity, EntityLocation}, + entity::{ + AllocAtWithoutReplacement, Entities, Entity, EntityDoesNotExistError, EntityLocation, + }, entity_disabling::DefaultQueryFilters, event::{Event, EventId, Events, SendBatchIds}, observer::Observers, @@ -53,7 +55,9 @@ use crate::{ system::Commands, world::{ command_queue::RawCommandQueue, - error::{EntityFetchError, TryDespawnError, TryInsertBatchError, TryRunScheduleError}, + error::{ + EntityDespawnError, EntityMutableFetchError, TryInsertBatchError, TryRunScheduleError, + }, }, }; use alloc::{boxed::Box, vec::Vec}; @@ -694,7 +698,7 @@ impl World { match self.get_entity(entities) { Ok(fetched) => fetched, - Err(entity) => panic_no_entity(self, entity), + Err(error) => panic_no_entity(self, error.entity), } } @@ -821,7 +825,7 @@ impl World { #[inline(never)] #[cold] #[track_caller] - fn panic_on_err(e: EntityFetchError) -> ! { + fn panic_on_err(e: EntityMutableFetchError) -> ! { panic!("{e}"); } @@ -833,25 +837,23 @@ impl World { /// Returns the components of an [`Entity`] through [`ComponentInfo`]. #[inline] - pub fn inspect_entity(&self, entity: Entity) -> impl Iterator { + pub fn inspect_entity( + &self, + entity: Entity, + ) -> Result, EntityDoesNotExistError> { let entity_location = self .entities() .get(entity) - .unwrap_or_else(|| panic!("Entity {entity} does not exist")); + .ok_or(EntityDoesNotExistError::new(entity, self.entities()))?; let archetype = self .archetypes() .get(entity_location.archetype_id) - .unwrap_or_else(|| { - panic!( - "Archetype {:?} does not exist", - entity_location.archetype_id - ) - }); + .expect("ArchetypeId was retrieved from an EntityLocation and should correspond to an Archetype"); - archetype + Ok(archetype .components() - .filter_map(|id| self.components().get_info(id)) + .filter_map(|id| self.components().get_info(id))) } /// Returns [`EntityRef`]s that expose read-only operations for the given @@ -869,7 +871,7 @@ impl World { /// # Errors /// /// If any of the given `entities` do not exist in the world, the first - /// [`Entity`] found to be missing will be returned in the [`Err`]. + /// [`Entity`] found to be missing will return an [`EntityDoesNotExistError`]. /// /// # Examples /// @@ -877,7 +879,10 @@ impl World { /// /// [`EntityHashSet`]: crate::entity::hash_set::EntityHashSet #[inline] - pub fn get_entity(&self, entities: F) -> Result, Entity> { + pub fn get_entity( + &self, + entities: F, + ) -> Result, EntityDoesNotExistError> { let cell = self.as_unsafe_world_cell_readonly(); // SAFETY: `&self` gives read access to the entire world, and prevents mutable access. unsafe { entities.fetch_ref(cell) } @@ -905,9 +910,9 @@ impl World { /// /// # Errors /// - /// - Returns [`EntityFetchError::NoSuchEntity`] if any of the given `entities` do not exist in the world. + /// - Returns [`EntityMutableFetchError::EntityDoesNotExist`] if any of the given `entities` do not exist in the world. /// - Only the first entity found to be missing will be returned. - /// - Returns [`EntityFetchError::AliasedMutability`] if the same entity is requested multiple times. + /// - Returns [`EntityMutableFetchError::AliasedMutability`] if the same entity is requested multiple times. /// /// # Examples /// @@ -918,7 +923,7 @@ impl World { pub fn get_entity_mut( &mut self, entities: F, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { let cell = self.as_unsafe_world_cell(); // SAFETY: `&mut self` gives mutable access to the entire world, // and prevents any other access to the world. @@ -1244,21 +1249,10 @@ impl World { &mut self, entity: Entity, f: impl FnOnce(&mut T) -> R, - ) -> Result, EntityFetchError> { + ) -> Result, EntityMutableFetchError> { let mut world = DeferredWorld::from(&mut *self); - let result = match world.modify_component(entity, f) { - Ok(result) => result, - Err(EntityFetchError::AliasedMutability(..)) => { - return Err(EntityFetchError::AliasedMutability(entity)) - } - Err(EntityFetchError::NoSuchEntity(..)) => { - return Err(EntityFetchError::NoSuchEntity( - entity, - self.entities().entity_does_not_exist_error_details(entity), - )) - } - }; + let result = world.modify_component(entity, f)?; self.flush(); Ok(result) @@ -1304,7 +1298,7 @@ impl World { /// Despawns the given `entity`, if it exists. This will also remove all of the entity's /// [`Components`](Component). /// - /// Returns a [`TryDespawnError`] if the entity does not exist. + /// Returns an [`EntityDespawnError`] if the entity does not exist. /// /// # Note /// @@ -1312,7 +1306,7 @@ impl World { /// to despawn descendants. For example, this will recursively despawn [`Children`](crate::hierarchy::Children). #[track_caller] #[inline] - pub fn try_despawn(&mut self, entity: Entity) -> Result<(), TryDespawnError> { + pub fn try_despawn(&mut self, entity: Entity) -> Result<(), EntityDespawnError> { self.despawn_with_caller(entity, MaybeLocation::caller()) } @@ -1321,17 +1315,11 @@ impl World { &mut self, entity: Entity, caller: MaybeLocation, - ) -> Result<(), TryDespawnError> { + ) -> Result<(), EntityDespawnError> { self.flush(); - if let Ok(entity) = self.get_entity_mut(entity) { - entity.despawn_with_caller(caller); - Ok(()) - } else { - Err(TryDespawnError { - entity, - details: self.entities().entity_does_not_exist_error_details(entity), - }) - } + let entity = self.get_entity_mut(entity)?; + entity.despawn_with_caller(caller); + Ok(()) } /// Clears the internal component tracker state. @@ -3607,7 +3595,7 @@ mod tests { entity_disabling::{DefaultQueryFilters, Disabled}, ptr::OwningPtr, resource::Resource, - world::error::EntityFetchError, + world::error::EntityMutableFetchError, }; use alloc::{ borrow::ToOwned, @@ -3992,39 +3980,39 @@ mod tests { let bar_id = TypeId::of::(); let baz_id = TypeId::of::(); assert_eq!( - to_type_ids(world.inspect_entity(ent0).collect()), + to_type_ids(world.inspect_entity(ent0).unwrap().collect()), [Some(foo_id), Some(bar_id), Some(baz_id)] .into_iter() .collect::>() ); assert_eq!( - to_type_ids(world.inspect_entity(ent1).collect()), + to_type_ids(world.inspect_entity(ent1).unwrap().collect()), [Some(foo_id), Some(bar_id)] .into_iter() .collect::>() ); assert_eq!( - to_type_ids(world.inspect_entity(ent2).collect()), + to_type_ids(world.inspect_entity(ent2).unwrap().collect()), [Some(bar_id), Some(baz_id)] .into_iter() .collect::>() ); assert_eq!( - to_type_ids(world.inspect_entity(ent3).collect()), + to_type_ids(world.inspect_entity(ent3).unwrap().collect()), [Some(foo_id), Some(baz_id)] .into_iter() .collect::>() ); assert_eq!( - to_type_ids(world.inspect_entity(ent4).collect()), + to_type_ids(world.inspect_entity(ent4).unwrap().collect()), [Some(foo_id)].into_iter().collect::>() ); assert_eq!( - to_type_ids(world.inspect_entity(ent5).collect()), + to_type_ids(world.inspect_entity(ent5).unwrap().collect()), [Some(bar_id)].into_iter().collect::>() ); assert_eq!( - to_type_ids(world.inspect_entity(ent6).collect()), + to_type_ids(world.inspect_entity(ent6).unwrap().collect()), [Some(baz_id)].into_iter().collect::>() ); } @@ -4171,20 +4159,34 @@ mod tests { world.entity_mut(e1).despawn(); - assert_eq!(Err(e1), world.get_entity(e1).map(|_| {})); - assert_eq!(Err(e1), world.get_entity([e1, e2]).map(|_| {})); + assert_eq!( + Err(e1), + world.get_entity(e1).map(|_| {}).map_err(|e| e.entity) + ); + assert_eq!( + Err(e1), + world.get_entity([e1, e2]).map(|_| {}).map_err(|e| e.entity) + ); assert_eq!( Err(e1), world .get_entity(&[e1, e2] /* this is an array not a slice */) .map(|_| {}) + .map_err(|e| e.entity) + ); + assert_eq!( + Err(e1), + world + .get_entity(&vec![e1, e2][..]) + .map(|_| {}) + .map_err(|e| e.entity) ); - assert_eq!(Err(e1), world.get_entity(&vec![e1, e2][..]).map(|_| {})); assert_eq!( Err(e1), world .get_entity(&EntityHashSet::from_iter([e1, e2])) .map(|_| {}) + .map_err(|e| e.entity) ); } @@ -4206,17 +4208,17 @@ mod tests { .is_ok()); assert_eq!( - Err(EntityFetchError::AliasedMutability(e1)), + Err(EntityMutableFetchError::AliasedMutability(e1)), world.get_entity_mut([e1, e2, e1]).map(|_| {}) ); assert_eq!( - Err(EntityFetchError::AliasedMutability(e1)), + Err(EntityMutableFetchError::AliasedMutability(e1)), world .get_entity_mut(&[e1, e2, e1] /* this is an array not a slice */) .map(|_| {}) ); assert_eq!( - Err(EntityFetchError::AliasedMutability(e1)), + Err(EntityMutableFetchError::AliasedMutability(e1)), world.get_entity_mut(&vec![e1, e2, e1][..]).map(|_| {}) ); // Aliased mutability isn't allowed by HashSets @@ -4228,25 +4230,25 @@ mod tests { assert!(matches!( world.get_entity_mut(e1).map(|_| {}), - Err(EntityFetchError::NoSuchEntity(e, ..)) if e == e1 + Err(EntityMutableFetchError::EntityDoesNotExist(e)) if e.entity == e1 )); assert!(matches!( world.get_entity_mut([e1, e2]).map(|_| {}), - Err(EntityFetchError::NoSuchEntity(e,..)) if e == e1)); + Err(EntityMutableFetchError::EntityDoesNotExist(e)) if e.entity == e1)); assert!(matches!( world .get_entity_mut(&[e1, e2] /* this is an array not a slice */) .map(|_| {}), - Err(EntityFetchError::NoSuchEntity(e, ..)) if e == e1)); + Err(EntityMutableFetchError::EntityDoesNotExist(e)) if e.entity == e1)); assert!(matches!( world.get_entity_mut(&vec![e1, e2][..]).map(|_| {}), - Err(EntityFetchError::NoSuchEntity(e, ..)) if e == e1, + Err(EntityMutableFetchError::EntityDoesNotExist(e)) if e.entity == e1, )); assert!(matches!( world .get_entity_mut(&EntityHashSet::from_iter([e1, e2])) .map(|_| {}), - Err(EntityFetchError::NoSuchEntity(e, ..)) if e == e1)); + Err(EntityMutableFetchError::EntityDoesNotExist(e)) if e.entity == e1)); } #[test] diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 570d33eeb2..cbaffb2991 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 crate::{ bundle::Bundles, change_detection::{MaybeLocation, MutUntyped, Ticks, TicksMut}, component::{ComponentId, ComponentTicks, Components, Mutable, StorageType, Tick, TickCells}, - entity::{Entities, Entity, EntityBorrow, EntityLocation}, + entity::{Entities, Entity, EntityBorrow, EntityDoesNotExistError, EntityLocation}, observer::Observers, prelude::Component, query::{DebugCheckedUnwrap, ReadOnlyQueryData}, @@ -351,9 +351,15 @@ impl<'w> UnsafeWorldCell<'w> { /// Retrieves an [`UnsafeEntityCell`] that exposes read and write operations for the given `entity`. /// Similar to the [`UnsafeWorldCell`], you are in charge of making sure that no aliasing rules are violated. #[inline] - pub fn get_entity(self, entity: Entity) -> Option> { - let location = self.entities().get(entity)?; - Some(UnsafeEntityCell::new(self, entity, location)) + pub fn get_entity( + self, + entity: Entity, + ) -> Result, EntityDoesNotExistError> { + let location = self + .entities() + .get(entity) + .ok_or(EntityDoesNotExistError::new(entity, self.entities()))?; + Ok(UnsafeEntityCell::new(self, entity, location)) } /// Gets a reference to the resource of the given type if it exists diff --git a/crates/bevy_render/src/render_phase/draw.rs b/crates/bevy_render/src/render_phase/draw.rs index 4343fda6d3..a12d336018 100644 --- a/crates/bevy_render/src/render_phase/draw.rs +++ b/crates/bevy_render/src/render_phase/draw.rs @@ -332,7 +332,9 @@ where let view = match self.view.get_manual(world, view) { Ok(view) => view, Err(err) => match err { - QueryEntityError::NoSuchEntity(_, _) => return Err(DrawError::ViewEntityNotFound), + QueryEntityError::EntityDoesNotExist(_) => { + 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 18ecfb16b6..777ca81a9d 100644 --- a/crates/bevy_transform/src/helper.rs +++ b/crates/bevy_transform/src/helper.rs @@ -52,11 +52,11 @@ fn map_error(err: QueryEntityError, ancestor: bool) -> ComputeGlobalTransformErr use ComputeGlobalTransformError::*; match err { QueryEntityError::QueryDoesNotMatch(entity, _) => MissingTransform(entity), - QueryEntityError::NoSuchEntity(entity, _) => { + QueryEntityError::EntityDoesNotExist(error) => { if ancestor { - MalformedHierarchy(entity) + MalformedHierarchy(error.entity) } else { - NoSuchEntity(entity) + NoSuchEntity(error.entity) } } QueryEntityError::AliasedMutability(_) => unreachable!(),