Support for `With<T: Bundle>`

[Zeenobit]

Objective

As far as I'm aware, there is currently no way to use a Bundle as a filter type. This applies to named bundles (i.e. using #[derive(Bundle)]), as well as unnamed bundles, such as (A, B, C, ...).

In other words, this transformation does not exist:

(A, B, C, ...) -> (With<A>, With<B>, With<C>, ...)

This makes the implementation of some generic systems unergonomic because for systems operating on an unknown number of generic components (i.e. a bundle), the user would have to define the filter query and the bundle separately. Basically the workaround is this:

#[derive(Bundle, Default)]
struct SomeBundle {
  a: A,
  b: B,
  ...
}

fn get_or_spawn<T: Bundle + Default, F: WorldQuery>(query: Query<Entity, F>, mut commands: Commands) -> Entity {
  if let Some(entity) = query.iter().next() {
    entity
  } else {
    commands.spawn(T::default()).id()
  }  
}

This is also bad because there is effectively no guarantee that F and the components within T match.

Solution

I propose a way to make this more ergonomic by using an associated type Filter on Bundle, which is automatically constructed by ECS macros. This type can then be used to create filtered queries for any generic Bundle or Component.

In short, this PR allows the same system above to be written as:

fn get_or_spawn<T: Bundle + Default>(query: Query<Entity, With<T>>, mut commands: Commands) -> Entity {
  ...
}

This implementation also works with nested bundles:

{ With<(A, B, C)> = With<((A, B), C) = With<(A, (B, C))> } -> (With<A>, With<B>, With<C>)

This would also be a step towards a cleaner implementation of Entity Kinds (#1634) as described in @JoJoJet 's comment: https://github.com/bevyengine/bevy/issues/1634#issuecomment-1275057017

Because instead of having to write Entity<(With<A>, With<B>, With<C>, ...)>, we'd be able to write Entity<With<(A, B, C, ...)>>, and use With<T> to derive the WorldQuery needed for working with it.

---

Changelog

• Added Filter associated type to Bundle

[DJMcNab]

https://github.com/bevyengine/bevy/blob/0b2a0e9de85b0de9b806e49e68c02b895b8098fe/crates/bevy_ecs/src/bundle.rs#L39-L56

[Zeenobit]

https://github.com/bevyengine/bevy/blob/0b2a0e9de85b0de9b806e49e68c02b895b8098fe/crates/bevy_ecs/src/bundle.rs#L39-L56

I fully understand the intent behind bundle. This is more to help with implementing generic systems that operate on bundles, not to query bundles. This also doesn't deal with overlapping issues, because With<A> is the same as (With<A>, With<A>). This doesn't allow one to access components of a bundle.

This just allows you to go from (A, B) to (With<A>, With<B>). The fact that we have Bundle implemented for (A, B) allows this to be implemented pretty easily as I've done it.

[joseph-gio]

This just allows you to go from (A, B) to (With<A>, With<B>). The fact that we have Bundle implemented for (A, B) allows this to be implemented pretty easily as I've done it.

Not sure how I feel about bundle filters yet, but I think a more natural way of doing it would be to "just" allow With<(A, B)>, i.e. use bundles for With<>.

[Zeenobit]

This just allows you to go from (A, B) to (With<A>, With<B>). The fact that we have Bundle implemented for (A, B) allows this to be implemented pretty easily as I've done it.

Not sure how I feel about bundle filters yet, but I think a more natural way of doing it would be to "just" allow With<(A, B)>, i.e. use bundles for With<>.

I like the With<(A, B)> approach. Any hints as to how it could be implemented like that? Also, given that SomeBundle { A, B } is equivalent to (A, B), would this imply that With<SomeBundle> would also be allowed?

Edit: What about Without<(A, B)>? Would we also want to support that? Same for Added and Changed, but I can't come up with a use case for that.

[joseph-gio]

Also, given that SomeBundle { A, B } is equivalent to (A, B), would this imply that With<SomeBundle> would also be allowed?

Yeah, that's why I'm hesitant. Not sure if I'm a fan of allowing With<> for named bundles.

I like the With<(A, B)> approach. Any hints as to how it could be implemented like that?

One way:

1. Rename With<> -> WithComponent<>, maybe make it #[doc(hidden)]
2. Define a new With<> filter type, which accepts bundles.
   • It can just be a thin wrapper around <T as Bundle>::Filter -- maybe even a type alias?

[alice-i-cecile]

Can't we just make With and friends accept a bundle, rather than a component? Which should still work for components, because single components are now bundles.

[Zeenobit]

Can't we just make With and friends accept a bundle, rather than a component? Which should still work for components, because single components are now bundles.

I'm not a big fan of allowing this pattern for anything but With, precisely for the reasons mentioned in @DJMcNab 's comment further up. But I don't see why it wouldn't be possible.

[joseph-gio]

An alternative way of doing With<T: Bundle> is to mimic the implementation of WithBundle<>, which has been removed: https://docs.rs/bevy_ecs/0.5.0/src/bevy_ecs/query/filter.rs.html#244

It's probably less efficient, but much less complex

[Zeenobit]

An alternative way of doing With<T: Bundle> is to mimic the implementation of WithBundle<>, which has been removed: https://docs.rs/bevy_ecs/0.5.0/src/bevy_ecs/query/filter.rs.html#244

It's probably less efficient, but much less complex

Less efficient in terms of implementation or runtime performance? If it's the latter, I don't think it'd be smart to compromise the performance of With<T: Component> just so we can use With<T: Bundle>. To me, With<(A, B)> should ideally be a zero cost abstraction over (With<A>, With<B>).

[Zeenobit]

The other issue we may need to consider is that if this is being used as a stepping stone for entity kinds (#1634), then we would need to provide some ability to "upcast" an Entity<(A, B)> to an Entity<A> and finally an Entity<()>.

I don't think we'd be able to achieve that without using type information at the macro level, because it all has to happen at compile time. So the complexity added here may be necessary for that.

[Zeenobit]

Thinking more on this, if instead of With<T: Bundle>, we implement entity kinds as Entity<T: Bundle>, and then implement WorldQuery for Entity<T> as a derivative of Entity and WithBundle<T> (from the snippet JoJo mentioned), then ...

Any query Query<Entity<(A, B, C)>> would be functionally equivalent to Query<Entity, With<(A, B, C)>>, which makes the latter form redundant.

This means the implementation of WithBundle can remain private (its function only accessible via a kinded entity query), and With would still be restricted to T: Component.

Because of this, a better incremental approach may be to just re-introduce WithBundle as a filter again. Any information on why this was removed initially would be helpful.

I would still be hesitant in using WithBundle if it would add a runtime overhead.

[joseph-gio]

Because of this, a better incremental approach may be to just re-introduce WithBundle as a filter again. Any information on why this was removed initially would be helpful.

Seems like it was removed due to #2620

[Zeenobit]

Given the discussion in https://github.com/bevyengine/bevy/issues/2620, I understand the objection to WithBundle and supporting With<T: Bundle> by extension.

So then would it be correct to assume kinded entities would also be an "anti-feature" as a result, because it's effectively the same problem/solution with a different syntax?

With the way bundles are implemented as-is in Bevy (i.e. (A, B) = Foo { A, B }), if we want to support Entity<With<(A, B)>> or Entity<(A, B)>, it means we have to support Entity<With<Foo>> or Entity<Foo>.

[DJMcNab]

Not necessarily; we could implement a trait over T: Component and tuples of other implementations for things which can be used within With.

That way With<A> and with With<(A, B)> would work, but not With<Foo>

[Zeenobit]

Not necessarily; we could implement a trait over T: Component and tuples of other implementations for things which can be used within With.

That way With<A> and with With<(A, B)> would work, but not With<Foo>

I don't quite see the benefit in the increased complexity of this approach (i.e. adding a new tuple of components that are like bundles in syntax, but not quite in usage), when the user could just do this to get what they want anyways:

type Foo = (A, B);
fn system(query: Query<(), With<Foo>>) { }

Putting artificial barriers in front of the user or taking power away from them because we don't trust in their ability to use the engine correctly, or because it may lead to poor OOP habits is a very pessimistic view of the user. :(

Edit: I'm not even sure if your suggestion would be possible without causing ambiguity in the compiler. I already thought about doing something similar initially. The only way I can think of doing that without causing ambiguous cases would be do have a IS_NAMED_BUNDLE flag or marker type on bundles that's populated by the macros.

[joseph-gio]

I don't quite see the benefit in the increased complexity of this approach (i.e. adding a new tuple of components that are like bundles in syntax, but not quite in usage)

It's not increasing complexity, so much as moving it to a different trait.

Here's a very rough example of how it could work: https://github.com/JoJoJet/bevy/commit/6271a9424be56fa76e1e092708c67ce369c4f65c

Now, this approach doesn't pave the way for entity kinds, but I don't think it needs to. We can always cross that bridge when we come to it.

[Zeenobit]

The implementation does make sense. And it does make the query_with_named_bundle_filter test fail as expected. While this is simpler than I imagined, I still don't think there is a gain in having a dedicated AnonymousBundle type. I think that's where the complexity lies, in terms of API, not the implementation.

And again, this is all to prevent the user from using named bundles as filters, when the user could just define their bundles like this:

type MyBundle = (A, B, C);

Which is even less verbose than:

#[derive(Bundle)]
struct MyBundle {
  a: A,
  b: B,
}

So to me, we'd just be adding this artificial barrier for the user because we decided that anonymous and named bundles are mostly the same, but not quite.

Instead, if we do allow Bundle filters without restriction on named bundles, we'd empower the user to decide when to use named bundles as filters, or otherwise, as needed.

If we don't want to allow named Bundle filters, then I propose we abandon the feature entirely, because it just feels like artificial complexity (however small) to enable that distinction.

Edit: Alternatively, we could also approach the problem by removing named bundles, and making all bundles be anonymous, with "named" ones being simple type declarations. Even that would be a better solution in my opinion. If we don't want to allow the user to use the names of bundles for anything other than to spawn/insert them, why do we have names for bundles to begin with?

[joseph-gio]

And again, this is all to prevent the user from using named bundles as filters, when the user could just define their bundles like this:

type MyBundle = (A, B, C);

That doesn't quite work. Type aliases can't be used as constructors, which means you can't do commands.spawn(MyBundle(a, b, c)). Named bundles also have the benefit of letting you define custom new functions, which you can't do for anonymous bundles.

You might be right about complexity, but I'm not really opinionated enough to comment on it.

[Zeenobit]

That doesn't quite work. Type aliases can't be used as constructors, which means you can't do commands.spawn(MyBundle(a, b, c)). Named bundles also have the benefit of letting you define custom new functions, which you can't do for anonymous bundles.

Until the user does this:

pub fn new_my_bundle(...) -> MyBundle { ... }

With your changes, this also compiles/passes just fine:

#[derive(Component, Default)]
struct Foo;

#[derive(Component, Default)]
struct Bar;

// #[derive(Bundle, Default)]
// struct FooBundle {
//     foo: Foo,
//     bar: Bar,
// }

type FooBundle2 = (Foo, Bar);

let mut world = World::new();
let e = world.spawn(FooBundle2::default()).id();

let mut q = world.query_filtered::<Entity, With<FooBundle2>>();

assert_eq!(q.single(&world), e);

At this point I'm leaning towards just abandoning the idea because I don't think it justifies this arbitrary distinction between Bundle and AnonymousBundle.

Let me know if you'd like me to update the PR with AnonymousBundle. But otherwise I'm gonna leave it open in case we want to revisit it in future.

[alice-i-cecile]

I don't think it justifies this arbitrary distinction between Bundle and AnonymousBundle.

Agree here, this distinction is fiddly and just going to frustrate users.