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 .include_entity() helper on query iterators #16560

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

Conversation

Nathan-Fenner
Copy link
Contributor

@Nathan-Fenner Nathan-Fenner commented Nov 30, 2024

Objective

Adds a helper method to the QueryIter struct called .include_entity(), which automatically adds (Entity, _) around whatever is currently being iterated. Since the underlying Entity is always tracked anyway, this can be done "for free" at zero additional runtime cost.

This helper makes it more ergonomic to evolve systems over time. For example, as we write a complex system, we encounter a case where we need the Entity that we're iterating over:

fn example(q_data: Query<(&mut Player, &Transform, &Health)>) {
   for (player, transform, _health) in q_data.iter() {
       do_thing1(player, transform);
    }

   for (mut player, transform, health) in q_data.iter_mut() {
       do_thing2(player, transform, health);
    }

   for (mut player, transform, _health) in q_data.iter() {
       do_thing3(todo!() /* need entity! */, player, transform);
    }
}

Despite the Entity already being tracked at runtime, we then need to go back and update multiple lines of code to request it:

fn example(q_data: Query<(Entity, &mut Player, &Transform, &Health)>) {   // changed
   for (_entity, player, transform, _health) in q_data.iter() {           // changed
       do_thing1(player, transform);
    }

   for (_entity, mut player, transform, _health) in q_data.iter_mut() {   // changed
       do_thing2(player, transform, health);
    }

   for (entity, mut player, transform, _health) in q_data.iter_mut() {    // changed
       do_thing3(entity, player, transform);                              // changed
    }
}

Of the 5 changed lines, only the last 2 are essential to the change we actually want to make. If we use include_entity() instead, then we get the much-more-explicit delta:

fn example(q_data: Query<(&mut Player, &Transform, &Health)>) {
   for (player, transform, _health) in q_data.iter() {
       do_thing1(player, transform);
    }

   for (mut player, transform, _health) in q_data.iter_mut() {
       do_thing2(player, transform, health);
    }

   for (entity, (mut player, transform, _health)) in q_data.iter_mut().include_entity() { // changed
       do_thing3(entity, player, transform);                                              // changed
    }
}

Why not (Entity, D)? Why a new IncludeEntity<D> type?

The underlying query state for the query (A, B) is (A::State, B::State). This means that for (Entity, D) the underlying state type is ((), D::State)), not D::State. Despite looking very similar, these types aren't guaranteed to have the same layout, and so we cannot transmute between them.

Testing

  • Needs tests before merging

@Nathan-Fenner
Copy link
Contributor Author

I consider this a better alternative to a previous PR I opened at #13764, which might be independently useful but is not as general

@IQuick143 IQuick143 added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Unsafe Touches with unsafe code in some way S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 30, 2024
@Nathan-Fenner Nathan-Fenner changed the title all .include_entity() helper on query iterators add .include_entity() helper on query iterators Dec 2, 2024
@hymm
Copy link
Contributor

hymm commented Dec 3, 2024

Definitely prefer this pr to the previous one as the call to transmute_lens in that pr allocates.

I lean towards not wanting this functionality as I prefer the simplicity of having only one way of adding entity to a query. But I don't feel too strongly about it if someone really wants it.

@BenjaminBrienen BenjaminBrienen added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 22, 2025
@Nathan-Fenner Nathan-Fenner force-pushed the nf/include_entity_iter branch from 13a9eb6 to 50c849e Compare January 27, 2025 04:39
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 C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes D-Unsafe Touches with unsafe code in some way S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants