Anti-feature request: remove WithBundle

[alice-i-cecile]

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

WithBundle promotes unidiomatic OOP-style code in Bevy that is hard to extend and maintain. It also doesn't behave the way many users would hope it does: it will detect any entities with the set of components within the bundle, rather than only entities spawned with the bundle.

It is also particularly odd to have the ability to filter on bundles but not request them (#2252), causing user confusion.

What solution would you like?

Remove WithBundle. Existing users can use combined With filters (ideally with marker components), or even write their own query filter types if they desperately need this.

What alternative(s) have you considered?

Complain about its usage in code reviews and discourage (or ignore it) in the docs.

Additional context

We should attempt to remove this sooner rather than later to reduce the pain. If and when bundles are properly stored on the entity (perhaps for editor purposes?), we can consider adding this and #2252.

[Hoidigan]

I feel like expecting bundles to include a marker component to be used in With filters is a good way to go about querying for bundles, especially since just querying for the items can give false positives if you're expecting only the entities that had the bundle inserted on them.
This would still allow people to manually insert the marker component onto an entity, but if you're doing the work to find and explicitly insert the marker component that specifies it as a bundle, then you're probably doing it on purpose. I don't feel like that would pose too much of an issue for people to accidentally do and then be confused about it.
The documentation of Bundle would need to be changed to show this.

As for adding the marker components, I feel like modifying the derive for Bundle would be the easiest and most friendly way to do this. The auto generated name could be something like BundleNameMarker for bundle BundleName.
It could also be included in the trait for easier access, perhaps as a const function or an associated type. (also generated by the macro).
Adding a required item to the trait would be breaking for any explicit implementing of the trait, I'm fairly sure.

Just removing the WithBundle is trivial, and it doesn't seem to break any bevy code outside of the query_bundle example, which we'd want to change to use whatever new method we decide on.
And then we'd also want to change documentation on Bundle.
I did a search for WithBundle and that does seem to be all the places it pops up, so that should cover everything unless I missed something.

[Hoidigan]

Also, I'd be willing to work on this, I'm not super confident with macros but now's as good a time as any to start :P

[NiklasEi]

I don't think adding marker components with all bundles would be helpful here. Adding a marker yourself is trivial and in my opinion the cleaner approach to this problem.

[Hoidigan]

Did you mean adding it automatically to all bundles through the derive, adding our own markers and exporting them alongside bundles manually, or did you mean expecting some form of marker (derived or otherwise) on bundles in general (people always have to make their own for any bundle)?

[Hoidigan]

I feel like having some sort of standard for providing a marker trait with a bundle, whether it's created through macros or written by hand, is a good idea. (I'm not particularly opposed to doing it either way)
It means that users have something to move over to from WithBundle and it makes it more modular and sharable with each crate, instead of having multiple plugins querying by possibly different marker types for the same bundle.
For example, In a case where plugin one and plugin two both depend on a crate exporting a bundle, Instead of PluginOneMarker and PluginTwoMarker, the user can query for BundleMarker (provided by that root crate) and get results generated by both plugins.

[alice-i-cecile]

We'd discussed something similar before: where you effectively need "names" for entity types for editor display. I'm not entirely convinced on which exact strategy we should use; especially at this stage.

In general, this style of thing doesn't seem very useful until you have an editor and a prefab workflow, and there are so many unknowns there that I worry we'd be prematurely optimizing.

For example, In a case where plugin one and plugin two both depend on a crate exporting a bundle, Instead of PluginOneMarker and PluginTwoMarker, the user can query for BundleMarker (provided by that root crate) and get results generated by both plugins.

This feels like it may be the opposite of what's desired: these bundles may be used in very different ways by the two plugins.

Did you mean adding it automatically to all bundles through the derive, adding our own markers and exporting them alongside bundles manually, or did you mean expecting some form of marker (derived or otherwise) on bundles in general (people always have to make their own for any bundle)?

I would prefer the second manual option for now, until we have a clearer idea of downstream use cases.

[Hoidigan]

I do agree, as far as I could tell we don't have a ton of formality around bundles so it does make sense that we want to leave our options open.

I would prefer the second manual option for now, until we have a clearer idea of downstream use cases.

In that light, then it does definitely make sense to do it all manually.

Would I want to add and export a marker for the bundles that we currently have in bevy repo?
Or does that fall out of the scope of this issue?

[alice-i-cecile]

Would I want to add and export a marker for the bundles that we currently have in bevy repo?

I think that's probably out of scope; for example there's a need to use marker components with cameras that is along the same lines but really deserves its own PR. For now, I think just make a dead-simple PR cutting the feature @Hoidigan.

As Cart was joking about on Discord, we can use this as a "scream test", and see who really needed this functionality, then design a better fitting feature set to meet their needs if and when they emerge <3

[afonsolage]

This should be marked as E-Good-First-Issue just for future reference?

[alice-i-cecile]

This should be marked as E-Good-First-Issue just for future reference?

Yep, I'm happy with that call now. I held off since there was still a little bit of discussion needed, and leading that conversation is often hard for new contributors :)