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

#[derive(Query)] #786

Closed
entropylost opened this issue Nov 4, 2020 · 15 comments
Closed

#[derive(Query)] #786

entropylost opened this issue Nov 4, 2020 · 15 comments
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@entropylost
Copy link
Contributor

What problem does this solve or what need does it fill?

Giant tuples within query (Query<(&Foo, &Bar, &mut Baz)>) is really annoying as then everything has to be referenced by .0, etc, and if I decide to add Entity to the list I have to shift everything.

Describe the solution would you like?

Make a derive macro #[derive(Query)] which implements HecsQuery for the structure in question; as such the structure can be used as a query.

Describe the alternative(s) you've considered?

None.

Additional context

This could also support Or by allowing deriving an enum.

@entropylost entropylost added the C-Feature A new feature, making something new possible label Nov 4, 2020
@entropylost
Copy link
Contributor Author

For example, I have the query Query<(&AI, Entity, &Health, &Transform, &Stats, &Unit)>, which is way too much in my opinion.

@karroffel karroffel added the A-ECS Entities, components, systems, and events label Nov 4, 2020
@Plecra
Copy link
Contributor

Plecra commented Nov 5, 2020

I wonder how much overlap this has with #[derive(Bundle)]. Would it be helpful for the derive to be implemented in a way that allowed you to write this declaration?

#[derive(Bundle, Query)]
struct Enemy {
    behaviour: Ai,
    health: Health,
    transform: Transform,
    extra: Stats,
}

Looks like this has already bene mentioned in #9

@smokku
Copy link
Member

smokku commented Nov 5, 2020

Does it make sense to have Entity as a part of the bundle?

@entropylost
Copy link
Contributor Author

Derive Bundle and derive query wouldn't work; queryies need references not the values.

@Plecra
Copy link
Contributor

Plecra commented Nov 5, 2020

I assumed it would be possible to map T to &T... would something prevent that?

@entropylost
Copy link
Contributor Author

That screws up mutable borrows, and doesn't really make sense (especially as it would have to generate a separate type).

@mvlabat
Copy link
Contributor

mvlabat commented Nov 6, 2020

@Plecra I think the only way to map T to &T is to introduce a new struct with a derive macro, which name would derive from the original one with some kind of suffix. This could work, but it would be a pain for IDEs and editors to understand this, and as @iMplode-nZ has pointed out, there doesn't seem to be an obvious way to express mutable borrows.

@mvlabat
Copy link
Contributor

mvlabat commented Nov 6, 2020

I'm also looking forward to seeing this feature in Bevy. Before stumbling upon on this issue, I've created #800 - this feature request covers both components and resources, and might give some additional idea about the motivation for the feature.

@mvlabat
Copy link
Contributor

mvlabat commented Nov 6, 2020

Does it make sense to have Entity as a part of the bundle?

@smokku, I think it makes perfect sense since Entity is queriable.

@smokku
Copy link
Member

smokku commented Nov 7, 2020

It makes sense for Query. My question is about Bundle.

@mvlabat
Copy link
Contributor

mvlabat commented Nov 7, 2020

@smokku right, I mean that everything that is queriable makes sense to be a part of a bundle. I don't think there's any reason not to support Entity, other than maybe some technical limitations (I don't know if there are any)

@memoryruins
Copy link
Contributor

Ralith/hecs#110 was opened recently to add a #[derive(Query)] in hecs

@tigregalis
Copy link
Contributor

tigregalis commented Nov 10, 2020

Was this addressed by #798 ?

  • SystemParam can now be derived. We now use this for DrawContext instead of a manual impl. You can use any types that impl SystemParam:
    #[derive(SystemParam)]
    struct MyParams<'a> {
      a: Res<'a, A>, 
      b: ResMut<'a, B>,
      commands: &'a mut Commands,
      #[system_param(ignore)]
      hello: bool // this will use Default
    }
    
    fn system(my_params: MyParams) {
      println!("{}", system.a);
    }

Edit: Sorry, never mind. This is for that inner param.

@entropylost
Copy link
Contributor Author

I would say having it being called Query or HECSQuery is simpler. (maybe bevy_hecs::Query)?
Or alternatively we could just use the hecs derive.

@mvlabat
Copy link
Contributor

mvlabat commented Nov 10, 2020

Yeah, I'd say it's something worth backporting to bevy_hecs, instead of implementing it inside bevy_ecs

@alice-i-cecile alice-i-cecile added the C-Usability A targeted quality-of-life change that makes Bevy easier to use label Feb 17, 2021
@bors bors bot closed this as completed in ba6b74b Feb 24, 2022
kurtkuehnert pushed a commit to kurtkuehnert/bevy that referenced this issue Mar 6, 2022
# Objective

- Closes bevyengine#786
- Closes bevyengine#2252
- Closes bevyengine#2588

This PR implements a derive macro that allows users to define their queries as structs with named fields.

## Example

```rust
#[derive(WorldQuery)]
#[world_query(derive(Debug))]
struct NumQuery<'w, T: Component, P: Component> {
    entity: Entity,
    u: UNumQuery<'w>,
    generic: GenericQuery<'w, T, P>,
}

#[derive(WorldQuery)]
#[world_query(derive(Debug))]
struct UNumQuery<'w> {
    u_16: &'w u16,
    u_32_opt: Option<&'w u32>,
}

#[derive(WorldQuery)]
#[world_query(derive(Debug))]
struct GenericQuery<'w, T: Component, P: Component> {
    generic: (&'w T, &'w P),
}

#[derive(WorldQuery)]
#[world_query(filter)]
struct NumQueryFilter<T: Component, P: Component> {
    _u_16: With<u16>,
    _u_32: With<u32>,
    _or: Or<(With<i16>, Changed<u16>, Added<u32>)>,
    _generic_tuple: (With<T>, With<P>),
    _without: Without<Option<u16>>,
    _tp: PhantomData<(T, P)>,
}

fn print_nums_readonly(query: Query<NumQuery<u64, i64>, NumQueryFilter<u64, i64>>) {
    for num in query.iter() {
        println!("{:#?}", num);
    }
}

#[derive(WorldQuery)]
#[world_query(mutable, derive(Debug))]
struct MutNumQuery<'w, T: Component, P: Component> {
    i_16: &'w mut i16,
    i_32_opt: Option<&'w mut i32>,
}

fn print_nums(mut query: Query<MutNumQuery, NumQueryFilter<u64, i64>>) {
    for num in query.iter_mut() {
        println!("{:#?}", num);
    }
}
```

## TODOs:
- [x] Add support for `&T` and `&mut T`
  - [x] Test
- [x] Add support for optional types
  - [x] Test
- [x] Add support for `Entity`
  - [x] Test
- [x] Add support for nested `WorldQuery`
  - [x] Test
- [x] Add support for tuples
  - [x] Test
- [x] Add support for generics
  - [x] Test
- [x] Add support for query filters
  - [x] Test
- [x] Add support for `PhantomData`
  - [x] Test
- [x] Refactor `read_world_query_field_type_info`
- [x] Properly document `readonly` attribute for nested queries and the static assertions that guarantee safety
  - [x] Test that we never implement `ReadOnlyFetch` for types that need mutable access
  - [x] Test that we insert static assertions for nested `WorldQuery` that a user marked as readonly
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-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants