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

constrain WorldQuery::init_state argument to ComponentInitializer #13442

Merged
merged 1 commit into from
May 30, 2024

Conversation

Victoronz
Copy link
Contributor

@Victoronz Victoronz commented May 20, 2024

Objective

In #13343, WorldQuery::get_state was constrained from &World as the argument to &Components, but WorldQuery::init_state hasn't yet been changed from &mut World to match.

Fixes #13358

Solution

Create a wrapper around &mut Components and &mut Storages that can be obtained from &mut World with a component_initializer method.
This new ComponentInitializer re-exposes the API on &mut Components minus the &mut Storages parameter where it was present. For the &Components API, it simply derefs to its components field.

Changelog

Added

The World::component_initializer method.
The ComponentInitializer struct that re-exposes Components API.

Changed

WorldQuery::init_state now takes &mut ComponentInitializer instead of &mut World.

Migration Guide

Instead of passing &mut World to WorldQuery::init_state directly, pass in a mutable reference to the struct returned from World::component_initializer.

@alice-i-cecile alice-i-cecile added this to the 0.14 milestone May 20, 2024
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A simple quality-of-life change that makes Bevy easier to use X-Contentious There are nontrivial implications that should be thought through S-Needs-Review Needs reviewer attention (from anyone!) to move forward D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels May 21, 2024
@alice-i-cecile alice-i-cecile added the C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide label May 22, 2024
Copy link
Contributor

@jdm jdm left a comment

Choose a reason for hiding this comment

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

This solution makes sense to me!

@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 May 30, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 30, 2024
Merged via the queue into bevyengine:main with commit 5cfb063 May 30, 2024
28 checks passed
hymm added a commit to hymm/bevy that referenced this pull request Jun 11, 2024
hymm added a commit to hymm/bevy that referenced this pull request Jun 11, 2024
github-merge-queue bot pushed a commit that referenced this pull request Jun 11, 2024
#13804)

…izer (#13442)"

This reverts commit 5cfb063.

- This PR broke bevy-trait-query, which needs to be able to write a
resource in init_state. See #13798 for more details.
- Note this doesn't fix everything as transmutes for bevy-trait-query
will still be broken,. But the current usage in that crate is UB, so we
need to find another solution.
mockersf pushed a commit that referenced this pull request Jun 11, 2024
#13804)

…izer (#13442)"

This reverts commit 5cfb063.

- This PR broke bevy-trait-query, which needs to be able to write a
resource in init_state. See #13798 for more details.
- Note this doesn't fix everything as transmutes for bevy-trait-query
will still be broken,. But the current usage in that crate is UB, so we
need to find another solution.
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-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide C-Usability A simple quality-of-life change that makes Bevy easier to use 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 X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WorldQuery::init_state signature is too generous
3 participants