Add `EntityCommands::with_child` and `EntityMut::with_child` #6716

alice-i-cecile · 2022-11-21T16:33:10Z

Objective

Adding a single child is very cumbersome, and requires the use of a complex closure.
Fixes Add a with_child method to BuildChildren #6712.

Solution

Add a with_child(b: impl Bundle) API.

Changelog

Added EntityCommands::with_child(b: impl Bundle) and an equivelent method on EntityMut (for direct world access).

Migration Guide

When spawning a single child with no children of its own, consider using commands.entity(parent).with_child(child_bundle) to make your code clearer.

DJMcNab · 2022-11-21T17:24:53Z

crates/bevy_hierarchy/src/child_builder.rs

@@ -252,6 +252,9 @@ impl<'w, 's, 'a> ChildBuilder<'w, 's, 'a> {

 /// Trait defining how to build children
 pub trait BuildChildren {
+    /// Spawns a new entity for the provided `bundle`, and adds it as a child entity to the parent
+    fn with_child(&mut self, bundle: impl Bundle) -> &mut Self;


I'd be inclined to make this return (), as this could reasonably return either Self or a version of Self for the new entity (especially as there's no way in this API to get the new child's id directly.

I think users would get that it's returning the child_builder cause that's a common pattern used in Bevy.

Returning self is actually quite useful: you can spawn multiple children under the same parent by chaining it. I should improve the docs to explain that though.

I didn't change the examples to use that pattern though, as it would be pointlessly controversial.

Yeah, I was a bit confused by that too. I think it makes sense as it is and overall is more useful this way, but I do think there's some potential for confusion here.

Yep, I also wanted to maintain consistency with with_children

crates/bevy_hierarchy/src/child_builder.rs

TimJentzsch

Seems to be common enough to warrant this addition.

I wonder if it would be possible to add a with_children that takes an iterable of children which are then all added?
I assume that wouldn't be possible because the children would have different types?

errors/B0004.md

IceSentry

I like it, a bit less nesting and less boilerplate in general is always appreciated.

alice-i-cecile · 2022-11-21T22:48:09Z

@TimJentzsch precisely, which is why the existing technique uses a closure.

crates/bevy_hierarchy/src/child_builder.rs

Co-authored-by: ira <[email protected]>

alice-i-cecile · 2022-11-22T01:19:38Z

Thanks @devil-ira; those are much nicer little code snippets than my first attempt.

JoJoJet

These examples look so much nicer!

alice-i-cecile · 2022-11-28T13:17:26Z

bors r+

# Objective - Adding a single child is very cumbersome, and requires the use of a complex closure. - Fixes #6712. ## Solution Add a `with_child(b: impl Bundle)` API. ## Changelog Added `EntityCommands::with_child(b: impl Bundle)` and an equivelent method on `EntityMut` (for direct world access). ## Migration Guide When spawning a single child with no children of its own, consider using `commands.entity(parent).with_child(child_bundle)` to make your code clearer.

bors · 2022-11-28T13:38:45Z

Build failed (retrying...):

check-compiles

alice-i-cecile · 2022-11-28T13:59:56Z

This clashed pretty hard: I think it's going to be more reliable to rebase more manually: I don't want to risk messing up the changes from #6727.

# Objective - Adding a single child is very cumbersome, and requires the use of a complex closure. - Fixes #6712. ## Solution Add a `with_child(b: impl Bundle)` API. ## Changelog Added `EntityCommands::with_child(b: impl Bundle)` and an equivelent method on `EntityMut` (for direct world access). ## Migration Guide When spawning a single child with no children of its own, consider using `commands.entity(parent).with_child(child_bundle)` to make your code clearer.

bors · 2022-11-28T14:13:53Z

Build failed (retrying...):

check-compiles

# Objective - Adding a single child is very cumbersome, and requires the use of a complex closure. - Fixes #6712. ## Solution Add a `with_child(b: impl Bundle)` API. ## Changelog Added `EntityCommands::with_child(b: impl Bundle)` and an equivelent method on `EntityMut` (for direct world access). ## Migration Guide When spawning a single child with no children of its own, consider using `commands.entity(parent).with_child(child_bundle)` to make your code clearer.

bors · 2022-11-28T14:38:52Z

Build failed:

check-compiles

mockersf · 2022-12-08T15:19:46Z

While this is an improvement, I feel like it's a bandaid on a deep wound...

alice-i-cecile · 2022-12-11T18:03:46Z

This needs to be rebased, and I'm not thrilled with the fact that this isn't chainable. Moving to draft.

mockersf · 2022-12-11T20:33:44Z

I don't think this should be merged:

this is at best a workaround
this makes the situation more confusing, there is already with_children, add_children, push_children, insert_children and add_child, a new one is not helping.
with_children and add_children both take a function, add_child takes an entity, with_child would take a bundle

alice-i-cecile · 2022-12-12T05:02:01Z

Firmly agree on the confusing diversity. Closing this out for now; I'd rather put the effort into other work to fix this mess for real in one form or another.

alice-i-cecile added 3 commits November 21, 2022 10:46

Add with_child API

c2badb0

Migrate simple cases

0aad5f0

More complex migrations

5e0d8df

alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Nov 21, 2022

DJMcNab reviewed Nov 21, 2022

View reviewed changes

Carter0 reviewed Nov 21, 2022

View reviewed changes

crates/bevy_hierarchy/src/child_builder.rs Show resolved Hide resolved

Carter0 approved these changes Nov 21, 2022

View reviewed changes

alice-i-cecile added 4 commits November 21, 2022 13:09

Clarify return type

8ad1809

Revert changes to elaborate test

8d1e998

Fix broken example

ed4c9a1

Fix doc test

083c6dd

TimJentzsch approved these changes Nov 21, 2022

View reviewed changes

errors/B0004.md Show resolved Hide resolved

Fix broken example

e52d7b1

Weibye approved these changes Nov 21, 2022

View reviewed changes

Re-add deleted comment

0cecd8e

IceSentry approved these changes Nov 21, 2022

View reviewed changes

alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Nov 21, 2022

tim-blackbird reviewed Nov 22, 2022

View reviewed changes

crates/bevy_hierarchy/src/child_builder.rs Outdated Show resolved Hide resolved

tim-blackbird reviewed Nov 22, 2022

View reviewed changes

crates/bevy_hierarchy/src/child_builder.rs Outdated Show resolved Hide resolved

Special case logic for better performance and less indirection

407aee0

Co-authored-by: ira <[email protected]>

JoJoJet approved these changes Nov 26, 2022

View reviewed changes

Merge branch 'main' into with-child

30c266c

ickshonpe mentioned this pull request Dec 8, 2022

Remove the style component from ui widgets #6886

Closed

ickshonpe approved these changes Dec 8, 2022

View reviewed changes

alice-i-cecile removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Dec 11, 2022

alice-i-cecile marked this pull request as draft December 11, 2022 18:03

mockersf added the X-Controversial There is active debate or serious implications around merging this PR label Dec 11, 2022

alice-i-cecile closed this Dec 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `EntityCommands::with_child` and `EntityMut::with_child` #6716

Add `EntityCommands::with_child` and `EntityMut::with_child` #6716

alice-i-cecile commented Nov 21, 2022

DJMcNab Nov 21, 2022

Carter0 Nov 21, 2022

alice-i-cecile Nov 21, 2022

IceSentry Nov 21, 2022

alice-i-cecile Nov 21, 2022

TimJentzsch left a comment

IceSentry left a comment

alice-i-cecile commented Nov 21, 2022

alice-i-cecile commented Nov 22, 2022

JoJoJet left a comment

alice-i-cecile commented Nov 28, 2022

bors bot commented Nov 28, 2022

alice-i-cecile commented Nov 28, 2022

bors bot commented Nov 28, 2022

bors bot commented Nov 28, 2022

mockersf commented Dec 8, 2022

alice-i-cecile commented Dec 11, 2022

mockersf commented Dec 11, 2022 •

edited

Loading

alice-i-cecile commented Dec 12, 2022

Add EntityCommands::with_child and EntityMut::with_child #6716

Add EntityCommands::with_child and EntityMut::with_child #6716

Conversation

alice-i-cecile commented Nov 21, 2022

Objective

Solution

Changelog

Migration Guide

DJMcNab Nov 21, 2022

Choose a reason for hiding this comment

Carter0 Nov 21, 2022

Choose a reason for hiding this comment

alice-i-cecile Nov 21, 2022

Choose a reason for hiding this comment

IceSentry Nov 21, 2022

Choose a reason for hiding this comment

alice-i-cecile Nov 21, 2022

Choose a reason for hiding this comment

TimJentzsch left a comment

Choose a reason for hiding this comment

IceSentry left a comment

Choose a reason for hiding this comment

alice-i-cecile commented Nov 21, 2022

alice-i-cecile commented Nov 22, 2022

JoJoJet left a comment

Choose a reason for hiding this comment

alice-i-cecile commented Nov 28, 2022

bors bot commented Nov 28, 2022

alice-i-cecile commented Nov 28, 2022

bors bot commented Nov 28, 2022

bors bot commented Nov 28, 2022

mockersf commented Dec 8, 2022

alice-i-cecile commented Dec 11, 2022

mockersf commented Dec 11, 2022 • edited Loading

alice-i-cecile commented Dec 12, 2022

Add `EntityCommands::with_child` and `EntityMut::with_child` #6716

Add `EntityCommands::with_child` and `EntityMut::with_child` #6716

mockersf commented Dec 11, 2022 •

edited

Loading