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

Restrict trait bound on F generic in query filters to ReadOnlyWorldQuery #6005

Closed
alice-i-cecile opened this issue Sep 17, 2022 · 3 comments
Closed
Labels
A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change

Comments

@alice-i-cecile
Copy link
Member

This is an interesting bound; we don't require ReadOnlyWorldQuery on the F in Query<Q, F>. Perhaps we should though?

Originally posted by @alice-i-cecile in #6004 (comment)

Query filters cannot mutate data (by definition); as a result we can strengthen the trait bound on the appropriate generics from WorldQuery to ReadOnlyWorldQuery.

This better communicates how these generics are supposed to be used, and catches mistakes that are likely to be user error at compile time.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change labels Sep 17, 2022
@alice-i-cecile
Copy link
Member Author

@BoxyUwU, I'd like your tak here.

For anyone considering tackling this, this should be super easy from an implementation perspective. Just make sure to hit QueryState too!

@BoxyUwU
Copy link
Member

BoxyUwU commented Sep 17, 2022

seems reasonable to me

bors bot pushed a commit that referenced this issue Sep 18, 2022
# Objective

Fixes Issue #6005.

## Solution

Replaced WorldQuery with ReadOnlyWorldQuery on F generic in Query filters and QueryState to restrict its trait bound.

## Migration Guide

Query filter (`F`) generics are now bound by `ReadOnlyWorldQuery`, rather than `WorldQuery`. If for some reason you were requesting `Query<&A, &mut B>`, please use `Query<&A, With<B>>` instead.
@james7132
Copy link
Member

Should be addressed as of #6008

james7132 pushed a commit to james7132/bevy that referenced this issue Oct 19, 2022
# Objective

Fixes Issue bevyengine#6005.

## Solution

Replaced WorldQuery with ReadOnlyWorldQuery on F generic in Query filters and QueryState to restrict its trait bound.

## Migration Guide

Query filter (`F`) generics are now bound by `ReadOnlyWorldQuery`, rather than `WorldQuery`. If for some reason you were requesting `Query<&A, &mut B>`, please use `Query<&A, With<B>>` instead.
james7132 pushed a commit to james7132/bevy that referenced this issue Oct 28, 2022
# Objective

Fixes Issue bevyengine#6005.

## Solution

Replaced WorldQuery with ReadOnlyWorldQuery on F generic in Query filters and QueryState to restrict its trait bound.

## Migration Guide

Query filter (`F`) generics are now bound by `ReadOnlyWorldQuery`, rather than `WorldQuery`. If for some reason you were requesting `Query<&A, &mut B>`, please use `Query<&A, With<B>>` instead.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective

Fixes Issue bevyengine#6005.

## Solution

Replaced WorldQuery with ReadOnlyWorldQuery on F generic in Query filters and QueryState to restrict its trait bound.

## Migration Guide

Query filter (`F`) generics are now bound by `ReadOnlyWorldQuery`, rather than `WorldQuery`. If for some reason you were requesting `Query<&A, &mut B>`, please use `Query<&A, With<B>>` instead.
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-Code-Quality A section of code that is hard to understand or change
Projects
None yet
Development

No branches or pull requests

3 participants