Add insert_batch and variations

[JaySpruce]

Objective

insert_or_spawn_batch exists, but a version for just inserting doesn't

• Closes Add Commands::insert_batch for faster component modification #2693
• Closes Add Commands::insert_batch #8384
• Adopts/supersedes Add Command::insert_batch and World::insert_batch #8600

Solution

Add insert_batch, along with the most common insert variations:

• World::insert_batch
• World::insert_batch_if_new
• World::try_insert_batch
• World::try_insert_batch_if_new
• Commands::insert_batch
• Commands::insert_batch_if_new
• Commands::try_insert_batch
• Commands::try_insert_batch_if_new

Testing

Added tests, and added a benchmark for insert_batch.
Performance is slightly better than insert_or_spawn_batch when only inserting:

old benchmark

This was before reworking it to remove the UnsafeWorldCell:

---

Showcase

Usage is the same as insert_or_spawn_batch:

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 entity_a = world.spawn_empty().id();
let entity_b = world.spawn_empty().id();
world.insert_batch([
    (entity_a, (A("a"), B(0.0))),
    (entity_b, (A("b"), B(1.0))),
]);

assert_eq!(world.get::<B>(entity_a), Some(&B(0.0)));

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[BD103]

Overall looks good! I have some nits and notes, but nothing blocking.

Do note that I'm unfamiliar with ECS internals, so I'd like someone with more experience to check over those parts of the PR.

[Trashtalk217]

This would be hugely helpful for deprecating insert_or_spawn_batch, which is part of #15459. I do however want to point to #15614 as an example of a way to add this functionality without doubling the API surface. Can you look if this may be possible for insert variants as well?

[JaySpruce]

I'm far from an expert, but I'm not sure if it's doable in the same way.

The World::entity variations deal with Single vs Plural and Static vs Dynamic, things that work nicely with Rust types.
The insert variations are Replace vs Keep and Any Conditional, which I can only think of ugly solutions for.

I'd imagine that we would want to include all the other insert functions, to keep things consistent. I've seen ideas about chaining commands to achieve a similar nice API, which would work for the insert Commands, but I don't know if the functions on the World can do that.

The best I've got is passing in an enum or something, but that's obvious so there's probably a reason that's not how it works already (too fragile, ugly, etc).

Notably, a lower level insert (on BundleInserter) does use an InsertMode enum to determine whether to replace or keep, it's just not exposed to users

Edit from days later: I think I missed the main part of the question, which is just adding insert and making it optionally function with batches. That's something that might need some higher up approval though

[JaySpruce]

Maybe this could be its own PR, but the original PR has some unhealthy usage of UnsafeWorldCell, so I think this is a worthwhile fix.

Here's the context:

The problem was that I needed access to both the World's Entities storage and a &mut World to make a new BundleInserter.

insert_or_spawn_batch snuck around this by making a BundleSpawner first, which internally made an UnsafeWorldCell that was then able to be used to access Entities while still using a &mut World.

insert_batch, of course, doesn't need a BundleSpawner. However, if you try to recreate the effect with a BundleInserter, you'll find that it additionally needs an ArchetypeId, which you get from an EntityLocation... which you get from Entities.

I originally used an UnsafeWorldCell directly to get around this, which worked, but it turns out the documentation specifically says to not do that.

So, eventually I figured out a (hopefully) proper solution: deal with the first entity in the iterator on its own, so you can make a BundleInserter with its ArchetypeId without triggering the borrow checker, and without dealing in unsafe (well, unsafe unsafe) operations.

And as a bonus, it's slightly faster now!

[JaySpruce]

Should I remove the try_ variants from the World? I've realized that they're only really useful for Commands

[hymm]

Am I understanding correctly that this could potentially allocate twice if we're moving multiple entities into the same archetype? And also make multiple moves of entities in the old table as we're moving entities out of it? Probably some room for future optimizations if so.

edit: I guess it would depend on how it's being used if it's worth it or not.

[JaySpruce]

I think keeping multiple inserters allocated would run into borrow problems, since they all need a &mut World

[hymm]

Should I remove the try_ variants from the World? I've realized that they're only really useful for Commands

I'm not sure I follow. We do have a try_despawn on world. I think it's more unlikely that an entity might not exist when working with World, but not impossible. You could be referencing an entity that is manually updated in a resource.

I think keeping multiple inserters allocated would run into borrow problems, since they all need a &mut World

It would definitely require something different from BundleInserter and probably a bunch of unsafe.

[JaySpruce]

I think it's more unlikely that an entity might not exist when working with World, but not impossible. You could be referencing an entity that is manually updated in a resource.

Just wasn't sure if it was possible, I'll leave them alone then