Make SystemIds easier to create and manage using SystemHandles

[asafigan]

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

Currently it isn't simple to create and manage SystemIds. For them to be widely used, there needs to be a simple way to work with them.

Reasons they are hard to create:

• &mut World is required to create them.
• Can't inline register_system in spawn arguments because both take &mut World.

Reasons they are hard to manage:

• It is hard to know when to remove the system.
  • They could be used by many Resources or Components

What solution would you like?

Manage SystemId lifetimes using a new type. I propose SystemHandle. It would act like an asset Handle where it keeps track of references and removes the system when it is no longer being used.

Also an easy way to create SystemHandles. I would suggest a new type that acts like one of AssetServer, Assets, or Commands.

In the future SystemHandle could be used by scenes to reference systems. Maybe the TypeId of the system could be used in a SystemPath. This would require the systems to be registered ahead of time. This can be done in Plugins like so: app.world.register_system(system).

What alternative(s) have you considered?

We could add a methods to Commands and ChildBuilder for creating handles instead of creating a new type to create SystemHandles. This has the drawback that you can't inline creating a SystemHandle in the arguments of spawn.

Is it possible to use the asset system for this and just use Handle<SystemId>? Would this be a good idea even if it were possible?

Additional context

I quickly wrote a proof of concept plugin:

Plugin

use crossbeam_channel::{Receiver, Sender};
use std::sync::{Arc, OnceLock};

use bevy::{
    ecs::system::{CommandQueue, SystemId, SystemParam},
    prelude::*,
};

pub struct OneShotSystemsPlugin;

impl Plugin for OneShotSystemsPlugin {
    fn build(&self, app: &mut App) {
        let (drop_sender, drop_receiver) = crossbeam_channel::unbounded();
        app.insert_resource(SystemHandleProvider {
            drop_sender,
            drop_receiver,
        })
        .add_systems(Last, clean_up_system_ids);
    }
}

#[derive(SystemParam)]
pub struct Systems<'w, 's> {
    queue: Deferred<'s, CommandQueue>,
    systems_server: Res<'w, SystemHandleProvider>,
}

#[derive(Resource)]
struct SystemHandleProvider {
    drop_sender: Sender<DropEvent>,
    drop_receiver: Receiver<DropEvent>,
}

impl<'w, 's> Systems<'w, 's> {
    pub fn add<M, S: IntoSystem<(), (), M> + Send + 'static>(&mut self, system: S) -> SystemHandle {
        let handle = SystemHandle::new(self.systems_server.drop_sender.clone());
        let clone = handle.inner.clone();
        self.queue.push(move |world: &mut World| {
            clone.id.set(world.register_system(system)).unwrap();
        });

        handle
    }
}

#[derive(Clone)]
pub struct SystemHandle {
    inner: StrongHandle,
}

impl SystemHandle {
    fn new(drop_sender: Sender<DropEvent>) -> Self {
        SystemHandle {
            inner: StrongHandle {
                id: default(),
                drop_sender,
            },
        }
    }
}

#[derive(Clone)]
struct StrongHandle {
    id: Arc<OnceLock<SystemId>>,
    drop_sender: Sender<DropEvent>,
}

impl Drop for StrongHandle {
    fn drop(&mut self) {
        let _ = self.drop_sender.send(DropEvent {
            id: self.id.clone(),
        });
    }
}

struct DropEvent {
    id: Arc<OnceLock<SystemId>>,
}

impl SystemHandle {
    pub fn id(&self) -> Option<SystemId> {
        self.inner.id.get().cloned()
    }
}

fn clean_up_system_ids(world: &mut World) {
    let provider = world.resource::<SystemHandleProvider>();
    let ids: Vec<_> = provider
        .drop_receiver
        .try_iter()
        .filter_map(|e| Arc::into_inner(e.id))
        .filter_map(|id| id.into_inner())
        .collect();

    for id in ids {
        world.remove_system(id).unwrap();
    }
}

Here is an example of how I use it to spawn a button:

Example usage

// system for setting up a main menu
fn setup_main_menu(mut commands: Commands, mut systems: Systems, asset_server: Res<AssetServer>) {
    commands
        .spawn(
           // center button
            NodeBundle {
                style: Style {
                    width: Val::Percent(100.0),
                    height: Val::Percent(100.0),
                    justify_content: JustifyContent::Center,
                    align_items: AlignItems::Center,
                    ..default()
                },
                ..default()
            }
        )
        .with_children(|builder| {
            spawn_button(
                builder,
                "Start",
                // use system the transitions to a new state
                systems.add(|mut next_state: ResMut<NextState<GameStates>>| {
                    next_state.set(GameStates::InGame);
                }),
            );
        });
}

// helper function for spawning a button
fn spawn_button(
    commands: &mut ChildBuilder,
    text: impl Into<String>,
    on_press_down: SystemHandle,
) {
    commands
        .spawn((
            ButtonBundle {
                style: Style {
                    align_items: AlignItems::Center,
                    justify_items: JustifyItems::Center,
                    align_content: AlignContent::Center,
                    justify_content: JustifyContent::Center,
                    padding: UiRect::all(Val::Px(10.0)),
                    ..default()
                },
                background_color: Color::BLACK,
                ..default()
            },
            OnPressDown(on_press_down),
        ))
        .with_children(|builder| {
            builder.spawn(TextBundle {
                text: Text {
                    sections: vec![TextSection::new(
                        text,
                        TextStyle {
                            font_size: 20.0,
                            color: Color::WHITE,
                            ..default()
                        },
                    )],
                    alignment: TextAlignment::Center,
                    ..default()
                },
                ..default()
            });
        });
}

#[derive(Event)]
pub struct PressDown {
    pub entity: Entity,
}

// system to send event when button is pressed
fn ui_press_down(
    interactions: Query<(Entity, &Interaction), Changed<Interaction>>,
    mut events: EventWriter<PressDown>,
) {
    events.send_batch(
        interactions
            .iter()
            .filter(|(_, x)| **x == Interaction::Pressed)
            .map(|(entity, _)| PressDown { entity }),
    );
}

#[derive(Component)]
pub struct OnPressDown(pub SystemHandle);

// system to trigger systems when `PressDown` events are fired
fn handle_on_press_down(
    mut events: EventReader<PressDown>,
    on_pressed: Query<&OnPressDown>,
    mut commands: Commands,
) {
    for on_pressed in on_pressed.iter_many(events.read().map(|x| x.entity)) {
        if let Some(id) = on_pressed.0.id() {
            commands.run_system(id);
        }
    }
}

[alice-i-cecile]

Related to #10382.

[asafigan]

Probably should favor not using SystemIds at all. For further information see my comment in #10582.