From 584d14808adfbe4c249fecebe7a4e95c2e29f4fb Mon Sep 17 00:00:00 2001 From: Christian Hughes <9044780+ItsDoot@users.noreply.github.com> Date: Mon, 7 Oct 2024 08:21:40 -0700 Subject: [PATCH] Allow `World::entity` family of functions to take multiple entities and get multiple references back (#15614) # Objective Following the pattern established in #15593, we can reduce the API surface of `World` by providing a single function to grab both a singular entity reference, or multiple entity references. ## Solution The following functions can now also take multiple entity IDs and will return multiple entity references back: - `World::entity` - `World::get_entity` - `World::entity_mut` - `World::get_entity_mut` - `DeferredWorld::entity_mut` - `DeferredWorld::get_entity_mut` If you pass in X, you receive Y: - give a single `Entity`, receive a single `EntityRef`/`EntityWorldMut` (matches current behavior) - give a `[Entity; N]`/`&[Entity; N]` (array), receive an equally-sized `[EntityRef; N]`/`[EntityMut; N]` - give a `&[Entity]` (slice), receive a `Vec`/`Vec` - give a `&EntityHashSet`, receive a `EntityHashMap`/`EntityHashMap` Note that `EntityWorldMut` is only returned in the single-entity case, because having multiple at the same time would lead to UB. Also, `DeferredWorld` receives an `EntityMut` in the single-entity case because it does not allow structural access. ## Testing - Added doc-tests on `World::entity`, `World::entity_mut`, and `DeferredWorld::entity_mut` - Added tests for aliased mutability and entity existence --- ## Showcase
Click to view showcase The APIs for fetching `EntityRef`s and `EntityMut`s from the `World` have been unified. ```rust // This code will be referred to by subsequent code blocks. let world = World::new(); let e1 = world.spawn_empty().id(); let e2 = world.spawn_empty().id(); let e3 = world.spawn_empty().id(); ``` Querying for a single entity remains mostly the same: ```rust // 0.14 let eref: EntityRef = world.entity(e1); let emut: EntityWorldMut = world.entity_mut(e1); let eref: Option = world.get_entity(e1); let emut: Option = world.get_entity_mut(e1); // 0.15 let eref: EntityRef = world.entity(e1); let emut: EntityWorldMut = world.entity_mut(e1); let eref: Result = world.get_entity(e1); let emut: Result = world.get_entity_mut(e1); ``` Querying for multiple entities with an array has changed: ```rust // 0.14 let erefs: [EntityRef; 2] = world.many_entities([e1, e2]); let emuts: [EntityMut; 2] = world.many_entities_mut([e1, e2]); let erefs: Result<[EntityRef; 2], Entity> = world.get_many_entities([e1, e2]); let emuts: Result<[EntityMut; 2], QueryEntityError> = world.get_many_entities_mut([e1, e2]); // 0.15 let erefs: [EntityRef; 2] = world.entity([e1, e2]); let emuts: [EntityMut; 2] = world.entity_mut([e1, e2]); let erefs: Result<[EntityRef; 2], Entity> = world.get_entity([e1, e2]); let emuts: Result<[EntityMut; 2], EntityFetchError> = world.get_entity_mut([e1, e2]); ``` Querying for multiple entities with a slice has changed: ```rust let ids = vec![e1, e2, e3]); // 0.14 let erefs: Result, Entity> = world.get_many_entities_dynamic(&ids[..]); let emuts: Result, QueryEntityError> = world.get_many_entities_dynamic_mut(&ids[..]); // 0.15 let erefs: Result, Entity> = world.get_entity(&ids[..]); let emuts: Result, EntityFetchError> = world.get_entity_mut(&ids[..]); let erefs: Vec = world.entity(&ids[..]); // Newly possible! let emuts: Vec = world.entity_mut(&ids[..]); // Newly possible! ``` Querying for multiple entities with an `EntityHashSet` has changed: ```rust let set = EntityHashSet::from_iter([e1, e2, e3]); // 0.14 let emuts: Result, QueryEntityError> = world.get_many_entities_from_set_mut(&set); // 0.15 let emuts: Result, EntityFetchError> = world.get_entity_mut(&set); let erefs: Result, EntityFetchError> = world.get_entity(&set); // Newly possible! let emuts: EntityHashMap = world.entity_mut(&set); // Newly possible! let erefs: EntityHashMap = world.entity(&set); // Newly possible! ```
## Migration Guide - `World::get_entity` now returns `Result<_, Entity>` instead of `Option<_>`. - Use `world.get_entity(..).ok()` to return to the previous behavior. - `World::get_entity_mut` and `DeferredWorld::get_entity_mut` now return `Result<_, EntityFetchError>` instead of `Option<_>`. - Use `world.get_entity_mut(..).ok()` to return to the previous behavior. - Type inference for `World::entity`, `World::entity_mut`, `World::get_entity`, `World::get_entity_mut`, `DeferredWorld::entity_mut`, and `DeferredWorld::get_entity_mut` has changed, and might now require the input argument's type to be explicitly written when inside closures. - The following functions have been deprecated, and should be replaced as such: - `World::many_entities` -> `World::entity::<[Entity; N]>` - `World::many_entities_mut` -> `World::entity_mut::<[Entity; N]>` - `World::get_many_entities` -> `World::get_entity::<[Entity; N]>` - `World::get_many_entities_dynamic` -> `World::get_entity::<&[Entity]>` - `World::get_many_entities_mut` -> `World::get_entity_mut::<[Entity; N]>` - The equivalent return type has changed from `Result<_, QueryEntityError>` to `Result<_, EntityFetchError>` - `World::get_many_entities_dynamic_mut` -> `World::get_entity_mut::<&[Entity]>1 - The equivalent return type has changed from `Result<_, QueryEntityError>` to `Result<_, EntityFetchError>` - `World::get_many_entities_from_set_mut` -> `World::get_entity_mut::<&EntityHashSet>` - The equivalent return type has changed from `Result, QueryEntityError>` to `Result, EntityFetchError>`. If necessary, you can still convert the `EntityHashMap` into a `Vec`. --- crates/bevy_ecs/src/lib.rs | 8 +- .../bevy_ecs/src/observer/entity_observer.rs | 2 +- .../bevy_ecs/src/reflect/entity_commands.rs | 4 +- crates/bevy_ecs/src/system/commands/mod.rs | 26 +- crates/bevy_ecs/src/system/system.rs | 2 +- crates/bevy_ecs/src/system/system_registry.rs | 12 +- crates/bevy_ecs/src/world/deferred_world.rs | 190 +++++- crates/bevy_ecs/src/world/entity_fetch.rs | 331 ++++++++++ crates/bevy_ecs/src/world/error.rs | 13 +- crates/bevy_ecs/src/world/mod.rs | 620 ++++++++++++------ crates/bevy_hierarchy/src/child_builder.rs | 2 +- crates/bevy_hierarchy/src/hierarchy.rs | 4 +- crates/bevy_remote/src/builtin_methods.rs | 4 +- crates/bevy_render/src/world_sync.rs | 4 +- crates/bevy_scene/src/bundle.rs | 2 +- crates/bevy_scene/src/scene_spawner.rs | 4 +- crates/bevy_scene/src/serde.rs | 2 +- crates/bevy_transform/src/commands.rs | 19 +- 18 files changed, 966 insertions(+), 283 deletions(-) create mode 100644 crates/bevy_ecs/src/world/entity_fetch.rs diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 4c7737328a..a62d23afb6 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1637,13 +1637,13 @@ mod tests { "new entity is created immediately after world_a's max entity" ); assert!(world_b.get::(e1).is_none()); - assert!(world_b.get_entity(e1).is_none()); + assert!(world_b.get_entity(e1).is_err()); assert!(world_b.get::(e2).is_none()); - assert!(world_b.get_entity(e2).is_none()); + assert!(world_b.get_entity(e2).is_err()); assert!(world_b.get::(e3).is_none()); - assert!(world_b.get_entity(e3).is_none()); + assert!(world_b.get_entity(e3).is_err()); world_b.get_or_spawn(e1).unwrap().insert(B(1)); assert_eq!( @@ -1694,7 +1694,7 @@ mod tests { let high_non_existent_but_reserved_entity = Entity::from_raw(5); assert!( - world_b.get_entity(high_non_existent_but_reserved_entity).is_none(), + world_b.get_entity(high_non_existent_but_reserved_entity).is_err(), "entities between high-newly allocated entity and continuous block of existing entities don't exist" ); diff --git a/crates/bevy_ecs/src/observer/entity_observer.rs b/crates/bevy_ecs/src/observer/entity_observer.rs index 0f9baba760..a5f332e6f3 100644 --- a/crates/bevy_ecs/src/observer/entity_observer.rs +++ b/crates/bevy_ecs/src/observer/entity_observer.rs @@ -19,7 +19,7 @@ impl Component for ObservedBy { }; for e in observed_by { let (total_entities, despawned_watched_entities) = { - let Some(mut entity_mut) = world.get_entity_mut(e) else { + let Ok(mut entity_mut) = world.get_entity_mut(e) else { continue; }; let Some(mut state) = entity_mut.get_mut::() else { diff --git a/crates/bevy_ecs/src/reflect/entity_commands.rs b/crates/bevy_ecs/src/reflect/entity_commands.rs index 3979eee229..60c89c7f7f 100644 --- a/crates/bevy_ecs/src/reflect/entity_commands.rs +++ b/crates/bevy_ecs/src/reflect/entity_commands.rs @@ -221,7 +221,7 @@ fn insert_reflect( .get_represented_type_info() .expect("component should represent a type."); let type_path = type_info.type_path(); - let Some(mut entity) = world.get_entity_mut(entity) else { + 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"); }; let Some(type_registration) = type_registry.get(type_info.type_id()) else { @@ -284,7 +284,7 @@ fn remove_reflect( type_registry: &TypeRegistry, component_type_path: Cow<'static, str>, ) { - let Some(mut entity) = world.get_entity_mut(entity) else { + let Ok(mut entity) = world.get_entity_mut(entity) else { return; }; let Some(type_registration) = type_registry.get_with_type_path(&component_type_path) else { diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index bb40dd48a1..2ba77daa30 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1757,7 +1757,7 @@ fn try_despawn() -> impl EntityCommand { fn insert(bundle: T, mode: InsertMode) -> impl EntityCommand { let caller = Location::caller(); move |entity: Entity, world: &mut World| { - if let Some(mut entity) = world.get_entity_mut(entity) { + if let Ok(mut entity) = world.get_entity_mut(entity) { entity.insert_with_caller( bundle, mode, @@ -1776,7 +1776,7 @@ fn insert_from_world(mode: InsertMode) -> impl EntityC let caller = Location::caller(); move |entity: Entity, world: &mut World| { let value = T::from_world(world); - if let Some(mut entity) = world.get_entity_mut(entity) { + if let Ok(mut entity) = world.get_entity_mut(entity) { entity.insert_with_caller( value, mode, @@ -1795,8 +1795,8 @@ fn insert_from_world(mode: InsertMode) -> impl EntityC fn try_insert(bundle: impl Bundle, mode: InsertMode) -> impl EntityCommand { #[cfg(feature = "track_change_detection")] let caller = Location::caller(); - move |entity, world: &mut World| { - if let Some(mut entity) = world.get_entity_mut(entity) { + move |entity: Entity, world: &mut World| { + if let Ok(mut entity) = world.get_entity_mut(entity) { entity.insert_with_caller( bundle, mode, @@ -1818,8 +1818,8 @@ unsafe fn insert_by_id( value: T, on_none_entity: impl FnOnce(Entity) + Send + 'static, ) -> impl EntityCommand { - move |entity, world: &mut World| { - if let Some(mut entity) = world.get_entity_mut(entity) { + move |entity: Entity, world: &mut World| { + if let Ok(mut entity) = world.get_entity_mut(entity) { // SAFETY: // - `component_id` safety is ensured by the caller // - `ptr` is valid within the `make` block; @@ -1837,7 +1837,7 @@ unsafe fn insert_by_id( /// For a [`Bundle`] type `T`, this will remove any components in the bundle. /// Any components in the bundle that aren't found on the entity will be ignored. fn remove(entity: Entity, world: &mut World) { - if let Some(mut entity) = world.get_entity_mut(entity) { + if let Ok(mut entity) = world.get_entity_mut(entity) { entity.remove::(); } } @@ -1848,7 +1848,7 @@ fn remove(entity: Entity, world: &mut World) { /// Panics if the provided [`ComponentId`] does not exist in the [`World`]. fn remove_by_id(component_id: ComponentId) -> impl EntityCommand { move |entity: Entity, world: &mut World| { - if let Some(mut entity) = world.get_entity_mut(entity) { + if let Ok(mut entity) = world.get_entity_mut(entity) { entity.remove_by_id(component_id); } } @@ -1856,7 +1856,7 @@ fn remove_by_id(component_id: ComponentId) -> impl EntityCommand { /// An [`EntityCommand`] that remove all components in the bundle and remove all required components for each component in the bundle. fn remove_with_requires(entity: Entity, world: &mut World) { - if let Some(mut entity) = world.get_entity_mut(entity) { + if let Ok(mut entity) = world.get_entity_mut(entity) { entity.remove_with_requires::(); } } @@ -1864,7 +1864,7 @@ fn remove_with_requires(entity: Entity, world: &mut World) { /// An [`EntityCommand`] that removes all components associated with a provided entity. fn clear() -> impl EntityCommand { move |entity: Entity, world: &mut World| { - if let Some(mut entity) = world.get_entity_mut(entity) { + if let Ok(mut entity) = world.get_entity_mut(entity) { entity.clear(); } } @@ -1875,7 +1875,7 @@ fn clear() -> impl EntityCommand { /// For a [`Bundle`] type `T`, this will remove all components except those in the bundle. /// Any components in the bundle that aren't found on the entity will be ignored. fn retain(entity: Entity, world: &mut World) { - if let Some(mut entity_mut) = world.get_entity_mut(entity) { + if let Ok(mut entity_mut) = world.get_entity_mut(entity) { entity_mut.retain::(); } } @@ -1919,8 +1919,8 @@ fn log_components(entity: Entity, world: &mut World) { fn observe( observer: impl IntoObserverSystem, ) -> impl EntityCommand { - move |entity, world: &mut World| { - if let Some(mut entity) = world.get_entity_mut(entity) { + move |entity: Entity, world: &mut World| { + if let Ok(mut entity) = world.get_entity_mut(entity) { entity.observe_entity(observer); } } diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index ded89235e7..c1c628cb2e 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -271,7 +271,7 @@ where /// let entity = world.run_system_once(|mut commands: Commands| { /// commands.spawn_empty().id() /// }).unwrap(); -/// # assert!(world.get_entity(entity).is_some()); +/// # assert!(world.get_entity(entity).is_ok()); /// ``` /// /// ## Immediate Queries diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 4b5508190a..fd4518e633 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -181,7 +181,7 @@ impl World { O: 'static, { match self.get_entity_mut(id.entity) { - Some(mut entity) => { + Ok(mut entity) => { let registered_system = entity .take::>() .ok_or(RegisteredSystemError::SelfRemove(id))?; @@ -191,7 +191,7 @@ impl World { system: registered_system.system, }) } - None => Err(RegisteredSystemError::SystemIdNotRegistered(id)), + Err(_) => Err(RegisteredSystemError::SystemIdNotRegistered(id)), } } @@ -327,7 +327,7 @@ impl World { // lookup let mut entity = self .get_entity_mut(id.entity) - .ok_or(RegisteredSystemError::SystemIdNotRegistered(id))?; + .map_err(|_| RegisteredSystemError::SystemIdNotRegistered(id))?; // take ownership of system trait object let RegisteredSystem { @@ -350,7 +350,7 @@ impl World { }; // return ownership of system trait object (if entity still exists) - if let Some(mut entity) = self.get_entity_mut(id.entity) { + if let Ok(mut entity) = self.get_entity_mut(id.entity) { entity.insert::>(RegisteredSystem { initialized, system, @@ -398,7 +398,7 @@ impl World { } self.resource_scope(|world, mut id: Mut>| { - if let Some(mut entity) = world.get_entity_mut(id.0.entity()) { + if let Ok(mut entity) = world.get_entity_mut(id.0.entity()) { if !entity.contains::>() { entity.insert(system_bundle(Box::new(IntoSystem::into_system(system)))); } @@ -538,7 +538,7 @@ where O: Send + 'static, { fn apply(self, world: &mut World) { - if let Some(mut entity) = world.get_entity_mut(self.entity) { + if let Ok(mut entity) = world.get_entity_mut(self.entity) { entity.insert(system_bundle(self.system)); } } diff --git a/crates/bevy_ecs/src/world/deferred_world.rs b/crates/bevy_ecs/src/world/deferred_world.rs index 8b3b046859..45e7a7cf87 100644 --- a/crates/bevy_ecs/src/world/deferred_world.rs +++ b/crates/bevy_ecs/src/world/deferred_world.rs @@ -11,12 +11,10 @@ use crate::{ query::{QueryData, QueryFilter}, system::{Commands, Query, Resource}, traversal::Traversal, + world::{error::EntityFetchError, WorldEntityFetch}, }; -use super::{ - unsafe_world_cell::{UnsafeEntityCell, UnsafeWorldCell}, - EntityMut, Mut, World, -}; +use super::{unsafe_world_cell::UnsafeWorldCell, Mut, World}; /// A [`World`] reference that disallows structural ECS changes. /// This includes initializing resources, registering components or spawning entities. @@ -81,35 +79,168 @@ impl<'w> DeferredWorld<'w> { unsafe { self.world.get_entity(entity)?.get_mut() } } - /// Retrieves an [`EntityMut`] that exposes read and write operations for the given `entity`. - /// Returns [`None`] if the `entity` does not exist. - /// Instead of unwrapping the value returned from this function, prefer [`Self::entity_mut`]. + /// Returns [`EntityMut`]s that expose read and write operations for the + /// given `entities`, returning [`Err`] if any of the given entities do not + /// exist. Instead of immediately unwrapping the value returned from this + /// function, prefer [`World::entity_mut`]. + /// + /// This function supports fetching a single entity or multiple entities: + /// - Pass an [`Entity`] to receive a single [`EntityMut`]. + /// - 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 an [`&EntityHashSet`] to receive an [`EntityHashMap`]. + /// + /// **As [`DeferredWorld`] does not allow structural changes, all returned + /// references are [`EntityMut`]s, which do not allow structural changes + /// (i.e. adding/removing components or despawning the entity).** + /// + /// # Errors + /// + /// - Returns [`EntityFetchError::NoSuchEntity`] 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. + /// + /// # Examples + /// + /// For examples, see [`DeferredWorld::entity_mut`]. + /// + /// [`EntityMut`]: crate::world::EntityMut + /// [`&EntityHashSet`]: crate::entity::EntityHashSet + /// [`EntityHashMap`]: crate::entity::EntityHashMap #[inline] - pub fn get_entity_mut(&mut self, entity: Entity) -> Option { - let location = self.entities.get(entity)?; - // SAFETY: if the Entity is invalid, the function returns early. - // Additionally, Entities::get(entity) returns the correct EntityLocation if the entity exists. - let entity_cell = UnsafeEntityCell::new(self.as_unsafe_world_cell(), entity, location); - // SAFETY: The UnsafeEntityCell has read access to the entire world. - let entity_ref = unsafe { EntityMut::new(entity_cell) }; - Some(entity_ref) + pub fn get_entity_mut( + &mut self, + entities: F, + ) -> Result, EntityFetchError> { + 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. + unsafe { entities.fetch_deferred_mut(cell) } } - /// Retrieves an [`EntityMut`] that exposes read and write operations for the given `entity`. - /// This will panic if the `entity` does not exist. Use [`Self::get_entity_mut`] if you want - /// to check for entity existence instead of implicitly panic-ing. + /// Returns [`EntityMut`]s that expose read and write operations for the + /// given `entities`. This will panic if any of the given entities do not + /// exist. Use [`DeferredWorld::get_entity_mut`] if you want to check for + /// entity existence instead of implicitly panicking. + /// + /// This function supports fetching a single entity or multiple entities: + /// - Pass an [`Entity`] to receive a single [`EntityMut`]. + /// - 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 an [`&EntityHashSet`] to receive an [`EntityHashMap`]. + /// + /// **As [`DeferredWorld`] does not allow structural changes, all returned + /// references are [`EntityMut`]s, which do not allow structural changes + /// (i.e. adding/removing components or despawning the entity).** + /// + /// # Panics + /// + /// If any of the given `entities` do not exist in the world. + /// + /// # Examples + /// + /// ## Single [`Entity`] + /// + /// ``` + /// # use bevy_ecs::{prelude::*, world::DeferredWorld}; + /// #[derive(Component)] + /// struct Position { + /// x: f32, + /// y: f32, + /// } + /// + /// # let mut world = World::new(); + /// # let entity = world.spawn(Position { x: 0.0, y: 0.0 }).id(); + /// let mut world: DeferredWorld = // ... + /// # DeferredWorld::from(&mut world); + /// + /// let mut entity_mut = world.entity_mut(entity); + /// let mut position = entity_mut.get_mut::().unwrap(); + /// position.y = 1.0; + /// assert_eq!(position.x, 0.0); + /// ``` + /// + /// ## Array of [`Entity`]s + /// + /// ``` + /// # use bevy_ecs::{prelude::*, world::DeferredWorld}; + /// #[derive(Component)] + /// struct Position { + /// x: f32, + /// y: f32, + /// } + /// + /// # let mut world = World::new(); + /// # let e1 = world.spawn(Position { x: 0.0, y: 0.0 }).id(); + /// # let e2 = world.spawn(Position { x: 1.0, y: 1.0 }).id(); + /// let mut world: DeferredWorld = // ... + /// # DeferredWorld::from(&mut world); + /// + /// let [mut e1_ref, mut e2_ref] = world.entity_mut([e1, e2]); + /// let mut e1_position = e1_ref.get_mut::().unwrap(); + /// e1_position.x = 1.0; + /// assert_eq!(e1_position.x, 1.0); + /// let mut e2_position = e2_ref.get_mut::().unwrap(); + /// e2_position.x = 2.0; + /// assert_eq!(e2_position.x, 2.0); + /// ``` + /// + /// ## Slice of [`Entity`]s + /// + /// ``` + /// # use bevy_ecs::{prelude::*, world::DeferredWorld}; + /// #[derive(Component)] + /// struct Position { + /// x: f32, + /// y: f32, + /// } + /// + /// # let mut world = World::new(); + /// # let e1 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// # let e2 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// # let e3 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// let mut world: DeferredWorld = // ... + /// # DeferredWorld::from(&mut world); + /// + /// let ids = vec![e1, e2, e3]; + /// for mut eref in world.entity_mut(&ids[..]) { + /// let mut pos = eref.get_mut::().unwrap(); + /// pos.y = 2.0; + /// assert_eq!(pos.y, 2.0); + /// } + /// ``` + /// + /// ## [`&EntityHashSet`] + /// + /// ``` + /// # use bevy_ecs::{prelude::*, entity::EntityHashSet, world::DeferredWorld}; + /// #[derive(Component)] + /// struct Position { + /// x: f32, + /// y: f32, + /// } + /// + /// # let mut world = World::new(); + /// # let e1 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// # let e2 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// # let e3 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// let mut world: DeferredWorld = // ... + /// # DeferredWorld::from(&mut world); + /// + /// let ids = EntityHashSet::from_iter([e1, e2, e3]); + /// for (_id, mut eref) in world.entity_mut(&ids) { + /// let mut pos = eref.get_mut::().unwrap(); + /// pos.y = 2.0; + /// assert_eq!(pos.y, 2.0); + /// } + /// ``` + /// + /// [`EntityMut`]: crate::world::EntityMut + /// [`&EntityHashSet`]: crate::entity::EntityHashSet + /// [`EntityHashMap`]: crate::entity::EntityHashMap #[inline] - pub fn entity_mut(&mut self, entity: Entity) -> EntityMut { - #[inline(never)] - #[cold] - fn panic_no_entity(entity: Entity) -> ! { - panic!("Entity {entity:?} does not exist"); - } - - match self.get_entity_mut(entity) { - Some(entity) => entity, - None => panic_no_entity(entity), - } + pub fn entity_mut(&mut self, entities: F) -> F::DeferredMut<'_> { + self.get_entity_mut(entities).unwrap() } /// Returns [`Query`] for the given [`QueryState`], which is used to efficiently @@ -411,6 +542,7 @@ impl<'w> DeferredWorld<'w> { } if let Some(traverse_to) = self .get_entity(entity) + .ok() .and_then(|entity| entity.get_components::()) .and_then(T::traverse) { diff --git a/crates/bevy_ecs/src/world/entity_fetch.rs b/crates/bevy_ecs/src/world/entity_fetch.rs new file mode 100644 index 0000000000..62d63ced54 --- /dev/null +++ b/crates/bevy_ecs/src/world/entity_fetch.rs @@ -0,0 +1,331 @@ +use core::mem::MaybeUninit; + +use crate::{ + entity::{Entity, EntityHash, EntityHashMap, EntityHashSet}, + world::{ + error::EntityFetchError, unsafe_world_cell::UnsafeWorldCell, EntityMut, EntityRef, + EntityWorldMut, + }, +}; + +/// Types that can be used to fetch [`Entity`] references from a [`World`]. +/// +/// Provided implementations are: +/// - [`Entity`]: Fetch a single entity. +/// - `[Entity; N]`/`&[Entity; N]`: Fetch multiple entities, receiving a +/// same-sized array of references. +/// - `&[Entity]`: Fetch multiple entities, receiving a vector of references. +/// - [`&EntityHashSet`](EntityHashSet): Fetch multiple entities, receiving a +/// hash map of [`Entity`] IDs to references. +/// +/// # Performance +/// +/// - The slice and array implementations perform an aliased mutabiltiy check +/// in [`WorldEntityFetch::fetch_mut`] that is `O(N^2)`. +/// - The [`EntityHashSet`] implementation performs no such check as the type +/// itself guarantees no duplicates. +/// - The single [`Entity`] implementation performs no such check as only one +/// reference is returned. +/// +/// # Safety +/// +/// Implementor must ensure that: +/// - No aliased mutability is caused by the returned references. +/// - [`WorldEntityFetch::fetch_ref`] returns only read-only references. +/// - [`WorldEntityFetch::fetch_deferred_mut`] returns only non-structurally-mutable references. +/// +/// [`World`]: crate::world::World +pub unsafe trait WorldEntityFetch { + /// The read-only reference type returned by [`WorldEntityFetch::fetch_ref`]. + type Ref<'w>; + + /// The mutable reference type returned by [`WorldEntityFetch::fetch_mut`]. + type Mut<'w>; + + /// The mutable reference type returned by [`WorldEntityFetch::fetch_deferred_mut`], + /// but without structural mutability. + type DeferredMut<'w>; + + /// Returns read-only reference(s) to the entities with the given + /// [`Entity`] IDs, as determined by `self`. + /// + /// # Safety + /// + /// It is the caller's responsibility to ensure that: + /// - The given [`UnsafeWorldCell`] has read-only access to the fetched entities. + /// - No other mutable references to the fetched entities exist at the same time. + /// + /// # Errors + /// + /// - Returns [`Entity`] if the entity does not exist. + unsafe fn fetch_ref(self, cell: UnsafeWorldCell<'_>) -> Result, Entity>; + + /// Returns mutable reference(s) to the entities with the given [`Entity`] + /// IDs, as determined by `self`. + /// + /// # Safety + /// + /// It is the caller's responsibility to ensure that: + /// - The given [`UnsafeWorldCell`] has mutable access to the fetched entities. + /// - No other references to the fetched entities exist at the same time. + /// + /// # Errors + /// + /// - Returns [`EntityFetchError::NoSuchEntity`] if the entity does not exist. + /// - Returns [`EntityFetchError::AliasedMutability`] if the entity was + /// requested mutably more than once. + unsafe fn fetch_mut(self, cell: UnsafeWorldCell<'_>) + -> Result, EntityFetchError>; + + /// Returns mutable reference(s) to the entities with the given [`Entity`] + /// IDs, as determined by `self`, but without structural mutability. + /// + /// No structural mutability means components cannot be removed from the + /// entity, new components cannot be added to the entity, and the entity + /// cannot be despawned. + /// + /// # Safety + /// + /// It is the caller's responsibility to ensure that: + /// - The given [`UnsafeWorldCell`] has mutable access to the fetched entities. + /// - No other references to the fetched entities exist at the same time. + /// + /// # Errors + /// + /// - Returns [`EntityFetchError::NoSuchEntity`] if the entity does not exist. + /// - Returns [`EntityFetchError::AliasedMutability`] if the entity was + /// requested mutably more than once. + unsafe fn fetch_deferred_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityFetchError>; +} + +// SAFETY: +// - No aliased mutability is caused because a single reference is returned. +// - No mutable references are returned by `fetch_ref`. +// - No structurally-mutable references are returned by `fetch_deferred_mut`. +unsafe impl WorldEntityFetch for Entity { + type Ref<'w> = EntityRef<'w>; + 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)?; + // SAFETY: caller ensures that the world cell has read-only access to the entity. + Ok(unsafe { EntityRef::new(ecell) }) + } + + unsafe fn fetch_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityFetchError> { + let location = cell + .entities() + .get(self) + .ok_or(EntityFetchError::NoSuchEntity(self))?; + // 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`. + Ok(unsafe { EntityWorldMut::new(world, self, location) }) + } + + unsafe fn fetch_deferred_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityFetchError> { + let ecell = cell + .get_entity(self) + .ok_or(EntityFetchError::NoSuchEntity(self))?; + // SAFETY: caller ensures that the world cell has mutable access to the entity. + Ok(unsafe { EntityMut::new(ecell) }) + } +} + +// SAFETY: +// - No aliased mutability is caused because the array is checked for duplicates. +// - No mutable references are returned by `fetch_ref`. +// - No structurally-mutable references are returned by `fetch_deferred_mut`. +unsafe impl WorldEntityFetch for [Entity; N] { + type Ref<'w> = [EntityRef<'w>; N]; + type Mut<'w> = [EntityMut<'w>; N]; + type DeferredMut<'w> = [EntityMut<'w>; N]; + + unsafe fn fetch_ref(self, cell: UnsafeWorldCell<'_>) -> Result, Entity> { + <&Self>::fetch_ref(&self, cell) + } + + unsafe fn fetch_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityFetchError> { + <&Self>::fetch_mut(&self, cell) + } + + unsafe fn fetch_deferred_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityFetchError> { + <&Self>::fetch_deferred_mut(&self, cell) + } +} + +// SAFETY: +// - No aliased mutability is caused because the array is checked for duplicates. +// - No mutable references are returned by `fetch_ref`. +// - No structurally-mutable references are returned by `fetch_deferred_mut`. +unsafe impl WorldEntityFetch for &'_ [Entity; N] { + type Ref<'w> = [EntityRef<'w>; N]; + type Mut<'w> = [EntityMut<'w>; N]; + type DeferredMut<'w> = [EntityMut<'w>; N]; + + unsafe fn fetch_ref(self, cell: UnsafeWorldCell<'_>) -> Result, Entity> { + 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)?; + // SAFETY: caller ensures that the world cell has read-only access to the entity. + *r = MaybeUninit::new(unsafe { EntityRef::new(ecell) }); + } + + // SAFETY: Each item was initialized in the loop above. + let refs = refs.map(|r| unsafe { MaybeUninit::assume_init(r) }); + + Ok(refs) + } + + unsafe fn fetch_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityFetchError> { + // 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])); + } + } + } + + 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))?; + // SAFETY: caller ensures that the world cell has mutable access to the entity. + *r = MaybeUninit::new(unsafe { EntityMut::new(ecell) }); + } + + // SAFETY: Each item was initialized in the loop above. + let refs = refs.map(|r| unsafe { MaybeUninit::assume_init(r) }); + + Ok(refs) + } + + unsafe fn fetch_deferred_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityFetchError> { + // 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) } + } +} + +// SAFETY: +// - No aliased mutability is caused because the slice is checked for duplicates. +// - No mutable references are returned by `fetch_ref`. +// - No structurally-mutable references are returned by `fetch_deferred_mut`. +unsafe impl WorldEntityFetch for &'_ [Entity] { + type Ref<'w> = Vec>; + type Mut<'w> = Vec>; + type DeferredMut<'w> = Vec>; + + unsafe fn fetch_ref(self, cell: UnsafeWorldCell<'_>) -> Result, Entity> { + let mut refs = Vec::with_capacity(self.len()); + for &id in self { + let ecell = cell.get_entity(id).ok_or(id)?; + // SAFETY: caller ensures that the world cell has read-only access to the entity. + refs.push(unsafe { EntityRef::new(ecell) }); + } + + Ok(refs) + } + + unsafe fn fetch_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityFetchError> { + // 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])); + } + } + } + + let mut refs = Vec::with_capacity(self.len()); + for &id in self { + let ecell = cell + .get_entity(id) + .ok_or(EntityFetchError::NoSuchEntity(id))?; + // SAFETY: caller ensures that the world cell has mutable access to the entity. + refs.push(unsafe { EntityMut::new(ecell) }); + } + + Ok(refs) + } + + unsafe fn fetch_deferred_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityFetchError> { + // 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) } + } +} + +// SAFETY: +// - No aliased mutability is caused because `EntityHashSet` guarantees no duplicates. +// - No mutable references are returned by `fetch_ref`. +// - No structurally-mutable references are returned by `fetch_deferred_mut`. +unsafe impl WorldEntityFetch for &'_ EntityHashSet { + type Ref<'w> = EntityHashMap>; + type Mut<'w> = EntityHashMap>; + type DeferredMut<'w> = EntityHashMap>; + + unsafe fn fetch_ref(self, cell: UnsafeWorldCell<'_>) -> Result, Entity> { + let mut refs = EntityHashMap::with_capacity_and_hasher(self.len(), EntityHash); + for &id in self { + let ecell = cell.get_entity(id).ok_or(id)?; + // SAFETY: caller ensures that the world cell has read-only access to the entity. + refs.insert(id, unsafe { EntityRef::new(ecell) }); + } + Ok(refs) + } + + unsafe fn fetch_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityFetchError> { + let mut refs = EntityHashMap::with_capacity_and_hasher(self.len(), EntityHash); + for &id in self { + let ecell = cell + .get_entity(id) + .ok_or(EntityFetchError::NoSuchEntity(id))?; + // SAFETY: caller ensures that the world cell has mutable access to the entity. + refs.insert(id, unsafe { EntityMut::new(ecell) }); + } + Ok(refs) + } + + unsafe fn fetch_deferred_mut( + self, + cell: UnsafeWorldCell<'_>, + ) -> Result, EntityFetchError> { + // 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 5fc4264e07..91cd2fa034 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -2,7 +2,7 @@ use thiserror::Error; -use crate::{component::ComponentId, schedule::InternedScheduleLabel}; +use crate::{component::ComponentId, entity::Entity, schedule::InternedScheduleLabel}; /// The error type returned by [`World::try_run_schedule`] if the provided schedule does not exist. /// @@ -21,3 +21,14 @@ pub enum EntityComponentError { #[error("The component with ID {0:?} was requested mutably more than once.")] AliasedMutability(ComponentId), } + +/// An error that occurs when fetching entities mutably from a world. +#[derive(Error, Debug, Clone, Copy, PartialEq, Eq)] +pub enum EntityFetchError { + /// The entity with the given ID does not exist. + #[error("The entity with ID {0:?} does not exist.")] + NoSuchEntity(Entity), + /// 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), +} diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 7dae46a97a..7c4daffb32 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -3,6 +3,7 @@ pub(crate) mod command_queue; mod component_constants; mod deferred_world; +mod entity_fetch; mod entity_ref; pub mod error; mod filtered_resource; @@ -19,6 +20,7 @@ pub use crate::{ }; pub use component_constants::*; pub use deferred_world::DeferredWorld; +pub use entity_fetch::WorldEntityFetch; pub use entity_ref::{ EntityMut, EntityMutExcept, EntityRef, EntityRefExcept, EntityWorldMut, Entry, FilteredEntityMut, FilteredEntityRef, OccupiedEntry, VacantEntry, @@ -43,14 +45,16 @@ use crate::{ schedule::{Schedule, ScheduleLabel, Schedules}, storage::{ResourceData, Storages}, system::{Commands, Resource}, - world::{command_queue::RawCommandQueue, error::TryRunScheduleError}, + world::{ + command_queue::RawCommandQueue, + error::{EntityFetchError, TryRunScheduleError}, + }, }; use bevy_ptr::{OwningPtr, Ptr}; use bevy_utils::tracing::warn; use core::{ any::TypeId, fmt, - mem::MaybeUninit, sync::atomic::{AtomicU32, Ordering}, }; @@ -595,13 +599,28 @@ impl World { self.components.get_resource_id(TypeId::of::()) } - /// Retrieves an [`EntityRef`] that exposes read-only operations for the given `entity`. - /// This will panic if the `entity` does not exist. Use [`World::get_entity`] if you want - /// to check for entity existence instead of implicitly panic-ing. + /// Returns [`EntityRef`]s that expose read-only operations for the given + /// `entities`. This will panic if any of the given entities do not exist. Use + /// [`World::get_entity`] if you want to check for entity existence instead + /// of implicitly panicking. + /// + /// This function supports fetching a single entity or multiple entities: + /// - 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 + /// [`EntityHashMap`](crate::entity::EntityHashMap). + /// + /// # Panics + /// + /// If any of the given `entities` do not exist in the world. + /// + /// # Examples + /// + /// ## Single [`Entity`] /// /// ``` - /// use bevy_ecs::{component::Component, world::World}; - /// + /// # use bevy_ecs::prelude::*; /// #[derive(Component)] /// struct Position { /// x: f32, @@ -610,12 +629,76 @@ impl World { /// /// let mut world = World::new(); /// let entity = world.spawn(Position { x: 0.0, y: 0.0 }).id(); + /// /// let position = world.entity(entity).get::().unwrap(); /// assert_eq!(position.x, 0.0); /// ``` + /// + /// ## Array of [`Entity`]s + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// #[derive(Component)] + /// struct Position { + /// x: f32, + /// y: f32, + /// } + /// + /// let mut world = World::new(); + /// let e1 = world.spawn(Position { x: 0.0, y: 0.0 }).id(); + /// let e2 = world.spawn(Position { x: 1.0, y: 1.0 }).id(); + /// + /// let [e1_ref, e2_ref] = world.entity([e1, e2]); + /// let e1_position = e1_ref.get::().unwrap(); + /// assert_eq!(e1_position.x, 0.0); + /// let e2_position = e2_ref.get::().unwrap(); + /// assert_eq!(e2_position.x, 1.0); + /// ``` + /// + /// ## Slice of [`Entity`]s + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// #[derive(Component)] + /// struct Position { + /// x: f32, + /// y: f32, + /// } + /// + /// let mut world = World::new(); + /// let e1 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// let e2 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// let e3 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// + /// let ids = vec![e1, e2, e3]; + /// for eref in world.entity(&ids[..]) { + /// assert_eq!(eref.get::().unwrap().y, 1.0); + /// } + /// ``` + /// + /// ## [`EntityHashSet`] + /// + /// ``` + /// # use bevy_ecs::{prelude::*, entity::EntityHashSet}; + /// #[derive(Component)] + /// struct Position { + /// x: f32, + /// y: f32, + /// } + /// + /// let mut world = World::new(); + /// let e1 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// let e2 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// let e3 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// + /// let ids = EntityHashSet::from_iter([e1, e2, e3]); + /// for (_id, eref) in world.entity(&ids) { + /// assert_eq!(eref.get::().unwrap().y, 1.0); + /// } + /// ``` #[inline] #[track_caller] - pub fn entity(&self, entity: Entity) -> EntityRef { + pub fn entity(&self, entities: F) -> F::Ref<'_> { #[inline(never)] #[cold] #[track_caller] @@ -623,19 +706,42 @@ impl World { panic!("Entity {entity:?} does not exist"); } - match self.get_entity(entity) { - Some(entity) => entity, - None => panic_no_entity(entity), + match self.get_entity(entities) { + Ok(fetched) => fetched, + Err(entity) => panic_no_entity(entity), } } - /// Retrieves an [`EntityWorldMut`] that exposes read and write operations for the given `entity`. - /// This will panic if the `entity` does not exist. Use [`World::get_entity_mut`] if you want - /// to check for entity existence instead of implicitly panic-ing. + /// Returns [`EntityMut`]s that expose read and write operations for the + /// given `entities`. This will panic if any of the given entities do not + /// exist. Use [`World::get_entity_mut`] if you want to check for entity + /// existence instead of implicitly panicking. + /// + /// This function supports fetching a single entity or multiple entities: + /// - Pass an [`Entity`] to receive a single [`EntityWorldMut`]. + /// - This reference type allows for structural changes to the entity, + /// 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 + /// [`EntityHashMap`](crate::entity::EntityHashMap). + /// + /// In order to perform structural changes on the returned entity reference, + /// such as adding or removing components, or despawning the entity, only a + /// single [`Entity`] can be passed to this function. Allowing multiple + /// entities at the same time with structural access would lead to undefined + /// behavior, so [`EntityMut`] is returned when requesting multiple entities. + /// + /// # Panics + /// + /// If any of the given `entities` do not exist in the world. + /// + /// # Examples + /// + /// ## Single [`Entity`] /// /// ``` - /// use bevy_ecs::{component::Component, world::World}; - /// + /// # use bevy_ecs::prelude::*; /// #[derive(Component)] /// struct Position { /// x: f32, @@ -644,23 +750,96 @@ impl World { /// /// let mut world = World::new(); /// let entity = world.spawn(Position { x: 0.0, y: 0.0 }).id(); + /// /// let mut entity_mut = world.entity_mut(entity); /// let mut position = entity_mut.get_mut::().unwrap(); - /// position.x = 1.0; + /// position.y = 1.0; + /// assert_eq!(position.x, 0.0); + /// entity_mut.despawn(); + /// # assert!(world.get_entity_mut(entity).is_err()); + /// ``` + /// + /// ## Array of [`Entity`]s + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// #[derive(Component)] + /// struct Position { + /// x: f32, + /// y: f32, + /// } + /// + /// let mut world = World::new(); + /// let e1 = world.spawn(Position { x: 0.0, y: 0.0 }).id(); + /// let e2 = world.spawn(Position { x: 1.0, y: 1.0 }).id(); + /// + /// let [mut e1_ref, mut e2_ref] = world.entity_mut([e1, e2]); + /// let mut e1_position = e1_ref.get_mut::().unwrap(); + /// e1_position.x = 1.0; + /// assert_eq!(e1_position.x, 1.0); + /// let mut e2_position = e2_ref.get_mut::().unwrap(); + /// e2_position.x = 2.0; + /// assert_eq!(e2_position.x, 2.0); + /// ``` + /// + /// ## Slice of [`Entity`]s + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// #[derive(Component)] + /// struct Position { + /// x: f32, + /// y: f32, + /// } + /// + /// let mut world = World::new(); + /// let e1 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// let e2 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// let e3 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// + /// let ids = vec![e1, e2, e3]; + /// for mut eref in world.entity_mut(&ids[..]) { + /// let mut pos = eref.get_mut::().unwrap(); + /// pos.y = 2.0; + /// assert_eq!(pos.y, 2.0); + /// } + /// ``` + /// + /// ## [`EntityHashSet`] + /// + /// ``` + /// # use bevy_ecs::{prelude::*, entity::EntityHashSet}; + /// #[derive(Component)] + /// struct Position { + /// x: f32, + /// y: f32, + /// } + /// + /// let mut world = World::new(); + /// let e1 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// let e2 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// let e3 = world.spawn(Position { x: 0.0, y: 1.0 }).id(); + /// + /// let ids = EntityHashSet::from_iter([e1, e2, e3]); + /// for (_id, mut eref) in world.entity_mut(&ids) { + /// let mut pos = eref.get_mut::().unwrap(); + /// pos.y = 2.0; + /// assert_eq!(pos.y, 2.0); + /// } /// ``` #[inline] #[track_caller] - pub fn entity_mut(&mut self, entity: Entity) -> EntityWorldMut { + pub fn entity_mut(&mut self, entities: F) -> F::Mut<'_> { #[inline(never)] #[cold] #[track_caller] - fn panic_no_entity(entity: Entity) -> ! { - panic!("Entity {entity:?} does not exist"); + fn panic_on_err(e: EntityFetchError) -> ! { + panic!("{e}"); } - match self.get_entity_mut(entity) { - Some(entity) => entity, - None => panic_no_entity(entity), + match self.get_entity_mut(entities) { + Ok(fetched) => fetched, + Err(e) => panic_on_err(e), } } @@ -690,18 +869,9 @@ impl World { /// world.despawn(id2); /// world.many_entities([id1, id2]); /// ``` + #[deprecated(since = "0.15.0", note = "Use `World::entity::<[Entity; N]>` instead")] pub fn many_entities(&mut self, entities: [Entity; N]) -> [EntityRef<'_>; N] { - #[inline(never)] - #[cold] - #[track_caller] - fn panic_no_entity(entity: Entity) -> ! { - panic!("Entity {entity:?} does not exist"); - } - - match self.get_many_entities(entities) { - Ok(refs) => refs, - Err(entity) => panic_no_entity(entity), - } + self.entity(entities) } /// Gets mutable access to multiple entities at once. @@ -732,21 +902,15 @@ impl World { /// # let id = world.spawn_empty().id(); /// world.many_entities_mut([id, id]); /// ``` + #[deprecated( + since = "0.15.0", + note = "Use `World::entity_mut::<[Entity; N]>` instead" + )] pub fn many_entities_mut( &mut self, entities: [Entity; N], ) -> [EntityMut<'_>; N] { - #[inline(never)] - #[cold] - #[track_caller] - fn panic_on_err(e: QueryEntityError) -> ! { - panic!("{e}"); - } - - match self.get_many_entities_mut(entities) { - Ok(borrows) => borrows, - Err(e) => panic_on_err(e), - } + self.entity_mut(entities) } /// Returns the components of an [`Entity`] through [`ComponentInfo`]. @@ -795,35 +959,31 @@ impl World { } } - /// Retrieves an [`EntityRef`] that exposes read-only operations for the given `entity`. - /// Returns [`None`] if the `entity` does not exist. - /// Instead of unwrapping the value returned from this function, prefer [`World::entity`]. + /// Returns [`EntityRef`]s that expose read-only operations for the given + /// `entities`, returning [`Err`] if any of the given entities do not exist. + /// Instead of immediately unwrapping the value returned from this function, + /// prefer [`World::entity`]. /// - /// ``` - /// use bevy_ecs::{component::Component, world::World}; + /// This function supports fetching a single entity or multiple entities: + /// - 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 + /// [`EntityHashMap`](crate::entity::EntityHashMap). /// - /// #[derive(Component)] - /// struct Position { - /// x: f32, - /// y: f32, - /// } + /// # Errors /// - /// let mut world = World::new(); - /// let entity = world.spawn(Position { x: 0.0, y: 0.0 }).id(); - /// let entity_ref = world.get_entity(entity).unwrap(); - /// let position = entity_ref.get::().unwrap(); - /// assert_eq!(position.x, 0.0); - /// ``` + /// 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`]. + /// + /// # Examples + /// + /// For examples, see [`World::entity`]. #[inline] - pub fn get_entity(&self, entity: Entity) -> Option { - let location = self.entities.get(entity)?; - // SAFETY: if the Entity is invalid, the function returns early. - // Additionally, Entities::get(entity) returns the correct EntityLocation if the entity exists. - let entity_cell = - UnsafeEntityCell::new(self.as_unsafe_world_cell_readonly(), entity, location); - // SAFETY: The UnsafeEntityCell has read access to the entire world. - let entity_ref = unsafe { EntityRef::new(entity_cell) }; - Some(entity_ref) + pub fn get_entity(&self, entities: F) -> Result, Entity> { + 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) } } /// Gets an [`EntityRef`] for multiple entities at once. @@ -846,19 +1006,15 @@ impl World { /// world.despawn(id2); /// assert!(world.get_many_entities([id1, id2]).is_err()); /// ``` + #[deprecated( + since = "0.15.0", + note = "Use `World::get_entity::<[Entity; N]>` instead" + )] pub fn get_many_entities( &self, entities: [Entity; N], ) -> Result<[EntityRef<'_>; N], Entity> { - let mut refs = [MaybeUninit::uninit(); N]; - for (r, id) in core::iter::zip(&mut refs, entities) { - *r = MaybeUninit::new(self.get_entity(id).ok_or(id)?); - } - - // SAFETY: Each item was initialized in the above loop. - let refs = refs.map(|r| unsafe { MaybeUninit::assume_init(r) }); - - Ok(refs) + self.get_entity(entities) } /// Gets an [`EntityRef`] for multiple entities at once, whose number is determined at runtime. @@ -883,16 +1039,55 @@ impl World { /// world.despawn(id2); /// assert!(world.get_many_entities_dynamic(&[id1, id2]).is_err()); /// ``` + #[deprecated( + since = "0.15.0", + note = "Use `World::get_entity::<&[Entity]>` instead" + )] pub fn get_many_entities_dynamic<'w>( &'w self, entities: &[Entity], ) -> Result>, Entity> { - let mut borrows = Vec::with_capacity(entities.len()); - for &id in entities { - borrows.push(self.get_entity(id).ok_or(id)?); - } + self.get_entity(entities) + } - Ok(borrows) + /// Returns [`EntityMut`]s that expose read and write operations for the + /// given `entities`, returning [`Err`] if any of the given entities do not + /// exist. Instead of immediately unwrapping the value returned from this + /// function, prefer [`World::entity_mut`]. + /// + /// This function supports fetching a single entity or multiple entities: + /// - Pass an [`Entity`] to receive a single [`EntityWorldMut`]. + /// - This reference type allows for structural changes to the entity, + /// 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 + /// [`EntityHashMap`](crate::entity::EntityHashMap). + /// + /// In order to perform structural changes on the returned entity reference, + /// such as adding or removing components, or despawning the entity, only a + /// single [`Entity`] can be passed to this function. Allowing multiple + /// entities at the same time with structural access would lead to undefined + /// behavior, so [`EntityMut`] is returned when requesting multiple entities. + /// + /// # Errors + /// + /// - Returns [`EntityFetchError::NoSuchEntity`] 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. + /// + /// # Examples + /// + /// For examples, see [`World::entity_mut`]. + #[inline] + pub fn get_entity_mut( + &mut self, + entities: F, + ) -> Result, EntityFetchError> { + 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. + unsafe { entities.fetch_mut(cell) } } /// Returns an [`Entity`] iterator of current entities. @@ -952,49 +1147,6 @@ impl World { }) } - /// Retrieves an [`EntityWorldMut`] that exposes read and write operations for the given `entity`. - /// Returns [`None`] if the `entity` does not exist. - /// Instead of unwrapping the value returned from this function, prefer [`World::entity_mut`]. - /// - /// ``` - /// use bevy_ecs::{component::Component, world::World}; - /// - /// #[derive(Component)] - /// struct Position { - /// x: f32, - /// y: f32, - /// } - /// - /// let mut world = World::new(); - /// let entity = world.spawn(Position { x: 0.0, y: 0.0 }).id(); - /// let mut entity_mut = world.get_entity_mut(entity).unwrap(); - /// let mut position = entity_mut.get_mut::().unwrap(); - /// position.x = 1.0; - /// ``` - #[inline] - pub fn get_entity_mut(&mut self, entity: Entity) -> Option { - let location = self.entities.get(entity)?; - // SAFETY: `entity` exists and `location` is that entity's location - Some(unsafe { EntityWorldMut::new(self, entity, location) }) - } - - /// Verify that no duplicate entities are present in the given slice. - /// Does NOT check if the entities actually exist in the world. - /// - /// # Errors - /// - /// If any entities are duplicated. - fn verify_unique_entities(entities: &[Entity]) -> Result<(), QueryEntityError<'static>> { - for i in 0..entities.len() { - for j in 0..i { - if entities[i] == entities[j] { - return Err(QueryEntityError::AliasedMutability(entities[i])); - } - } - } - Ok(()) - } - /// Gets mutable access to multiple entities. /// /// # Errors @@ -1015,42 +1167,20 @@ impl World { /// // Trying to access the same entity multiple times will fail. /// assert!(world.get_many_entities_mut([id1, id1]).is_err()); /// ``` + #[deprecated( + since = "0.15.0", + note = "Use `World::get_entity_mut::<[Entity; N]>` instead" + )] pub fn get_many_entities_mut( &mut self, entities: [Entity; N], ) -> Result<[EntityMut<'_>; N], QueryEntityError> { - Self::verify_unique_entities(&entities)?; - - // SAFETY: Each entity is unique. - unsafe { self.get_entities_mut_unchecked(entities) } - } - - /// # Safety - /// `entities` must contain no duplicate [`Entity`] IDs. - unsafe fn get_entities_mut_unchecked( - &mut self, - entities: [Entity; N], - ) -> Result<[EntityMut<'_>; N], QueryEntityError> { - let world_cell = self.as_unsafe_world_cell(); - - let mut cells = [MaybeUninit::uninit(); N]; - for (cell, id) in core::iter::zip(&mut cells, entities) { - *cell = MaybeUninit::new( - world_cell - .get_entity(id) - .ok_or(QueryEntityError::NoSuchEntity(id))?, - ); - } - // SAFETY: Each item was initialized in the loop above. - let cells = cells.map(|c| unsafe { MaybeUninit::assume_init(c) }); - - // SAFETY: - // - `world_cell` has exclusive access to the entire world. - // - The caller ensures that each entity is unique, so none - // of the borrows will conflict with one another. - let borrows = cells.map(|c| unsafe { EntityMut::new(c) }); - - Ok(borrows) + self.get_entity_mut(entities).map_err(|e| match e { + EntityFetchError::NoSuchEntity(entity) => QueryEntityError::NoSuchEntity(entity), + EntityFetchError::AliasedMutability(entity) => { + QueryEntityError::AliasedMutability(entity) + } + }) } /// Gets mutable access to multiple entities, whose number is determined at runtime. @@ -1074,14 +1204,20 @@ impl World { /// // Trying to access the same entity multiple times will fail. /// assert!(world.get_many_entities_dynamic_mut(&[id1, id1]).is_err()); /// ``` + #[deprecated( + since = "0.15.0", + note = "Use `World::get_entity_mut::<&[Entity]>` instead" + )] pub fn get_many_entities_dynamic_mut<'w>( &'w mut self, entities: &[Entity], ) -> Result>, QueryEntityError> { - Self::verify_unique_entities(entities)?; - - // SAFETY: Each entity is unique. - unsafe { self.get_entities_dynamic_mut_unchecked(entities.iter().copied()) } + self.get_entity_mut(entities).map_err(|e| match e { + EntityFetchError::NoSuchEntity(entity) => QueryEntityError::NoSuchEntity(entity), + EntityFetchError::AliasedMutability(entity) => { + QueryEntityError::AliasedMutability(entity) + } + }) } /// Gets mutable access to multiple entities, contained in a [`EntityHashSet`]. @@ -1111,41 +1247,22 @@ impl World { /// let mut entities = world.get_many_entities_from_set_mut(&set).unwrap(); /// let entity1 = entities.get_mut(0).unwrap(); /// ``` + #[deprecated( + since = "0.15.0", + note = "Use `World::get_entity_mut::<&EntityHashSet>` instead." + )] pub fn get_many_entities_from_set_mut<'w>( &'w mut self, entities: &EntityHashSet, ) -> Result>, QueryEntityError> { - // SAFETY: Each entity is unique. - unsafe { self.get_entities_dynamic_mut_unchecked(entities.iter().copied()) } - } - - /// # Safety - /// `entities` must produce no duplicate [`Entity`] IDs. - unsafe fn get_entities_dynamic_mut_unchecked( - &mut self, - entities: impl ExactSizeIterator, - ) -> Result>, QueryEntityError> { - let world_cell = self.as_unsafe_world_cell(); - - let mut cells = Vec::with_capacity(entities.len()); - for id in entities { - cells.push( - world_cell - .get_entity(id) - .ok_or(QueryEntityError::NoSuchEntity(id))?, - ); - } - - let borrows = cells - .into_iter() - // SAFETY: - // - `world_cell` has exclusive access to the entire world. - // - The caller ensures that each entity is unique, so none - // of the borrows will conflict with one another. - .map(|c| unsafe { EntityMut::new(c) }) - .collect(); - - Ok(borrows) + self.get_entity_mut(entities) + .map(|fetched| fetched.into_values().collect()) + .map_err(|e| match e { + EntityFetchError::NoSuchEntity(entity) => QueryEntityError::NoSuchEntity(entity), + EntityFetchError::AliasedMutability(entity) => { + QueryEntityError::AliasedMutability(entity) + } + }) } /// Spawns a new [`Entity`] and returns a corresponding [`EntityWorldMut`], which can be used @@ -1332,7 +1449,7 @@ impl World { /// ``` #[inline] pub fn get(&self, entity: Entity) -> Option<&T> { - self.get_entity(entity)?.get() + self.get_entity(entity).ok()?.get() } /// Retrieves a mutable reference to the given `entity`'s [`Component`] of the given type. @@ -1381,7 +1498,7 @@ impl World { /// let mut world = World::new(); /// let entity = world.spawn(Position { x: 0.0, y: 0.0 }).id(); /// assert!(world.despawn(entity)); - /// assert!(world.get_entity(entity).is_none()); + /// assert!(world.get_entity(entity).is_err()); /// assert!(world.get::(entity).is_none()); /// ``` #[track_caller] @@ -1406,7 +1523,7 @@ impl World { log_warning: bool, ) -> bool { self.flush(); - if let Some(entity) = self.get_entity_mut(entity) { + if let Ok(entity) = self.get_entity_mut(entity) { entity.despawn(); true } else { @@ -3282,8 +3399,10 @@ mod tests { use crate::{ change_detection::DetectChangesMut, component::{ComponentDescriptor, ComponentInfo, StorageType}, + entity::EntityHashSet, ptr::OwningPtr, system::Resource, + world::error::EntityFetchError, }; use alloc::sync::Arc; use bevy_ecs_macros::Component; @@ -3822,21 +3941,102 @@ mod tests { } #[test] - fn test_verify_unique_entities() { + fn get_entity() { let mut world = World::new(); - let entity1 = world.spawn(()).id(); - let entity2 = world.spawn(()).id(); - let entity3 = world.spawn(()).id(); - let entity4 = world.spawn(()).id(); - let entity5 = world.spawn(()).id(); - assert!( - World::verify_unique_entities(&[entity1, entity2, entity3, entity4, entity5]).is_ok() + let e1 = world.spawn_empty().id(); + let e2 = world.spawn_empty().id(); + + assert!(world.get_entity(e1).is_ok()); + assert!(world.get_entity([e1, e2]).is_ok()); + assert!(world + .get_entity(&[e1, e2] /* this is an array not a slice */) + .is_ok()); + assert!(world.get_entity(&vec![e1, e2][..]).is_ok()); + assert!(world + .get_entity(&EntityHashSet::from_iter([e1, e2])) + .is_ok()); + + 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, e2] /* this is an array not a slice */) + .map(|_| {}) + ); + assert_eq!(Err(e1), world.get_entity(&vec![e1, e2][..]).map(|_| {})); + assert_eq!( + Err(e1), + world + .get_entity(&EntityHashSet::from_iter([e1, e2])) + .map(|_| {}) + ); + } + + #[test] + fn get_entity_mut() { + let mut world = World::new(); + + let e1 = world.spawn_empty().id(); + let e2 = world.spawn_empty().id(); + + assert!(world.get_entity_mut(e1).is_ok()); + assert!(world.get_entity_mut([e1, e2]).is_ok()); + assert!(world + .get_entity_mut(&[e1, e2] /* this is an array not a slice */) + .is_ok()); + assert!(world.get_entity_mut(&vec![e1, e2][..]).is_ok()); + assert!(world + .get_entity_mut(&EntityHashSet::from_iter([e1, e2])) + .is_ok()); + + assert_eq!( + Err(EntityFetchError::AliasedMutability(e1)), + world.get_entity_mut([e1, e2, e1]).map(|_| {}) + ); + assert_eq!( + Err(EntityFetchError::AliasedMutability(e1)), + world + .get_entity_mut(&[e1, e2, e1] /* this is an array not a slice */) + .map(|_| {}) + ); + assert_eq!( + Err(EntityFetchError::AliasedMutability(e1)), + world.get_entity_mut(&vec![e1, e2, e1][..]).map(|_| {}) + ); + // Aliased mutability isn't allowed by HashSets + assert!(world + .get_entity_mut(&EntityHashSet::from_iter([e1, e2, e1])) + .is_ok()); + + 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)), + 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)), + world + .get_entity_mut(&EntityHashSet::from_iter([e1, e2])) + .map(|_| {}) ); - assert!(World::verify_unique_entities(&[entity1, entity1, entity2, entity5]).is_err()); - assert!(World::verify_unique_entities(&[ - entity1, entity2, entity3, entity4, entity5, entity1 - ]) - .is_err()); } } diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 47c74bb4f6..ad5819479d 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -45,7 +45,7 @@ fn update_parent(world: &mut World, child: Entity, new_parent: Entity) -> Option /// /// Removes the [`Children`] component from the parent if it's empty. fn remove_from_children(world: &mut World, parent: Entity, child: Entity) { - let Some(mut parent) = world.get_entity_mut(parent) else { + let Ok(mut parent) = world.get_entity_mut(parent) else { return; }; let Some(mut children) = parent.get_mut::() else { diff --git a/crates/bevy_hierarchy/src/hierarchy.rs b/crates/bevy_hierarchy/src/hierarchy.rs index 351f36ae7d..4a9d71ace7 100644 --- a/crates/bevy_hierarchy/src/hierarchy.rs +++ b/crates/bevy_hierarchy/src/hierarchy.rs @@ -312,7 +312,7 @@ mod tests { // The parent's Children component should be removed. assert!(world.entity(parent).get::().is_none()); // The child should be despawned. - assert!(world.get_entity(child).is_none()); + assert!(world.get_entity(child).is_err()); } #[test] @@ -340,6 +340,6 @@ mod tests { assert!(children.is_some()); assert_eq!(children.unwrap().len(), 2_usize); // The original child should be despawned. - assert!(world.get_entity(child).is_none()); + assert!(world.get_entity(child).is_err()); } } diff --git a/crates/bevy_remote/src/builtin_methods.rs b/crates/bevy_remote/src/builtin_methods.rs index 168999d622..f2e9581892 100644 --- a/crates/bevy_remote/src/builtin_methods.rs +++ b/crates/bevy_remote/src/builtin_methods.rs @@ -601,7 +601,7 @@ pub fn process_remote_list_request(In(params): In>, world: &World) fn get_entity(world: &World, entity: Entity) -> Result, BrpError> { world .get_entity(entity) - .ok_or_else(|| BrpError::entity_not_found(entity)) + .map_err(|_| BrpError::entity_not_found(entity)) } /// Mutably retrieves an entity from the [`World`], returning an error if the @@ -609,7 +609,7 @@ fn get_entity(world: &World, entity: Entity) -> Result, BrpError> fn get_entity_mut(world: &mut World, entity: Entity) -> Result, BrpError> { world .get_entity_mut(entity) - .ok_or_else(|| BrpError::entity_not_found(entity)) + .map_err(|_| BrpError::entity_not_found(entity)) } /// Returns the [`TypeId`] and [`ComponentId`] of the components with the given diff --git a/crates/bevy_render/src/world_sync.rs b/crates/bevy_render/src/world_sync.rs index cec1ce748c..e0f81fdc02 100644 --- a/crates/bevy_render/src/world_sync.rs +++ b/crates/bevy_render/src/world_sync.rs @@ -148,7 +148,7 @@ pub(crate) fn entity_sync_system(main_world: &mut World, render_world: &mut Worl for record in pending.drain(..) { match record { EntityRecord::Added(e) => { - if let Some(mut entity) = world.get_entity_mut(e) { + if let Ok(mut entity) = world.get_entity_mut(e) { match entity.entry::() { bevy_ecs::world::Entry::Occupied(_) => { panic!("Attempting to synchronize an entity that has already been synchronized!"); @@ -162,7 +162,7 @@ pub(crate) fn entity_sync_system(main_world: &mut World, render_world: &mut Worl } } EntityRecord::Removed(e) => { - if let Some(ec) = render_world.get_entity_mut(e) { + if let Ok(ec) = render_world.get_entity_mut(e) { ec.despawn(); }; } diff --git a/crates/bevy_scene/src/bundle.rs b/crates/bevy_scene/src/bundle.rs index b0f8b38a77..0024b2f777 100644 --- a/crates/bevy_scene/src/bundle.rs +++ b/crates/bevy_scene/src/bundle.rs @@ -180,7 +180,7 @@ mod tests { app.update(); // the scene entity does not exist anymore - assert!(app.world().get_entity(scene_entity).is_none()); + assert!(app.world().get_entity(scene_entity).is_err()); // the root entity does not have any children anymore assert!(app.world().entity(entity).get::().is_none()); diff --git a/crates/bevy_scene/src/scene_spawner.rs b/crates/bevy_scene/src/scene_spawner.rs index b354b52b09..1fa42e6019 100644 --- a/crates/bevy_scene/src/scene_spawner.rs +++ b/crates/bevy_scene/src/scene_spawner.rs @@ -191,7 +191,7 @@ impl SceneSpawner { pub fn despawn_instance_sync(&mut self, world: &mut World, instance_id: &InstanceId) { if let Some(instance) = self.spawned_instances.remove(instance_id) { for &entity in instance.entity_map.values() { - if let Some(mut entity_mut) = world.get_entity_mut(entity) { + if let Ok(mut entity_mut) = world.get_entity_mut(entity) { entity_mut.remove_parent(); entity_mut.despawn_recursive(); }; @@ -427,7 +427,7 @@ pub fn scene_spawner_system(world: &mut World) { scene_spawner .scenes_with_parent .retain(|(instance, parent)| { - let retain = world.get_entity(*parent).is_some(); + let retain = world.get_entity(*parent).is_ok(); if !retain { dead_instances.insert(*instance); diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index d854d280a1..0cf4efa128 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -775,7 +775,7 @@ mod tests { assert!(dst_world .query_filtered::<&MyEntityRef, With>() .iter(&dst_world) - .all(|r| world.get_entity(r.0).is_none())); + .all(|r| world.get_entity(r.0).is_err())); } #[test] diff --git a/crates/bevy_transform/src/commands.rs b/crates/bevy_transform/src/commands.rs index 4e02105bfe..2860ddf381 100644 --- a/crates/bevy_transform/src/commands.rs +++ b/crates/bevy_transform/src/commands.rs @@ -29,9 +29,15 @@ impl Command for AddChildInPlace { hierarchy_command.apply(world); // FIXME: Replace this closure with a `try` block. See: https://github.com/rust-lang/rust/issues/31436. let mut update_transform = || { - let parent = *world.get_entity(self.parent)?.get::()?; - let child_global = *world.get_entity(self.child)?.get::()?; - let mut child_entity = world.get_entity_mut(self.child)?; + let parent = *world + .get_entity(self.parent) + .ok()? + .get::()?; + let child_global = *world + .get_entity(self.child) + .ok()? + .get::()?; + let mut child_entity = world.get_entity_mut(self.child).ok()?; let mut child = child_entity.get_mut::()?; *child = child_global.reparented_to(&parent); Some(()) @@ -54,8 +60,11 @@ impl Command for RemoveParentInPlace { hierarchy_command.apply(world); // FIXME: Replace this closure with a `try` block. See: https://github.com/rust-lang/rust/issues/31436. let mut update_transform = || { - let child_global = *world.get_entity(self.child)?.get::()?; - let mut child_entity = world.get_entity_mut(self.child)?; + let child_global = *world + .get_entity(self.child) + .ok()? + .get::()?; + let mut child_entity = world.get_entity_mut(self.child).ok()?; let mut child = child_entity.get_mut::()?; *child = child_global.compute_transform(); Some(())