[Merged by Bors] - Make Commands and World apis consistent #1703

cart · 2021-03-20T03:17:47Z

This makes the Commands apis consistent with World apis. This moves to a "type state" pattern (like World) where the "current entity" is stored in an EntityCommands builder.

In general this tends to cuts down on indentation and line count. It comes at the cost of needing to type commands more and adding more semicolons to terminate expressions.

I also added spawn_bundle to Commands because this is a common enough operation that I think its worth providing a shorthand.

cart · 2021-03-20T03:21:40Z

This is a pretty big change to user facing apis, so please weigh in if you have alternative ideas. I'd like to avoid touching these any more than we need to.

alice-i-cecile · 2021-03-20T03:44:01Z

User-facing changes to Commands, collected for your convenience:

spawn: no longer takes a bundle. Now much easier to spawn entities with zero / one component.
spawn_bundle: new convenience method to get back old behavior.
entity: Does... something? It's not documented, but I think it sets the current entity to use with the various EntityCommands
id: Seems to be the new replacement for current_entity?
add: replaces add_command
insert_bundle: replaces with_bundle
insert: replaces with
current_entity / with_current_entity: removed
insert / remove / despawn / various parent-child commands: now use commands.entity(entity).remove()

alice-i-cecile · 2021-03-20T03:50:47Z

Now, thoughts. I love these changes. The API is much clearer, more consistent and more explicit, without making anything significantly harder to do.

The previous API often worked in... difficult to grasp and challenging to intuit ways as soon as you stepped beyond copying directly from the examples. The new EntityCommands seems much nicer to write custom commands for.

This API also seems like an excellent step towards batching commands due to the increased explicitness and clarity.

IMO these are the right changes to make, and I'd much rather break things all at once and earlier than do it later. The consistency with World in particular is really valuable for reducing mental overhead for intermediate and advanced users.

alice-i-cecile · 2021-03-20T04:03:37Z

examples/2d/contributors.rs

-            })
-            .with_bundle(SpriteBundle {
+        let e = commands
+            .spawn_bundle((


This is much nicer to read if you use .spawn().insert_bundle().insert_bundle().

alice-i-cecile · 2021-03-20T04:10:11Z

Is it worth solving #792 quickly before 0.5 to make this API really ergonomic? It fits in well with these changes and would be useful to folks trying to refactor their related code.

mockersf · 2021-03-20T08:50:39Z

the new api looks nice 👍

wilk10 · 2021-03-22T07:16:35Z

i like the new API! thanks a lot for that.

however:

entity: Does... something? It's not documented, but I think it sets the current entity to use with the various EntityCommands

id: Seems to be the new replacement for current_entity?

i do agree with the confusion. i feel that they should contain a verb (eg: get_id) so that they consistent with being commands?

cart · 2021-03-22T18:40:29Z

Is it worth solving #792 quickly before 0.5 to make this API really ergonomic? It fits in well with these changes and would be useful to folks trying to refactor their related code.

I'd prefer not to. Adding more features than we already are feels like to much risk at this point. Bundle merging also feels like a pretty superficial change from a user perspective (and non breaking). Folks are free to explore this and we can make a call when the code prior to release if the in front of us, but I think it would be a stretch.

i do agree with the confusion. i feel that they should contain a verb (eg: get_id) so that they consistent with being commands?

get_id() isn't a command, its a normal "getter". That means it should be in the x() form instead of get_x().

I do think the naming of world.entity(e1), commands.entity(e1), and world.entity_mut(e1) commands are worth discussing (especially in relationship to id()). These commands return "entity builder" structs for a specific entity (the same struct returned by spawn() and spawn_bundle()).

We could consider alternative names like world.entity_builder(e1) / world.entity_builder_mut(e1), but thats more of a mouth-full for a pretty common operation (especially when we account for the Option<EntityMut> variants: world.get_entity_builder_mut(e1)).

alice-i-cecile · 2021-03-22T18:49:31Z

I'd prefer not to. Adding more features than we already are feels like to much risk at this point. Bundle merging also feels like a pretty superficial change from a user perspective (and non breaking). Folks are free to explore this and we can make a call when the code prior to release if the in front of us, but I think it would be a stretch.

I'm alright with this; we can explore it in another PR, probably after 0.5's release.

get_id() isn't a command, its a normal "getter". That means it should be in the x() form instead of get_x().

Your explanation was helpful, and I think that brevity is important to consider here. I think that good docstrings and examples should alleviate the confusion well enough here to stick with the shorter names.

cart · 2021-03-22T19:01:41Z

Folks are free to explore this and we can make a call when the code prior to release if the in front of us, but I think it would be a stretch.

Wow what a dumpster fire of a sentence. I meant to say: "Folks are free to explore this and we can make a call prior to release if the code is in front of us".

Your explanation was helpful, and I think that brevity is important to consider here. I think that good docstrings and examples should alleviate the confusion well enough here to stick with the shorter names.

Cool cool. I'll add the docs.

alice-i-cecile

I've read over the new docs and I think this is ready to merge. I'd prefer to do it sooner than later to reduce merge conflict headaches and give plugin authors a headstart on updating.

rparrett · 2021-03-22T21:30:15Z

commands.entity(child).despawn();
commands.despawn_recursive(entity);

It seems like this inconsistency may have been overlooked, or maybe the hierarchy/state/alien cake examples didn't get updated.

cart · 2021-03-23T00:23:16Z

It seems like this inconsistency may have been overlooked, or maybe the hierarchy/state/alien cake examples didn't get updated.

Yup it was overlooked. Fixed!

cart · 2021-03-23T00:23:25Z

bors r+

Resolves #1253 #1562 This makes the Commands apis consistent with World apis. This moves to a "type state" pattern (like World) where the "current entity" is stored in an `EntityCommands` builder. In general this tends to cuts down on indentation and line count. It comes at the cost of needing to type `commands` more and adding more semicolons to terminate expressions. I also added `spawn_bundle` to Commands because this is a common enough operation that I think its worth providing a shorthand.

bors · 2021-03-23T00:41:47Z

Pull request successfully merged into main.

Build succeeded:

This reverts commit 81b53d1.

recatek · 2021-03-23T04:05:48Z

Little late to the party here, but I think insert would be initially confusing to me if I were just starting out, since it seems like I'm adding the component to a list. Maybe attach is a better metaphor for associating a component with an entity? I understand that with doesn't make sense in a world context.

)"" This reverts commit 8860534.

The only API to add a parent/child relationship between existing entities is through commands, there is no easy way to do it from `World`. Manually inserting the components is not completely possible since `PreviousParent` has no public constructor. This PR adds two methods to set entities as children of an `EntityMut`: `insert_children` and `push_children`. ~~The API is similar to the one on `Commands`, except that the parent is the `EntityMut`.~~ The API is the same as in #1703. However, the `Parent` and `Children` components are defined in `bevy_transform` which depends on `bevy_ecs`, while `EntityMut` is defined in `bevy_ecs`, so the methods are added to the `BuildWorldChildren` trait instead. If #1545 is merged this should be fixed too. I'm aware cart was experimenting with entity hierarchies, but unless it's a coming soon this PR would be useful to have meanwhile. Co-authored-by: Carter Anderson <mcanders1@gmail.com>

cart added the A-ECS Entities, components, systems, and events label Mar 20, 2021

cart force-pushed the command-world-harmony branch from 4da8266 to 7b64840 Compare March 20, 2021 03:25

alice-i-cecile reviewed Mar 20, 2021

View reviewed changes

alice-i-cecile added the C-Usability A simple quality-of-life change that makes Bevy easier to use label Mar 20, 2021

rparrett mentioned this pull request Mar 20, 2021

Intermittent panic in Alien Cake Addict example #1704

Closed

alice-i-cecile mentioned this pull request Mar 20, 2021

[Merged by Bors] - make ComponentTicks::set_changed public #1711

Closed

alice-i-cecile added the help wanted label Mar 20, 2021

alice-i-cecile added this to the Bevy 0.5 milestone Mar 20, 2021

alice-i-cecile mentioned this pull request Mar 21, 2021

Create manual set_changed and set_added methods #1718

Closed

Davier mentioned this pull request Mar 22, 2021

[Merged by Bors] - Add insert_children and push_children to EntityMut #1728

Closed

cart added 3 commits March 22, 2021 12:33

Make Commands and World apis consistent

f610c3a

Improve docs and fix doc tests

91ebb41

fmt

44cd3fc

cart force-pushed the command-world-harmony branch from c036000 to 44cd3fc Compare March 22, 2021 19:34

contributors spawn tweak

55ce72e

alice-i-cecile approved these changes Mar 22, 2021

View reviewed changes

cart added 2 commits March 22, 2021 17:15

fix pbr example

4b15095

despawn recursive

aef482d

bors bot changed the title ~~Make Commands and World apis consistent~~ [Merged by Bors] - Make Commands and World apis consistent Mar 23, 2021

bors bot closed this Mar 23, 2021

siler added a commit to siler/bevy that referenced this pull request Mar 23, 2021

Revert "Make Commands and World apis consistent (bevyengine#1703)"

8860534

This reverts commit 81b53d1.

aevyrie added a commit to aevyrie/bevy_mod_raycast that referenced this pull request Mar 23, 2021

Fixes for upstream bevyengine/bevy#1703

74274dd

aevyrie added a commit to aevyrie/bevy_mod_picking that referenced this pull request Mar 23, 2021

Fixes for upstream bevyengine/bevy#1703

9e181c9

aevyrie added a commit to aevyrie/bevy_mod_bounding that referenced this pull request Mar 23, 2021

Fixes for upstream bevyengine/bevy#1703

5f30c9e

cart mentioned this pull request Mar 23, 2021

Adapt Commands apis for consistency with new World apis #1562

Closed

siler added a commit to siler/bevy that referenced this pull request Mar 24, 2021

Revert "Revert "Make Commands and World apis consistent (bevyengine#1703

4323ac1

)"" This reverts commit 8860534.

ledyba mentioned this pull request Apr 1, 2021

Cannot easily spawn an entity with one component #607

Closed

alice-i-cecile mentioned this pull request Apr 3, 2021

"with_children" does not support adding single component #277

Closed

SuperSamus mentioned this pull request Apr 7, 2021

0.4 -> 0.5 Migration Guide #1601

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Make Commands and World apis consistent #1703

[Merged by Bors] - Make Commands and World apis consistent #1703

cart commented Mar 20, 2021

cart commented Mar 20, 2021

alice-i-cecile commented Mar 20, 2021 •

edited

Loading

alice-i-cecile commented Mar 20, 2021

alice-i-cecile Mar 20, 2021 •

edited

Loading

alice-i-cecile commented Mar 20, 2021

mockersf commented Mar 20, 2021

wilk10 commented Mar 22, 2021

cart commented Mar 22, 2021

alice-i-cecile commented Mar 22, 2021

cart commented Mar 22, 2021 •

edited

Loading

alice-i-cecile left a comment

rparrett commented Mar 22, 2021 •

edited

Loading

cart commented Mar 23, 2021

cart commented Mar 23, 2021

bors bot commented Mar 23, 2021

recatek commented Mar 23, 2021 •

edited

Loading

[Merged by Bors] - Make Commands and World apis consistent #1703

[Merged by Bors] - Make Commands and World apis consistent #1703

Conversation

cart commented Mar 20, 2021

cart commented Mar 20, 2021

alice-i-cecile commented Mar 20, 2021 • edited Loading

alice-i-cecile commented Mar 20, 2021

alice-i-cecile Mar 20, 2021 • edited Loading

Choose a reason for hiding this comment

alice-i-cecile commented Mar 20, 2021

mockersf commented Mar 20, 2021

wilk10 commented Mar 22, 2021

cart commented Mar 22, 2021

alice-i-cecile commented Mar 22, 2021

cart commented Mar 22, 2021 • edited Loading

alice-i-cecile left a comment

Choose a reason for hiding this comment

rparrett commented Mar 22, 2021 • edited Loading

cart commented Mar 23, 2021

cart commented Mar 23, 2021

bors bot commented Mar 23, 2021

recatek commented Mar 23, 2021 • edited Loading

alice-i-cecile commented Mar 20, 2021 •

edited

Loading

alice-i-cecile Mar 20, 2021 •

edited

Loading

cart commented Mar 22, 2021 •

edited

Loading

rparrett commented Mar 22, 2021 •

edited

Loading

recatek commented Mar 23, 2021 •

edited

Loading