One shot system cannot recursively call itself

[Maxime-Verrier]

Bevy version

0.15.3

What you did

I was creating a effect system that run one shot system, and can take effect as parameters to run them from that one shot system (which mean i can call the same one shot system from the same system, but it's not rly recursive)

What went wrong

I discovered that you cannot call X one shot system from the same system

Isolated minimal example

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

fn main() {
    let mut app = App::new();
    let id = app.register_system(one_shot_system);

    app.add_plugins(DefaultPlugins);
    
    app.insert_resource(TestRes {
        id: id,
        counter: 5
    });

    app.add_systems(Startup, test_startup);
    app.run();
}

pub fn test_startup(mut commands: Commands, test: Res<TestRes>) {
    commands.run_system(test.id);
}

#[derive(Resource)]
pub struct TestRes {
    id: SystemId,
    counter: u32
}

pub fn one_shot_system(mut commands: Commands, mut test: ResMut<TestRes>) {
    println!("{:?}", test.counter);
    test.counter = test.counter-1;

    if test.counter > 0 {
        commands.run_system(test.id);
    }
}

This only call one_shot_sytem once and the commands.run_system inside it do nothing.

Of course this is not rly my use case but it's the same bug in the end.

Apparently i can just go with observers since i was told they can recursivly call themself, which i tried to avoid since in my use case it's mean 500 entities, one for each effect), but in the end it's probably nothing and have 0 perf issue.

But i wanted to know if this was intended or not cause it's documented nowhere (or i'm blind, it's a posibility i just have reading issues).

I think this happens cause the system may be removed from the world while it's executed

[SpecificProtagonist]

I think this happens cause the system may be removed from the world while it's executed

Yup. The problem is that applying a command queue requires exclusive access to it (both because running commands consumes them and to clear the queue), and a system's command queue is stored in the system itself.

To fix this, I think need to move the command queue out of the system and then double buffer them. Iirc flecs uses a thread-local queue?

Apparently i can just go with observers since i was told they can recursivly call themself, which i tried to avoid since in my use case it's mean 500 entities, one for each effect), but in the end it's probably nothing and have 0 perf issue.

Should be fine, as entities shouldn't hurt performance just by existing.

[grind086]

There's a note about this on World::run_system.

After some testing it looks like you can make it work with a custom System implementation that unconditionally uses System::queue_deferred instead of System::apply_deferred. This may have other consequences, so I wouldn't recommend it.

use std::borrow::Cow;

use bevy::{
    ecs::{
        archetype::ArchetypeComponentId,
        component::{ComponentId, Tick},
        query::Access,
        schedule::InternedSystemSet,
        system::{FunctionSystem, IntoSystem, ReadOnlySystemParam, SystemId},
        world::{DeferredWorld, unsafe_world_cell::UnsafeWorldCell},
    },
    prelude::*,
};

fn main() {
    let mut app = App::new();
    let id = app.register_system(QueueFunctionSystem {
        inner: IntoSystem::into_system(one_shot_system),
    });

    app.add_plugins(DefaultPlugins);

    app.insert_resource(TestRes { id: id, counter: 5 });

    app.add_systems(Startup, test_startup);
    app.run();
}

pub fn test_startup(mut commands: Commands, test: Res<TestRes>) {
    commands.run_system(test.id);
}

#[derive(Resource)]
pub struct TestRes {
    id: SystemId,
    counter: u32,
}

pub fn one_shot_system(mut commands: Commands, mut test: ResMut<TestRes>) {
    println!("{:?}", test.counter);
    test.counter = test.counter - 1;

    if test.counter > 0 {
        let id = test.id;
        commands.queue(move |world: &mut World| {
            world.commands().run_system(id);
        });
    }
}

struct QueueFunctionSystem<Marker, F: SystemParamFunction<Marker>> {
    inner: FunctionSystem<Marker, F>,
}

impl<Marker, F> System for QueueFunctionSystem<Marker, F>
where
    Marker: 'static,
    F: SystemParamFunction<Marker>,
{
    type In = F::In;
    type Out = F::Out;

    #[inline]
    fn name(&self) -> Cow<'static, str> {
        self.inner.name()
    }

    #[inline]
    fn component_access(&self) -> &Access<ComponentId> {
        self.inner.component_access()
    }

    #[inline]
    fn archetype_component_access(&self) -> &Access<ArchetypeComponentId> {
        self.inner.archetype_component_access()
    }

    #[inline]
    fn is_send(&self) -> bool {
        self.inner.is_send()
    }

    #[inline]
    fn is_exclusive(&self) -> bool {
        self.inner.is_send()
    }

    #[inline]
    fn has_deferred(&self) -> bool {
        self.inner.has_deferred()
    }

    #[inline]
    unsafe fn run_unsafe(
        &mut self,
        input: SystemIn<'_, Self>,
        world: UnsafeWorldCell,
    ) -> Self::Out {
        unsafe { self.inner.run_unsafe(input, world) }
    }

    #[inline]
    fn apply_deferred(&mut self, world: &mut World) {
        // Queue instead of applying
        self.inner.queue_deferred(world.into());
    }

    #[inline]
    fn queue_deferred(&mut self, world: DeferredWorld) {
        self.inner.queue_deferred(world);
    }

    #[inline]
    unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool {
        unsafe { self.inner.validate_param_unsafe(world) }
    }

    #[inline]
    fn initialize(&mut self, world: &mut World) {
        self.inner.initialize(world);
    }

    fn update_archetype_component_access(&mut self, world: UnsafeWorldCell) {
        self.inner.update_archetype_component_access(world);
    }

    #[inline]
    fn check_change_tick(&mut self, change_tick: Tick) {
        self.inner.check_change_tick(change_tick);
    }

    fn default_system_sets(&self) -> Vec<InternedSystemSet> {
        self.inner.default_system_sets()
    }

    fn get_last_run(&self) -> Tick {
        self.inner.get_last_run()
    }

    fn set_last_run(&mut self, last_run: Tick) {
        self.inner.set_last_run(last_run);
    }
}

unsafe impl<Marker, F> ReadOnlySystem for QueueFunctionSystem<Marker, F>
where
    Marker: 'static,
    F: SystemParamFunction<Marker>,
    F::Param: ReadOnlySystemParam,
{
}

[Maxime-Verrier]

There's a note about this on World::run_system.

After some testing it looks like you can make it work with a custom System implementation that unconditionally uses System::queue_deferred instead of System::apply_deferred. This may have other consequences, so I wouldn't recommend it.

Guess i have to use Observer for anything that can call itself, haven't tested them yet so i hope they support recursive.

Entities have 0 cost if they do nothing right ? (outside the few bits of memory), i can have as many observers i want ?

Also i'm not sure if i should close this issue. Not rly used to the github ecosystem to know.

[alice-i-cecile]

Definitely a bug! Please leave this open.