bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
35.96k stars 3.55k forks source link

Hierarchy traversal and syntactic sugar for `EntityCommands` #10060

Open tintincastro opened 1 year ago

tintincastro commented 1 year ago

What problem does this solve or what need does it fill?

I love EntityCommands daisy chaining but to me, it doesn't work anymore when dealing with parent/children entities, since I have to stop the flow to get parent or child from queries (that could be get ridden of to unclutter the system signature).

Also, I think that in general, it could be easily improved by adding a bit of syntactic sugar.

What solution would you like?

As an example of what I mean, here is some pseudocode of how I think it would greatly improve concision and readability:

fn my_system_with_syntactic_sugar(
    mut commands: Commands,
    // -- SNIP --
) {
    // -- SNIP --
    let some_condition = false;
    // -- SNIP --

    commands
        .entity(some_known_child_entity)
        .insert(SomeMarker)
        .parent()
            .insert_if(some_condition, SomeRelatedMarker)
            .on_children(|child_entity_commands| {
                child_entity_commands
                    .remove_if::<AnotherRelatedMarker>(!some_condition)
                    .despawn_recursive_if(some_condition)
                ;
            })
    ;
}

Whereas today, in bevy 0.11, I'm writing this as:

fn my_system_without_syntactic_sugar(
    mut commands: Commands,
    parent_query: </* SNIP */>,
    children_query: </* SNIP */>,
    // -- SNIP --
) {
    // -- SNIP --
    let some_condition = false;
    // -- SNIP --

    commands
        .entity(some_known_child_entity)
        .insert(SomeMarker)
    ;

    let parent_entity = parent_query.get(some_known_child_entity);

    if some_condition {
        commands
            .entity(parent_entity)
            .insert(SomeRelatedMarker)
        ;
    }

    children_query
        .for_each(|child_entity| {
            if (some_condition) {
                commands
                    .entity(child_entity)
                    .despawn_recursive()
                ;
            } else {
                commands
                    .entity(child_entity)
                    .remove::<AnotherRelatedMarker>
                ;
            }
        })
    ;
}

Additional context

I'm still a Bevy newbie and still in the process of learning Rust, so there's maybe already a better way to express those kind of things that I don't know of, so please feel free to correct me!

Performance?

Also, I'm not savy enough about bevy internals (and Rust in general☺) to evaluate the performance hit of storing references to parent/children entities in EntityCommands, so maybe making it optional would be better (e.g. a HierarchyEntityCommands that could be accessed using commands.entity_in_hierarchy(some_known_child_entity) ? (Although maybe maintenance cost would be too high…).

Thanks in advance for your input, have a nice day!

SkiFire13 commented 1 year ago

Also, I'm not savy enough about bevy internals (and Rust in general☺) to evaluate the performance hit of storing references to parent/children entities in EntityCommands, so maybe making it optional would be better (e.g. a HierarchyEntityCommands that could be accessed using commands.entity_in_hierarchy(some_known_child_entity) ? (Although maybe maintenance cost would be too high…).

I think the problem would be Commands being able to access Parent/Children rather than fetching/storing them when calling entity/entity_in_hierarchy. It means any Command would now have to include a Query<&Parent> and Query<&Children>, and thus conflict with any query that access them mutably. Maybe a separate HierarchyCommands struct could be added though.

tintincastro commented 1 year ago

You're right, it didn't cross my mind, but it's true that the command and hierarchy between entity are quite far from each other as concerns goes.

It might be a stupid idea, but how about using some proxies structs instead? Like a ParentProxy that expresses nothing more than the fact that it is designated as the parent of a specific entity. It would just store a reference to the known entity, and the commands to apply.

The corresponding parent/children queries would then be issued when the initial command is applied to the world, and the stored commands themselves issued on the queried entities if they still exist, and do nothing but issue a warning when it's not the case.

The task is left to the calling user to ensure that no other systems comes in-between to mutate or break the hierarchy, and it could be indicated with the calling name, so my example would then be written like this:

// -- SNIP --
commands
    .entity(some_known_child_entity)
    .insert(SomeMarker)
    .parent_proxy()
        .insert_if(some_condition, SomeRelatedMarker)
        .on_children_proxy(|child_entity_commands| {
            child_entity_commands
                .remove_if::<AnotherRelatedMarker>(!some_condition)
                .despawn_recursive_if(some_condition)
            ;
        })
;

(Somehow it feels a bit more like bevy_ecs becomes bevy_orm ☺!)

tintincastro commented 1 year ago

For clarity's sake, here's how I would envision it:

From the moment the user would call parent_proxy(), he would be no more operating on an EntityCommands, but rather on a hypothetical ProxyEntityCommands which provides only a subset of EntityCommands to store deferred operations to be applied later on the World. For instance, there would be no id() method available on a ProxyEntityCommands struct.

(On the other topic at hand, I let the _if methods from my initial example, but it my mind, it is clear that the condition is evaluated at the calling time, so the command is simply not stored in the proxies if some_condition is not met.)