Skip to content

Conversation

@lassade
Copy link
Contributor

@lassade lassade commented Nov 21, 2020

Fixes #889,

TLDR;

The Problem

  1. A entity is been added twice in Children component while spawning a scene
  2. Performance takes a massive hit, transform_propagate_system took 13ms in release with a small skeleton tree ~20 bones
  3. Affects gltf loading
  4. Right now the only "correct" way of creating a Scene is using WorldBuilder.with(Parent(...)) (still wrong for other reasons)

The Fix

  1. Impl FromResources and MapEntities for PrevousParent
  2. Checking for duplicates in the Children using a debug_assert
  3. Children.0 and PreviousParent.0 are now pub(crate), their contents are a reflect of Parent

Conclusion
This is the best short term fix, it will make so that the only true correct way of creating Scene will be WorldBuilder.with_children which has no caviats

@Moxinilian Moxinilian added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times labels Nov 21, 2020
@lassade
Copy link
Contributor Author

lassade commented Nov 21, 2020

@Moxinilian the CI error is very strange can you force a re-run to see if fixes?

@Moxinilian
Copy link
Member

This is an issue with all CI currently, I don't have permission to do anything about it. I'll make sure cart is aware.

Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

I think this is a reasonable short term fix. I made one small change (see comment below)

}
}

impl DerefMut for 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 need some way to re-order children. order already matters for things like UI, and once we have an editor it will matter for "scene organization".

I added this to enable those scenarios:

children.swap(a_index, b_index);

@cart cart merged commit c1e499d into bevyengine:master Nov 22, 2020
@fopsdev fopsdev mentioned this pull request Jan 24, 2021
@lassade lassade deleted the fix_dup_children branch April 5, 2021 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Children is been added twice

3 participants