refactor: Simplify lifetimes for Commands and related types

[joseph-gio]

Objective

It would be convenient to be able to call functions with Commands as a parameter without having to move your own instance of Commands. Since this struct is composed entirely of references, we can easily get an owned instance of Commands by shortening the lifetime.

Solution

Add Commands::reborrow, EntiyCommands::reborrow, and Deferred::reborrow, which returns an owned version of themselves with a shorter lifetime.

Remove unnecessary lifetimes from EntityCommands. The 'w and 's lifetimes only have to be separate for Commands because it's used as a SystemParam -- this is not the case for EntityCommands.

---

Changelog

Added Commands::reborrow. This is useful if you have &mut Commands but need Commands. Also added EntityCommands::reborrow and Deferred:reborrow which serve the same purpose.

Migration Guide

The lifetimes for EntityCommands have been simplified.

// Before (Bevy 0.12)
struct MyStruct<'w, 's, 'a> {
     commands: EntityCommands<'w, 's, 'a>,
}

// After (Bevy 0.13)
struct MyStruct<'a> {
    commands: EntityCommands<'a>,
}

The method EntityCommands::commands now returns Commands rather than &mut Commands.

// Before (Bevy 0.12)
let commands = entity_commands.commands();
commands.spawn(...);

// After (Bevy 0.13)
let mut commands = entity_commands.commands();
commands.spawn(...);

[alice-i-cecile]

Really nice! The logic makes sense to me, the docs on reborrow are unusually helpful, and the simplified lifetimes are great.

[hymm]

Just a nit about the migration guide. Definitely nice not to see 3 lifetimes on EntityCommands anymore.

[eidloi]

FYI this gave me a mental breakdown. It is no longer possible to create a unified extension over both commands and entity commands, which means I have to throw away like ~10kloc and start over the UI widget library I have been building for the past 3 months. On that note, I find it hard to believe the lifetimes were that much of a problem. They were complicated to use when writing some extensions, yes, but they did let you write stuff you would have needed the world otherwise for. Cheers.

[hymm]

@eidloi I'd be fine with reverting the lifetime change. Could you give a small example of code that doesn't compile anymore with this change?

[eidloi]

No, @hymm, it is fine, meltdown's over, moving forward - I'll drop supporting EntityCommands entirely. This was my original code:

use bevy::{
    ecs::{
        bundle::Bundle,
        entity::Entity,
        system::{Commands, EntityCommands},
    },
    hierarchy::BuildChildren,
};

pub struct UiBuilder<'w, 's, 'a> {
    commands: &'a mut Commands<'w, 's>,
    entity: Option<Entity>,
}

impl<'w, 's> UiBuilder<'w, 's, '_> {
    pub fn id(&self) -> Option<Entity> {
        self.entity
    }

    pub fn commands(&mut self) -> &mut Commands<'w, 's> {
        self.commands
    }

    pub fn entity_commands<'a>(&'a mut self) -> Result<EntityCommands<'w, 's, 'a>, &'static str> {
        if let Some(entity) = self.entity {
            Ok(self.commands().entity(entity))
        } else {
            Err("No entity set for UiBuilder")
        }
    }

    pub fn spawn<'a>(&'a mut self, bundle: impl Bundle) -> EntityCommands<'w, 's, 'a> {
        let mut new_entity = Entity::PLACEHOLDER;

        if let Some(entity) = self.id() {
            self.commands().entity(entity).with_children(|parent| {
                new_entity = parent.spawn(bundle).id();
            });
        } else {
            new_entity = self.commands().spawn(bundle).id();
        }

        self.commands().entity(new_entity)
    }
}

pub trait UiBuilderExt<'w, 's, 'a> {
    fn ui_builder(&'a mut self) -> UiBuilder<'w, 's, 'a>;
}

impl<'w, 's, 'a> UiBuilderExt<'w, 's, 'a> for EntityCommands<'w, 's, 'a> {
    fn ui_builder(&'a mut self) -> UiBuilder<'w, 's, 'a> {
        let entity = self.id();
        let mut builder = self.commands().ui_builder();
        builder.entity = entity.into();

        builder
    }
}

impl<'w, 's, 'a> UiBuilderExt<'w, 's, 'a> for Commands<'w, 's> {
    fn ui_builder(&'a mut self) -> UiBuilder<'w, 's, 'a> {
        UiBuilder::<'w, 's, 'a> {
            commands: self,
            entity: None,
        }
    }
}

Which let me do things like:

commands.entity(root_entity).ui_builder().column(
        ColumnConfig {
            width: Val::Percent(100.),
            background_color: Color::rgb(0.7, 0.7, 0.7),
            ..default()
        },
        |column| {
            column.row(
                RowConfig {
                    height: Val::Px(50.),
                    background_color: Color::rgb(0.8, 0.8, 0.8),
                },
                |row| {
                    row.column(
                        ColumnConfig {
                            width: Val::Px(100.),
                            background_color: Color::CYAN,
                        },
                        |_| {},
                    )
                    .insert((
                        Interaction::default(),
                        ChangeableBackgroundColor,
                        ChangeableBorderSize,
                        GenerateContextMenu::default(),
                    ));

                    row.checkbox(String::from("Panel").into())
                        .insert((ToggleDisplay(floating_panel_id_2),));
                    row.checkbox(Option::<String>::None);
                    row.slider(
                        SliderConfig::horizontal(Some("Slider"), 0., 100., 50., true).into(),
                    );
                    row.radio_group(vec!["Draggable", "Resizable", "Both"], false)
                        .insert(PanelFeatureControl(floating_panel_id));
                },
            );           
        },
    );

Notice the inserts in the chain there? Bottom line is that the callbacks inside would receive a builder, but would return the EntityCommands of the corresponding frame entity, so you could decorate it with whatever. This was a necessary addition to 1) be able to grab the commands to spawn entities next to each other while still being able to return the EntityCommands of the external one (something which a ChildBuilder wouldn't let you do) and 2) to have a common struct that can be extended by end users as well and not just from Bevy. An example of 1):

pub trait UiScrollViewExt<'w, 's> {
    fn scroll_view<'a>(
        &'a mut self,
        restrict_to: Option<ScrollAxis>,
        spawn_children: impl FnOnce(&mut UiBuilder),
    ) -> EntityCommands<'w, 's, 'a>;
}

impl<'w, 's> UiScrollViewExt<'w, 's> for UiBuilder<'w, 's, '_> {
    fn scroll_view<'a>(
        &'a mut self,
        restrict_to: Option<ScrollAxis>,
        spawn_children: impl FnOnce(&mut UiBuilder),
    ) -> EntityCommands<'w, 's, 'a> {
        let mut viewport = Entity::PLACEHOLDER;
        let mut content_container = Entity::PLACEHOLDER;
        let mut horizontal_scroll_id: Option<Entity> = None;
        let mut horizontal_scroll_handle_id: Option<Entity> = None;
        let mut vertical_scroll_id: Option<Entity> = None;
        let mut vertical_scroll_handle_id: Option<Entity> = None;

        let mut scroll_view = self.container(ScrollView::frame(), |frame| {
            let scroll_axes = if let Some(restrict_to) = restrict_to {
                vec![restrict_to]
            } else {
                vec![ScrollAxis::Horizontal, ScrollAxis::Vertical]
            };

            let scroll_view_id = frame.id().unwrap();
            viewport = frame
                .container(
                    (
                        ScrollView::viewport(),
                        ScrollViewViewport {
                            scroll_view: scroll_view_id,
                        },
                    ),
                    |viewport| {
                        content_container = viewport
                            .container(
                                ScrollView::content(scroll_view_id, restrict_to),
                                spawn_children,
                            )
                            .id();
                    },
                )
                .id();

            frame.container(ScrollView::scroll_bar_container(), |scroll_bar_container| {
                for axis in scroll_axes.iter() {
                    let mut handle_id = Entity::PLACEHOLDER;
                    let mut scroll_bar = scroll_bar_container.container(
                        ScrollView::scroll_bar(*axis),
                        |scroll_bar| {
                            handle_id = scroll_bar
                                .spawn((
                                    ScrollView::scroll_bar_handle(*axis),
                                    ScrollBarHandle {
                                        axis: *axis,
                                        scroll_view: scroll_view_id,
                                    },
                                ))
                                .id();
                        },
                    );
                    scroll_bar.insert(ScrollBar {
                        axis: *axis,
                        scroll_view: scroll_view_id,
                        handle: handle_id,
                    });
                    match axis {
                        ScrollAxis::Horizontal => {
                            horizontal_scroll_id = scroll_bar.id().into();
                            horizontal_scroll_handle_id = handle_id.into();
                        }
                        ScrollAxis::Vertical => {
                            vertical_scroll_id = scroll_bar.id().into();
                            vertical_scroll_handle_id = handle_id.into();
                        }
                    }
                }
            });
        });

        scroll_view.insert(ScrollView {
            viewport,
            content_container,
            horizontal_scroll_bar: horizontal_scroll_id,
            horizontal_scroll_bar_handle: horizontal_scroll_handle_id,
            vertical_scroll_bar: vertical_scroll_id,
            vertical_scroll_bar_handle: vertical_scroll_handle_id,
            ..default()
        });

        scroll_view
    }
}

The scroll view is a tricky set, but not unique in the sense that you would normally populate a node deep in a stack of nodes.

Anyway, this change to the lifetimes also meant that EntityCommands now behave more like the ChildBuilder with an entirely useless access to it's commands (for any serious purposes I mean) so I could no longer meaningfully chain things due to the reduced lifetime. It might seem you get access to an owned Commands in it, but in reality it's the same original Commands with a heavily scoped lifetime so Rust will just stop you from using it.

That said, I decided to ditch the extension on EntityCommands entirely but implement my own insert on the builder. This will make it clear that you are in a UI building chain. while still letting you add your own components. Users would still be able to grab the ID and use the commands they already have access to to grab the EntityCommands if need be.

[eidloi]

Oh, and of course UI elements can be both root nodes and children of other entities, hence the need to support Commands and EntityCommands under the umbrella.