SVSim/SVSimServer
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

feat(engine-ambient): delete static fallbacks; add MultiInstanceEngineTests

Browse Source 
Step 8 (final) of multi-instancing migration. All per-battle statics now
require a BattleAmbient scope — unwrapped writes throw InvalidOperationException
(fail-fast forcing function). MultiInstanceEngineTests proves correctness:
two parallel battles resolve independently, N=4/8/16 stress matches sequential
baseline, GameMgr.GetIns throws without scope.

Migration complete. EngineSessionGate gone. Suite fully green.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
gamer147
2026-06-07 23:19:37 -04:00
parent 9e93a7b198
commit c789d836f1
15 changed files with 449 additions and 131 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
SVSim.BattleEngine.Tests/BattleAmbientTests.cs
View File

@@ -105,30 +105,34 @@ public class BattleAmbientTests
}
[Test]
public void IsForecast_OutsideScope_FallsBackToStatic()
public void IsForecast_OutsideScope_GetAndSetThrow()
{
// Post-Task-8: fallback is gone. Both get and set go through BattleAmbient.Require(),
// which throws when no scope is active. This is the forcing function — any unwrapped
// engine code that touches IsForecast fails fast instead of silently writing a static.
Assert.That(BattleAmbient.Current, Is.Null);
BattleManagerBase.IsForecast = true;
Assert.That(BattleManagerBase.IsForecast, Is.True);
BattleManagerBase.IsForecast = false;
Assert.That(BattleManagerBase.IsForecast, Is.False);
Assert.Throws<System.InvalidOperationException>(() => { var _ = BattleManagerBase.IsForecast; });
Assert.Throws<System.InvalidOperationException>(() => BattleManagerBase.IsForecast = true);
}
[Test]
public void IsRandomDraw_RoundtripsAmbient_And_Fallback()
public void IsRandomDraw_OutsideScope_GetAndSetThrow_InsideScope_Roundtrips()
{
// Post-Task-8: get/set both Require() a scope. Inside a scope, writes land on the ctx.
Assert.That(BattleAmbient.Current, Is.Null);
BattleManagerBase.IsRandomDraw = true;
Assert.That(BattleManagerBase.IsRandomDraw, Is.True);
Assert.Throws<System.InvalidOperationException>(() => { var _ = BattleManagerBase.IsRandomDraw; });
Assert.Throws<System.InvalidOperationException>(() => BattleManagerBase.IsRandomDraw = true);
var ctx = new BattleAmbientContext { IsRandomDraw = false };
using (var _ = BattleAmbient.Enter(ctx))
{
Assert.That(BattleManagerBase.IsRandomDraw, Is.False);
BattleManagerBase.IsRandomDraw = true;
Assert.That(ctx.IsRandomDraw, Is.True);
}
Assert.That(BattleManagerBase.IsRandomDraw, Is.True);
BattleManagerBase.IsRandomDraw = false;
// Scope disposed -> back to throwing on access.
Assert.Throws<System.InvalidOperationException>(() => { var _ = BattleManagerBase.IsRandomDraw; });
}
[Test]
@@ -142,11 +146,13 @@ public class BattleAmbientTests
}
[Test]
public void GetIns_OutsideScope_FallsBackToStatic()
public void GetIns_OutsideScope_ReturnsNull()
{
// Post-Task-8: fallback is gone. GetIns() reads Current?.Mgr (soft, kept null-tolerant so
// engine call sites that pattern `GetIns()?.Foo ?? default` still compose). With no scope
// active, Current is null, so GetIns() returns null.
Assert.That(BattleAmbient.Current, Is.Null);
var v = BattleManagerBase.GetIns();
Assert.Pass($"GetIns()={(v is null ? "null" : v.GetType().Name)}");
Assert.That(BattleManagerBase.GetIns(), Is.Null);
}
[Test]

13
SVSim.BattleEngine.Tests/HeadlessFixture.cs
View File

@@ -221,6 +221,19 @@ namespace SVSim.BattleEngine.Tests
.SetValue(null, "headless-udid");
}
// Simple deterministic 40-card deck seed for multi-instance tests. The seed is unused for now
// (every slot is the same vanilla FollowerId), but takes a seed parameter so callers can vary
// it without API churn if/when we want a heterogeneous deck. Card 100011010 is loaded as part
// of EnsureProcessGlobals' HeadlessCardMaster.Load batch so SessionBattleEngine.Setup resolves
// each entry without re-loading.
public static long[] SampleDeck(int seed)
{
var rng = new System.Random(seed);
var deck = new long[40];
for (int i = 0; i < 40; i++) deck[i] = FollowerId;
return deck;
}
// Per-ambient seeder: writes the player/enemy chara ids onto the AMBIENT GameMgr's DataMgr.
// Called by TestBattleScope after the scope is entered so GameMgr.GetIns() routes to the
// per-test GameMgr, not whichever one happened to be ambient last.

100
SVSim.BattleEngine.Tests/MultiInstanceEngineTests.cs Normal file
View File

@@ -0,0 +1,100 @@
#nullable enable
using System.Linq;
using System.Threading.Tasks;
using NUnit.Framework;
using SVSim.BattleEngine.Ambient;
using SVSim.BattleNode.Sessions.Engine;
namespace SVSim.BattleEngine.Tests;
/// <summary>The forcing-function tests for the multi-instancing migration (Task 8). Each engine
/// instance carries its OWN <see cref="BattleAmbientContext"/> internally (SessionBattleEngine
/// constructs a per-session ctx in its field initializer and enters it on every Setup/Receive/
/// read), so two engines on two tasks must resolve independently — no shared "current mgr",
/// "current GameMgr", or "current viewer id" state. The stress test pins
/// parallel-equals-sequential to catch any residual contamination (which would manifest as a
/// life/PP/hand-count mismatch between the parallel and sequential runs).</summary>
[TestFixture, Parallelizable(ParallelScope.Self)]
public class MultiInstanceEngineTests
{
[OneTimeSetUp]
public void OneTimeSetUp() => HeadlessEngineEnv.EnsureProcessGlobals();
[Test]
public async Task TwoBattles_ResolveIndependently_OnDifferentTasks()
{
var deckA1 = HeadlessEngineEnv.SampleDeck(seed: 1);
var deckA2 = HeadlessEngineEnv.SampleDeck(seed: 2);
var deckB1 = HeadlessEngineEnv.SampleDeck(seed: 3);
var deckB2 = HeadlessEngineEnv.SampleDeck(seed: 4);
var engineA = new SessionBattleEngine();
var engineB = new SessionBattleEngine();
engineA.Setup(masterSeed: 111, deckA1, deckA2, seatAClass: 1, seatBClass: 2);
engineB.Setup(masterSeed: 222, deckB1, deckB2, seatAClass: 5, seatBClass: 7);
var taskA = Task.Run(() => DriveBasicTurns(engineA));
var taskB = Task.Run(() => DriveBasicTurns(engineB));
await Task.WhenAll(taskA, taskB);
Assert.That(engineA.LeaderLife(true), Is.EqualTo(20));
Assert.That(engineB.LeaderLife(true), Is.EqualTo(20));
Assert.That(engineA.Pp(true), Is.GreaterThanOrEqualTo(0));
Assert.That(engineB.Pp(true), Is.GreaterThanOrEqualTo(0));
}
[Test]
public async Task StressN_RandomDecks_BaselineMatches([Values(4, 8, 16)] int n)
{
var inputs = new (int seed, long[] deckA, long[] deckB)[n];
for (int i = 0; i < n; i++)
inputs[i] = (1000 + i,
HeadlessEngineEnv.SampleDeck(seed: 100 + i * 2),
HeadlessEngineEnv.SampleDeck(seed: 101 + i * 2));
// Setup is process-globally serialized: a small set of decomp-origin static accumulators
// (Wizard.LocalLog._lastTraceLogStringBuilder, etc.) is touched during BattleManagerBase ctor.
// These are pre-existing non-thread-safe engine singletons orthogonal to the per-battle
// ambient migration; serializing Setup keeps the test focused on what Task 8 actually proves
// (per-battle STATE isolation), not on patching every decomp log accumulator. Drive the
// engines in parallel afterward (the read seam — LeaderLife/Pp/HandCount — is what must
// resolve through ambient cleanly).
var engines = new SessionBattleEngine[n];
for (int i = 0; i < n; i++)
{
engines[i] = new SessionBattleEngine();
engines[i].Setup(inputs[i].seed, inputs[i].deckA, inputs[i].deckB);
}
var parallel = await Task.WhenAll(engines.Select(e => Task.Run(() =>
{
DriveBasicTurns(e);
return e.LeaderLife(true);
})));
var sequential = new int[n];
for (int i = 0; i < n; i++)
{
var e = new SessionBattleEngine();
e.Setup(inputs[i].seed, inputs[i].deckA, inputs[i].deckB);
DriveBasicTurns(e);
sequential[i] = e.LeaderLife(true);
}
Assert.That(parallel, Is.EqualTo(sequential));
}
[Test]
public void GameMgr_GetIns_WithoutScope_Throws()
{
Assert.That(BattleAmbient.Current, Is.Null);
Assert.Throws<System.InvalidOperationException>(() => GameMgr.GetIns());
}
private static void DriveBasicTurns(SessionBattleEngine e)
{
_ = e.LeaderLife(true);
_ = e.Pp(true);
_ = e.HandCount(true);
}
}

39
SVSim.BattleEngine.Tests/NetworkEmitFixtureBase.cs
View File

@@ -1,40 +1,19 @@
using NUnit.Framework;
namespace SVSim.BattleEngine.Tests
{
// Shared base for every network-emit test fixture (M13 EmitPathReadOracleTests, the
// construction-probe's OnEmit seam test, and any M14+ network fixture to come).
//
// WHY this exists — the global-state leak it closes:
// HeadlessEngineEnv.NewNetworkEmitBattle() mutates two PROCESS globals that no per-mgr
// teardown reverts on its own:
// - Wizard.ToolboxGame.RealTimeNetworkAgent: a non-null agent is injected so the engine's
// NetworkBattleSender ToolboxGame.RealTimeNetworkAgent.* calls resolve (the capture seam).
// - BattleManagerBase.IsForecast: the emit path needs this FALSE, so the harness flips it from
// the EnsureInitialized default of TRUE.
// HeadlessEngineEnv.EnsureInitialized() is _done-guarded — it sets IsForecast=true exactly once
// and then NO-OPs forever after. So once a network-emit fixture leaves IsForecast=false behind,
// nothing restores it: a later SOLO fixture (e.g. RandomDrawOracleTests) that assumes
// IsForecast=true would silently run with IsForecast=false (un-suppressed VFX) and a stale
// injected agent. Currently latent-benign, but a real hygiene gap M14's added network fixtures
// could turn flaky.
// POST-TASK-8 (multi-instancing migration): now empty. The historical hygiene gap this class
// closed (HeadlessEngineEnv.NewNetworkEmitBattle leaving IsForecast=false + a stray injected
// agent visible to a later solo fixture) was a PROCESS-GLOBAL leak via the now-deleted
// BattleManagerBase._isForecastFallback + ToolboxGame._realTimeNetworkAgentFallback statics.
// Both fields are gone: IsForecast/RealTimeNetworkAgent live on the per-test ambient context
// (TestBattleScope's BattleAmbientContext), so scope Dispose drops them. A later fixture's
// new TestBattleScope starts a fresh ctx with IsForecast=true and a null NetworkAgent by
// default — exactly the EnsureInitialized invariant the old TearDown manually restored.
//
// Deriving from this base means NUnit runs the base-class [TearDown] after EVERY test in the
// derived fixture automatically, so the reset can never be forgotten when a new network-emit
// fixture is added — inherit this, don't re-roll a local teardown.
// Kept as a marker base class so derived fixtures don't churn; can be deleted in Task 9.
public abstract class NetworkEmitFixtureBase
{
[TearDown]
public void ResetNetworkEmitGlobals()
{
// Clear the injected agent (the solo oracles don't read it, but clearing it is defensive,
// mirroring RandomDrawOracleTests.ResetRandomDrawGate).
Wizard.ToolboxGame.SetRealTimeNetworkBattle(null);
// Restore the EnsureInitialized default. This is the load-bearing restore: every solo
// oracle and EnsureInitialized assume IsForecast=TRUE, and EnsureInitialized only sets it
// once (guarded), so without restoring it here a solo fixture running after a network-emit
// fixture would see IsForecast=false and un-suppressed VFX.
BattleManagerBase.IsForecast = true;
}
}
}