Deprecate insert_or_spawn function family (#18147)

# Objective

Based on #18054, this PR builds on #18035 to deprecate:

- `Commands::insert_or_spawn_batch`
- `Entities::alloc_at_without_replacement`
- `Entities::alloc_at`
- `World::insert_or_spawn_batch`
- `World::insert_or_spawn_batch_with_caller`

## Testing

Just deprecation, so no new tests. Note that as of writing #18035 is
still under testing and review.

## Open Questions

- [x] Should `entity::AllocAtWithoutReplacement` be deprecated? It is
internal and only used in `Entities::alloc_at_without_replacement`.
**EDIT:** Now deprecated.

## Migration Guide

The following functions have been deprecated:

- `Commands::insert_or_spawn_batch`
- `World::insert_or_spawn_batch`
- `World::insert_or_spawn_batch_with_caller`

These functions, when used incorrectly, can cause major performance
problems and are generally viewed as anti-patterns and foot guns. These
are planned to be removed altogether in 0.17.

Instead of these functions consider doing one of the following:

Option A) Instead of despawing entities and re-spawning them at a
particular id, insert the new `Disabled` component without despawning
the entity, and use `try_insert_batch` or `insert_batch` and remove
`Disabled` instead of re-spawning it.

Option B) Instead of giving special meaning to an entity id, simply use
`spawn_batch` and ensure entity references are valid when despawning.

---------

Co-authored-by: JaySpruce <jsprucebruce@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
This commit is contained in:
Eagster 2025-03-06 12:04:16 -05:00 committed by GitHub
parent 47509ef6a9
commit ed7b366b24
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 51 additions and 0 deletions

View File

@ -106,6 +106,10 @@ pub fn insert_commands(criterion: &mut Criterion) {
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);
});

View File

@ -246,6 +246,9 @@ impl Hash for Entity {
}
}
#[deprecated(
note = "This is exclusively used with the now deprecated `Entities::alloc_at_without_replacement`."
)]
pub(crate) enum AllocAtWithoutReplacement {
Exists(EntityLocation),
DidNotExist,
@ -697,6 +700,9 @@ impl Entities {
///
/// Returns the location of the entity currently using the given ID, if any. Location should be
/// written immediately.
#[deprecated(
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();
@ -730,6 +736,13 @@ impl Entities {
/// Allocate a specific entity ID, overwriting its generation.
///
/// Returns the location of the entity currently using the given ID, if any.
#[deprecated(
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,

View File

@ -1702,6 +1702,10 @@ mod tests {
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!(
@ -1742,6 +1746,10 @@ mod tests {
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!(

View File

@ -681,6 +681,9 @@ impl<'w, 's> Commands<'w, 's> {
/// 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(
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,
@ -688,6 +691,11 @@ impl<'w, 's> Commands<'w, 's> {
{
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,

View File

@ -30,6 +30,10 @@ 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::{
@ -2154,18 +2158,28 @@ impl World {
/// assert_eq!(world.get::<B>(e0), Some(&B(0.0)));
/// ```
#[track_caller]
#[deprecated(
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(
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,
@ -2203,6 +2217,10 @@ impl World {
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)