From 7ccb203b37a19e5a04ec03728e2d7476eb2178e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E6=9E=97=E4=BC=9F?= Date: Tue, 8 Aug 2023 07:00:48 +0800 Subject: [PATCH] Prevent setting parent as itself (#8980) # Objective - Fixes https://github.com/bevyengine/bevy/issues/8979. ## Solution - Panic when parent and child are same entity. - Add checks into EntityCommands and EntityMut methods --- crates/bevy_hierarchy/src/child_builder.rs | 63 ++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 466d6f7b95..19bada425f 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. /// @@ -346,12 +366,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 +387,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 +409,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 +424,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 +436,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 } @@ -466,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 @@ -474,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 /// @@ -491,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. @@ -511,6 +565,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); }); @@ -525,6 +582,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); }); @@ -541,6 +601,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); });