Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Command::insert_batch and World::insert_batch #8600

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Connor-McMillin
Copy link
Contributor

Objective

Solution

  • Instead of checking for Entities being pending via alloc_with_no_replacement, this function directly calls world.entities.get(entity) to find the current EntityLocation (which may be invalid). It panics if it runs into an invalid entity

Changelog

This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

  • What changed as a result of this PR?
  • If applicable, organize changes under "Added", "Changed", or "Fixed" sub-headings
  • Stick to one or two sentences. If more detail is needed for a particular change, consider adding it to the "Solution" section
    • If you can't summarize the work, your change may be unreasonably large / unrelated. Consider splitting your PR to make it easier to review and merge!

Migration Guide

This section is optional. If there are no breaking changes, you can delete this section.

  • If this PR is a breaking change (relative to the last release of Bevy), describe how a user might need to migrate their code to support these changes
  • Simply adding new functionality is not a breaking change.
  • Fixing behavior that was definitely a bug, rather than a questionable design choice is not a breaking change.

@james7132 james7132 self-requested a review May 14, 2023 23:20
@james7132 james7132 added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events labels May 14, 2023
@nicopap nicopap self-requested a review August 7, 2023 08:04
@nicopap nicopap added this to the 0.12 milestone Aug 17, 2023
@alice-i-cecile alice-i-cecile added C-Performance A change motivated by improving speed, memory usage or compile times and removed C-Feature A new feature, making something new possible labels Sep 29, 2023
@alice-i-cecile
Copy link
Member

@Connor-McMillin can you update the PR description to fill out the template and make / show some benchmarking results to justify this?

@alice-i-cecile alice-i-cecile removed this from the 0.12 milestone Sep 29, 2023
github-merge-queue bot pushed a commit that referenced this pull request Oct 13, 2024
# Objective

`insert_or_spawn_batch` exists, but a version for just inserting doesn't
- Closes #2693 
- Closes #8384 
- Adopts/supersedes #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:


![Code_HPnUN0QeWe](https://github.com/user-attachments/assets/53091e4f-6518-43f4-a63f-ae57d5470c66)

<details>
<summary>old benchmark</summary>

This was before reworking it to remove the `UnsafeWorldCell`:


![Code_QhXJb8sjlJ](https://github.com/user-attachments/assets/1061e2a7-a521-48e1-a799-1b6b8d1c0b93)
</details>

---

## 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)));

```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Commands::insert_batch
4 participants