diff --git a/benches/benches/bevy_ecs/world/commands.rs b/benches/benches/bevy_ecs/world/commands.rs index 8ad87862eb..d378bc7f4a 100644 --- a/benches/benches/bevy_ecs/world/commands.rs +++ b/benches/benches/bevy_ecs/world/commands.rs @@ -92,28 +92,6 @@ pub fn insert_commands(criterion: &mut Criterion) { command_queue.apply(&mut world); }); }); - group.bench_function("insert_or_spawn_batch", |bencher| { - let mut world = World::default(); - let mut command_queue = CommandQueue::default(); - let mut entities = Vec::new(); - for _ in 0..entity_count { - entities.push(world.spawn_empty().id()); - } - - bencher.iter(|| { - let mut commands = Commands::new(&mut command_queue, &world); - let mut values = Vec::with_capacity(entity_count); - for entity in &entities { - values.push((*entity, (Matrix::default(), Vec3::default()))); - } - #[expect( - deprecated, - reason = "This needs to be supported for now, and therefore still needs the benchmark." - )] - commands.insert_or_spawn_batch(values); - command_queue.apply(&mut world); - }); - }); group.bench_function("insert_batch", |bencher| { let mut world = World::default(); let mut command_queue = CommandQueue::default(); diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 7bba07aac6..3126c9d198 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -243,16 +243,6 @@ impl Hash for Entity { } } -#[deprecated( - since = "0.16.0", - note = "This is exclusively used with the now deprecated `Entities::alloc_at_without_replacement`." -)] -pub(crate) enum AllocAtWithoutReplacement { - Exists(EntityLocation), - DidNotExist, - ExistsWithWrongGeneration, -} - impl Entity { /// Construct an [`Entity`] from a raw `index` value and a non-zero `generation` value. /// Ensure that the generation value is never greater than `0x7FFF_FFFF`. @@ -690,87 +680,6 @@ impl Entities { } } - /// Allocate a specific entity ID, overwriting its generation. - /// - /// Returns the location of the entity currently using the given ID, if any. Location should be - /// written immediately. - #[deprecated( - since = "0.16.0", - note = "This can cause extreme performance problems when used after freeing a large number of entities and requesting an arbitrary entity. See #18054 on GitHub." - )] - pub fn alloc_at(&mut self, entity: Entity) -> Option { - self.verify_flushed(); - - let loc = if entity.index() as usize >= self.meta.len() { - self.pending - .extend((self.meta.len() as u32)..entity.index()); - let new_free_cursor = self.pending.len() as IdCursor; - *self.free_cursor.get_mut() = new_free_cursor; - self.meta - .resize(entity.index() as usize + 1, EntityMeta::EMPTY); - None - } else if let Some(index) = self.pending.iter().position(|item| *item == entity.index()) { - self.pending.swap_remove(index); - let new_free_cursor = self.pending.len() as IdCursor; - *self.free_cursor.get_mut() = new_free_cursor; - None - } else { - Some(mem::replace( - &mut self.meta[entity.index() as usize].location, - EntityMeta::EMPTY.location, - )) - }; - - self.meta[entity.index() as usize].generation = entity.generation; - - loc - } - - /// Allocate a specific entity ID, overwriting its generation. - /// - /// Returns the location of the entity currently using the given ID, if any. - #[deprecated( - since = "0.16.0", - note = "This can cause extreme performance problems when used after freeing a large number of entities and requesting an arbitrary entity. See #18054 on GitHub." - )] - #[expect( - deprecated, - reason = "We need to support `AllocAtWithoutReplacement` for now." - )] - pub(crate) fn alloc_at_without_replacement( - &mut self, - entity: Entity, - ) -> AllocAtWithoutReplacement { - self.verify_flushed(); - - let result = if entity.index() as usize >= self.meta.len() { - self.pending - .extend((self.meta.len() as u32)..entity.index()); - let new_free_cursor = self.pending.len() as IdCursor; - *self.free_cursor.get_mut() = new_free_cursor; - self.meta - .resize(entity.index() as usize + 1, EntityMeta::EMPTY); - AllocAtWithoutReplacement::DidNotExist - } else if let Some(index) = self.pending.iter().position(|item| *item == entity.index()) { - self.pending.swap_remove(index); - let new_free_cursor = self.pending.len() as IdCursor; - *self.free_cursor.get_mut() = new_free_cursor; - AllocAtWithoutReplacement::DidNotExist - } else { - let current_meta = &self.meta[entity.index() as usize]; - if current_meta.location.archetype_id == ArchetypeId::INVALID { - AllocAtWithoutReplacement::DidNotExist - } else if current_meta.generation == entity.generation { - AllocAtWithoutReplacement::Exists(current_meta.location) - } else { - return AllocAtWithoutReplacement::ExistsWithWrongGeneration; - } - }; - - self.meta[entity.index() as usize].generation = entity.generation; - result - } - /// Destroy an entity, allowing it to be reused. /// /// Must not be called while reserved entities are awaiting `flush()`. diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index c366a73885..b5b6ea0125 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -152,7 +152,6 @@ mod tests { use core::{ any::TypeId, marker::PhantomData, - num::NonZero, sync::atomic::{AtomicUsize, Ordering}, }; use std::sync::Mutex; @@ -1697,97 +1696,6 @@ mod tests { assert_eq!(0, query_min_size![(&A, &B), Or<(Changed, Changed)>]); } - #[test] - fn insert_or_spawn_batch() { - let mut world = World::default(); - let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(1); - - let values = vec![(e0, (B(0), C)), (e1, (B(1), C))]; - - #[expect( - deprecated, - reason = "This needs to be supported for now, and therefore still needs the test." - )] - world.insert_or_spawn_batch(values).unwrap(); - - assert_eq!( - world.get::(e0), - Some(&A(0)), - "existing component was preserved" - ); - assert_eq!( - world.get::(e0), - Some(&B(0)), - "pre-existing entity received correct B component" - ); - assert_eq!( - world.get::(e1), - Some(&B(1)), - "new entity was spawned and received correct B component" - ); - assert_eq!( - world.get::(e0), - Some(&C), - "pre-existing entity received C component" - ); - assert_eq!( - world.get::(e1), - Some(&C), - "new entity was spawned and received C component" - ); - } - - #[test] - fn insert_or_spawn_batch_invalid() { - let mut world = World::default(); - let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(1); - let e2 = world.spawn_empty().id(); - let invalid_e2 = - Entity::from_raw_and_generation(e2.index(), NonZero::::new(2).unwrap()); - - let values = vec![(e0, (B(0), C)), (e1, (B(1), C)), (invalid_e2, (B(2), C))]; - - #[expect( - deprecated, - reason = "This needs to be supported for now, and therefore still needs the test." - )] - let result = world.insert_or_spawn_batch(values); - - assert_eq!( - result, - Err(vec![invalid_e2]), - "e2 failed to be spawned or inserted into" - ); - - assert_eq!( - world.get::(e0), - Some(&A(0)), - "existing component was preserved" - ); - assert_eq!( - world.get::(e0), - Some(&B(0)), - "pre-existing entity received correct B component" - ); - assert_eq!( - world.get::(e1), - Some(&B(1)), - "new entity was spawned and received correct B component" - ); - assert_eq!( - world.get::(e0), - Some(&C), - "pre-existing entity received C component" - ); - assert_eq!( - world.get::(e1), - Some(&C), - "new entity was spawned and received C component" - ); - } - #[test] fn insert_batch() { let mut world = World::default(); diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 2eed9f6f2a..c8b36e0220 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -12,12 +12,11 @@ pub use parallel_scope::*; use alloc::boxed::Box; use core::marker::PhantomData; -use log::error; use crate::{ self as bevy_ecs, bundle::{Bundle, InsertMode, NoBundleEffect}, - change_detection::{MaybeLocation, Mut}, + change_detection::Mut, component::{Component, ComponentId, Mutable}, entity::{Entities, Entity, EntityClonerBuilder, EntityDoesNotExistError}, error::{ignore, warn, BevyError, CommandWithEntity, ErrorContext, HandleError}, @@ -624,57 +623,6 @@ impl<'w, 's> Commands<'w, 's> { } } - /// Pushes a [`Command`] to the queue for creating entities, if needed, - /// and for adding a bundle to each entity. - /// - /// `bundles_iter` is a type that can be converted into an ([`Entity`], [`Bundle`]) iterator - /// (it can also be a collection). - /// - /// When the command is applied, - /// for each (`Entity`, `Bundle`) pair in the given `bundles_iter`, - /// the `Entity` is spawned, if it does not exist already. - /// Then, the `Bundle` is added to the entity. - /// - /// This method is equivalent to iterating `bundles_iter`, - /// calling [`spawn`](Self::spawn) for each bundle, - /// and passing it to [`insert`](EntityCommands::insert), - /// but it is faster due to memory pre-allocation. - /// - /// # Note - /// - /// Spawning a specific `entity` value is rarely the right choice. Most apps should use [`Commands::spawn_batch`]. - /// This method should generally only be used for sharing entities across apps, and only when they have a scheme - /// worked out to share an ID space (which doesn't happen by default). - #[track_caller] - #[deprecated( - since = "0.16.0", - note = "This can cause extreme performance problems when used with lots of arbitrary free entities. See #18054 on GitHub." - )] - pub fn insert_or_spawn_batch(&mut self, bundles_iter: I) - where - I: IntoIterator + Send + Sync + 'static, - B: Bundle, - { - let caller = MaybeLocation::caller(); - self.queue(move |world: &mut World| { - - #[expect( - deprecated, - reason = "This needs to be supported for now, and the outer item is deprecated too." - )] - if let Err(invalid_entities) = world.insert_or_spawn_batch_with_caller( - bundles_iter, - caller, - ) { - error!( - "{caller}: Failed to 'insert or spawn' bundle of type {} into the following invalid entities: {:?}", - core::any::type_name::(), - invalid_entities - ); - } - }); - } - /// Adds a series of [`Bundles`](Bundle) to each [`Entity`] they are paired with, /// based on a batch of `(Entity, Bundle)` pairs. /// diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 9bd8d699c6..e6bc1606a0 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -30,10 +30,6 @@ pub use filtered_resource::*; pub use identifier::WorldId; pub use spawn_batch::*; -#[expect( - deprecated, - reason = "We need to support `AllocAtWithoutReplacement` for now." -)] use crate::{ archetype::{ArchetypeId, ArchetypeRow, Archetypes}, bundle::{ @@ -46,9 +42,7 @@ use crate::{ ComponentTicks, Components, ComponentsQueuedRegistrator, ComponentsRegistrator, Mutable, RequiredComponents, RequiredComponentsError, Tick, }, - entity::{ - AllocAtWithoutReplacement, Entities, Entity, EntityDoesNotExistError, EntityLocation, - }, + entity::{Entities, Entity, EntityDoesNotExistError, EntityLocation}, entity_disabling::DefaultQueryFilters, event::{Event, EventId, Events, SendBatchIds}, observer::Observers, @@ -2222,176 +2216,6 @@ impl World { unsafe { self.as_unsafe_world_cell().get_non_send_resource_mut() } } - /// For a given batch of ([`Entity`], [`Bundle`]) pairs, either spawns each [`Entity`] with the given - /// bundle (if the entity does not exist), or inserts the [`Bundle`] (if the entity already exists). - /// This is faster than doing equivalent operations one-by-one. - /// Returns `Ok` if all entities were successfully inserted into or spawned. Otherwise it returns an `Err` - /// with a list of entities that could not be spawned or inserted into. A "spawn or insert" operation can - /// only fail if an [`Entity`] is passed in with an "invalid generation" that conflicts with an existing [`Entity`]. - /// - /// # Note - /// Spawning a specific `entity` value is rarely the right choice. Most apps should use [`World::spawn_batch`]. - /// This method should generally only be used for sharing entities across apps, and only when they have a scheme - /// worked out to share an ID space (which doesn't happen by default). - /// - /// ``` - /// use bevy_ecs::{entity::Entity, world::World, component::Component}; - /// #[derive(Component)] - /// struct A(&'static str); - /// #[derive(Component, PartialEq, Debug)] - /// struct B(f32); - /// - /// let mut world = World::new(); - /// let e0 = world.spawn_empty().id(); - /// let e1 = world.spawn_empty().id(); - /// world.insert_or_spawn_batch(vec![ - /// (e0, (A("a"), B(0.0))), // the first entity - /// (e1, (A("b"), B(1.0))), // the second entity - /// ]); - /// - /// assert_eq!(world.get::(e0), Some(&B(0.0))); - /// ``` - #[track_caller] - #[deprecated( - since = "0.16.0", - note = "This can cause extreme performance problems when used with lots of arbitrary free entities. See #18054 on GitHub." - )] - pub fn insert_or_spawn_batch(&mut self, iter: I) -> Result<(), Vec> - where - I: IntoIterator, - I::IntoIter: Iterator, - B: Bundle, - { - #[expect( - deprecated, - reason = "This needs to be supported for now, and the outer function is deprecated too." - )] - self.insert_or_spawn_batch_with_caller(iter, MaybeLocation::caller()) - } - - /// Split into a new function so we can pass the calling location into the function when using - /// as a command. - #[inline] - #[deprecated( - since = "0.16.0", - note = "This can cause extreme performance problems when used with lots of arbitrary free entities. See #18054 on GitHub." - )] - pub(crate) fn insert_or_spawn_batch_with_caller( - &mut self, - iter: I, - caller: MaybeLocation, - ) -> Result<(), Vec> - where - I: IntoIterator, - I::IntoIter: Iterator, - B: Bundle, - { - self.flush(); - let change_tick = self.change_tick(); - - // SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too. - let mut registrator = - unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; - let bundle_id = self - .bundles - .register_info::(&mut registrator, &mut self.storages); - enum SpawnOrInsert<'w> { - Spawn(BundleSpawner<'w>), - Insert(BundleInserter<'w>, ArchetypeId), - } - - impl<'w> SpawnOrInsert<'w> { - fn entities(&mut self) -> &mut Entities { - match self { - SpawnOrInsert::Spawn(spawner) => spawner.entities(), - SpawnOrInsert::Insert(inserter, _) => inserter.entities(), - } - } - } - // SAFETY: we initialized this bundle_id in `init_info` - let mut spawn_or_insert = SpawnOrInsert::Spawn(unsafe { - BundleSpawner::new_with_id(self, bundle_id, change_tick) - }); - - let mut invalid_entities = Vec::new(); - for (entity, bundle) in iter { - #[expect( - deprecated, - reason = "This needs to be supported for now, and the outer function is deprecated too." - )] - match spawn_or_insert - .entities() - .alloc_at_without_replacement(entity) - { - AllocAtWithoutReplacement::Exists(location) => { - match spawn_or_insert { - SpawnOrInsert::Insert(ref mut inserter, archetype) - if location.archetype_id == archetype => - { - // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter - unsafe { - inserter.insert( - entity, - location, - bundle, - InsertMode::Replace, - caller, - RelationshipHookMode::Run, - ) - }; - } - _ => { - // SAFETY: we initialized this bundle_id in `init_info` - let mut inserter = unsafe { - BundleInserter::new_with_id( - self, - location.archetype_id, - bundle_id, - change_tick, - ) - }; - // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter - unsafe { - inserter.insert( - entity, - location, - bundle, - InsertMode::Replace, - caller, - RelationshipHookMode::Run, - ) - }; - spawn_or_insert = - SpawnOrInsert::Insert(inserter, location.archetype_id); - } - }; - } - AllocAtWithoutReplacement::DidNotExist => { - if let SpawnOrInsert::Spawn(ref mut spawner) = spawn_or_insert { - // SAFETY: `entity` is allocated (but non existent), bundle matches inserter - unsafe { spawner.spawn_non_existent(entity, bundle, caller) }; - } else { - // SAFETY: we initialized this bundle_id in `init_info` - let mut spawner = - unsafe { BundleSpawner::new_with_id(self, bundle_id, change_tick) }; - // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter - unsafe { spawner.spawn_non_existent(entity, bundle, caller) }; - spawn_or_insert = SpawnOrInsert::Spawn(spawner); - } - } - AllocAtWithoutReplacement::ExistsWithWrongGeneration => { - invalid_entities.push(entity); - } - } - } - - if invalid_entities.is_empty() { - Ok(()) - } else { - Err(invalid_entities) - } - } - /// For a given batch of ([`Entity`], [`Bundle`]) pairs, /// adds the `Bundle` of components to each `Entity`. /// This is faster than doing equivalent operations one-by-one. diff --git a/release-content/migration-guides/remove_deprecated_batch_spawning.md b/release-content/migration-guides/remove_deprecated_batch_spawning.md new file mode 100644 index 0000000000..9ab5ab0bbf --- /dev/null +++ b/release-content/migration-guides/remove_deprecated_batch_spawning.md @@ -0,0 +1,19 @@ +--- +title: Removed Deprecated Batch Spawning Methods +pull_requests: [18148] +--- + +The following deprecated functions have been removed: + +- `Commands::insert_or_spawn_batch` +- `World::insert_or_spawn_batch` +- `World::insert_or_spawn_batch_with_caller` + +These functions, when used incorrectly, could cause major performance problems and were generally viewed as anti-patterns and foot guns. +They were deprecated in 0.16 for being unnecessary with the retained render world and easily misused. + +Instead of these functions consider doing one of the following: + +Option A) Instead of despawing entities, insert the `Disabled` component, and instead of respawning them at particular ids, use `try_insert_batch` or `insert_batch` and remove `Disabled`. + +Option B) Instead of giving special meaning to an entity id, simply use `spawn_batch` and ensure entity references are valid when despawning.