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 helper method Query::with_entity #13764

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Nathan-Fenner
Copy link
Contributor

Objective

While I'm writing systems, I frequently find myself suddenly wanting to have access to the Entity that I'm iterating over with a Query<&MyComp> - so I have to refactor to Query<(Entity, &MyComp)>. This means changing the parameter type and other iteration sites over the same query, even if those sites don't use the Entity.

Solution

This PR adds a convenience method, .with_entity() analogous to e.g. .enumerate() on Iterator - it returns a QueryLens over the same data, but with Entity added to the data selector (in other words, it turns a Query<D, F> into a QueryLens<(Entity, D), F>).

Testing

  • There's a trivial doctest that should make sure it compiles; this is essentially just a convenience wrapper around transmute_lens_filtered but without the need to specify generic types as a caller

Changelog

  • Added Query::with_entity() convenience function that returns a QueryLens over the same results but also including the Entity id

@Nathan-Fenner Nathan-Fenner marked this pull request as draft June 9, 2024 02:20
@Victoronz Victoronz added C-Enhancement A new feature A-ECS Entities, components, systems, and events D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jun 9, 2024
@alice-i-cecile
Copy link
Member

I like this quite a bit, but I think we should change the name to transmute_something. That way it will be clearer in the auto-complete / methods for beginners, and will be grouped with its siblings. transmute_with_entity is fine, but I like transmute_include_entity a bit better since there's no With involved.

@Victoronz
Copy link
Contributor

In 13.0, Entity is the only parameter that can be unconditionally added, but in 14.0, &Archetype, and EntityLocation work like this as well, and this list might expand in the future.
Maybe it makes more sense to have a generic transmute_include/transmute_add?

@hymm
Copy link
Contributor

hymm commented Jun 9, 2024

Why do you need to transmute to add an entity instead of just including it in the original query?

@Nathan-Fenner
Copy link
Contributor Author

Maybe it makes more sense to have a generic transmute_include/transmute_add?

Maybe, but the main point of this particular method is to avoid needing to specify (lengthy) generic types and get better inference - so even if we had a generic method, I'd still want the Entity-specific version, since it's common (and the only one of those that I actually have wanted so far)

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-Enhancement A new feature D-Straightforward Simple bug fixes and API improvements, docs, test and examples 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.

None yet

4 participants