SystemId scene templating

[ItsDoot]

Objective

• Simplified alternative to Arcify one-shot systems to enable SystemId templating #24072

I have a bunch of Bundle-style code that I want to replace with the new bsn! Scene-style macro:

pub fn rest_ui(/* system params */) {
    // my ui system...
}

// From
pub fn rest(mut commands: Commands) -> impl Bundle {
    (
        Rest,
        Name::new("Rest"),
        Activity {
            render: commands.register_system(rest_ui),
        },
    )
}

// To (ideally)
pub fn rest() -> impl Scene {
    bsn! {
        Rest
        Name("Rest")
        Activity {
            render: rest_ui
        }
    }
}

Solution

This solution is more inspired by how HandleTemplate works.

1. Added SystemIdTemplate; it stores either a SystemId or a Arc<Mutex<Either<SystemId, Box<dyn System>>>>
2. Added a system_value function for wrapping system functions (see Future Work for potentially removing the need)

Testing

• Added a unit test for SystemIdTemplate.
• Added to the callbacks example demonstrating how to spawn SystemIds via BSN scenes.

Future work

• I believe we can remove the need for wrapping with system_value by introducing SuperFrom/SuperInto traits a la Dioxus and using it in the bsn! macros in-place of the implicit .into()s.

---

Showcase

You can now spawn components containing SystemIds via bsn! macros:

#[derive(Component, FromTemplate)]
struct Callback {
    system_id: SystemId<(), ()>,
}

fn my_scene() -> impl Scene {
    bsn! {
        Callback {
            system_id: system_value(|| {
                println!("This is a callback spawned via a scene.");
            })
        }
    }
}

[laundmo]

Glad to see my idea from #24072 (comment) had some merit! I won't approve yet, since i've only looked through it on my phone, but it already seems far simpler.

[SkiFire13]

This was proposed basically 1:1 in #24026 (this was later changed though). The issue is that it's unclear who owns these systems, that is who is responsible for unregistering them once they are no longer needed. Given that recreating the template will spawn the system again this basically becomes a memory leak.

[laundmo]

This was proposed basically 1:1 in #24026 (this was later changed though). The issue is that it's unclear who owns these systems, that is who is responsible for unregistering them once they are no longer needed. Given that recreating the template will spawn the system again this basically becomes a memory leak.

Hm, are you sure this is the case even tho in build_template it only registers the system the first time its called, switching over to storing the SystemId after the first call?

[SkiFire13]

Hm, are you sure this is the case even tho in build_template it only registers the system the first time its called, switching over to storing the SystemId after the first call?

If you recreate the template (e.g. you call my_scene() again) then you will create a new instance of the system. And since the system is not scoped to the scene once the scene is despawned the system entity will be leaked.

[chescock]

The issue is that it's unclear who owns these systems, that is who is responsible for unregistering them once they are no longer needed. Given that recreating the template will spawn the system again this basically becomes a memory leak.

Would it work to use register_system_cached instead of register_system here?

Then it's not a leak because we could always use the SystemId again, even by recreating the scene. It also avoids some edge cases if you try to use a scene with multiple worlds. And you could probably require the IntoSystem to be Clone and avoid needing Arcs, since ZST systems are usually Clone anyway.

If the systems need to capture values, then maybe those values could be stored as components on the parent entity instead of in the system? The system could take In<Entity> and then have a Query to look up the values. I don't know enough about the actual use cases to know how awkward that transformation would be.

[SkiFire13]

Would it work to use register_system_cached instead of register_system here?

You cannot use register_system_cached with a BoxedSystem<I, O>. You need to have a concrete system type that is also a ZST. I don't see an easy way to do that here.

[chescock]

You cannot use register_system_cached with a BoxedSystem<I, O>. You need to have a concrete system type that is also a ZST. I don't see an easy way to do that here.

Right, you'd have to wrap the register call. It would be approximately Box::new(move |world| world.register_system_cached(s)). Except I think Box<dyn Fn> can't be Clone, so you'd need something like Box<dyn Template> with a separate template type.

[ItsDoot]

Hm, are you sure this is the case even tho in build_template it only registers the system the first time its called, switching over to storing the SystemId after the first call?

If you recreate the template (e.g. you call my_scene() again) then you will create a new instance of the system. And since the system is not scoped to the scene once the scene is despawned the system entity will be leaked.

I've come up with a bevy_asset::Handle style solution to this problem in #24114, dubbed SystemHandles.

[ItsDoot]

Was going to just do a git merge but it turned into a headache so I went with a clean rebase on top of current main.