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

Allow filters to take a Bundle instead of a singular Component, and make the conjunction behavior configurable #15327

Open
ItsDoot opened this issue Sep 20, 2024 · 5 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes X-Controversial There is active debate or serious implications around merging this PR

Comments

@ItsDoot
Copy link
Contributor

ItsDoot commented Sep 20, 2024

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

With the merging of #14791, bevy itself is moving away from the usage of Bundle structs as a means of adding many components to an entity at once, and recommends (but doesn't require that) ecosystem crates follow suit. Therefore, Bundles are more free to be used in other cases with less of a worry about introducing footguns, since they are being phased out from common usage (but not completely removed).

The most immediate helpful place Bundles would find usage in is filters:

// Why should we have to write all of this:
Query<Entity, (With<A>, With<B>, With<C>)>
// When we should be able to write this:
Query<Entity, With<(A, B, C)>>

// And if we need to `OR` them:
Query<Entity, Or<(With<A>, With<B>, With<C>)>>
// We should be able to do:
Query<Entity, With<(A, B, C), Any>>

What solution would you like?

These filters:

struct With<T: Component>;
struct Without<T: Component>;
struct Changed<T: Component>;
struct Added<T: Component>;

Become:

struct With<B: Bundle, Join = All>;
struct Without<B: Bundle, Join = All>;
struct Changed<B: Bundle, Join = All>;
struct Added<B: Bundle, Join = All>;

The additional Join generic parameter specifies how the tuple conjunction is performed:

  • All: With<(A, B, C), All> means With<A> AND With<B> AND With<C>
  • Any: With<(A, B, C), Any> means With<A> OR With<B> OR With<C>

We should determine if the default conjunction for Without should be Any instead of All.

Note: std::any::Any already exists. We should try to find an alternative naming scheme that doesn't clash with std types or pre-existing bevy types, but we may have to resort to doing so anyways if no better alternative is found.

What alternative(s) have you considered?

Bundle tuples only

To reduce controversy, #9255 proposed implementing With<B: Bundle> and other filters only for Bundles made of tuples. I believe this to no longer be necessary as Bundles are being phased out in favor of required components, so it's believed that developers will have less of a draw towards thinking in terms of Bundles (which would have been a poor-man's way of doing OOP).

Conjunction-first

Previously suggested is a flipping of the filter type and conjunction type:

Any<(A, B, C), With>
Any<(A, B, C), Without>
All<(A, B, C), With>
// ...

However that ran into issues with HKTs (higher kinded types), and is verbose in the single-component case.

Additional context

@ItsDoot ItsDoot added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Sep 20, 2024
@ItsDoot
Copy link
Contributor Author

ItsDoot commented Sep 20, 2024

I expect this still to be controversial, but hopefully less so within the context we have today, and with (hopefully) all information centralized.

@tguichaoua
Copy link
Contributor

tguichaoua commented Sep 20, 2024

I pretty like the idea of having a join mode on the filters to choose between "and" and "or".
But I think it's regretable we loose the fluent syntax with With<(A, B, C), All>.

What about having the join mode before the tuple ?

  • Filter<T> for single component (the current behaviour of filters)
  • Filter<AllOf, (A, B, C)> for A and B and C
  • Filter<AnyOf, (A, B, C)> for A or B or C

With this fluent syntax we can "translate" With<AllOf, (A, B, C)> as "with all of A and B and C" or With<AnyOf, (A, B, C)> as "with any of A or B or C".

I make a POC here.


When you're saying "bundle" you mean structures that implement Bundle or tuples of components ?
Because if I understand correctly #14437, bundle structures are about to be deprecated.

@iiYese
Copy link
Contributor

iiYese commented Sep 20, 2024

Having the conjunction as the first param makes the trivial case (single component) more verbose & is a breaking change. You can't do what you're suggesting because that requires specialization or negative trait bounds. Bundles aren't going to be deprecated at most they're going to be repurposed.

@tguichaoua
Copy link
Contributor

Having the conjunction as the first param makes the trivial case (single component) more verbose & is a breaking change.

I want to keep Filter<T> for the single component case.

You can't do what you're suggesting because that requires specialization or negative trait bounds.

I don't get where specialization or negative bounds are needed. If you look at the POC, only one parameter of the filter is generic and the other one is fixed.

@iiYese
Copy link
Contributor

iiYese commented Sep 20, 2024

I don't get where specialization or negative bounds are needed

Ah ok you don't need it if you just have impls for each conjecture instead of a trait for conjectures. That would work & it wouldn't break existing filters.

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-Usability A targeted quality-of-life change that makes Bevy easier to use D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

No branches or pull requests

3 participants