Add ChildBuilder::commands method

[Havvy]

Objective

Access the Commands directly from within a with_children closure.

Solution

Add a .commands() method to ChildBuilder to access Commands directly.

[Ixentus]

parent.commands().insert_resource(ImportantLocation(entity)); I wonder what it means to insert a resource into a parent?

[NiklasEi]

I agree with @Ixentus and imagine that this could be quite confusing. In the case of your documentation example, I think it would be clearer to remember the entity and insert the resource later on after spawning.
Do you have other use cases where you need command access inside the with_children closure?

[mockersf]

While this doesn't expose anything that's not already possible through ChildBuilder::add_command, as mentioned above I think it isn't a good api to expose as it can bring errors in understanding.

I would prefer to have specialised functions on the ChildBuilder to issue the commands you want, with a clear name.

[Havvy]

Would it help if the method name said it wasn't attached to the parent? Something like commands_unattached_to_parent or raw_commands? Another alternative is to make a method commands_without_parent(|commands| ...) but adding another layer of closures seemed pointless.

And yes, this doesn't formally add anything that ChildBuilder::add_command doesn't already do, so if you don't like this method, then that one should also be removed.

For my use case, I think storing the entity in a local to run after the fact breaks the locality of reading, making it harder to understand what the code is doing. The declared local, setting the local, and then actually inserting the resource with the local will all be in completely different locations. Like, they are multiple screens away.

[alice-i-cecile]

And yes, this doesn't formally add anything that ChildBuilder::add_command doesn't already do, so if you don't like this method, then that one should also be removed.

There's something to be said for both "make the hard things hard" and "there should be only one way to do things". I think Ixentus raises an important point about the confusion this could induce.

In general I'm not thrilled with the level of mental hoops required to work with ChildBuilder. My preference is to deprecate it completely in favor of relations (#1627), but that's a long way off.

[Ixentus]

So if I understand correctly, you're adding Commands to the Childbuilder to use Commands inside it. However you can trivially do this by adding a second mut Commands to the system parameters:

fn setup_locations(mut commands: Commands, mut commandsb: Commands) {
    commands
        .spawn()
        .insert(Transform::from_xyz(1.0, 0.0, 0.0))
        .insert(GlobalTransform::default())
        .with_children(|parent| {
            let entity = parent
                .spawn()
                .insert(Transform::from_xyz(1.0, 0.0, 0.0))
                .insert(GlobalTransform::default())
                .id();

            commandsb.insert_resource(ImportantLocation(entity));
        });
}

I'm not sure this is even documented though, so this PR made sense :)

[NathanSWard]

This seems like a footgun waiting to happen...

[Havvy]

It's a footgun on the already-present add_commands method.

[Havvy]

I don't forsee this being approved, so I'm going to close it. Do note that the underlying motivation is still there, and that add_command still exists with the same footguns.