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

Migrate from Query::single and friends to Single #15872

Merged
merged 11 commits into from
Oct 13, 2024

Conversation

pablo-lua
Copy link
Contributor

Objective

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.

@pablo-lua pablo-lua added C-Examples An addition or correction to our examples C-Code-Quality A section of code that is hard to understand or change A-Cross-Cutting Impacts the entire engine S-Needs-Review Needs reviewer attention (from anyone!) to move forward D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Oct 12, 2024
@alice-i-cecile
Copy link
Member

I've had to open this up in Codespaces to review it, as Github's webpage hates PRs with many files changed 🙃 That said, looks great! I particularly appreciate the use of Option<Single> in tests to ensure that we're still panicking if things aren't found.

@rparrett
Copy link
Contributor

rparrett commented Oct 13, 2024

Guessing this is going to have conflicts after #15857. Can take a look after those get fixed up.

@alice-i-cecile alice-i-cecile added this to the 0.15 milestone Oct 13, 2024
crates/bevy_app/src/app.rs Outdated Show resolved Hide resolved
crates/bevy_app/src/app.rs Outdated Show resolved Hide resolved
crates/bevy_app/src/app.rs Outdated Show resolved Hide resolved
crates/bevy_app/src/app.rs Outdated Show resolved Hide resolved
examples/audio/spatial_audio_2d.rs Outdated Show resolved Hide resolved
@pablo-lua
Copy link
Contributor Author

@BenjaminBrienen All comments addressed.

@pablo-lua pablo-lua 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-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 13, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 13, 2024
Merged via the queue into bevyengine:main with commit d96a9d1 Oct 13, 2024
28 checks passed
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-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace all uses of Query::single with Single
4 participants