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.

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] - Remove BuildWorldChildren impl from WorldChildBuilder #6727

Closed
wants to merge 2 commits into from

Conversation

tim-blackbird
Copy link
Contributor

Objective

Remove an obscure and inconsistent bit of API.
Simplify the WorldChildBuilder code.

No idea why this even exists.

An example of the removed API:

world.spawn_empty().with_children(|parent| {
    parent.spawn_empty();
    parent.push_children(&[some_entity]); // Does *not* add children to the parent.
    // It's actually identical to:
    parent.spawn_empty().push_children(&[some_entity]);
});

world.spawn_empty().with_children(|parent| {
    // This just panics.
    parent.push_children(&[some_entity]);
});

This exists only on WorldChildBuilder; ChildBuilder does not have this API.

Yeet.

Migration Guide

Hierarchy editing methods such as with_children and push_children have been removed from WorldChildBuilder.
You can edit the hierarchy via EntityMut instead.

@alice-i-cecile alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide A-Hierarchy Parent-child entity hierarchies labels Nov 22, 2022
push_child_unchecked(self.world, parent_entity, entity);
self.current_entity = Some(entity);
let entity = self.world.spawn(Parent(self.parent)).id();
push_child_unchecked(self.world, self.parent, entity);
if let Some(mut added) = self.world.get_resource_mut::<Events<HierarchyEvent>>() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are event's resources?

Why not just send the event, isn't a child always being added?

Copy link
Member

Choose a reason for hiding this comment

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

The if let here is checking if the event resource itself exists (to reduce random panics).

@james7132 james7132 added this to the 0.10 milestone Nov 22, 2022
@james7132 james7132 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Nov 22, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Nov 23, 2022
# Objective
Remove an obscure and inconsistent bit of API.
Simplify the `WorldChildBuilder` code.

No idea why this even exists.

An example of the removed API:
```rust
world.spawn_empty().with_children(|parent| {
    parent.spawn_empty();
    parent.push_children(&[some_entity]); // Does *not* add children to the parent.
    // It's actually identical to:
    parent.spawn_empty().push_children(&[some_entity]);
});

world.spawn_empty().with_children(|parent| {
    // This just panics.
    parent.push_children(&[some_entity]);
});
```
This exists only on `WorldChildBuilder`; `ChildBuilder` does not have this API.

Yeet.

## Migration Guide
Hierarchy editing methods such as `with_children` and `push_children` have been removed from `WorldChildBuilder`.
You can edit the hierarchy via `EntityMut` instead.

Co-authored-by: devil-ira <[email protected]>
@bors
Copy link
Contributor

bors bot commented Nov 23, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Nov 23, 2022
# Objective
Remove an obscure and inconsistent bit of API.
Simplify the `WorldChildBuilder` code.

No idea why this even exists.

An example of the removed API:
```rust
world.spawn_empty().with_children(|parent| {
    parent.spawn_empty();
    parent.push_children(&[some_entity]); // Does *not* add children to the parent.
    // It's actually identical to:
    parent.spawn_empty().push_children(&[some_entity]);
});

world.spawn_empty().with_children(|parent| {
    // This just panics.
    parent.push_children(&[some_entity]);
});
```
This exists only on `WorldChildBuilder`; `ChildBuilder` does not have this API.

Yeet.

## Migration Guide
Hierarchy editing methods such as `with_children` and `push_children` have been removed from `WorldChildBuilder`.
You can edit the hierarchy via `EntityMut` instead.

Co-authored-by: devil-ira <[email protected]>
@bors bors bot changed the title Remove BuildWorldChildren impl from WorldChildBuilder [Merged by Bors] - Remove BuildWorldChildren impl from WorldChildBuilder Nov 23, 2022
@bors bors bot closed this Nov 23, 2022
@tim-blackbird tim-blackbird deleted the remove-jank branch November 24, 2022 11:21
taiyoungjang pushed a commit to taiyoungjang/bevy that referenced this pull request Dec 15, 2022
…#6727)

# Objective
Remove an obscure and inconsistent bit of API.
Simplify the `WorldChildBuilder` code.

No idea why this even exists.

An example of the removed API:
```rust
world.spawn_empty().with_children(|parent| {
    parent.spawn_empty();
    parent.push_children(&[some_entity]); // Does *not* add children to the parent.
    // It's actually identical to:
    parent.spawn_empty().push_children(&[some_entity]);
});

world.spawn_empty().with_children(|parent| {
    // This just panics.
    parent.push_children(&[some_entity]);
});
```
This exists only on `WorldChildBuilder`; `ChildBuilder` does not have this API.

Yeet.

## Migration Guide
Hierarchy editing methods such as `with_children` and `push_children` have been removed from `WorldChildBuilder`.
You can edit the hierarchy via `EntityMut` instead.

Co-authored-by: devil-ira <[email protected]>
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
…#6727)

# Objective
Remove an obscure and inconsistent bit of API.
Simplify the `WorldChildBuilder` code.

No idea why this even exists.

An example of the removed API:
```rust
world.spawn_empty().with_children(|parent| {
    parent.spawn_empty();
    parent.push_children(&[some_entity]); // Does *not* add children to the parent.
    // It's actually identical to:
    parent.spawn_empty().push_children(&[some_entity]);
});

world.spawn_empty().with_children(|parent| {
    // This just panics.
    parent.push_children(&[some_entity]);
});
```
This exists only on `WorldChildBuilder`; `ChildBuilder` does not have this API.

Yeet.

## Migration Guide
Hierarchy editing methods such as `with_children` and `push_children` have been removed from `WorldChildBuilder`.
You can edit the hierarchy via `EntityMut` instead.

Co-authored-by: devil-ira <[email protected]>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…#6727)

# Objective
Remove an obscure and inconsistent bit of API.
Simplify the `WorldChildBuilder` code.

No idea why this even exists.

An example of the removed API:
```rust
world.spawn_empty().with_children(|parent| {
    parent.spawn_empty();
    parent.push_children(&[some_entity]); // Does *not* add children to the parent.
    // It's actually identical to:
    parent.spawn_empty().push_children(&[some_entity]);
});

world.spawn_empty().with_children(|parent| {
    // This just panics.
    parent.push_children(&[some_entity]);
});
```
This exists only on `WorldChildBuilder`; `ChildBuilder` does not have this API.

Yeet.

## Migration Guide
Hierarchy editing methods such as `with_children` and `push_children` have been removed from `WorldChildBuilder`.
You can edit the hierarchy via `EntityMut` instead.

Co-authored-by: devil-ira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Hierarchy Parent-child entity hierarchies C-Code-Quality A section of code that is hard to understand or change M-Needs-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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants