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.

Sign up for GitHub

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

Jump to bottom

EntityRef/Mut get_components (immutable variants only) #15089

Merged
merged 9 commits into from
Sep 9, 2024
Merged

EntityRef/Mut get_components (immutable variants only) #15089

merged 9 commits into from
Sep 9, 2024

Conversation

ItsDoot
Copy link
Contributor

@ItsDoot ItsDoot commented Sep 8, 2024

Objective

Smaller scoped version of #13375 without the _mut variants which currently have unsoundness issues.

Solution

Same as #13375, but without the _mut variants.

Testing

Migration Guide

  • Renamed FilteredEntityRef::components to FilteredEntityRef::accessed_components and FilteredEntityMut::components to FilteredEntityMut::accessed_components.

@ItsDoot ItsDoot added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events 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 Sep 8, 2024
@ItsDoot ItsDoot added this to the 0.15 milestone Sep 8, 2024
chescock
chescock approved these changes Sep 8, 2024
View reviewed changes
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!

It seems like this should have needed a safety comment somewhere explaining how it relies on ReadOnlyQueryData. Maybe there should be a safety requirement on init_fetch that the access is consistent, and that is normally discharged by calling update_component_access and having it not panic? (That shouldn't block this PR, of course.)

@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 Sep 8, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 9, 2024
This was referenced Sep 9, 2024
Closed
Closed
Merged via the queue into bevyengine:main with commit 79f6fcd Sep 9, 2024
27 checks passed
@ItsDoot ItsDoot deleted the get_components branch September 9, 2024 17:40
@ItsDoot ItsDoot mentioned this pull request Nov 29, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chescock chescock chescock approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@cart cart Awaiting requested review from cart

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible 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
Projects
None yet
Milestone
0.15
Development

Successfully merging this pull request may close these issues.
4 participants
@ItsDoot @alice-i-cecile @chescock @cart