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

Update QueryBuilder to change the type after each transformation #14596

Closed

Conversation

cBournhonesque
Copy link
Contributor

Objective

  1. I get a rust error that I don't know how to resolve
error[E0505]: cannot move out of `self` because it is borrowed
   --> crates/bevy_ecs/src/query/builder.rs:209:38
    |
42  | impl<'w, D: QueryData, F: QueryFilter> QueryBuilder<'w, D, F> {
    |      -- lifetime `'w` defined here
...
200 |     pub fn or<F2: QueryFilter>(mut self, f: impl Fn(QueryBuilder<'w>) -> QueryBuilder<'w, (), F2>) -> QueryBuilder<'w, D, Or<(F, F2)>> {
    |                                -------- binding `self` declared here
201 |         let mut builder = QueryBuilder::new(self.world);
    |                           -----------------------------
    |                           |                 |
    |                           |                 borrow of `*self.world` occurs here
    |                           argument requires that `*self.world` is borrowed for `'w`
...
209 |         unsafe { std::mem::transmute(self) }
    |                                      ^^^^ move out of `self` occurs here
  1. The system used to finish building the SystemBuilder often won't have a signature that matches the output of the QueryBuilder.
    For example I get this error:
warning: build failed, waiting for other jobs to finish...
error[E0271]: type mismatch resolving `<Query<'_, '_, (), With<A>> as BuildableSystemParam>::OutputBuilder == QueryBuilder<'_, (), (_, With<A>)>`
   --> crates/bevy_ecs/src/system/builder.rs:191:14
    |
191 |             .builder(|w: &mut World| {
    |              ^^^^^^^ type mismatch resolving `<Query<'_, '_, (), With<A>> as BuildableSystemParam>::OutputBuilder == QueryBuilder<'_, (), (_, With<A>)>`
    |
note: expected this to be `query::builder::QueryBuilder<'_, (), filter::With<system::builder::tests::A>>`
   --> crates/bevy_ecs/src/system/system_param.rs:261:26
    |
261 |     type OutputBuilder = QueryBuilder<'w, D, F>;
    |                          ^^^^^^^^^^^^^^^^^^^^^^
    = note: expected struct `query::builder::QueryBuilder<'_, _, filter::With<system::builder::tests::A>>`
               found struct `query::builder::QueryBuilder<'_, _, (_, filter::With<system::builder::tests::A>)>`

However I still think it's worthwhile posting the draft just to demonstrate the approach and to get some feedback
@SkiFire13 @james-j-obrien

Maybe it's actually a feature (and not a bug) to have a Query that iterates on data that doesn't exactly match the types (D, F)?
I know it's already used in QueryLens and QueryBuilder, so maybe it's something we'd like to keep?

Maybe a better solution would then be #14348 (comment)

@cBournhonesque cBournhonesque added the A-ECS Entities, components, systems, and events label Aug 2, 2024
@SkiFire13
Copy link
Contributor

Maybe it's actually a feature (and not a bug) to have a Query that iterates on data that doesn't exactly match the types (D, F)?

I think that's the case, otherwise transmute wouldn't make much sense.

@alice-i-cecile alice-i-cecile added D-Complex Quite challenging from either a design or technical perspective. Ask for help! P-Unsound A bug that results in undefined compiler behavior S-Needs-Review Needs reviewer attention (from anyone!) to move forward S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 5, 2024
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 D-Complex Quite challenging from either a design or technical perspective. Ask for help! P-Unsound A bug that results in undefined compiler behavior S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unsoundness in dynamic queries: sparse filters are ignored if the typed query is dense
3 participants