Skip to content

Add SystemIdTemplate to allow one-shot system in bsn!#24026

Draft
hxYuki wants to merge 5 commits intobevyengine:mainfrom
hxYuki:system-id-template
Draft

Add SystemIdTemplate to allow one-shot system in bsn!#24026
hxYuki wants to merge 5 commits intobevyengine:mainfrom
hxYuki:system-id-template

Conversation

@hxYuki
Copy link
Copy Markdown
Contributor

@hxYuki hxYuki commented Apr 29, 2026

Objective

Solution

  • Add SystemIdTemplate
  • Implement FromTemplate for SystemId
  • Add a helper function system_value<M> to convert a system to SystemIdTemplate

Testing

  • A ad-hoc test nested in 3d_scene.

Showcase

#[derive(Component, FromTemplate)]
struct Callback(SystemId);

fn callback_scene() -> impl SceneList {
    bsn_list! {
        Callback(system_value(|| {
            println!("Hello from the callback!");
        })),
        Callback(system_value(callback_system)),
    }
}

fn callback_system(){
    println!("Hello from the system!");
}

Co-authored-by: Copilot <copilot@github.com>
@github-actions
Copy link
Copy Markdown
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide, as well as our policy regarding AI usage, and we look forward to reviewing your pull request shortly ✨

@hxYuki
Copy link
Copy Markdown
Contributor Author

hxYuki commented Apr 29, 2026

Not sure where to place these codes. I've searched impl FromTemplate for in repo and got only Entity here.

Comment thread crates/bevy_ecs/src/template.rs Outdated

impl Default for SystemIdTemplate {
fn default() -> Self {
Self::BoxedSystem(Arc::new(Mutex::new(Some(Box::new(IntoSystem::into_system(|| {}))))))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should be the default value for a SystemId?

Comment thread crates/bevy_ecs/src/template.rs Outdated
Comment thread crates/bevy_ecs/src/template.rs Outdated
Comment thread crates/bevy_ecs/src/template.rs Outdated
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events A-Scenes Composing and serializing ECS objects labels Apr 29, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in ECS Apr 29, 2026
@alice-i-cecile alice-i-cecile added D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Apr 29, 2026
Co-authored-by: Copilot <copilot@github.com>
Comment thread crates/bevy_ecs/src/template.rs Outdated
Comment thread crates/bevy_ecs/src/template.rs Outdated
@hxYuki hxYuki requested a review from SkiFire13 May 3, 2026 09:41
Copy link
Copy Markdown
Contributor

@SkiFire13 SkiFire13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this is becoming pretty complex and I'm kinda losing track of how everything fits together.

let system_refs_entity = world.get::<RefToSystem>(entity).unwrap().0;
let system_refs = &world.get::<SystemRefs>(system_refs_entity).unwrap().0;

if system_refs.is_empty() { // a system is no more referenced, so we can clean it up.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is guaranteed to run after relationship has been removed from SystemRefs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relationship connection is maintained in on_discard and this happens in on_remove.

As documented in lifecycle.rs:

An on_discard hook always runs before any on_remove hooks (if the component is being removed from the entity).

Relationship must have already been removed before this hook.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I would document it here though.


/// These are used to track one-shot systems registered from templates, so that they can be cleaned up when the template is no longer in use and to prevent duplicate registrations.
///
/// A template instance will create a linked entity for each holding one-shot system. Each system will create a entity with `SytemIdRecorder` saved in `SceneSystemRegistry`, which counting references to it via `RefToSystem`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI warning looks legit ("Sytem" instead of "System")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yhea, didn't run ci locally yet as drafting.

Comment on lines +500 to +510
#[derive(Resource, Default)]
struct SceneSystemRegistry {
type_map: bevy_platform::collections::HashMap<core::any::TypeId, Entity>,
id_map: bevy_platform::collections::HashMap<SystemId<(), ()>, Entity>,
}

#[derive(Component, Clone)]
struct SystemIdRecorder {
type_id: core::any::TypeId,
system_id: SystemId<(), ()>,
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with the documentation it's not clear to me what these are actually doing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

erDiagram
    TemplateEntity ||--o{ MiddleEntity : "link lifetime"
    MiddleEntity ||--|{ SystemEntity : "reference count"
    SceneSystemRegistry ||--|{ SystemEntity : stores
    TemplateBuild }|..|{ SceneSystemRegistry:"check system existence, create relationship"

    TemplateEntity {
        CallbackComponet systemId
    }
    MiddleEntity {
        LinkLifetimeWith templateEntity
        RefToSystem systemEntity
    }
    SceneSystemRegistry {
        Hashmap system_id_map_to_entity
        Hashmap type_id_map_to_entity
    }
    SystemEntity {
        SystemId systemId
        TypeId systemType
    }
Loading

It simulates a many-to-many relationship between template and system. It should be a little simpler than it looks if we have many-to-many relationship.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can understand MiddleEntity, however SceneSystemRegistry is still unclear.

system_id_map_to_entity looks like it's just the same as SystemId::entity? While type_id_map_to_entity doesn't look correct since there might be different systems with the same type.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may get a BoxedSystem or a SystemId on build_template. I need to connect the two "key" together. And a Relatioship targets to an Entity. So I spawn new entities to save these.

I forgot the SystemId has a field. Maybe I can reuse its entity and get the map eliminated.

there might be different systems with the same type.

What is it exactly? I tried fn callback_1(mut call_counter:Local<u32>) and fn callback_2(mut call_counter:Local<u32>), they have different TypeId. And I copied the assertion in World::register_system_cached to system_value, I assumed the TypeId have same mechanism like type parameter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fn make_system(a: u32) -> impl System {
    bevy_ecs::system::IntoSystem::into_system(move || {
        println!("{a}");
    })
}

make_system(0) and make_system(1) have the same type but are different systems (running them will produce different outputs). In general closures used as systems cause this issue.

register_system_cached has a ZST check precisely to avoid this issue (because ZST systems are always equal).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events A-Scenes Composing and serializing ECS objects C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

Allow registering one-shot system in bsn!

3 participants