[IN-33941] OnStopRunning is not called if SystemGroup is disabled

[plucky sparrow]

The extreme repo

[UpdateInGroup(typeof(InitializationSystemGroup))]
public partial class TestToggleSystem : SystemBase
{
    private SimulationSystemGroup simulationSystemGroup;
    protected override void OnCreate() => simulationSystemGroup = World.GetExistingSystemManaged<SimulationSystemGroup>();
    protected override void OnUpdate() => simulationSystemGroup.Enabled = !simulationSystemGroup.Enabled;
}
public partial struct TestSystem : ISystem, ISystemStartStop
{
    public void OnStartRunning(ref SystemState state) => Debug.Log("Start");
    public void OnStopRunning(ref SystemState state) => Debug.Log("Stop");
}

And results in only OnStartRunning being spammed over and over
Reported as IN-33941

Expected result:
Start Stop alternate

Actual result:
Start spams

Toggling system group enable causes imbalance in OnStartRunning/OnStopRunning

[IN-33941] Toggling system group enable causes imbalance in OnStartRunning/OnStopRunning

[plucky sparrow]

hang on, this is much worse than i expected. It's not related to toggling at all.

OnStopRunning is just never called if the SystemGroup is disabled now

[IN-33941] OnStopRunning is not called if SystemGroup is disabled

[bold basalt]

@obsidian flume Does this fall under Kernels jurisdiction? :3

[plucky sparrow]

I'm pretty confident this worked in pre.15
I'm about to test if it worked in pre.44

As expected it doesn't work on pre.44 (change set was too small)

OK

it's only failing with ISystem

it works as expected with SystemBase. i don't know if i've ever tested with ISystem before.

public partial class TestSystem2 : SystemBase
{
    protected override void OnStartRunning() => Debug.Log("Start2");
    protected override void OnStopRunning() => Debug.Log("Stop2");```

umm yeah

        internal override void OnStopRunningInternal()
        {
            OnStopRunning();

            foreach (var sys in m_managedSystemsToUpdate)
            {
                if (sys == null)
                    continue;

                if (sys.m_StatePtr == null)
                    continue;

                if (!sys.m_StatePtr->PreviouslyEnabled)
                    continue;

                sys.m_StatePtr->PreviouslyEnabled = false;
                sys.OnStopRunningInternal();
            }

            for (int i = 0; i < m_UnmanagedSystemsToUpdate.Length; ++i)
            {
                var sys = World.Unmanaged.ResolveSystemState(m_UnmanagedSystemsToUpdate[i]);

                if (sys == null || !sys->PreviouslyEnabled)
                    continue;

                sys->PreviouslyEnabled = false;

                // Optional callback here
            }
        }```

it's just never been implemented for unmanaged systems 😐

not optional 😄

all this seems to need is

            for (int i = 0; i < m_UnmanagedSystemsToUpdate.Length; ++i)
            {
                var sys = World.Unmanaged.ResolveSystemState(m_UnmanagedSystemsToUpdate[i]);

                if (sys == null || !sys->PreviouslyEnabled)
                    continue;

                sys->PreviouslyEnabled = false;
                SystemBaseRegistry.CallOnStopRunning(sys);
            }```

let's find out...!

yep fixed

just needs that 1 line of code

well this explains a lot of random issues i've seen

I'll add it to my tab 😄

[static anvil]

well that's a frickin whoops. thanks for the report!

[plucky sparrow]

this wasn't fix 😦

[static anvil]

@obsidian flume iirc landed it in the upcoming version?