Condense `insert` / `insert_bundle` and related methods

[alice-i-cecile]

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

Various commands operate on either components or bundles. The end user doesn't want to think up which one they're using, and this leads to code duplication and API bloat.

What solution would you like?

Condense .insert_bundle, .remove_bundle and so on down into single methods that operate on either bundles or components.

My preference would be to use a shared trait that is implied by both the Component and Bundle trait to do so.

What alternative(s) have you considered?

As a hack, you could make derive(Component) also implement Bundle, and then only keep the bundle forms around.

Additional context

Enabled by #1843 (and #2254). Inspired by #2252, which encourages a hard split between components and bundles.

[BoxyUwU]

thoughts:

If components are an opt-in component via some derive(Component) macro we could also have the derive spit out a trait impl for Bundle too. This would make every component be a one element bundle.

Now if everything is a bundle we can remove insert_bundle remove_bundle and just have only insert and remove that works off a T: Bundle bound as it would include both "components" and bundles.

without derive(Component) if you do .insert(Bundle { ... }) you silently get wrong behaviour
with derive(Component) you get a compile error
with this it compiles and works exactly how you would expect it to work

frizi said that we have a #[bundle] attribute for our derive(Bundle) macro which allows us to let our bundles contain nested bundles as fields, but if every component is a bundle we can get rid of that attribute and just pretend its on every field because every field is a bundle now.

both of these seem like pretty clear ergonomic wins, question I guess is whether people would find .insert(Bundle { .. }) to be unintuitive

[alice-i-cecile]

I like both of those a lot, honestly. To me, merging the bundle and component methods just feels like allowing a function with a variable number of arguments that we're splatting.

IME, beginners have a very fuzzy distinction between bundles and components in the first place. When combined with #2252, this would allow for them to mostly not care since methods on one will tend to work on the other seamlessly.

[cart]

While this might work today, I think this will conflict with some of our (well ... my) future plans for Bundles. Bundles are "groups of components that together produce some intended behavior". However what components live in these Bundles can change. If we save a scene with SpriteBundle components, then add a new component to SpriteBundle (which is needed to make it behave like a sprite), then try to load the scene with the new behavior, it won't work.

If we store the fact that an entity has a SpriteBundle, we can make it forward compatible for certain types of changes. If Scenes store that bundle info, we can even omit components in the scene file when they have default values because we can just "fill in" the default components at runtime. This will likely also help with ergonomic "hand composed" scenes (ex: for UI scenes).

This also plays into "Bundle Queries". Currently the bundle filter will match anything that has all components in a bundle, but imo the filter would be more useful / aligned with user intent if it only matched things that actually added the Bundle.

In short: I think treating Bundles as semantically different than Components has a lot of benefits and having separate apis leaves that path open for us.

[alice-i-cecile]

This also plays into "Bundle Queries". Currently the bundle filter will match anything that has all components in a bundle, but imo the filter would be more useful / aligned with user intent if it only matched things that actually added the Bundle.

Ping @aevyrie

[alice-i-cecile]

In short: I think treating Bundles as semantically different than Components has a lot of benefits and having separate apis leaves that path open for us.

I think that's sensible. If we had negative trait bounds I think we could enforce that components and bundles are mutually exclusive at the type level, enabling us to better handle patterns like I want here and merge things back together without the auto-impl hack that will mess with your plans. That'll be a while though :/

[cart]

I don't think we need to make them mutually exclusive. I don't think derive(Component, Bundle) is incompatible with the design above. It would mean that if you add a Bundle as a Component, it wouldn't behave like a Bundle. But, like, thats what the user asked for / why they decided to derive both.

[alice-i-cecile]

If we save a scene with SpriteBundle components, then add a new component to SpriteBundle (which is needed to make it behave like a sprite), then try to load the scene with the new behavior, it won't work.

FWIW, I think archetype invariants are a cleaner and more explicit way to handle this pattern, along with automated migration tools. Erroring on "missing component X, consider using bevyup my_asset" is much less surprising than silently adding default values to saved entities across versions.

[cart]

I do like that idea, but it does ignore the fact that Bundles often set Components to context-specific defaults (as defined by the Bundle). If Bundle information is discarded, that context is erased. An archetype invariant that detects a missing component might not be able to construct a default value for that component (because it doesn't implement default) or it might pick the "wrong" value because Default doesn't take into account the "bundle context". For example, Transform::default() won't necessarily be the same as the default Transform value as defined by BallBundle. Maybe ball transforms start at an offset position.

[alice-i-cecile]

Right, hmm. What if we steal the idea of "defining components" and use them to guide the migration tools? More to chew on for the future... That one is going to need an RFC to work out I think. My lingering feeling is that I'm reluctant to sacrifice common-case ergonomics for easier one-off asset migrations.

Well, in the meantime I think we should try and forge ahead with #1481 and #2252 (plus scene improvements and probably #792) and maybe we'll get some nice clarity on how this should be designed.

[aevyrie]

Bundles are "groups of components that together produce some intended behavior". However what components live in these Bundles can change. If we save a scene with SpriteBundle components, then add a new component to SpriteBundle (which is needed to make it behave like a sprite), then try to load the scene with the new behavior, it won't work.

This, to me, feels a lot like traits. I think I'm aligned with you, especially for bundle queries (#2252 (comment)). I want to retain the ability to implement traits or methods for a bundle itself, because then I can access and mutate the fields of that bundle as a standalone struct, but I also get the bonus of field-level change detection and composability.

This also plays into "Bundle Queries". Currently the bundle filter will match anything that has all components in a bundle, but imo the filter would be more useful / aligned with user intent if it only matched things that actually added the Bundle.

What happens if I need, for example, a Transform as part of multiple bundles, and I add both bundles to my entity? What I'm proposing is matching anything with all components in the bundle (a bundle filter), but return the result from the query as the bundle struct. This gives me the composability of current entities, the ergonomics of allowing traits/methods over bundles, without the magic of bundles being semantically "special".