-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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.
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
Add hierarchy example #565
Conversation
examples/ecs/hierarchy.rs
Outdated
commands.despawn(child); | ||
} | ||
|
||
if time.seconds_since_startup >= 4.0 && children.len() == 2 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To better illustrate the "despawn recursive" behavior, it might be better to despawn the "parent" entity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point! Although I do like the idea of showing that despawn_recursive also takes care of cleaning up the parent's children. Maybe keep this one in and add another step where it despawns the parent afterwards? As well as some clearer comments to go with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that we aren't directly illustrating despawn_recursive
if it doesn't despawn multiple entities. If we leave the current line as-is, it only despawns one entity. If after the fact we despawn the parent recursively, that also only despawns one entity because there will be no children left.
I think this has my preference:
// To demonstrate removing children, we'll start to remove the children after a couple of seconds
if time.seconds_since_startup >= 2.0 && children.len() == 3 {
// Using .despawn() on an entity does not remove it from its parent's list of children!
// It must be done manually if using .despawn()
// NOTE: this is a bug. eventually Bevy will update the children list automatically
let child = children.pop().unwrap();
commands.despawn(child);
}
if time.seconds_since_startup >= 4.0 {
// Alternatively, you can use .despawn_recursive()
// This will remove the entity from its parent's list of children, as well as despawn
// any children the entity has.
commands.despawn_recursive(parent);
}
I added a "note" that explains that the "children list maintenance" will eventually be fixed
Clippy error isn't your fault here. Its a new nightly clippy rule. Should be fixed once we merge this: #577 |
Good to know, I did happen to have a legitimate error I missed, but that should be fixed. |
I just force-pushed a rebase on master. I tried coming up with a better workflow for forcing a new CI build from latest master, but sadly I couldn't (without adding PR orchestrator github actions). |
add ecs/hierarchy example
Fixes #513