bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Feature: Component composition.

Open mintlu8 opened this issue 1 year ago • 10 comments

Objective

Allow specialized component insertion behavior. The primary objective is allowing inserting a component multiple times between two apply_deferred calls for the clear cut case of feature composition. This opens up a lot of freedom for library developers, as previously it was difficult to modify an already inserted component without race condition or a specialized command.

For example this can be used to build a bitflags or a vec from multiple compose calls, without the need to aggregate data in one place.

Solution

Added support for specialized insertion behavior for pre-existing components. Added compose and try_compose to EntityCommands as an alternative to insert and try_insert.

compose takes a &mut Self and composes it with an incoming Self.


Changelog

  • Added method compose to Component with a default implementation identical to insert.
  • Added type level (head, tail) deconstruction support for Bundle to support bundle compositions.
  • Added attribute compose to derive macro Component.
  • Added compose and try_compose to EntityCommands as an alternative to insert and try_insert.
  • Bumped MSRV to 1.75.0 for RPITIT support, which is needed to provide default implementations for Bundle.

Migration Guide

Component and Bundle are now Sized, which might be a breaking change.

mintlu8 avatar Jan 16 '24 06:01 mintlu8

The "compose" naming seems a bit confusing. But this looks like an "upsert" functionality? Where if it doesn't exist it updates instead? I don't think adding the method to Component is the right call here at least, might make more sense for the Command to require a trait instead. That way there would also be no need to add the Sized

NiseVoid avatar Jan 16 '24 08:01 NiseVoid

The "compose" naming seems a bit confusing. But this looks like an "upsert" functionality? Where if it doesn't exist it updates instead? I don't think adding the method to Component is the right call here at least, might make more sense for the Command to require a trait instead. That way there would also be no need to add the Sized

An extra trait would not work with bundles without specialization. What I want from this PR is a function that can be called on every component for feature composition, hence the name. An extra trait would make it only callable on its implementors. I don't really like upsert but something like insert_or_update could be an alternate name.

Still compose is a much better name in my opinion for conveying intent of composing behaviors. Something like update sounds extremely vague, like a bad operator overload that makes no sense.

Unless I'm mistaken I don't think a dynamic sized struct could be inserted into a ECS table so I think the breakage is minimal.

mintlu8 avatar Jan 16 '24 08:01 mintlu8

I think this would make more sense as a 3rd party crate. I don't see the immediate benefit of adding it to bevy.

As NiseVoid mentions, instead of adding the method on the Component trait, you could create your own trait, and expose a custom EntityCommand implementation and an extension trait that adds compose to EntityCommands (similarly to how bevy adds add_children in bevy_hierarchy).

The only benefit of adding the method to Component is the ability to call compose with arbitrary components. But is it really a benefit? If I call compose with a component, it's because I think it will have a different behavior than insert right? So why allow calling compose with a component that has no particular behavior on insertion? That would not be the intended behavior. Shouldn't a type error be better in this case?

Now, maybe you are advocating that compose should straight up replace insert. And here, I think you might have a point. Like I've difficulties coming up with counterarguments why it shouldn't.

Edit: Actually yeah: It makes the Component macro more complex, and this is a negative tradeoff, that needs to be balanced with a quantified benefit. I think showing a compelling use case (even in the form of a 3rd party crate) could help the case.

nicopap avatar Jan 16 '24 08:01 nicopap

Now, maybe you are advocating that compose should straight up replace insert. And here, I think you might have a point. Like I've difficulties coming up with counterarguments why it shouldn't.

Kind of, but more as an alternative instead of a replacement.

The use case is mostly UI framework ergonomics. I want to make the widget abstraction and the user both be able to add reactive function calls in the same frame without race conditions from insert.

mintlu8 avatar Jan 16 '24 08:01 mintlu8

Something like update sounds extremely vague, like a bad operator overload that makes no sense.

Yea, update isn't necessarily very clear either, since the name sound kind of like what insert already doe (add it if it doesn't exist, or overwrite it if it does). If it took a closure insert_or_update_with would be a very clear name however :thinking:

NiseVoid avatar Jan 16 '24 08:01 NiseVoid

i don't think Component: Sized should be controversial. IIRC, all bounds are T: Component (+ Sized) since that's a pretty important invariant for storage, and dyn Component is both already impossible and unlikely ever to be useful (since we have ReflectComponent).

(NB: this isn't a comment on the S-Controversial label being attached to this issue)

soqb avatar Jan 16 '24 16:01 soqb

i think better names than compose:

  1. reduce
  • matches Iterator::reduce
  • which takes in n inputs, returns 0..=1 outputs
  1. fold
  • matches Iterator::fold
  • which always results in a value, whereas this operation doesn't.

soqb avatar Jan 16 '24 17:01 soqb

Your PR increases Bevy Minimum Supported Rust Version. Please update the rust-version field in the root Cargo.toml file.

github-actions[bot] avatar Jan 16 '24 18:01 github-actions[bot]

Update: Implemented compose on Bundle. This requires Bundle to be Sized and contains necessary usage of RPITIT, that bumps MSRV to 1.75.

mintlu8 avatar Jan 16 '24 18:01 mintlu8

Thought about this a little bit and alternative names could be either mathematical like union, join or something about synchronization like sync_insert. par_insert etc.

mintlu8 avatar Jan 17 '24 05:01 mintlu8