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

Replace all uses of Query::single with Single #15866

Closed
alice-i-cecile opened this issue Oct 12, 2024 · 2 comments · Fixed by #15872
Closed

Replace all uses of Query::single with Single #15866

alice-i-cecile opened this issue Oct 12, 2024 · 2 comments · Fixed by #15872
Labels
A-Cross-Cutting Impacts the entire engine C-Code-Quality A section of code that is hard to understand or change C-Examples An addition or correction to our examples D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

Query::single and Query::single_mut are convenient lazy helpers with a nasty side effect: they panic unless exactly one matching entity is found!

While this is fine, in theory, for polished, tiny example code, it teaches bad habits to our users.

What solution would you like?

We should swap over all panicking uses over to the shiny new Single system param.

In rare cases, swapping to Query::get_single or even Option<Single<&Component>> might be preferred, to allow for more detailed error handling or to ensure the system runs to do secondary work even if the single entity it's looking for isn't found.

What alternative(s) have you considered?

We could remove these methods entirely, per #14275. This is a much larger and more controversial change. Even if this is chosen, this migration needs to be done anyways, so we might as well do it now.

Additional context

The nascent bevy_cli project introduced a lint to check for this class of error: TheBevyFlock/bevy_cli#95. At some point, we'll want to run that Bevy-specific linter in CI for this project, but it's not ready yet.

@alice-i-cecile alice-i-cecile added A-Cross-Cutting Impacts the entire engine C-Code-Quality A section of code that is hard to understand or change C-Examples An addition or correction to our examples D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon labels Oct 12, 2024
@BD103
Copy link
Member

BD103 commented Oct 12, 2024

I opened TheBevyFlock/bevy_cli#136, which would make the bevy::panicking_query_methods lint suggest Single when query.single() and related are used. I'm not sure whether the Bevy Linter should support unstable APIs before they are released, but it's certainly something to look into.

@MiniaczQ
Copy link
Contributor

About this issue, is there a plan for migrate QueryState::single methods? Or only Query::single methods?

from @pablo-lua in https://discord.com/channels/691052431525675048/749335865876021248/1294672821687025707

Both

from @alice-i-cecile

github-merge-queue bot pushed a commit that referenced this issue Oct 13, 2024
# Objective

- closes #15866

## Solution

- Simply migrate where possible.

## Testing

- Expect that CI will do most of the work. Examples is another way of
testing this, as most of the work is in that area.
---

## Notes
For now, this PR doesn't migrate `QueryState::single` and friends as for
now, this look like another issue. So for example, QueryBuilders that
used single or `World::query` that used single wasn't migrated. If there
is a easy way to migrate those, please let me know.

Most of the uses of `Query::single` were removed, the only other uses
that I found was related to tests of said methods, so will probably be
removed when we remove `Query::single`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Cross-Cutting Impacts the entire engine C-Code-Quality A section of code that is hard to understand or change C-Examples An addition or correction to our examples D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants