Skip to content

Track spawn Tick of entities, offer methods, query data SpawnDetails and query filter Spawned #19047

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

Merged
merged 43 commits into from
May 8, 2025

Conversation

urben1680
Copy link
Contributor

@urben1680 urben1680 commented May 3, 2025

Objective

In my own project I was encountering the issue to find out which entities were spawned after applying commands. I began maintaining a vector of all entities with generational information before and after applying the command and diffing it. This was awfully complicated though and has no constant complexity but grows with the number of entities.

Solution

Looking at EntyMeta it seemed obvious to me that struct can track the tick just as it does with MaybeLocation, updated from the same call. After that it became almost a given to also introduce query data SpawnDetails which offers methods to get the spawn tick and location, and query filter Spawned that filters entities out that were not spawned since the last run.

Testing

I expanded a few tests and added new ones, though maybe I forgot a group of tests that should be extended too. I basically searched bevy_ecs for mentions of Changed and Added to see where the tests and docs are.

Benchmarks of spawn/despawn can be found here.


Showcase

From the added docs, systems with equal complexity since the filter is not archetypal:

fn system1(q: Query<Entity, Spawned>) {
    for entity in &q { /* entity spawned */ }
}

fn system2(query: Query<(Entity, SpawnDetails)>) {
    for (entity, spawned) in &query {
        if spawned.is_spawned() { /* entity spawned */ }
    }
}

SpawnedDetails has a few more methods:

fn print_spawn_details(query: Query<(Entity, SpawnDetails)>) {
    for (entity, spawn_details) in &query {
        if spawn_details.is_spawned() {
            print!("new ");
        }
        println!(
            "entity {:?} spawned at {:?} by {:?}",
            entity,
            spawn_details.spawned_at(),
            spawn_details.spawned_by()
        );        
    }
}

Changes

No public api was changed, I only added to it. That is why I added no migration guide.

  • query data SpawnDetails
  • query filter Spawned
  • method Entities::entity_get_spawned_or_despawned_at
  • method EntityRef::spawned_at
  • method EntityMut::spawned_at
  • method EntityWorldMut::spawned_at
  • method UnsafeEntityCell::spawned_at
  • method FilteredEntityRef::spawned_at
  • method FilteredEntityMut::spawned_at
  • method EntityRefExcept::spawned_at
  • method EntityMutExcept::spawned_at

@alice-i-cecile alice-i-cecile added S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events X-Contentious There are nontrivial implications that should be thought through labels May 3, 2025
@alice-i-cecile
Copy link
Member

I really like this idea: it's useful and consistent. That said, we should see how this affects spawning performance, both in the worst case and on more realistic cases.

Copy link
Contributor

@ElliottjPierce ElliottjPierce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Over all, looks good. Just some minor questions, etc.

One thing we should consider is benching this. I have a feeling that having a separate vec for SpawnedOrDespawned, separate from EntityMeta would be faster. Almost every iteration cares about where the entity is. Asking when it was spawned and by who is more like a map thing IIUC. But this might be better as a separate PR.

Another option is making an observer and event for Spawned. I feel like that's going to be the primary use of this, and exposing an observer might be better anyway for this use case. Then again, it makes sense to ask when an entity was spawned. Ex: This particle entity has been around for 100 ticks; despawn it. (Not that each particle should be an entity.) In other words, this might be a cheaper alternative to a timer component in some cases.

Anyway, that's my thoughts. I'm generally on board with this, provided the separate vecs and observer APIs are investigated (maybe in future prs).

urben1680 and others added 2 commits May 3, 2025 23:17
Co-authored-by: Eagster <79881080+ElliottjPierce@users.noreply.github.com>
Co-authored-by: Eagster <79881080+ElliottjPierce@users.noreply.github.com>
@urben1680
Copy link
Contributor Author

urben1680 commented May 3, 2025

Regarding introducing an event for observers, I considered it but I think the PR is big enough as is. I also imagine spawns triggering observers will have a more noticeable performance impact on all spawns than what I did here.

@urben1680
Copy link
Contributor Author

urben1680 commented May 4, 2025

I did another change.

Previously, Entities::set_spawned_or_despawned_by was called at the end of spawns/despawns, which was fine for logging something you usually deactivate during releases.

However, since with this change the method is no longer noop even when you deactivate tracking locations, this always added to the number of accesses on Entities.

So I removed that method entirely and added Entities::set_spawn_despawn that is used instead of Entities::set in the spawning/despawning context.

Entities::set is still used at

  • BundleInserter::insert
  • EntityWorldMut::move_entity_from_remove (which makes me think Add BundleRemover #18521 does not need to adjust here)

Entities::set_spawn_despawn is now used instead at

  • BundleSpawner::spawn_non_existent
  • EntityWorldMut::despawn_with_caller (I don't know why this method has two set calls, please tell me)
  • World::spawn_at_empty_internal

When doing spawn benchmarks, this change greatly reduced the regressions from up to +6% down to +/-1%.
Despawn benchmarks are still a bit wildly going in +/- 5%.

I will do a separate post with benchmarks once I am sure I do them correctly.

@alice-i-cecile alice-i-cecile removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 6, 2025
@alice-i-cecile alice-i-cecile added this to the 0.17 milestone May 6, 2025
Copy link
Contributor

github-actions bot commented May 6, 2025

It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note.

Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes.

@NthTensor
Copy link
Contributor

At minimum, you can probably just copy over your showcase section to a new file following the instructions posted above.

This is super neat!

@urben1680
Copy link
Contributor Author

urben1680 commented May 6, 2025

I wrote release notes. I wanted to do it detailed, I hope not too much.

rendered (up to date to following commits)

I was also close to give a short recap that Ticks becoming too old is something to keep in mind but I was not sure if this belongs there.

@urben1680 urben1680 requested a review from alice-i-cecile May 6, 2025 19:24
@NthTensor
Copy link
Contributor

Oh these are perfect! Thank you!!!

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels May 7, 2025
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff! Thanks for the release note; it'll need an editing pass but capturing the motivation and usage is the key thing at this stage.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 7, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch May 7, 2025
@alice-i-cecile
Copy link
Member

Merge conflicts, sorry!

@urben1680
Copy link
Contributor Author

@alice-i-cecile resolved them. 👍

@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 8, 2025
Merged via the queue into bevyengine:main with commit 732b2e0 May 8, 2025
32 checks passed
@urben1680 urben1680 deleted the track-spawn-tick branch May 8, 2025 16:13
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request May 9, 2025
…ls` and query filter `Spawned` (bevyengine#19047)

In my own project I was encountering the issue to find out which
entities were spawned after applying commands. I began maintaining a
vector of all entities with generational information before and after
applying the command and diffing it. This was awfully complicated though
and has no constant complexity but grows with the number of entities.

Looking at `EntyMeta` it seemed obvious to me that struct can track the
tick just as it does with `MaybeLocation`, updated from the same call.
After that it became almost a given to also introduce query data
`SpawnDetails` which offers methods to get the spawn tick and location,
and query filter `Spawned` that filters entities out that were not
spawned since the last run.

I expanded a few tests and added new ones, though maybe I forgot a group
of tests that should be extended too. I basically searched `bevy_ecs`
for mentions of `Changed` and `Added` to see where the tests and docs
are.

Benchmarks of spawn/despawn can be found
[here](bevyengine#19047 (comment)).

---

From the added docs, systems with equal complexity since the filter is
not archetypal:
```rs
fn system1(q: Query<Entity, Spawned>) {
    for entity in &q { /* entity spawned */ }
}

fn system2(query: Query<(Entity, SpawnDetails)>) {
    for (entity, spawned) in &query {
        if spawned.is_spawned() { /* entity spawned */ }
    }
}
```

`SpawnedDetails` has a few more methods:

```rs
fn print_spawn_details(query: Query<(Entity, SpawnDetails)>) {
    for (entity, spawn_details) in &query {
        if spawn_details.is_spawned() {
            print!("new ");
        }
        println!(
            "entity {:?} spawned at {:?} by {:?}",
            entity,
            spawn_details.spawned_at(),
            spawn_details.spawned_by()
        );
    }
}
```

No public api was changed, I only added to it. That is why I added no
migration guide.

- query data `SpawnDetails`
- query filter `Spawned`
- method `Entities::entity_get_spawned_or_despawned_at`
- method `EntityRef::spawned_at`
- method `EntityMut::spawned_at`
- method `EntityWorldMut::spawned_at`
- method `UnsafeEntityCell::spawned_at`
- method `FilteredEntityRef::spawned_at`
- method `FilteredEntityMut::spawned_at`
- method `EntityRefExcept::spawned_at`
- method `EntityMutExcept::spawned_at`

---------

Co-authored-by: Eagster <79881080+ElliottjPierce@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
andrewzhurov pushed a commit to andrewzhurov/bevy that referenced this pull request May 17, 2025
…ls` and query filter `Spawned` (bevyengine#19047)

# Objective

In my own project I was encountering the issue to find out which
entities were spawned after applying commands. I began maintaining a
vector of all entities with generational information before and after
applying the command and diffing it. This was awfully complicated though
and has no constant complexity but grows with the number of entities.

## Solution

Looking at `EntyMeta` it seemed obvious to me that struct can track the
tick just as it does with `MaybeLocation`, updated from the same call.
After that it became almost a given to also introduce query data
`SpawnDetails` which offers methods to get the spawn tick and location,
and query filter `Spawned` that filters entities out that were not
spawned since the last run.

## Testing

I expanded a few tests and added new ones, though maybe I forgot a group
of tests that should be extended too. I basically searched `bevy_ecs`
for mentions of `Changed` and `Added` to see where the tests and docs
are.

Benchmarks of spawn/despawn can be found
[here](bevyengine#19047 (comment)).

---

## Showcase

From the added docs, systems with equal complexity since the filter is
not archetypal:
```rs
fn system1(q: Query<Entity, Spawned>) {
    for entity in &q { /* entity spawned */ }
}

fn system2(query: Query<(Entity, SpawnDetails)>) {
    for (entity, spawned) in &query {
        if spawned.is_spawned() { /* entity spawned */ }
    }
}
```

`SpawnedDetails` has a few more methods:

```rs
fn print_spawn_details(query: Query<(Entity, SpawnDetails)>) {
    for (entity, spawn_details) in &query {
        if spawn_details.is_spawned() {
            print!("new ");
        }
        println!(
            "entity {:?} spawned at {:?} by {:?}",
            entity,
            spawn_details.spawned_at(),
            spawn_details.spawned_by()
        );        
    }
}
```

## Changes

No public api was changed, I only added to it. That is why I added no
migration guide.

- query data `SpawnDetails`
- query filter `Spawned`
- method `Entities::entity_get_spawned_or_despawned_at`
- method `EntityRef::spawned_at`
- method `EntityMut::spawned_at`
- method `EntityWorldMut::spawned_at`
- method `UnsafeEntityCell::spawned_at`
- method `FilteredEntityRef::spawned_at`
- method `FilteredEntityMut::spawned_at`
- method `EntityRefExcept::spawned_at`
- method `EntityMutExcept::spawned_at`

---------

Co-authored-by: Eagster <79881080+ElliottjPierce@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
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-Feature A new feature, making something new possible M-Needs-Release-Note Work that should be called out in the blog due to impact S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants