Skip to content

Commit

Permalink
Prevent setting parent as itself (#8980)
Browse files Browse the repository at this point in the history 
# Objective

- Fixes #8979.

## Solution

- Panic when parent and child are same entity.
- Add checks into EntityCommands and EntityMut methods
  • Loading branch information
@lewiszlw
lewiszlw committed Aug 7, 2023
1 parent 10797d4 commit 7ccb203
Showing 1 changed file with 63 additions and 0 deletions.
63 changes: 63 additions & 0 deletions crates/bevy_hierarchy/src/child_builder.rs
View file
Expand Up @@ -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
///
Expand All @@ -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.
///
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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
}
Expand All @@ -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,
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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
///
Expand All @@ -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.
Expand All @@ -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);
});
Expand All @@ -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);
});
Expand All @@ -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);
});
Expand Down

0 comments on commit 7ccb203

Please sign in to comment.