From f045aaa8c58c6c7eb52476fdb23918bb30402c88 Mon Sep 17 00:00:00 2001 From: Linwei Zhang Date: Wed, 28 Jun 2023 12:54:05 +0800 Subject: [PATCH 1/6] Prevent setting parent as itself --- 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 54c942144ecc4..109ce00dee683 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -170,6 +170,9 @@ pub struct AddChild { impl Command for AddChild { fn apply(self, world: &mut World) { + if self.child == self.parent { + panic!("Cannot add entity as a child of itself."); + } world.entity_mut(self.parent).add_child(self.child); } } From fc9560f85333f983ac9b7a8328abfe17a4ad5110 Mon Sep 17 00:00:00 2001 From: Linwei Zhang Date: Wed, 28 Jun 2023 14:17:34 +0800 Subject: [PATCH 2/6] Apply fix to more child builder commands --- crates/bevy_hierarchy/src/child_builder.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 109ce00dee683..76e8d8c3be991 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -187,6 +187,9 @@ pub struct InsertChildren { impl Command for InsertChildren { fn apply(self, world: &mut World) { + if self.children.contains(&self.parent) { + panic!("Cannot insert entity as a child of itself."); + } world .entity_mut(self.parent) .insert_children(self.index, &self.children); @@ -202,6 +205,9 @@ pub struct PushChildren { impl Command for PushChildren { fn apply(self, world: &mut World) { + if self.children.contains(&self.parent) { + panic!("Cannot push entity as a child of itself."); + } world.entity_mut(self.parent).push_children(&self.children); } } @@ -238,6 +244,9 @@ pub struct ReplaceChildren { impl Command for ReplaceChildren { fn apply(self, world: &mut World) { + if self.children.contains(&self.parent) { + panic!("Cannot replace entity as a child of itself."); + } clear_children(self.parent, world); world.entity_mut(self.parent).push_children(&self.children); } From f7edb55f4e9460642b8767bbf9a1486f06378966 Mon Sep 17 00:00:00 2001 From: Linwei Zhang Date: Wed, 28 Jun 2023 16:28:27 +0800 Subject: [PATCH 3/6] Move checks into EntityMut --- crates/bevy_hierarchy/src/child_builder.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 76e8d8c3be991..61fe37accb47a 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -170,9 +170,6 @@ pub struct AddChild { impl Command for AddChild { fn apply(self, world: &mut World) { - if self.child == self.parent { - panic!("Cannot add entity as a child of itself."); - } world.entity_mut(self.parent).add_child(self.child); } } @@ -187,9 +184,6 @@ pub struct InsertChildren { impl Command for InsertChildren { fn apply(self, world: &mut World) { - if self.children.contains(&self.parent) { - panic!("Cannot insert entity as a child of itself."); - } world .entity_mut(self.parent) .insert_children(self.index, &self.children); @@ -205,9 +199,6 @@ pub struct PushChildren { impl Command for PushChildren { fn apply(self, world: &mut World) { - if self.children.contains(&self.parent) { - panic!("Cannot push entity as a child of itself."); - } world.entity_mut(self.parent).push_children(&self.children); } } @@ -244,9 +235,6 @@ pub struct ReplaceChildren { impl Command for ReplaceChildren { fn apply(self, world: &mut World) { - if self.children.contains(&self.parent) { - panic!("Cannot replace entity as a child of itself."); - } clear_children(self.parent, world); world.entity_mut(self.parent).push_children(&self.children); } @@ -523,6 +511,9 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { fn add_child(&mut self, child: Entity) -> &mut Self { let parent = self.id(); + if child == parent { + panic!("Cannot add entity as a child of itself."); + } self.world_scope(|world| { update_old_parent(world, child, parent); }); @@ -537,6 +528,9 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { fn push_children(&mut self, children: &[Entity]) -> &mut Self { let parent = self.id(); + if children.contains(&parent) { + panic!("Cannot push entity as a child of itself."); + } self.world_scope(|world| { update_old_parents(world, parent, children); }); @@ -553,6 +547,9 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { fn insert_children(&mut self, index: usize, children: &[Entity]) -> &mut Self { let parent = self.id(); + if children.contains(&parent) { + panic!("Cannot insert entity as a child of itself."); + } self.world_scope(|world| { update_old_parents(world, parent, children); }); From 4c58a99151abfcffed0748b72f8c1a9f46129d22 Mon Sep 17 00:00:00 2001 From: Linwei Zhang Date: Wed, 28 Jun 2023 19:01:25 +0800 Subject: [PATCH 4/6] Add checks into EntityCommands extension methods --- crates/bevy_hierarchy/src/child_builder.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 61fe37accb47a..2be30c61ae537 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -346,12 +346,18 @@ impl<'w, 's, 'a> BuildChildren for EntityCommands<'w, 's, 'a> { spawn_children(&mut builder); let children = builder.push_children; + if children.children.contains(&parent) { + panic!("Entity cannot be a child of itself."); + } self.commands().add(children); self } fn push_children(&mut self, children: &[Entity]) -> &mut Self { let parent = self.id(); + if children.contains(&parent) { + panic!("Cannot push entity as a child of itself."); + } self.commands().add(PushChildren { children: SmallVec::from(children), parent, @@ -361,6 +367,9 @@ impl<'w, 's, 'a> BuildChildren for EntityCommands<'w, 's, 'a> { fn insert_children(&mut self, index: usize, children: &[Entity]) -> &mut Self { let parent = self.id(); + if children.contains(&parent) { + panic!("Cannot insert entity as a child of itself."); + } self.commands().add(InsertChildren { children: SmallVec::from(children), index, @@ -380,6 +389,9 @@ impl<'w, 's, 'a> BuildChildren for EntityCommands<'w, 's, 'a> { fn add_child(&mut self, child: Entity) -> &mut Self { let parent = self.id(); + if child == parent { + panic!("Cannot add entity as a child of itself."); + } self.commands().add(AddChild { child, parent }); self } @@ -392,6 +404,9 @@ impl<'w, 's, 'a> BuildChildren for EntityCommands<'w, 's, 'a> { fn replace_children(&mut self, children: &[Entity]) -> &mut Self { let parent = self.id(); + if children.contains(&parent) { + panic!("Cannot replace entity as a child of itself."); + } self.commands().add(ReplaceChildren { children: SmallVec::from(children), parent, @@ -401,6 +416,9 @@ impl<'w, 's, 'a> BuildChildren for EntityCommands<'w, 's, 'a> { fn set_parent(&mut self, parent: Entity) -> &mut Self { let child = self.id(); + if child == parent { + panic!("Cannot set parent to itself"); + } self.commands().add(AddChild { child, parent }); self } From 6de656bdcc80d01054f82c114102172ba03b5d12 Mon Sep 17 00:00:00 2001 From: Linwei Zhang Date: Mon, 3 Jul 2023 17:00:22 +0800 Subject: [PATCH 5/6] Add panics doc --- crates/bevy_hierarchy/src/child_builder.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 2be30c61ae537..8a7dc5182bbd1 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -297,12 +297,20 @@ pub trait BuildChildren { /// If the children were previously children of another parent, that parent's [`Children`] component /// will have those children removed from its list. Removing all children from a parent causes its /// [`Children`] component to be removed from the entity. + /// + /// # Panics + /// + /// Panics if any of the children are the same as the parent. fn push_children(&mut self, children: &[Entity]) -> &mut Self; /// Inserts children at the given index. /// /// If the children were previously children of another parent, that parent's [`Children`] component /// will have those children removed from its list. Removing all children from a parent causes its /// [`Children`] component to be removed from the entity. + /// + /// # Panics + /// + /// Panics if any of the children are the same as the parent. fn insert_children(&mut self, index: usize, children: &[Entity]) -> &mut Self; /// Removes the given children /// @@ -313,18 +321,30 @@ pub trait BuildChildren { /// If the children were previously children of another parent, that parent's [`Children`] component /// will have those children removed from its list. Removing all children from a parent causes its /// [`Children`] component to be removed from the entity. + /// + /// # Panics + /// + /// Panics if the child is the same as the parent. fn add_child(&mut self, child: Entity) -> &mut Self; /// Removes all children from this entity. The [`Children`] component will be removed if it exists, otherwise this does nothing. fn clear_children(&mut self) -> &mut Self; /// Removes all current children from this entity, replacing them with the specified list of entities. /// /// The removed children will have their [`Parent`] component removed. + /// + /// # Panics + /// + /// Panics if any of the children are the same as the parent. fn replace_children(&mut self, children: &[Entity]) -> &mut Self; /// Sets the parent of this entity. /// /// If this entity already had a parent, the parent's [`Children`] component will have this /// child removed from its list. Removing all children from a parent causes its [`Children`] /// component to be removed from the entity. + /// + /// # Panics + /// + /// Panics if the parent is the same as the child. fn set_parent(&mut self, parent: Entity) -> &mut Self; /// Removes the [`Parent`] of this entity. /// From 1814e73031991a5e7fdfeeabeaf77cb6e6b4058c Mon Sep 17 00:00:00 2001 From: Linwei Zhang Date: Mon, 3 Jul 2023 17:03:05 +0800 Subject: [PATCH 6/6] Add more panics doc --- crates/bevy_hierarchy/src/child_builder.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 8a7dc5182bbd1..ade0459e66676 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -504,6 +504,10 @@ pub trait BuildWorldChildren { /// If the children were previously children of another parent, that parent's [`Children`] component /// will have those children removed from its list. Removing all children from a parent causes its /// [`Children`] component to be removed from the entity. + /// + /// # Panics + /// + /// Panics if the child is the same as the parent. fn add_child(&mut self, child: Entity) -> &mut Self; /// Pushes children to the back of the builder's children. For any entities that are @@ -512,12 +516,20 @@ pub trait BuildWorldChildren { /// If the children were previously children of another parent, that parent's [`Children`] component /// will have those children removed from its list. Removing all children from a parent causes its /// [`Children`] component to be removed from the entity. + /// + /// # Panics + /// + /// Panics if any of the children are the same as the parent. fn push_children(&mut self, children: &[Entity]) -> &mut Self; /// Inserts children at the given index. /// /// If the children were previously children of another parent, that parent's [`Children`] component /// will have those children removed from its list. Removing all children from a parent causes its /// [`Children`] component to be removed from the entity. + /// + /// # Panics + /// + /// Panics if any of the children are the same as the parent. fn insert_children(&mut self, index: usize, children: &[Entity]) -> &mut Self; /// Removes the given children /// @@ -529,6 +541,10 @@ pub trait BuildWorldChildren { /// If this entity already had a parent, the parent's [`Children`] component will have this /// child removed from its list. Removing all children from a parent causes its [`Children`] /// component to be removed from the entity. + /// + /// # Panics + /// + /// Panics if the parent is the same as the child. fn set_parent(&mut self, parent: Entity) -> &mut Self; /// Removes the [`Parent`] of this entity.