diff --git a/src/Engine.Kernel/Scheduling/SystemAccessScope.cs b/src/Engine.Kernel/Scheduling/SystemAccessScope.cs index 6007a27..8fe0e84 100644 --- a/src/Engine.Kernel/Scheduling/SystemAccessScope.cs +++ b/src/Engine.Kernel/Scheduling/SystemAccessScope.cs @@ -27,6 +27,29 @@ internal static class SystemAccessScope return new Restore(previous); } + /// + /// Clears any active scope for the duration of a bulk, whole-world + /// operation — GameWorld.Restore is the one real caller. Restore + /// rebuilds arbitrary component types from a snapshot via + /// GameObject.AddComponent, and it can be invoked from inside a + /// running system (a Play/Stop button click handled during Stage. + /// Render, say) as easily as from unconstrained editor/host code. This + /// class's own doc comment already says editor code and scene + /// construction are unconstrained by design; without this, that's only + /// true when the call happens to originate outside any system's own + /// scope, not because Restore is actually exempt — a real bug, not a + /// hypothetical one: entering then exiting Play mode from the editor's + /// own Stop button throws, because ExitPlay runs inside EditorPlugin's + /// Stage.Render system, which (correctly) never declares Writes<T>() + /// for component types it has no compile-time knowledge of. + /// + public static IDisposable Suspend() + { + var previous = Current.Value; + Current.Value = null; + return new Restore(previous); + } + /// Querying or fetching a component counts as a read — either /// Reads<T>() or Writes<T>() satisfies it. public static void CheckRead(Type componentType) diff --git a/src/Engine.Kernel/World/GameWorld.cs b/src/Engine.Kernel/World/GameWorld.cs index c54058d..bcdeacb 100644 --- a/src/Engine.Kernel/World/GameWorld.cs +++ b/src/Engine.Kernel/World/GameWorld.cs @@ -113,6 +113,12 @@ public sealed class GameWorld : IWorld public void Restore(string snapshot) { + // Suspended for the whole rebuild, not just around AddComponent: + // Restore is a bulk, whole-world reset regardless of which system + // (if any) happened to call it — see SystemAccessScope.Suspend's + // own doc comment for why this is a real, not hypothetical, fix. + using var _ = SystemAccessScope.Suspend(); + foreach (var root in Roots.ToArray()) Destroy(root); diff --git a/tests/Engine.Kernel.Tests/WorldSnapshotTests.cs b/tests/Engine.Kernel.Tests/WorldSnapshotTests.cs index c14dbd3..411de14 100644 --- a/tests/Engine.Kernel.Tests/WorldSnapshotTests.cs +++ b/tests/Engine.Kernel.Tests/WorldSnapshotTests.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using Engine.Kernel.Scheduling; using Engine.Kernel.World; namespace Engine.Kernel.Tests; @@ -103,4 +104,30 @@ public class WorldSnapshotTests stopwatch.ElapsedMilliseconds < 100, $"Snapshot + Restore of 300 GameObjects took {stopwatch.ElapsedMilliseconds} ms, expected < 100 ms."); } + + // A real bug, caught by hand: entering then exiting Play mode via + // engine.editor's own Stop button threw InvalidOperationException, + // because ExitPlay runs Restore from inside EditorPlugin's Stage. + // Render system — which correctly declares no Reads/Writes at all, it + // has no compile-time knowledge of QuadRenderer or any other game's + // component types — and Restore rebuilds those very types via + // AddComponent. Every other test in this file calls Restore directly, + // outside any system's scope, which is exactly why none of them caught + // this. This one reproduces the real call shape: a system with no + // declared access, calling Restore, run through the real Schedule. + [Fact] + public void Restore_Called_From_A_System_With_No_Declared_Access_Does_Not_Throw() + { + var world = new GameWorld(); + var go = world.CreateGameObject("Hero"); + go.AddComponent().Value = 100; + var snapshot = world.Snapshot(); + + var schedule = new Schedule(); + schedule.Add(Stage.Update, (IWorld w) => w.Restore(snapshot)); // no Reads/Writes — like EditorPlugin's DrawUi + + var exception = Record.Exception(() => schedule.RunStage(Stage.Update, world)); + + Assert.Null(exception); + } }