Skip to content

Commit

Permalink
Rename push children to add children (#15196)
Browse files Browse the repository at this point in the history
# Objective

- Makes naming between add_child and add_children more consistent
- Fixes #15101 

## Solution

renamed push_children to add_children

## Testing

- Did you test these changes? If so, how?
Ran tests + grep search for any instance of `push_child`

- Are there any parts that need more testing?

- How can other people (reviewers) test your changes? Is there anything
specific they need to know?

- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
ran tests on WSL2

---

## Migration Guide

> This section is optional. If there are no breaking changes, you can
delete this section.

- If this PR is a breaking change (relative to the last release of
Bevy), describe how a user might need to migrate their code to support
these changes

rename any use of `push_children()` to the updated `add_children()`
  • Loading branch information
nealtaylor98 authored Sep 16, 2024
1 parent 522d82b commit 23a77ca
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 79 deletions.
90 changes: 45 additions & 45 deletions crates/bevy_hierarchy/src/child_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fn push_events(world: &mut World, events: impl IntoIterator<Item = HierarchyEven
/// Adds `child` to `parent`'s [`Children`], without checking if it is already present there.
///
/// This might cause unexpected results when removing duplicate children.
fn push_child_unchecked(world: &mut World, parent: Entity, child: Entity) {
fn add_child_unchecked(world: &mut World, parent: Entity, child: Entity) {
let mut parent = world.entity_mut(parent);
if let Some(mut children) = parent.get_mut::<Children>() {
children.0.push(child);
Expand Down Expand Up @@ -161,14 +161,14 @@ fn clear_children(parent: Entity, world: &mut World) {

/// Command that adds a child to an entity.
#[derive(Debug)]
pub struct PushChild {
pub struct AddChild {
/// Parent entity to add the child to.
pub parent: Entity,
/// Child entity to add.
pub child: Entity,
}

impl Command for PushChild {
impl Command for AddChild {
fn apply(self, world: &mut World) {
world.entity_mut(self.parent).add_child(self.child);
}
Expand All @@ -192,14 +192,14 @@ impl Command for InsertChildren {

/// Command that pushes children to the end of the entity's [`Children`].
#[derive(Debug)]
pub struct PushChildren {
pub struct AddChildren {
parent: Entity,
children: SmallVec<[Entity; 8]>,
}

impl Command for PushChildren {
impl Command for AddChildren {
fn apply(self, world: &mut World) {
world.entity_mut(self.parent).push_children(&self.children);
world.entity_mut(self.parent).add_children(&self.children);
}
}

Expand Down Expand Up @@ -236,7 +236,7 @@ pub struct ReplaceChildren {
impl Command for ReplaceChildren {
fn apply(self, world: &mut World) {
clear_children(self.parent, world);
world.entity_mut(self.parent).push_children(&self.children);
world.entity_mut(self.parent).add_children(&self.children);
}
}

Expand Down Expand Up @@ -276,7 +276,7 @@ impl Command for RemoveParent {
/// ```
pub struct ChildBuilder<'a> {
commands: Commands<'a, 'a>,
push_children: PushChildren,
add_children: AddChildren,
}

/// Trait for building children entities and adding them to a parent entity. This is used in
Expand Down Expand Up @@ -313,18 +313,18 @@ impl ChildBuild for ChildBuilder<'_> {

fn spawn(&mut self, bundle: impl Bundle) -> EntityCommands {
let e = self.commands.spawn(bundle);
self.push_children.children.push(e.id());
self.add_children.children.push(e.id());
e
}

fn spawn_empty(&mut self) -> EntityCommands {
let e = self.commands.spawn_empty();
self.push_children.children.push(e.id());
self.add_children.children.push(e.id());
e
}

fn parent_entity(&self) -> Entity {
self.push_children.parent
self.add_children.parent
}

fn add_command<C: Command>(&mut self, command: C) -> &mut Self {
Expand Down Expand Up @@ -362,7 +362,7 @@ pub trait BuildChildren {
/// # Panics
///
/// Panics if any of the children are the same as the parent.
fn push_children(&mut self, children: &[Entity]) -> &mut Self;
fn add_children(&mut self, children: &[Entity]) -> &mut Self;

/// Inserts children at the given index.
///
Expand Down Expand Up @@ -428,14 +428,14 @@ impl BuildChildren for EntityCommands<'_> {
let parent = self.id();
let mut builder = ChildBuilder {
commands: self.commands(),
push_children: PushChildren {
add_children: AddChildren {
children: SmallVec::default(),
parent,
},
};

spawn_children(&mut builder);
let children = builder.push_children;
let children = builder.add_children;
if children.children.contains(&parent) {
panic!("Entity cannot be a child of itself.");
}
Expand All @@ -446,16 +446,16 @@ impl BuildChildren for EntityCommands<'_> {
fn with_child<B: Bundle>(&mut self, bundle: B) -> &mut Self {
let parent = self.id();
let child = self.commands().spawn(bundle).id();
self.commands().add(PushChild { parent, child });
self.commands().add(AddChild { parent, child });
self
}

fn push_children(&mut self, children: &[Entity]) -> &mut Self {
fn add_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 {
self.commands().add(AddChildren {
children: SmallVec::from(children),
parent,
});
Expand Down Expand Up @@ -489,7 +489,7 @@ impl BuildChildren for EntityCommands<'_> {
if child == parent {
panic!("Cannot add entity as a child of itself.");
}
self.commands().add(PushChild { child, parent });
self.commands().add(AddChild { child, parent });
self
}

Expand All @@ -516,7 +516,7 @@ impl BuildChildren for EntityCommands<'_> {
if child == parent {
panic!("Cannot set parent to itself");
}
self.commands().add(PushChild { child, parent });
self.commands().add(AddChild { child, parent });
self
}

Expand All @@ -539,7 +539,7 @@ impl ChildBuild for WorldChildBuilder<'_> {

fn spawn(&mut self, bundle: impl Bundle) -> EntityWorldMut {
let entity = self.world.spawn((bundle, Parent(self.parent))).id();
push_child_unchecked(self.world, self.parent, entity);
add_child_unchecked(self.world, self.parent, entity);
push_events(
self.world,
[HierarchyEvent::ChildAdded {
Expand All @@ -552,7 +552,7 @@ impl ChildBuild for WorldChildBuilder<'_> {

fn spawn_empty(&mut self) -> EntityWorldMut {
let entity = self.world.spawn(Parent(self.parent)).id();
push_child_unchecked(self.world, self.parent, entity);
add_child_unchecked(self.world, self.parent, entity);
push_events(
self.world,
[HierarchyEvent::ChildAdded {
Expand Down Expand Up @@ -613,7 +613,7 @@ impl BuildChildren for EntityWorldMut<'_> {
self
}

fn push_children(&mut self, children: &[Entity]) -> &mut Self {
fn add_children(&mut self, children: &[Entity]) -> &mut Self {
if children.is_empty() {
return self;
}
Expand Down Expand Up @@ -691,7 +691,7 @@ impl BuildChildren for EntityWorldMut<'_> {
}

fn replace_children(&mut self, children: &[Entity]) -> &mut Self {
self.clear_children().push_children(children)
self.clear_children().add_children(children)
}
}

Expand Down Expand Up @@ -838,7 +838,7 @@ mod tests {

let [a, b, c] = std::array::from_fn(|_| world.spawn_empty().id());

world.entity_mut(a).push_children(&[b, c]);
world.entity_mut(a).add_children(&[b, c]);
world.entity_mut(b).remove_parent();

assert_parent(world, b, None);
Expand Down Expand Up @@ -920,7 +920,7 @@ mod tests {
let mut queue = CommandQueue::default();
{
let mut commands = Commands::new(&mut queue, &world);
commands.entity(entities[0]).push_children(&entities[1..3]);
commands.entity(entities[0]).add_children(&entities[1..3]);
}
queue.apply(&mut world);

Expand Down Expand Up @@ -981,7 +981,7 @@ mod tests {
let mut queue = CommandQueue::default();
{
let mut commands = Commands::new(&mut queue, &world);
commands.entity(entities[0]).push_children(&entities[1..3]);
commands.entity(entities[0]).add_children(&entities[1..3]);
}
queue.apply(&mut world);

Expand Down Expand Up @@ -1019,7 +1019,7 @@ mod tests {
let mut queue = CommandQueue::default();
{
let mut commands = Commands::new(&mut queue, &world);
commands.entity(entities[0]).push_children(&entities[1..3]);
commands.entity(entities[0]).add_children(&entities[1..3]);
}
queue.apply(&mut world);

Expand Down Expand Up @@ -1060,7 +1060,7 @@ mod tests {
.spawn_batch(vec![C(1), C(2), C(3), C(4), C(5)])
.collect::<Vec<Entity>>();

world.entity_mut(entities[0]).push_children(&entities[1..3]);
world.entity_mut(entities[0]).add_children(&entities[1..3]);

let parent = entities[0];
let child1 = entities[1];
Expand Down Expand Up @@ -1103,7 +1103,7 @@ mod tests {
.spawn_batch(vec![C(1), C(2), C(3)])
.collect::<Vec<Entity>>();

world.entity_mut(entities[0]).push_children(&entities[1..3]);
world.entity_mut(entities[0]).add_children(&entities[1..3]);

let parent = entities[0];
let child1 = entities[1];
Expand All @@ -1130,7 +1130,7 @@ mod tests {
.spawn_batch(vec![C(1), C(2), C(3), C(4), C(5)])
.collect::<Vec<Entity>>();

world.entity_mut(entities[0]).push_children(&entities[1..3]);
world.entity_mut(entities[0]).add_children(&entities[1..3]);

let parent = entities[0];
let child1 = entities[1];
Expand Down Expand Up @@ -1170,15 +1170,15 @@ mod tests {
let parent2 = entities[1];
let child = entities[2];

// push child into parent1
world.entity_mut(parent1).push_children(&[child]);
// add child into parent1
world.entity_mut(parent1).add_children(&[child]);
assert_eq!(
world.get::<Children>(parent1).unwrap().0.as_slice(),
&[child]
);

// move only child from parent1 with `push_children`
world.entity_mut(parent2).push_children(&[child]);
// move only child from parent1 with `add_children`
world.entity_mut(parent2).add_children(&[child]);
assert!(world.get::<Children>(parent1).is_none());

// move only child from parent2 with `insert_children`
Expand All @@ -1204,21 +1204,21 @@ mod tests {

let mut queue = CommandQueue::default();

// push child into parent1
// add child into parent1
{
let mut commands = Commands::new(&mut queue, &world);
commands.entity(parent1).push_children(&[child]);
commands.entity(parent1).add_children(&[child]);
queue.apply(&mut world);
}
assert_eq!(
world.get::<Children>(parent1).unwrap().0.as_slice(),
&[child]
);

// move only child from parent1 with `push_children`
// move only child from parent1 with `add_children`
{
let mut commands = Commands::new(&mut queue, &world);
commands.entity(parent2).push_children(&[child]);
commands.entity(parent2).add_children(&[child]);
queue.apply(&mut world);
}
assert!(world.get::<Children>(parent1).is_none());
Expand Down Expand Up @@ -1249,20 +1249,20 @@ mod tests {
}

#[test]
fn regression_push_children_same_archetype() {
fn regression_add_children_same_archetype() {
let mut world = World::new();
let child = world.spawn_empty().id();
world.spawn_empty().push_children(&[child]);
world.spawn_empty().add_children(&[child]);
}

#[test]
fn push_children_idempotent() {
fn add_children_idempotent() {
let mut world = World::new();
let child = world.spawn_empty().id();
let parent = world
.spawn_empty()
.push_children(&[child])
.push_children(&[child])
.add_children(&[child])
.add_children(&[child])
.id();

let mut query = world.query::<&Children>();
Expand All @@ -1271,9 +1271,9 @@ mod tests {
}

#[test]
fn push_children_does_not_insert_empty_children() {
fn add_children_does_not_insert_empty_children() {
let mut world = World::new();
let parent = world.spawn_empty().push_children(&[]).id();
let parent = world.spawn_empty().add_children(&[]).id();

let mut query = world.query::<&Children>();
let children = query.get(&world, parent);
Expand Down
8 changes: 4 additions & 4 deletions crates/bevy_hierarchy/src/query_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ mod tests {

let [a, b, c, d] = std::array::from_fn(|i| world.spawn(A(i)).id());

world.entity_mut(a).push_children(&[b, c]);
world.entity_mut(c).push_children(&[d]);
world.entity_mut(a).add_children(&[b, c]);
world.entity_mut(c).add_children(&[d]);

let mut system_state = SystemState::<(Query<&Children>, Query<&A>)>::new(world);
let (children_query, a_query) = system_state.get(world);
Expand All @@ -191,8 +191,8 @@ mod tests {

let [a, b, c] = std::array::from_fn(|i| world.spawn(A(i)).id());

world.entity_mut(a).push_children(&[b]);
world.entity_mut(b).push_children(&[c]);
world.entity_mut(a).add_children(&[b]);
world.entity_mut(b).add_children(&[c]);

let mut system_state = SystemState::<(Query<&Parent>, Query<&A>)>::new(world);
let (parent_query, a_query) = system_state.get(world);
Expand Down
Loading

0 comments on commit 23a77ca

Please sign in to comment.