Remove insert_or_spawn
function family (#18148)
# Objective Based on and closes #18054, this PR builds on #18035 and #18147 to remove: - `Commands::insert_or_spawn_batch` - `Entities::alloc_at_without_replacement` - `Entities::alloc_at` - `entity::AllocAtWithoutReplacement` - `World::insert_or_spawn_batch` - `World::insert_or_spawn_batch_with_caller` ## Testing Just removing unused, deprecated code, so no new tests. Note that as of writing, #18035 is still under testing and review. ## Future Work Per [this](https://github.com/bevyengine/bevy/issues/18054#issuecomment-2689088899) comment on #18054, there may be additional performance improvements possible to the entity allocator now that `alloc_at` no longer is supported. At a glance, I don't see anything obvious to improve, but it may be worth further investigation in the future. --------- Co-authored-by: JaySpruce <jsprucebruce@gmail.com>
This commit is contained in:
parent
3b24f520b9
commit
bfc76c589e
@ -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();
|
||||
|
@ -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<EntityLocation> {
|
||||
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()`.
|
||||
|
@ -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<A>, Changed<B>)>]);
|
||||
}
|
||||
|
||||
#[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::<A>(e0),
|
||||
Some(&A(0)),
|
||||
"existing component was preserved"
|
||||
);
|
||||
assert_eq!(
|
||||
world.get::<B>(e0),
|
||||
Some(&B(0)),
|
||||
"pre-existing entity received correct B component"
|
||||
);
|
||||
assert_eq!(
|
||||
world.get::<B>(e1),
|
||||
Some(&B(1)),
|
||||
"new entity was spawned and received correct B component"
|
||||
);
|
||||
assert_eq!(
|
||||
world.get::<C>(e0),
|
||||
Some(&C),
|
||||
"pre-existing entity received C component"
|
||||
);
|
||||
assert_eq!(
|
||||
world.get::<C>(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::<u32>::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::<A>(e0),
|
||||
Some(&A(0)),
|
||||
"existing component was preserved"
|
||||
);
|
||||
assert_eq!(
|
||||
world.get::<B>(e0),
|
||||
Some(&B(0)),
|
||||
"pre-existing entity received correct B component"
|
||||
);
|
||||
assert_eq!(
|
||||
world.get::<B>(e1),
|
||||
Some(&B(1)),
|
||||
"new entity was spawned and received correct B component"
|
||||
);
|
||||
assert_eq!(
|
||||
world.get::<C>(e0),
|
||||
Some(&C),
|
||||
"pre-existing entity received C component"
|
||||
);
|
||||
assert_eq!(
|
||||
world.get::<C>(e1),
|
||||
Some(&C),
|
||||
"new entity was spawned and received C component"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn insert_batch() {
|
||||
let mut world = World::default();
|
||||
|
@ -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<I, B>(&mut self, bundles_iter: I)
|
||||
where
|
||||
I: IntoIterator<Item = (Entity, B)> + Send + Sync + 'static,
|
||||
B: Bundle<Effect: NoBundleEffect>,
|
||||
{
|
||||
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::<B>(),
|
||||
invalid_entities
|
||||
);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/// Adds a series of [`Bundles`](Bundle) to each [`Entity`] they are paired with,
|
||||
/// based on a batch of `(Entity, Bundle)` pairs.
|
||||
///
|
||||
|
@ -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::<B>(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<I, B>(&mut self, iter: I) -> Result<(), Vec<Entity>>
|
||||
where
|
||||
I: IntoIterator,
|
||||
I::IntoIter: Iterator<Item = (Entity, B)>,
|
||||
B: Bundle<Effect: NoBundleEffect>,
|
||||
{
|
||||
#[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<I, B>(
|
||||
&mut self,
|
||||
iter: I,
|
||||
caller: MaybeLocation,
|
||||
) -> Result<(), Vec<Entity>>
|
||||
where
|
||||
I: IntoIterator,
|
||||
I::IntoIter: Iterator<Item = (Entity, B)>,
|
||||
B: Bundle<Effect: NoBundleEffect>,
|
||||
{
|
||||
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::<B>(&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.
|
||||
|
@ -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.
|
Loading…
Reference in New Issue
Block a user