Skip to content

Merge bundles together #792

Closed
Closed

Description

@RustyYato

I ran into an issue using Commands::spawn_batch to spawn in a bunch of entities that have some custom components and also needed a SpriteComponents bundle for rendering, and the only way I could find to do this was to use commands.spawn(...).with_bundle(...) or inline the fields of SpriteComponents. Both of these are sub-optimal solutions in my eyes.

One way to solve this would be to add a combinator for Bundle like

trait Bundle {
    // ...

    fn and<B: Bundle>(self, other: B) -> And<Self, B> where Self: Sized {
        And { first: self, second: other }
    }
}

struct And<A, B> {
    first: A,
    second: B,
}

impl<A: Bundle, B: Bundle> Bundle for And<A, B> {
    // ...
}

such that

fn some_startup(mut commands: Commands) {
    commands.spawn(bundle_a).with_bundle(bundle_b);
}

// has the same effect as

fn some_startup(mut commands: Commands) {
    commands.spawn(bundle_a.and(bundle_b));
}

This would allow you to combine bundles in places where you can only pass in a single bundle, for example Commands::spawn_batch.

Activity

  1. added
    A-ECSEntities, components, systems, and events
    C-FeatureA new feature, making something new possible
    on Nov 4, 2020
  2. alice-i-cecile commented on Jan 17, 2021

    @alice-i-cecile
    Member

    While this is being looked at, it might be useful to think about how we can flatten nested bundles. Bundles which contain a SpriteBundle or similar are particularly natural, but don't seem to work in the obvious way.

  3. alice-i-cecile commented on Jan 17, 2021

    @alice-i-cecile
    Member

    In Discord, there was discussion of an alternate method to solve this problem: simply flattening the bundles entirely.

    On the face of it, to make this non-ambiguous, you'd have to disallow non-bundle tuple components. I'm not convinced this is a huge loss. They're a very strange way of thinking about components; I think specialization is better handled by making the component type generic and I don't see any other obvious uses.

  4. alice-i-cecile commented on Jan 22, 2021

    @alice-i-cecile
    Member

    To demonstrate a particular example of this problem, here's an example showing the painful interaction between spawn_batch and Bundles.

    Suppose we're adding a number of entities at once with .spawn_batch:

    .spawn_batch((0..1_000).map(|_| Bundle {
                    pos: Position(0.0),
                    vel: Velocity(0.0),
                }))

    If we also want to add a Ball marker component, we can easily insert this:

    .spawn_batch((0..1_000).map(|_| Bundle {
                    pos: Position(0.0),
                    vel: Velocity(0.0),
                    ball: Ball(),
                }))

    But if we're using a premade bundle, like SpriteBundle, this becomes exceptionally painful, since we can't just modify its internals. Instead, we're forced to remake the entire bundle from scratch, causing our code to break if SpriteBundle is ever updated by Bevy. This problem gets worse if we want to combine multiple pre-existing bundles into a single bundle to be spawned with spawn_batch.

    A method for extend and extend_bundle on Bundle would help alleviate this pain substantially.

  5. RustyStriker commented on Jan 30, 2021

    @RustyStriker

    I think a good and more intuitive solution would be moving the .with functions to the Bundle instead, that way if you use a bundle in any way you will simply be able to use the same .with functions resulting in a more consistent code.

    EDIT: I think besides a more consistent code base, having the with on the bundle will allow you to separate the spawn command from the building of complex objects which will result in a better looking code.

  6. mwillsey commented on Feb 1, 2021

    @mwillsey

    @RustyStriker I think your proposed solution will still require some kind of "flattening" operator.

    struct With<B: Bundle, C: Component>(B, C)
    impl Bundle for With { ... }
    
    trait Bundle {
       ...
       fn with<C: Component>(Self, component: C) -> With<Self, C>
    }

    Since the With type will have to implement Bundle.

    I was just taking a look at implementing some kind of flatten attribute:

    #[derive(Bundle)]
    struct MyBundle {
       #[flatten] sprite: SpriteBundle
       other: OtherComponent,
    }

    Right now, it seems like actually implementing Bundle for something that has a Bundle won't be possible, since Bundle::get has this "check before you read" requirement. That makes calling get on any inner bundles impossible, since they might check their components, pass, and then start reading before you called get on any other bundles.

    I think the Bundle API will have to be tweaked at the very least to fix this checking behavior before we can get nested bundles. They are other issues as well: for example with_ids and with_static_ids will be very hard to implement without allocating Vecs, since we still have to preserve sorted-ness for type ids across all nested bundles.

  7. cart commented on Feb 1, 2021

    @cart
    Member

    I believe the Bevy ECS rewrite I'm wrapping up will make this sort of thing much easier. The Bundle api is much simpler now.

  8. added
    C-UsabilityA targeted quality-of-life change that makes Bevy easier to use
    on Feb 17, 2021
  9. alice-i-cecile commented on Feb 26, 2021

    @alice-i-cecile
    Member

    This is (maybe) planned for #1525, and if not, should be much easier to implement once it's merged.

  10. alice-i-cecile commented on Mar 7, 2021

    @alice-i-cecile
    Member

    This is largely solved by #1525, documented in #1570. Does this do everything you want it to?

  11. cart commented on Mar 7, 2021

    @cart
    Member

    I think theres still value in the "bundle builder" apis mentioned above. #1525 adds the ability to nest bundle types, but more dynamic world.spawn().insert_bundle(a_bundle.with(b_bundle)) seems useful.

  12. RustyYato commented on Mar 7, 2021

    @RustyYato
    Author

    Yes it looks like it is solved by #1525. I still think there is value in ad hoc composition here using something like Bundle::with/Bundle::and. I think this should be rather simple to implement because the existing derives handle the heavy lifting. This would allow me to streamline combining custom components/bundles with exiting premade ones.

  13. TheRawMeatball commented on Mar 7, 2021

    @TheRawMeatball
    Member

    Personally I don't think this is very useful since you could just do world.spawn().insert_bundle(a_bundle).insert_bundle(b_bundle).

  14. alice-i-cecile commented on Mar 7, 2021

    @alice-i-cecile
    Member

    Personally I don't think this is very useful since you could just do world.spawn().insert_bundle(a_bundle).insert_bundle(b_bundle).

    And hopefully the same thing with commands shortly too.

    But there's a reasonably important use case I think you're missing @TheRawMeatball: bundle builder functions that are nested a couple layers deep. These have cropped up for me a few times, where:

    1. I want to create and manipulate bundles in a pipeline based on some shared functionality and input data
    2. I want to combine them more than one layer deep, so I can't just use the pattern you described. On 0.4, it was also rough to use that pattern with spawn_batch .

    Moreover, bundles will be getting many more uses with #1481, better scenes and so on, so adding a few core QoL methods like this will pay off there too.

    Is there some appropriate collections trait we can impl for bundles? That really feels like the most idiomatic solution here: it's very strange to have something close to a Vec of components that doesn't behave at all like one.

  15. RustyYato commented on Mar 7, 2021

    @RustyYato
    Author

    @TheRawMeatball As I showed in my initial issue, that doesn't work in some special cases.

    I ran into an issue using Commands::spawn_batch to spawn in a bunch of entities that have some custom components and also needed a SpriteComponents bundle for rendering, and the only way I could find to do this was to use commands.spawn(...).with_bundle(...) or inline the fields of SpriteComponents. Both of these are sub-optimal solutions in my eyes.

    I think @alice-i-cecile has listed off more cases in newer/upcoming versions. But I'd like to approach this from another angle. Having ad-hoc bundles is useful for many of the same reasons why tuples are useful. When you need to package multiple things together, but you don't want to give them a name. Either because it's verbose, or because there isn't a great name to give them.

    @alice-i-cecile No, I don't believe there are any standard collection traits. If this feels like a Vec<_>, maybe you can draw inspiration from there on what to add. (Based on what makes sense for Bundle). For example Bundle::and as proposed at the beginning looks kind-of like Vec::extend/Vec::append. Bundle::with as proposed by @mwillsey looks kind of like Vec::push.

  16. added
    S-Needs-DesignThis issue requires design work to think about how it would best be accomplished
    on Dec 12, 2021
  17. SUPERCILEX commented on Apr 29, 2022

    @SUPERCILEX
    Contributor

    Isn't this issue obsoleted by #[bundle] flattening attribute? You could define a quick new bundle that includes the other.

  18. alice-i-cecile commented on Apr 29, 2022

    @alice-i-cecile
    Member

    It helps, but doesn't solve it. You can do that to define bundles, but not to work with them dynamically.

  19. SUPERCILEX commented on Apr 29, 2022

    @SUPERCILEX
    Contributor

    Ohhhh, right dang.

  20. james7132 commented on Oct 19, 2022

    @james7132
    Member

    Following #2975, this seems to be resolved as you can now just use tuple bundles to group multiple bundles and components together.

    fn test(mut commands: Commands) {
      commands.spawn((
        Camera3dBundle::default(),
        TransformBundle::default(),
        SpriteBundle::default()
      ));
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-ECSEntities, components, systems, and eventsC-FeatureA new feature, making something new possibleC-UsabilityA targeted quality-of-life change that makes Bevy easier to useS-Needs-DesignThis issue requires design work to think about how it would best be accomplished

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions