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

Introduce methods on QueryState to obtain a Query #15858

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

Conversation

chescock
Copy link
Contributor

Objective

Simplify and expand the API for QueryState.

QueryState has a lot of methods that mirror those on Query. These are then multiplied by variants that take &World, &mut World, and UnsafeWorldCell. In addition, many of them have _manual variants that take &QueryState and avoid calling update_archetypes(). Not all of the combinations exist, however, so some operations are not possible.

Solution

Introduce methods to get a Query from a QueryState. That will reduce duplication between the types, and ensure that the full Query API is always available for QueryState.

Introduce methods on Query that consume the query to return types with the full 'w lifetime. This avoids issues with borrowing where things like query_state.query(&world).get(entity) don't work because they borrow from the temporary Query.

Finally, implement Copy for read-only Querys. get_inner and iter_inner currently take &self, so changing them to consume self would be a breaking change. By making Query: Copy, they can consume a copy of self and continue to work.

The consuming methods also let us simplify the implementation of methods on Query, by doing fn foo(&self) { self.as_readonly().foo_inner() } and fn foo_mut(&mut self) { self.reborrow().foo_inner() }. That structure makes it more difficult to accidentally extend lifetimes, since the safe as_readonly() and reborrow() methods shrink them appropriately. The optimizer is able to see that they are both identity functions and inline them, so there should be no performance cost.

Note that this change would conflict with #15848. If QueryState is stored as a Cow, then the consuming methods cannot be implemented, and Copy cannot be implemented.

Future Work

The next step is to mark the methods on QueryState as #[deprecated], and move the implementations into Query.

Migration Guide

Query::to_readonly has been renamed to Query::as_readonly.

…ry to consume Self and return the original lifetime.
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 11, 2024
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Oct 11, 2024
@alice-i-cecile alice-i-cecile added M-Needs-Release-Note Work that should be called out in the blog due to impact X-Contentious There are nontrivial implications that should be thought through C-Code-Quality A section of code that is hard to understand or change labels Oct 11, 2024
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-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide M-Needs-Release-Note Work that should be called out in the blog due to impact S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants