Skip to content

Refactor hierarchy-related commands to remove structs#17029

Merged
alice-i-cecile merged 4 commits intobevyengine:mainfrom
JaySpruce:refactor_hierarchy_commands
Dec 30, 2024
Merged

Refactor hierarchy-related commands to remove structs#17029
alice-i-cecile merged 4 commits intobevyengine:mainfrom
JaySpruce:refactor_hierarchy_commands

Conversation

@JaySpruce
Copy link
Copy Markdown
Member

@JaySpruce JaySpruce commented Dec 29, 2024

Objective

Continuation of #16999.

This PR handles the following:

  • Many hierarchy-related commands are wrappers around World and EntityWorldMut methods and can be moved to closures:
    • AddChild
    • InsertChildren
    • AddChildren
    • RemoveChildren
    • ClearChildren
    • ReplaceChildren
    • RemoveParent
    • DespawnRecursive
    • DespawnChildrenRecursive
    • AddChildInPlace
    • RemoveParentInPlace
  • SendEvent is a wrapper around World methods and can be moved to a closure (and its file deleted).

Migration Guide

If you were queuing the structs of hierarchy-related commands or SendEvent directly, you will need to change them to the methods implemented on EntityCommands (or Commands for SendEvent):

Struct Method
commands.queue(AddChild { child, parent }); commands.entity(parent).add_child(child); OR commands.entity(child).set_parent(parent);
commands.queue(AddChildren { children, parent }); commands.entity(parent).add_children(children);
commands.queue(InsertChildren { children, parent }); commands.entity(parent).insert_children(children);
commands.queue(RemoveChildren { children, parent }); commands.entity(parent).remove_children(children);
commands.queue(ReplaceChildren { children, parent }); commands.entity(parent).replace_children(children);
commands.queue(ClearChildren { parent }); commands.entity(parent).clear_children();
commands.queue(RemoveParent { child }); commands.entity(child).remove_parent()
commands.queue(DespawnRecursive { entity, warn: true }); commands.entity(entity).despawn_recursive();
commands.queue(DespawnRecursive { entity, warn: false }); commands.entity(entity).try_despawn_recursive();
commands.queue(DespawnChildrenRecursive { entity, warn: true }); commands.entity(entity).despawn_descendants();
commands.queue(DespawnChildrenRecursive { entity, warn: false}); commands.entity(entity).try_despawn_descendants();
commands.queue(SendEvent { event }); commands.send_event(event);

@alice-i-cecile alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change A-Hierarchy D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Dec 29, 2024
@github-actions
Copy link
Copy Markdown
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Good changes and very clear. Many of these structs are pub though: can you add a quick migration guide to the PR description?

@alice-i-cecile alice-i-cecile added the X-Uncontroversial This work is generally agreed upon label Dec 29, 2024
@BenjaminBrienen BenjaminBrienen added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Dec 29, 2024
@BenjaminBrienen BenjaminBrienen added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Dec 29, 2024
Copy link
Copy Markdown
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

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

Makes sense to streamline these commands, nice work!

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 30, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 30, 2024
Merged via the queue into bevyengine:main with commit 9ac7e17 Dec 30, 2024
@JaySpruce JaySpruce deleted the refactor_hierarchy_commands branch December 30, 2024 21:31
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
## Objective

Continuation of bevyengine#16999.

This PR handles the following:
- Many hierarchy-related commands are wrappers around `World` and
`EntityWorldMut` methods and can be moved to closures:
  - `AddChild`
  - `InsertChildren`
  - `AddChildren`
  - `RemoveChildren`
  - `ClearChildren`
  - `ReplaceChildren`
  - `RemoveParent`
  - `DespawnRecursive`
  - `DespawnChildrenRecursive`
  - `AddChildInPlace`
  - `RemoveParentInPlace`
- `SendEvent` is a wrapper around `World` methods and can be moved to a
closure (and its file deleted).

## Migration Guide

If you were queuing the structs of hierarchy-related commands or
`SendEvent` directly, you will need to change them to the methods
implemented on `EntityCommands` (or `Commands` for `SendEvent`):

| Struct | Method |

|--------------------------------------------------------------------|---------------------------------------------------------------------------------------------|
| `commands.queue(AddChild { child, parent });` |
`commands.entity(parent).add_child(child);` OR
`commands.entity(child).set_parent(parent);` |
| `commands.queue(AddChildren { children, parent });` |
`commands.entity(parent).add_children(children);` |
| `commands.queue(InsertChildren { children, parent });` |
`commands.entity(parent).insert_children(children);` |
| `commands.queue(RemoveChildren { children, parent });` |
`commands.entity(parent).remove_children(children);` |
| `commands.queue(ReplaceChildren { children, parent });` |
`commands.entity(parent).replace_children(children);` |
| `commands.queue(ClearChildren { parent });` |
`commands.entity(parent).clear_children();` |
| `commands.queue(RemoveParent { child });` |
`commands.entity(child).remove_parent()` |
| `commands.queue(DespawnRecursive { entity, warn: true });` |
`commands.entity(entity).despawn_recursive();` |
| `commands.queue(DespawnRecursive { entity, warn: false });` |
`commands.entity(entity).try_despawn_recursive();` |
| `commands.queue(DespawnChildrenRecursive { entity, warn: true });` |
`commands.entity(entity).despawn_descendants();` |
| `commands.queue(DespawnChildrenRecursive { entity, warn: false});` |
`commands.entity(entity).try_despawn_descendants();` |
| `commands.queue(SendEvent { event });` | `commands.send_event(event);`
|
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events and removed A-Hierarchy labels Jan 28, 2025
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
## Objective

Continuation of bevyengine#16999.

This PR handles the following:
- Many hierarchy-related commands are wrappers around `World` and
`EntityWorldMut` methods and can be moved to closures:
  - `AddChild`
  - `InsertChildren`
  - `AddChildren`
  - `RemoveChildren`
  - `ClearChildren`
  - `ReplaceChildren`
  - `RemoveParent`
  - `DespawnRecursive`
  - `DespawnChildrenRecursive`
  - `AddChildInPlace`
  - `RemoveParentInPlace`
- `SendEvent` is a wrapper around `World` methods and can be moved to a
closure (and its file deleted).

## Migration Guide

If you were queuing the structs of hierarchy-related commands or
`SendEvent` directly, you will need to change them to the methods
implemented on `EntityCommands` (or `Commands` for `SendEvent`):

| Struct | Method |

|--------------------------------------------------------------------|---------------------------------------------------------------------------------------------|
| `commands.queue(AddChild { child, parent });` |
`commands.entity(parent).add_child(child);` OR
`commands.entity(child).set_parent(parent);` |
| `commands.queue(AddChildren { children, parent });` |
`commands.entity(parent).add_children(children);` |
| `commands.queue(InsertChildren { children, parent });` |
`commands.entity(parent).insert_children(children);` |
| `commands.queue(RemoveChildren { children, parent });` |
`commands.entity(parent).remove_children(children);` |
| `commands.queue(ReplaceChildren { children, parent });` |
`commands.entity(parent).replace_children(children);` |
| `commands.queue(ClearChildren { parent });` |
`commands.entity(parent).clear_children();` |
| `commands.queue(RemoveParent { child });` |
`commands.entity(child).remove_parent()` |
| `commands.queue(DespawnRecursive { entity, warn: true });` |
`commands.entity(entity).despawn_recursive();` |
| `commands.queue(DespawnRecursive { entity, warn: false });` |
`commands.entity(entity).try_despawn_recursive();` |
| `commands.queue(DespawnChildrenRecursive { entity, warn: true });` |
`commands.entity(entity).despawn_descendants();` |
| `commands.queue(DespawnChildrenRecursive { entity, warn: false});` |
`commands.entity(entity).try_despawn_descendants();` |
| `commands.queue(SendEvent { event });` | `commands.send_event(event);`
|
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-Code-Quality A section of code that is hard to understand or change D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants