Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add with_child to simplify spawning when there will only be one child #14594

Merged
merged 2 commits into from
Aug 2, 2024

Conversation

rparrett
Copy link
Contributor

@rparrett rparrett commented Aug 2, 2024

Objective

This idea came up in the context of a hypothetical "text sections as entities" where text sections are children of a text bundle.

commands
    .spawn(TextBundle::default())
    .with_children(|parent| {
        parent.spawn(TextSection::from("Hello"));
    });

This is a bit cumbersome (but powerful and probably the way things are headed). bsn! will eventually make this nicer, but in the mean time, this might improve ergonomics for the common case where there is only one TextSection.

Solution

Add a with_child method to the BuildChildren trait that spawns a single bundle and adds it as a child to the entity.

commands
    .spawn(TextBundle::default())
    .with_child(TextSection::from("Hello"));

Testing

I added some tests, and modified the button example to use the new method.

If any potential co-authors want to improve the tests, that would be great.

Alternatives

  • Some sort of macro. See https://github.com/tigregalis/bevy_spans_ent/blob/main/examples/macro.rs#L20. I don't love this, personally, and it would probably be obsoleted by bsn!.
  • Wait for bsn!
  • Add with_children_batch that takes an Into<Iterator> of bundles.
    with_children_batch(vec![TextSection::from("Hello")])
    This is maybe not as useful as it sounds -- it only works with homogeneous bundles, so no marker components or styles.
  • If this doesn't seem valuable, doing nothing is cool with me.

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Hierarchy D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 2, 2024
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good: nice and simple but should make simple cases easier. The longer we can shield new users from closures the better.

Copy link
Contributor

@TimJentzsch TimJentzsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small change, but can make the code a lot easier to read.
Quite a common pattern in UI, but can also be useful in other contexts.

I like it!

Copy link
Contributor

@Olle-Lukowski Olle-Lukowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is simply a nice-to-have, those are always welcome IMO.

@Olle-Lukowski Olle-Lukowski added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 2, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 2, 2024
Merged via the queue into bevyengine:main with commit 5b29402 Aug 2, 2024
32 checks passed
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events and removed A-Hierarchy labels Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants