Skip to content

Deprecated Query::many and many_mut #18183

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

Merged
merged 2 commits into from
Mar 7, 2025

Conversation

alice-i-cecile
Copy link
Member

Objective

Alternative to and closes #18120.

Sibling to #18082, see that PR for broader reasoning.

Folks weren't sold on the name many (get_many is clearer, and this is rare), and that PR is much more complex.

Solution

  • Simply deprecate Query::many and Query::many_mut
  • Clean up internal usages

Mentions of this in the docs can wait until it's fully removed in the 0.17 cycle IMO: it's much easier to catch the problems when doing that.

Testing

CI!

Migration Guide

Query::many and Query::many_mut have been deprecated to reduce panics and API duplication. Use Query::get_many and Query::get_many_mut instead, and handle the Result.

@alice-i-cecile
Copy link
Member Author

@chescock @bushrat011899 @Carter0, review this one for me?

@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 D-Trivial Nice and easy! A great choice to get started with Bevy X-Contentious There are nontrivial implications that should be thought through labels Mar 6, 2025
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Mar 6, 2025
Copy link
Contributor

@chescock chescock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

And the only uses I see in docs are on the many methods themselves, so I think you may have updated all the docs after all :).

Co-authored-by: Chris Russell <[email protected]>
@alice-i-cecile
Copy link
Member Author

And the only uses I see in docs are on the many methods themselves, so I think you may have updated all the docs after all

There's an entry for them in an algorithmic complexity table on Query that I didn't clean up this time :)

@alice-i-cecile alice-i-cecile enabled auto-merge March 6, 2025 23:01
@alice-i-cecile alice-i-cecile 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 Mar 6, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 6, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 7, 2025
Merged via the queue into bevyengine:main with commit 5bc1d68 Mar 7, 2025
31 checks passed
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-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it 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