Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Add insert_children and push_children to EntityMut #1728

Closed
wants to merge 2 commits into from
Closed

[Merged by Bors] - Add insert_children and push_children to EntityMut #1728

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3eaca13
Add `insert_children` and `push_children` to EntityMut
Davier Mar 21, 2021
116ce80
simplify / optimize and add test
cart Mar 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions crates/bevy_transform/src/hierarchy/child_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ impl<'w> WorldChildBuilder<'w> {

pub trait BuildWorldChildren {
fn with_children(&mut self, spawn_children: impl FnOnce(&mut WorldChildBuilder)) -> &mut Self;
fn push_children(&mut self, children: &[Entity]) -> &mut Self;
fn insert_children(&mut self, index: usize, children: &[Entity]) -> &mut Self;
}

impl<'w> BuildWorldChildren for EntityMut<'w> {
Expand All @@ -282,6 +284,43 @@ impl<'w> BuildWorldChildren for EntityMut<'w> {
self.update_location();
self
}

fn push_children(&mut self, children: &[Entity]) -> &mut Self {
let parent = self.id();
// SAFETY: update_location is called after the entity is modified
let world = unsafe { self.world_mut() };
for child in children.iter() {
world
.entity_mut(*child)
.insert_bundle((Parent(parent), PreviousParent(parent))); // FIXME: dom't erase the previous parent (see #1545)
}
if let Some(mut children_component) = world.get_mut::<Children>(parent) {
children_component.0.extend(children.iter().cloned());
} else {
world.entity_mut(parent).insert(Children::with(children));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can optimize this by using self (which is world.entity_mut(parent)). Additionally, we haven't updated the entity location, so theres no need to call self.update_location() here.

I'll just push the changes so we can get this merged asap.

Copy link
Member

@cart cart Mar 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I should say: "we haven't modified the entity location using the unsafe world reference". Using self.insert will handle the location stuff for us.

self.update_location()
}
self
}

fn insert_children(&mut self, index: usize, children: &[Entity]) -> &mut Self {
let parent = self.id();
// SAFETY: update_location is called after the entity is modified
let world = unsafe { self.world_mut() };
for child in children.iter() {
world
.entity_mut(*child)
.insert_bundle((Parent(parent), PreviousParent(parent))); // FIXME: dom't erase the previous parent (see #1545)
}

if let Some(mut children_component) = world.get_mut::<Children>(parent) {
children_component.0.insert_from_slice(index, children);
} else {

This comment was marked as outdated.

Sign in to view

world.entity_mut(parent).insert(Children::with(children));
self.update_location()
}
self
}
}

impl<'w> BuildWorldChildren for WorldChildBuilder<'w> {
Expand All @@ -300,6 +339,48 @@ impl<'w> BuildWorldChildren for WorldChildBuilder<'w> {
self.current_entity = self.parent_entities.pop();
self
}

fn push_children(&mut self, children: &[Entity]) -> &mut Self {
let parent = self
.current_entity
.expect("Cannot add children without a parent. Try creating an entity first.");
self.parent_entities.push(parent);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We aren't creating a "nested WorldChildBuilder context" here like we do in with_children, so theres no need to push the parent entity (and in fact, that will break things).


for child in children.iter() {
self.world
.entity_mut(*child)
.insert_bundle((Parent(parent), PreviousParent(parent))); // FIXME: dom't erase the previous parent (see #1545)
}
if let Some(mut children_component) = self.world.get_mut::<Children>(parent) {
children_component.0.extend(children.iter().cloned());
} else {
self.world
.entity_mut(parent)
.insert(Children::with(children));
}
self
}

fn insert_children(&mut self, index: usize, children: &[Entity]) -> &mut Self {
let parent = self
.current_entity
.expect("Cannot add children without a parent. Try creating an entity first.");
self.parent_entities.push(parent);

for child in children.iter() {
self.world
.entity_mut(*child)
.insert_bundle((Parent(parent), PreviousParent(parent))); // FIXME: dom't erase the previous parent (see #1545)
}
if let Some(mut children_component) = self.world.get_mut::<Children>(parent) {
children_component.0.insert_from_slice(index, children);
} else {
self.world
.entity_mut(parent)
.insert(Children::with(children));
}
self
}
}

#[cfg(test)]
Expand Down