Skip to content

Commit

Permalink
Fix PreviousParent lag by merging parent update systems (#713)
Browse files Browse the repository at this point in the history
* Sync previous parent in parent_update_system

* Previous parent does not need to be an option now

* Remove previous parent after parent deletion
  • Loading branch information
svents authored Oct 29, 2020
1 parent bf2a917 commit 7734b1e
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 48 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_transform/src/components/parent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl MapEntities for Parent {
}

#[derive(Debug, Copy, Clone, Eq, PartialEq)]
pub struct PreviousParent(pub Option<Entity>);
pub struct PreviousParent(pub Entity);

impl Deref for Parent {
type Target = Entity;
Expand Down
22 changes: 8 additions & 14 deletions crates/bevy_transform/src/hierarchy/child_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ impl Command for InsertChildren {
fn write(self: Box<Self>, world: &mut World, _resources: &mut Resources) {
for child in self.children.iter() {
world
.insert(
*child,
(Parent(self.parent), PreviousParent(Some(self.parent))),
)
.insert(*child, (Parent(self.parent), PreviousParent(self.parent)))
.unwrap();
}
{
Expand Down Expand Up @@ -53,10 +50,7 @@ impl Command for PushChildren {
fn write(self: Box<Self>, world: &mut World, _resources: &mut Resources) {
for child in self.children.iter() {
world
.insert(
*child,
(Parent(self.parent), PreviousParent(Some(self.parent))),
)
.insert(*child, (Parent(self.parent), PreviousParent(self.parent)))
.unwrap();
}
{
Expand Down Expand Up @@ -255,11 +249,11 @@ mod tests {

assert_eq!(
*world.get::<PreviousParent>(child1).unwrap(),
PreviousParent(Some(parent))
PreviousParent(parent)
);
assert_eq!(
*world.get::<PreviousParent>(child2).unwrap(),
PreviousParent(Some(parent))
PreviousParent(parent)
);
}

Expand Down Expand Up @@ -291,11 +285,11 @@ mod tests {

assert_eq!(
*world.get::<PreviousParent>(child1).unwrap(),
PreviousParent(Some(parent))
PreviousParent(parent)
);
assert_eq!(
*world.get::<PreviousParent>(child2).unwrap(),
PreviousParent(Some(parent))
PreviousParent(parent)
);

commands.insert_children(parent, 1, &entities[3..]);
Expand All @@ -310,11 +304,11 @@ mod tests {
assert_eq!(*world.get::<Parent>(child4).unwrap(), Parent(parent));
assert_eq!(
*world.get::<PreviousParent>(child3).unwrap(),
PreviousParent(Some(parent))
PreviousParent(parent)
);
assert_eq!(
*world.get::<PreviousParent>(child4).unwrap(),
PreviousParent(Some(parent))
PreviousParent(parent)
);
}
}
50 changes: 18 additions & 32 deletions crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,64 +3,53 @@ use bevy_ecs::{Commands, Entity, IntoQuerySystem, Query, System, Without};
use bevy_utils::HashMap;
use smallvec::SmallVec;

pub fn missing_previous_parent_system(
mut commands: Commands,
mut query: Query<Without<PreviousParent, (Entity, &Parent)>>,
) {
// Add missing `PreviousParent` components
for (entity, _parent) in &mut query.iter() {
log::trace!("Adding missing PreviousParent to {:?}", entity);
commands.insert_one(entity, PreviousParent(None));
}
}

pub fn parent_update_system(
mut commands: Commands,
mut removed_parent_query: Query<Without<Parent, (Entity, &PreviousParent)>>,
// TODO: ideally this only runs when the Parent component has changed
mut changed_parent_query: Query<(Entity, &Parent, &mut PreviousParent)>,
mut changed_parent_query: Query<(Entity, &Parent, Option<&mut PreviousParent>)>,
children_query: Query<&mut Children>,
) {
// Entities with a missing `Parent` (ie. ones that have a `PreviousParent`), remove
// them from the `Children` of the `PreviousParent`.
for (entity, previous_parent) in &mut removed_parent_query.iter() {
log::trace!("Parent was removed from {:?}", entity);
if let Some(previous_parent_entity) = previous_parent.0 {
if let Ok(mut previous_parent_children) =
children_query.get_mut::<Children>(previous_parent_entity)
{
log::trace!(" > Removing {:?} from it's prev parent's children", entity);
previous_parent_children.0.retain(|e| *e != entity);
}
if let Ok(mut previous_parent_children) =
children_query.get_mut::<Children>(previous_parent.0)
{
log::trace!(" > Removing {:?} from it's prev parent's children", entity);
previous_parent_children.0.retain(|e| *e != entity);
commands.remove_one::<PreviousParent>(entity);
}
}

// Tracks all newly created `Children` Components this frame.
let mut children_additions = HashMap::<Entity, SmallVec<[Entity; 8]>>::default();

// Entities with a changed Parent (that also have a PreviousParent, even if None)
for (entity, parent, mut previous_parent) in &mut changed_parent_query.iter() {
for (entity, parent, possible_previous_parent) in &mut changed_parent_query.iter() {
log::trace!("Parent changed for {:?}", entity);

// If the `PreviousParent` is not None.
if let Some(previous_parent_entity) = previous_parent.0 {
if let Some(mut previous_parent) = possible_previous_parent {
// New and previous point to the same Entity, carry on, nothing to see here.
if previous_parent_entity == parent.0 {
if previous_parent.0 == parent.0 {
log::trace!(" > But the previous parent is the same, ignoring...");
continue;
}

// Remove from `PreviousParent.Children`.
if let Ok(mut previous_parent_children) =
children_query.get_mut::<Children>(previous_parent_entity)
children_query.get_mut::<Children>(previous_parent.0)
{
log::trace!(" > Removing {:?} from prev parent's children", entity);
(*previous_parent_children).0.retain(|e| *e != entity);
}
}

// Set `PreviousParent = Parent`.
*previous_parent = PreviousParent(Some(parent.0));
// Set `PreviousParent = Parent`.
*previous_parent = PreviousParent(parent.0);
} else {
log::trace!("Adding missing PreviousParent to {:?}", entity);
commands.insert_one(entity, PreviousParent(parent.0));
};

// Add to the parent's `Children` (either the real component, or
// `children_additions`).
Expand Down Expand Up @@ -99,10 +88,7 @@ pub fn parent_update_system(
}

pub fn hierarchy_maintenance_systems() -> Vec<Box<dyn System>> {
vec![
missing_previous_parent_system.system(),
parent_update_system.system(),
]
vec![parent_update_system.system()]
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_transform/src/hierarchy/world_child_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ impl<'a, 'b> WorldChildBuilder<'a, 'b> {
.expect("There should always be a parent at this point.");
self.world_builder
.spawn(components)
.with_bundle((Parent(parent_entity), PreviousParent(Some(parent_entity))));
.with_bundle((Parent(parent_entity), PreviousParent(parent_entity)));
let entity = self.world_builder.current_entity.unwrap();
{
let world = &mut self.world_builder.world;
Expand Down

0 comments on commit 7734b1e

Please sign in to comment.