From 3697221543ec3d952205cc374533a4a51829c7f3 Mon Sep 17 00:00:00 2001 From: Alex Saveau Date: Mon, 9 May 2022 16:12:01 -0700 Subject: [PATCH 1/9] Support returning data out of with_children Signed-off-by: Alex Saveau --- crates/bevy_hierarchy/src/child_builder.rs | 79 ++++++++++++++++------ 1 file changed, 60 insertions(+), 19 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 3dd47da3878a6..50911d084555d 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -1,11 +1,15 @@ -use crate::prelude::{Children, Parent, PreviousParent}; +use std::ops::{Deref, DerefMut}; + +use smallvec::SmallVec; + use bevy_ecs::{ bundle::Bundle, entity::Entity, system::{Command, Commands, EntityCommands}, world::{EntityMut, World}, }; -use smallvec::SmallVec; + +use crate::prelude::{Children, Parent, PreviousParent}; /// Command that adds a child to an entity #[derive(Debug)] @@ -163,9 +167,12 @@ impl<'w, 's, 'a> ChildBuilder<'w, 's, 'a> { } /// Trait defining how to build children -pub trait BuildChildren { +pub trait BuildChildren<'w, 's, 'a> { /// Creates a [`ChildBuilder`] with the given children built in the given closure - fn with_children(&mut self, f: impl FnOnce(&mut ChildBuilder)) -> &mut Self; + fn with_children( + &'a mut self, + f: impl FnOnce(&mut ChildBuilder) -> T, + ) -> WithChildren<'w, 's, 'a, T>; /// Pushes children to the back of the builder's children fn push_children(&mut self, children: &[Entity]) -> &mut Self; /// Inserts children at the given index @@ -176,10 +183,35 @@ pub trait BuildChildren { fn add_child(&mut self, child: Entity) -> &mut Self; } -impl<'w, 's, 'a> BuildChildren for EntityCommands<'w, 's, 'a> { - fn with_children(&mut self, spawn_children: impl FnOnce(&mut ChildBuilder)) -> &mut Self { +/// Result of a [BuildChildren::with_children] call that provides access to the underlying commands +/// and the closure's output. +pub struct WithChildren<'w, 's, 'a, T> { + /// The output of the [BuildChildren::with_children] closure. + pub out: T, + commands: &'a mut EntityCommands<'w, 's, 'a>, +} + +impl<'w, 's, 'a, T> Deref for WithChildren<'w, 's, 'a, T> { + type Target = EntityCommands<'w, 's, 'a>; + + fn deref(&self) -> &Self::Target { + self.commands + } +} + +impl<'w, 's, 'a, T> DerefMut for WithChildren<'w, 's, 'a, T> { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.commands + } +} + +impl<'w, 's, 'a> BuildChildren<'w, 's, 'a> for EntityCommands<'w, 's, 'a> { + fn with_children( + &'a mut self, + spawn_children: impl FnOnce(&mut ChildBuilder) -> T, + ) -> WithChildren<'w, 's, 'a, T> { let parent = self.id(); - let push_children = { + let (out, push_children) = { let mut builder = ChildBuilder { commands: self.commands(), push_children: PushChildren { @@ -187,12 +219,14 @@ impl<'w, 's, 'a> BuildChildren for EntityCommands<'w, 's, 'a> { parent, }, }; - spawn_children(&mut builder); - builder.push_children + (spawn_children(&mut builder), builder.push_children) }; self.commands().add(push_children); - self + WithChildren { + out, + commands: self, + } } fn push_children(&mut self, children: &[Entity]) -> &mut Self { @@ -460,15 +494,18 @@ impl<'w> BuildWorldChildren for WorldChildBuilder<'w> { #[cfg(test)] mod tests { - use super::{BuildChildren, BuildWorldChildren}; - use crate::prelude::{Children, Parent, PreviousParent}; + use smallvec::{smallvec, SmallVec}; + use bevy_ecs::{ component::Component, entity::Entity, system::{CommandQueue, Commands}, world::World, }; - use smallvec::{smallvec, SmallVec}; + + use crate::prelude::{Children, Parent, PreviousParent}; + + use super::{BuildChildren, BuildWorldChildren}; #[derive(Component)] struct C(u32); @@ -479,13 +516,17 @@ mod tests { let mut queue = CommandQueue::default(); let mut commands = Commands::new(&mut queue, &world); - let mut children = Vec::new(); let parent = commands.spawn().insert(C(1)).id(); - commands.entity(parent).with_children(|parent| { - children.push(parent.spawn().insert(C(2)).id()); - children.push(parent.spawn().insert(C(3)).id()); - children.push(parent.spawn().insert(C(4)).id()); - }); + let children = commands + .entity(parent) + .with_children(|parent| { + [ + parent.spawn().insert(C(2)).id(), + parent.spawn().insert(C(3)).id(), + parent.spawn().insert(C(4)).id(), + ] + }) + .out; queue.apply(&mut world); assert_eq!( From f8128e5ef6bfca77374e39d15f2b91c28508bbc6 Mon Sep 17 00:00:00 2001 From: Alex Saveau Date: Mon, 9 May 2022 16:28:07 -0700 Subject: [PATCH 2/9] Make clippy happy Signed-off-by: Alex Saveau --- crates/bevy_hierarchy/src/child_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 50911d084555d..1316ddf79f6a1 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -186,7 +186,7 @@ pub trait BuildChildren<'w, 's, 'a> { /// Result of a [BuildChildren::with_children] call that provides access to the underlying commands /// and the closure's output. pub struct WithChildren<'w, 's, 'a, T> { - /// The output of the [BuildChildren::with_children] closure. + /// The output of the [`BuildChildren::with_children`] closure. pub out: T, commands: &'a mut EntityCommands<'w, 's, 'a>, } From c5c4d21486ef0144d19e3f5ebfec3f4880985c7e Mon Sep 17 00:00:00 2001 From: Alex Saveau Date: Mon, 9 May 2022 16:32:51 -0700 Subject: [PATCH 3/9] Dang it Signed-off-by: Alex Saveau --- crates/bevy_hierarchy/src/child_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 1316ddf79f6a1..1fcf2296d5b22 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -183,7 +183,7 @@ pub trait BuildChildren<'w, 's, 'a> { fn add_child(&mut self, child: Entity) -> &mut Self; } -/// Result of a [BuildChildren::with_children] call that provides access to the underlying commands +/// Result of a [`BuildChildren::with_children`] call that provides access to the underlying commands /// and the closure's output. pub struct WithChildren<'w, 's, 'a, T> { /// The output of the [`BuildChildren::with_children`] closure. From 2101e88f8b5d0c1dae0390383556f034f12110f0 Mon Sep 17 00:00:00 2001 From: Alex Saveau Date: Mon, 9 May 2022 16:53:46 -0700 Subject: [PATCH 4/9] More lifetime pain and suffering Signed-off-by: Alex Saveau --- crates/bevy_hierarchy/src/child_builder.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 1fcf2296d5b22..b5e59c4a11b87 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -169,10 +169,10 @@ impl<'w, 's, 'a> ChildBuilder<'w, 's, 'a> { /// Trait defining how to build children pub trait BuildChildren<'w, 's, 'a> { /// Creates a [`ChildBuilder`] with the given children built in the given closure - fn with_children( - &'a mut self, + fn with_children<'b, T>( + &'b mut self, f: impl FnOnce(&mut ChildBuilder) -> T, - ) -> WithChildren<'w, 's, 'a, T>; + ) -> WithChildren<'w, 's, 'a, 'b, T>; /// Pushes children to the back of the builder's children fn push_children(&mut self, children: &[Entity]) -> &mut Self; /// Inserts children at the given index @@ -185,13 +185,13 @@ pub trait BuildChildren<'w, 's, 'a> { /// Result of a [`BuildChildren::with_children`] call that provides access to the underlying commands /// and the closure's output. -pub struct WithChildren<'w, 's, 'a, T> { +pub struct WithChildren<'w, 's, 'a, 'b, T> { /// The output of the [`BuildChildren::with_children`] closure. pub out: T, - commands: &'a mut EntityCommands<'w, 's, 'a>, + commands: &'b mut EntityCommands<'w, 's, 'a>, } -impl<'w, 's, 'a, T> Deref for WithChildren<'w, 's, 'a, T> { +impl<'w, 's, 'a, 'b, T> Deref for WithChildren<'w, 's, 'a, 'b, T> { type Target = EntityCommands<'w, 's, 'a>; fn deref(&self) -> &Self::Target { @@ -199,17 +199,17 @@ impl<'w, 's, 'a, T> Deref for WithChildren<'w, 's, 'a, T> { } } -impl<'w, 's, 'a, T> DerefMut for WithChildren<'w, 's, 'a, T> { +impl<'w, 's, 'a, 'b, T> DerefMut for WithChildren<'w, 's, 'a, 'b, T> { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.commands } } impl<'w, 's, 'a> BuildChildren<'w, 's, 'a> for EntityCommands<'w, 's, 'a> { - fn with_children( - &'a mut self, + fn with_children<'b, T>( + &'b mut self, spawn_children: impl FnOnce(&mut ChildBuilder) -> T, - ) -> WithChildren<'w, 's, 'a, T> { + ) -> WithChildren<'w, 's, 'a, 'b, T> { let parent = self.id(); let (out, push_children) = { let mut builder = ChildBuilder { From 6a224ff46ef248f8759dbe7b4babf3c7056e46e4 Mon Sep 17 00:00:00 2001 From: Alex Saveau Date: Mon, 16 May 2022 16:14:01 -0700 Subject: [PATCH 5/9] Switch to add_children method Signed-off-by: Alex Saveau --- crates/bevy_hierarchy/src/child_builder.rs | 74 +++++++++------------- 1 file changed, 29 insertions(+), 45 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index b5e59c4a11b87..0b5570cdddaad 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -1,5 +1,3 @@ -use std::ops::{Deref, DerefMut}; - use smallvec::SmallVec; use bevy_ecs::{ @@ -167,12 +165,11 @@ impl<'w, 's, 'a> ChildBuilder<'w, 's, 'a> { } /// Trait defining how to build children -pub trait BuildChildren<'w, 's, 'a> { +pub trait BuildChildren { + /// Creates a [`ChildBuilder`] with the given children built in the given closure + fn with_children(&mut self, f: impl FnOnce(&mut ChildBuilder)) -> &mut Self; /// Creates a [`ChildBuilder`] with the given children built in the given closure - fn with_children<'b, T>( - &'b mut self, - f: impl FnOnce(&mut ChildBuilder) -> T, - ) -> WithChildren<'w, 's, 'a, 'b, T>; + fn add_children(&mut self, f: impl FnOnce(&mut ChildBuilder) -> T) -> T; /// Pushes children to the back of the builder's children fn push_children(&mut self, children: &[Entity]) -> &mut Self; /// Inserts children at the given index @@ -183,33 +180,26 @@ pub trait BuildChildren<'w, 's, 'a> { fn add_child(&mut self, child: Entity) -> &mut Self; } -/// Result of a [`BuildChildren::with_children`] call that provides access to the underlying commands -/// and the closure's output. -pub struct WithChildren<'w, 's, 'a, 'b, T> { - /// The output of the [`BuildChildren::with_children`] closure. - pub out: T, - commands: &'b mut EntityCommands<'w, 's, 'a>, -} - -impl<'w, 's, 'a, 'b, T> Deref for WithChildren<'w, 's, 'a, 'b, T> { - type Target = EntityCommands<'w, 's, 'a>; - - fn deref(&self) -> &Self::Target { - self.commands - } -} +impl<'w, 's, 'a> BuildChildren for EntityCommands<'w, 's, 'a> { + fn with_children(&mut self, spawn_children: impl FnOnce(&mut ChildBuilder)) -> &mut Self { + let parent = self.id(); + let push_children = { + let mut builder = ChildBuilder { + commands: self.commands(), + push_children: PushChildren { + children: SmallVec::default(), + parent, + }, + }; + spawn_children(&mut builder); + builder.push_children + }; -impl<'w, 's, 'a, 'b, T> DerefMut for WithChildren<'w, 's, 'a, 'b, T> { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.commands + self.commands().add(push_children); + self } -} -impl<'w, 's, 'a> BuildChildren<'w, 's, 'a> for EntityCommands<'w, 's, 'a> { - fn with_children<'b, T>( - &'b mut self, - spawn_children: impl FnOnce(&mut ChildBuilder) -> T, - ) -> WithChildren<'w, 's, 'a, 'b, T> { + fn add_children(&mut self, spawn_children: impl FnOnce(&mut ChildBuilder) -> T) -> T { let parent = self.id(); let (out, push_children) = { let mut builder = ChildBuilder { @@ -223,10 +213,7 @@ impl<'w, 's, 'a> BuildChildren<'w, 's, 'a> for EntityCommands<'w, 's, 'a> { }; self.commands().add(push_children); - WithChildren { - out, - commands: self, - } + out } fn push_children(&mut self, children: &[Entity]) -> &mut Self { @@ -517,16 +504,13 @@ mod tests { let mut commands = Commands::new(&mut queue, &world); let parent = commands.spawn().insert(C(1)).id(); - let children = commands - .entity(parent) - .with_children(|parent| { - [ - parent.spawn().insert(C(2)).id(), - parent.spawn().insert(C(3)).id(), - parent.spawn().insert(C(4)).id(), - ] - }) - .out; + let children = commands.entity(parent).add_children(|parent| { + [ + parent.spawn().insert(C(2)).id(), + parent.spawn().insert(C(3)).id(), + parent.spawn().insert(C(4)).id(), + ] + }); queue.apply(&mut world); assert_eq!( From cc4a311a38c5befda4fdf9f1e0c9eefd33b3d1a9 Mon Sep 17 00:00:00 2001 From: Alex Saveau Date: Mon, 16 May 2022 21:19:47 -0700 Subject: [PATCH 6/9] Add docs example Signed-off-by: Alex Saveau --- crates/bevy_hierarchy/src/child_builder.rs | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 0b5570cdddaad..7b869ede79dfa 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -169,6 +169,32 @@ pub trait BuildChildren { /// Creates a [`ChildBuilder`] with the given children built in the given closure fn with_children(&mut self, f: impl FnOnce(&mut ChildBuilder)) -> &mut Self; /// Creates a [`ChildBuilder`] with the given children built in the given closure + /// + /// Compared to [`with_children`][BuildChildren::with_children], this method lets you + /// return data out of the closure like so: + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # use bevy_hierarchy::*; + /// # + /// # #[derive(Component)] + /// # struct SomethingElse; + /// # + /// # #[derive(Component)] + /// # struct MoreStuff; + /// # + /// # fn foo(mut commands: Commands) { + /// let mut parent_commands = commands.spawn(); + /// let child_id = parent_commands.add_children(|parent| { + /// parent.spawn().id() + /// }); + /// + /// parent_commands.insert(SomethingElse); + /// commands.entity(child_id).with_children(|parent| { + /// parent.spawn().insert(MoreStuff); + /// }); + /// # } + /// ``` fn add_children(&mut self, f: impl FnOnce(&mut ChildBuilder) -> T) -> T; /// Pushes children to the back of the builder's children fn push_children(&mut self, children: &[Entity]) -> &mut Self; From 8cbeeca50907a5a922cedb7770727e43456bcc29 Mon Sep 17 00:00:00 2001 From: Alex Saveau Date: Mon, 16 May 2022 23:20:34 -0700 Subject: [PATCH 7/9] Update crates/bevy_hierarchy/src/child_builder.rs Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> --- crates/bevy_hierarchy/src/child_builder.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 7b869ede79dfa..ce2c8a35ce704 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -170,8 +170,10 @@ pub trait BuildChildren { fn with_children(&mut self, f: impl FnOnce(&mut ChildBuilder)) -> &mut Self; /// Creates a [`ChildBuilder`] with the given children built in the given closure /// - /// Compared to [`with_children`][BuildChildren::with_children], this method lets you - /// return data out of the closure like so: + /// Compared to [`with_children`][BuildChildren::with_children], this method returns the + /// the value returned from the closure, but doesn't allow chaining. + /// + /// ## Example /// /// ``` /// # use bevy_ecs::prelude::*; From 2b48828aaf499b86d0486fb9bd25dd8b1afd1f20 Mon Sep 17 00:00:00 2001 From: Alex Saveau Date: Mon, 16 May 2022 23:27:11 -0700 Subject: [PATCH 8/9] Address review feedback Signed-off-by: Alex Saveau --- crates/bevy_hierarchy/src/child_builder.rs | 43 ++++++++-------------- 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index ce2c8a35ce704..a64fd0b664a8e 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -171,11 +171,11 @@ pub trait BuildChildren { /// Creates a [`ChildBuilder`] with the given children built in the given closure /// /// Compared to [`with_children`][BuildChildren::with_children], this method returns the - /// the value returned from the closure, but doesn't allow chaining. - /// + /// the value returned from the closure, but doesn't allow chaining. + /// /// ## Example /// - /// ``` + /// ```no_run /// # use bevy_ecs::prelude::*; /// # use bevy_hierarchy::*; /// # @@ -210,38 +210,25 @@ pub trait BuildChildren { impl<'w, 's, 'a> BuildChildren for EntityCommands<'w, 's, 'a> { fn with_children(&mut self, spawn_children: impl FnOnce(&mut ChildBuilder)) -> &mut Self { - let parent = self.id(); - let push_children = { - let mut builder = ChildBuilder { - commands: self.commands(), - push_children: PushChildren { - children: SmallVec::default(), - parent, - }, - }; - spawn_children(&mut builder); - builder.push_children - }; - - self.commands().add(push_children); + self.add_children(spawn_children); self } fn add_children(&mut self, spawn_children: impl FnOnce(&mut ChildBuilder) -> T) -> T { let parent = self.id(); - let (out, push_children) = { - let mut builder = ChildBuilder { - commands: self.commands(), - push_children: PushChildren { - children: SmallVec::default(), - parent, - }, - }; - (spawn_children(&mut builder), builder.push_children) + let mut builder = ChildBuilder { + commands: self.commands(), + push_children: PushChildren { + children: SmallVec::default(), + parent, + }, }; - self.commands().add(push_children); - out + let result = spawn_children(&mut builder); + let children = builder.push_children; + self.commands().add(children); + + result } fn push_children(&mut self, children: &[Entity]) -> &mut Self { From c1eb1ea83e796fd1264715e7affecb1e2e7e65fd Mon Sep 17 00:00:00 2001 From: Alex Saveau Date: Tue, 17 May 2022 14:15:17 -0700 Subject: [PATCH 9/9] Add doc line Signed-off-by: Alex Saveau --- crates/bevy_hierarchy/src/child_builder.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index a64fd0b664a8e..54d65646cf357 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -167,6 +167,9 @@ impl<'w, 's, 'a> ChildBuilder<'w, 's, 'a> { /// Trait defining how to build children pub trait BuildChildren { /// Creates a [`ChildBuilder`] with the given children built in the given closure + /// + /// Compared to [`add_children`][BuildChildren::add_children], this method returns self + /// to allow chaining. fn with_children(&mut self, f: impl FnOnce(&mut ChildBuilder)) -> &mut Self; /// Creates a [`ChildBuilder`] with the given children built in the given closure ///