fixed commands

crates/bevy_ecs/src/entity/mod.rs

@@ -978,8 +978,14 @@ impl Entities {
     }
 }
 
+/// An error that occurs when a specified [`Entity`] can not be constructed.
+#[derive(thiserror::Error, Debug, Clone, Copy, PartialEq, Eq)]
 pub enum ConstructionError {
+    #[error(
+        "The entity's id was invalid: either erroneously created or with the wrong generation."
+    )]
     InvalidId,
+    #[error("The entity can not be constructed as it already has a location.")]
     AlreadyConstructed,
 }
 

crates/bevy_ecs/src/system/commands/mod.rs

@@ -18,7 +18,7 @@ use crate::{
     bundle::{Bundle, InsertMode, NoBundleEffect},
     change_detection::{MaybeLocation, Mut},
     component::{Component, ComponentId, Mutable},
-    entity::{Entities, Entity, EntityClonerBuilder, EntityDoesNotExistError},
+    entity::{Entities, EntitiesAllocator, Entity, EntityClonerBuilder},
     error::{ignore, warn, BevyError, CommandWithEntity, ErrorContext, HandleError},
     event::Event,
     observer::{Observer, TriggerTargets},
@@ -99,7 +99,7 @@ use crate::{
 /// [`ApplyDeferred`]: crate::schedule::ApplyDeferred
 pub struct Commands<'w, 's> {
     queue: InternalQueue<'s>,
-    entities: &'w Entities,
+    entities: &'w EntitiesAllocator,
 }
 
 // SAFETY: All commands [`Command`] implement [`Send`]
@@ -176,7 +176,7 @@ const _: () = {
             world: UnsafeWorldCell<'w>,
             change_tick: bevy_ecs::component::Tick,
         ) -> Self::Item<'w, 's> {
-            let(f0, f1) =  <(Deferred<'s, CommandQueue>, &'w Entities) as bevy_ecs::system::SystemParam>::get_param(&mut state.state, system_meta, world, change_tick);
+            let(f0, f1) =  <(Deferred<'s, CommandQueue>, &'w EntitiesAllocator) as bevy_ecs::system::SystemParam>::get_param(&mut state.state, system_meta, world, change_tick);
             Commands {
                 queue: InternalQueue::CommandQueue(f0),
                 entities: f1,
@@ -200,11 +200,11 @@ enum InternalQueue<'s> {
 impl<'w, 's> Commands<'w, 's> {
     /// Returns a new `Commands` instance from a [`CommandQueue`] and a [`World`].
     pub fn new(queue: &'s mut CommandQueue, world: &'w World) -> Self {
-        Self::new_from_entities(queue, &world.entities)
+        Self::new_from_entities(queue, &world.allocator)
     }
 
     /// Returns a new `Commands` instance from a [`CommandQueue`] and an [`Entities`] reference.
-    pub fn new_from_entities(queue: &'s mut CommandQueue, entities: &'w Entities) -> Self {
+    pub fn new_from_entities(queue: &'s mut CommandQueue, entities: &'w EntitiesAllocator) -> Self {
         Self {
             queue: InternalQueue::CommandQueue(Deferred(queue)),
             entities,
@@ -220,7 +220,7 @@ impl<'w, 's> Commands<'w, 's> {
     /// * Caller ensures that `queue` must outlive `'w`
     pub(crate) unsafe fn new_raw_from_entities(
         queue: RawCommandQueue,
-        entities: &'w Entities,
+        entities: &'w EntitiesAllocator,
     ) -> Self {
         Self {
             queue: InternalQueue::RawCommandQueue(queue),
@@ -304,24 +304,14 @@ impl<'w, 's> Commands<'w, 's> {
     ///   with the same combination of components.
     #[track_caller]
     pub fn spawn_empty(&mut self) -> EntityCommands {
-        let entity = self.entities.reserve_entity();
+        let entity = self.entities.alloc();
-        let mut entity_commands = EntityCommands {
-            entity,
-            commands: self.reborrow(),
-        };
         let caller = MaybeLocation::caller();
-        entity_commands.queue(move |entity: EntityWorldMut| {
+        self.queue(move |world: &mut World| {
-            let index = entity.id().index();
+            world
-            let world = entity.into_world_mut();
+                .construct_empty_with_caller(entity, caller)
-            let tick = world.change_tick();
+                .map(|_| ())
-            // SAFETY: Entity has been flushed
-            unsafe {
-                world
-                    .entities_mut()
-                    .mark_construct_or_destruct(index, caller, tick);
-            }
         });
-        entity_commands
+        self.entity(entity)
     }
 
     /// Spawns a new [`Entity`] with the given components
@@ -368,37 +358,14 @@ impl<'w, 's> Commands<'w, 's> {
     ///   with the same combination of components.
     #[track_caller]
     pub fn spawn<T: Bundle>(&mut self, bundle: T) -> EntityCommands {
-        let entity = self.entities.reserve_entity();
+        let entity = self.entities.alloc();
-        let mut entity_commands = EntityCommands {
-            entity,
-            commands: self.reborrow(),
-        };
         let caller = MaybeLocation::caller();
-
+        self.queue(move |world: &mut World| {
-        entity_commands.queue(move |mut entity: EntityWorldMut| {
+            world
-            // Store metadata about the spawn operation.
+                .construct_with_caller(entity, bundle, caller)
-            // This is the same as in `spawn_empty`, but merged into
+                .map(|_| ())
-            // the same command for better performance.
-            let index = entity.id().index();
-            entity.world_scope(|world| {
-                let tick = world.change_tick();
-                // SAFETY: Entity has been flushed
-                unsafe {
-                    world
-                        .entities_mut()
-                        .mark_construct_or_destruct(index, caller, tick);
-                }
-            });
-
-            entity.insert_with_caller(
-                bundle,
-                InsertMode::Replace,
-                caller,
-                crate::relationship::RelationshipHookMode::Run,
-            );
         });
-        // entity_command::insert(bundle, InsertMode::Replace)
+        self.entity(entity)
-        entity_commands
     }
 
     /// Returns the [`EntityCommands`] for the given [`Entity`].
@@ -437,61 +404,6 @@ impl<'w, 's> Commands<'w, 's> {
         }
     }
 
-    /// Returns the [`EntityCommands`] for the requested [`Entity`] if it exists.
-    ///
-    /// This method does not guarantee that commands queued by the returned `EntityCommands`
-    /// will be successful, since the entity could be despawned before they are executed.
-    ///
-    /// # Errors
-    ///
-    /// Returns [`EntityDoesNotExistError`] if the requested entity does not exist.
-    ///
-    /// # Example
-    ///
-    /// ```
-    /// # use bevy_ecs::prelude::*;
-    /// #[derive(Resource)]
-    /// struct PlayerEntity {
-    ///     entity: Entity
-    /// }
-    ///
-    /// #[derive(Component)]
-    /// struct Label(&'static str);
-    ///
-    /// fn example_system(mut commands: Commands, player: Res<PlayerEntity>) -> Result {
-    ///     // Get the entity if it still exists and store the `EntityCommands`.
-    ///     // If it doesn't exist, the `?` operator will propagate the returned error
-    ///     // to the system, and the system will pass it to an error handler.
-    ///     let mut entity_commands = commands.get_entity(player.entity)?;
-    ///
-    ///     // Add a component to the entity.
-    ///     entity_commands.insert(Label("hello world"));
-    ///
-    ///     // Return from the system successfully.
-    ///     Ok(())
-    /// }
-    /// # bevy_ecs::system::assert_is_system(example_system);
-    /// ```
-    ///
-    /// # See also
-    ///
-    /// - [`entity`](Self::entity) for the infallible version.
-    #[inline]
-    #[track_caller]
-    pub fn get_entity(
-        &mut self,
-        entity: Entity,
-    ) -> Result<EntityCommands, EntityDoesNotExistError> {
-        if self.entities.contains(entity) {
-            Ok(EntityCommands {
-                entity,
-                commands: self.reborrow(),
-            })
-        } else {
-            Err(EntityDoesNotExistError::new(entity, self.entities))
-        }
-    }
-
     /// Spawns multiple entities with the same combination of components,
     /// based on a batch of [`Bundles`](Bundle).
     ///

crates/bevy_ecs/src/system/commands/parallel_scope.rs

@@ -1,7 +1,7 @@
 use bevy_utils::Parallel;
 
 use crate::{
-    entity::Entities,
+    entity::EntitiesAllocator,
     prelude::World,
     system::{Deferred, SystemBuffer, SystemMeta, SystemParam},
 };
@@ -51,7 +51,7 @@ struct ParallelCommandQueue {
 #[derive(SystemParam)]
 pub struct ParallelCommands<'w, 's> {
     state: Deferred<'s, ParallelCommandQueue>,
-    entities: &'w Entities,
+    entities: &'w EntitiesAllocator,
 }
 
 impl SystemBuffer for ParallelCommandQueue {

crates/bevy_ecs/src/system/system_param.rs

@@ -4,7 +4,7 @@ use crate::{
     bundle::Bundles,
     change_detection::{MaybeLocation, Ticks, TicksMut},
     component::{ComponentId, ComponentTicks, Components, Tick},
-    entity::Entities,
+    entity::{Entities, EntitiesAllocator},
     query::{
         Access, FilteredAccess, FilteredAccessSet, QueryData, QueryFilter, QuerySingleError,
         QueryState, ReadOnlyQueryData,
@@ -1552,6 +1552,27 @@ unsafe impl<'a> SystemParam for &'a Entities {
     }
 }
 
+// SAFETY: Only reads World entities
+unsafe impl<'a> ReadOnlySystemParam for &'a EntitiesAllocator {}
+
+// SAFETY: no component value access
+unsafe impl<'a> SystemParam for &'a EntitiesAllocator {
+    type State = ();
+    type Item<'w, 's> = &'w EntitiesAllocator;
+
+    fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {}
+
+    #[inline]
+    unsafe fn get_param<'w, 's>(
+        _state: &'s mut Self::State,
+        _system_meta: &SystemMeta,
+        world: UnsafeWorldCell<'w>,
+        _change_tick: Tick,
+    ) -> Self::Item<'w, 's> {
+        world.entities_allocator()
+    }
+}
+
 // SAFETY: Only reads World bundles
 unsafe impl<'a> ReadOnlySystemParam for &'a Bundles {}
 

crates/bevy_ecs/src/world/mod.rs

@@ -267,7 +267,7 @@ impl World {
     #[inline]
     pub fn commands(&mut self) -> Commands {
         // SAFETY: command_queue is stored on world and always valid while the world exists
-        unsafe { Commands::new_raw_from_entities(self.command_queue.clone(), &self.entities) }
+        unsafe { Commands::new_raw_from_entities(self.command_queue.clone(), &self.allocator) }
     }
 
     /// Registers a new [`Component`] type and returns the [`ComponentId`] created for it.
@@ -1046,7 +1046,8 @@ impl World {
         // - Command queue access does not conflict with entity access.
         let raw_queue = unsafe { cell.get_raw_command_queue() };
         // SAFETY: `&mut self` ensures the commands does not outlive the world.
-        let commands = unsafe { Commands::new_raw_from_entities(raw_queue, cell.entities()) };
+        let commands =
+            unsafe { Commands::new_raw_from_entities(raw_queue, cell.entities_allocator()) };
 
         (fetcher, commands)
     }
@@ -1089,10 +1090,19 @@ impl World {
         &mut self,
         entity: Entity,
         bundle: B,
+    ) -> Result<EntityWorldMut<'_>, ConstructionError> {
+        self.construct_with_caller(entity, bundle, MaybeLocation::caller())
+    }
+
+    pub(crate) fn construct_with_caller<B: Bundle>(
+        &mut self,
+        entity: Entity,
+        bundle: B,
+        caller: MaybeLocation,
     ) -> Result<EntityWorldMut<'_>, ConstructionError> {
         self.entities.validate_construction(entity)?;
         // SAFETY: We just ensured it was valid.
-        Ok(unsafe { self.construct_unchecked(entity, bundle, MaybeLocation::caller()) })
+        Ok(unsafe { self.construct_unchecked(entity, bundle, caller) })
     }
 
     /// Constructs `bundle` on `entity`.
@@ -1127,46 +1137,34 @@ impl World {
         entity
     }
 
-    /// Spawns a new [`Entity`] and returns a corresponding [`EntityWorldMut`], which can be used
-    /// to add components to the entity or retrieve its id.
-    ///
-    /// ```
-    /// use bevy_ecs::{component::Component, world::World};
-    ///
-    /// #[derive(Component)]
-    /// struct Position {
-    ///   x: f32,
-    ///   y: f32,
-    /// }
-    /// #[derive(Component)]
-    /// struct Label(&'static str);
-    /// #[derive(Component)]
-    /// struct Num(u32);
-    ///
-    /// let mut world = World::new();
-    /// let entity = world.spawn_empty()
-    ///     .insert(Position { x: 0.0, y: 0.0 }) // add a single component
-    ///     .insert((Num(1), Label("hello"))) // add a bundle of components
-    ///     .id();
-    ///
-    /// let position = world.entity(entity).get::<Position>().unwrap();
-    /// assert_eq!(position.x, 0.0);
-    /// ```
     #[track_caller]
-    pub fn spawn_empty(&mut self) -> EntityWorldMut {
+    pub fn construct_empty(
-        self.flush();
+        &mut self,
-        let entity = self.allocator.alloc();
+        entity: Entity,
-        // SAFETY: entity was just allocated
+    ) -> Result<EntityWorldMut<'_>, ConstructionError> {
-        unsafe { self.spawn_at_empty_internal(entity, MaybeLocation::caller()) }
+        self.construct_empty_with_caller(entity, MaybeLocation::caller())
     }
 
-    /// # Safety
+    pub(crate) fn construct_empty_with_caller(
-    /// must be called on an entity that was just allocated
-    unsafe fn spawn_at_empty_internal(
         &mut self,
         entity: Entity,
         caller: MaybeLocation,
-    ) -> EntityWorldMut {
+    ) -> Result<EntityWorldMut<'_>, ConstructionError> {
+        self.entities.validate_construction(entity)?;
+        // SAFETY: We just ensured it was valid.
+        Ok(unsafe { self.construct_empty_unchecked(entity, caller) })
+    }
+
+    /// Constructs `bundle` on `entity`.
+    ///
+    /// # Safety
+    ///
+    /// `entity` must be valid and have no location.
+    pub(crate) unsafe fn construct_empty_unchecked(
+        &mut self,
+        entity: Entity,
+        caller: MaybeLocation,
+    ) -> EntityWorldMut<'_> {
         let archetype = self.archetypes.empty_mut();
         // PERF: consider avoiding allocating entities in the empty archetype unless needed
         let table_row = self.storages.tables[archetype.table_id()].allocate(entity);
@@ -1256,6 +1254,42 @@ impl World {
         unsafe { self.construct_unchecked(entity, bundle, caller) }
     }
 
+    /// Spawns a new [`Entity`] and returns a corresponding [`EntityWorldMut`], which can be used
+    /// to add components to the entity or retrieve its id.
+    ///
+    /// ```
+    /// use bevy_ecs::{component::Component, world::World};
+    ///
+    /// #[derive(Component)]
+    /// struct Position {
+    ///   x: f32,
+    ///   y: f32,
+    /// }
+    /// #[derive(Component)]
+    /// struct Label(&'static str);
+    /// #[derive(Component)]
+    /// struct Num(u32);
+    ///
+    /// let mut world = World::new();
+    /// let entity = world.spawn_empty()
+    ///     .insert(Position { x: 0.0, y: 0.0 }) // add a single component
+    ///     .insert((Num(1), Label("hello"))) // add a bundle of components
+    ///     .id();
+    ///
+    /// let position = world.entity(entity).get::<Position>().unwrap();
+    /// assert_eq!(position.x, 0.0);
+    /// ```
+    #[track_caller]
+    pub fn spawn_empty(&mut self) -> EntityWorldMut {
+        self.spawn_empty_with_caller(MaybeLocation::caller())
+    }
+
+    pub(crate) fn spawn_empty_with_caller(&mut self, caller: MaybeLocation) -> EntityWorldMut {
+        let entity = self.allocator.alloc();
+        // SAFETY: entity was just allocated
+        unsafe { self.construct_empty_unchecked(entity, caller) }
+    }
+
     /// Spawns a batch of entities with the same component [`Bundle`] type. Takes a given
     /// [`Bundle`] iterator and returns a corresponding [`Entity`] iterator.
     /// This is more efficient than spawning entities and adding components to them individually

crates/bevy_ecs/src/world/unsafe_world_cell.rs

@@ -6,7 +6,10 @@ use crate::{
     bundle::Bundles,
     change_detection::{MaybeLocation, MutUntyped, Ticks, TicksMut},
     component::{ComponentId, ComponentTicks, Components, Mutable, StorageType, Tick, TickCells},
-    entity::{ContainsEntity, Entities, Entity, EntityDoesNotExistError, EntityLocation},
+    entity::{
+        ContainsEntity, Entities, EntitiesAllocator, Entity, EntityDoesNotExistError,
+        EntityLocation,
+    },
     error::{DefaultErrorHandler, ErrorHandler},
     observer::Observers,
     prelude::Component,
@@ -259,6 +262,14 @@ impl<'w> UnsafeWorldCell<'w> {
         &unsafe { self.world_metadata() }.entities
     }
 
+    /// Retrieves this world's [`Entities`] collection.
+    #[inline]
+    pub fn entities_allocator(self) -> &'w EntitiesAllocator {
+        // SAFETY:
+        // - we only access world metadata
+        &unsafe { self.world_metadata() }.allocator
+    }
+
     /// Retrieves this world's [`Archetypes`] collection.
     #[inline]
     pub fn archetypes(self) -> &'w Archetypes {