diff --git a/SVSim.BattleNode/Sessions/BattleSession.cs b/SVSim.BattleNode/Sessions/BattleSession.cs index 0eee1b9..2042c6c 100644 --- a/SVSim.BattleNode/Sessions/BattleSession.cs +++ b/SVSim.BattleNode/Sessions/BattleSession.cs @@ -52,11 +52,6 @@ public sealed class BattleSession /// was already processed during the Swap feed. private bool _engineReadyFed; - /// True once this session has acquired the process-wide - /// (and is therefore the single active engine owner). Drives the matching Release at battle - /// end so the next session can take the engine. - private bool _engineOwned; - /// Serializes dispatch. Both participants' read loops raise FrameEmitted on their own /// threads, and a dispatch ( + the relay PushAsync calls) mutates /// shared, non-thread-safe state — the dictionaries and each @@ -191,24 +186,18 @@ public sealed class BattleSession if (A is RealParticipant rpA) rpA.Outbound.Clear(); if (B is RealParticipant rpB) rpB.Outbound.Clear(); - try - { - await Task.WhenAll( - A.TerminateAsync(BattleFinishReason.NormalFinish), - B.TerminateAsync(BattleFinishReason.NormalFinish)) - .ConfigureAwait(false); + // Per-session BattleAmbientContext on the engine isolates per-battle state across concurrent + // sessions (Task 7 of multi-instancing migration), so the historical single-active-engine gate + // (and its matching try/finally Release) is gone — engine setup is unconditional per session, + // and there is no teardown obligation that must run on a throw from the participant tear-down. + await Task.WhenAll( + A.TerminateAsync(BattleFinishReason.NormalFinish), + B.TerminateAsync(BattleFinishReason.NormalFinish)) + .ConfigureAwait(false); - await A.DisposeAsync().ConfigureAwait(false); - await B.DisposeAsync().ConfigureAwait(false); - _dispatchGate.Dispose(); - } - finally - { - // Release the single-active-engine gate exactly once, in a finally so a throw from the - // terminate/dispose teardown above can never leak it to the next session (the gate is a - // process-global static shared across all sessions, incl. across tests in one process). - if (_engineOwned) Engine.EngineSessionGate.Release(); - } + await A.DisposeAsync().ConfigureAwait(false); + await B.DisposeAsync().ConfigureAwait(false); + _dispatchGate.Dispose(); } private Task OnFrameFromA(MsgEnvelope env, CancellationToken ct) => HandleFrameAsync(A, env, ct); @@ -349,18 +338,13 @@ public sealed class BattleSession if (_engineSetupAttempted) return; _engineSetupAttempted = true; - // Single-active-engine gate: the engine's process-global turn state can't back two concurrent - // battles, so only one session may own it (carried-risk B). On failure we DON'T set the engine - // up — it stays not-ready and ShadowIngest no-ops on !IsReady — and log the limitation loudly - // (not a silent fallback). Per-session isolation (dropping the gate) is the tracked follow-up. - if (!Engine.EngineSessionGate.TryAcquire()) - { - _log.LogWarning("BattleSession {Bid}: another battle owns the engine; this battle runs " + - "WITHOUT engine-sourced fields (single-active-engine limitation — per-session isolation pending)", - BattleId); - return; - } - _engineOwned = true; + // Per-session BattleAmbientContext on the engine isolates per-battle state across concurrent + // sessions (Task 7 of multi-instancing migration), so the historical single-active-engine gate + // (EngineSessionGate.TryAcquire) is gone and engine setup is unconditional. A genuine setup + // failure still surfaces via ComputeFrames' shadow-engine try/catch (it logs + swallows so the + // relay never sees an engine exception, ND6), and IsReady stays false in that case so + // ShadowIngest/ShadowFeedServerFrames no-op for the rest of the battle. + // // Seed the engine's StableRandom with BattleSeeds.Stable(MasterSeed) — the SAME value the // Matched frame ships to both clients (InitBattleHandler.cs:28). The clients seed their // System.Random with Matched.seed (BattleManagerBase.cs:721), so the engine's stream must diff --git a/SVSim.BattleNode/Sessions/Dispatch/Handlers/PlayActionsHandler.cs b/SVSim.BattleNode/Sessions/Dispatch/Handlers/PlayActionsHandler.cs index d7b5d86..4250f21 100644 --- a/SVSim.BattleNode/Sessions/Dispatch/Handlers/PlayActionsHandler.cs +++ b/SVSim.BattleNode/Sessions/Dispatch/Handlers/PlayActionsHandler.cs @@ -50,8 +50,8 @@ internal sealed class PlayActionsHandler : IFrameHandler // engine.Receive for THIS frame before this handler runs, so the engine has resolved the play and // PlayedCardCost reads the discounted cost it actually charged (spellboost + board modifiers folded // in BY CONSTRUCTION — no bookkeeping). Sender's seat == ctx.A (BattleSession.ShadowIngest uses the - // same ReferenceEquals(from, A) mapping). Degrades to 0 when the engine isn't owned/ready for this - // session (single-active-engine gate) so a non-engine session never crashes. + // same ReferenceEquals(from, A) mapping). Degrades to 0 when the engine isn't ready for this session + // (Setup failed and the ComputeFrames try/catch swallowed it, ND6) so a non-engine session never crashes. bool senderSeat = ReferenceEquals(ctx.From, ctx.A); int playedCost = ctx.Engine.PlayedCardCost(senderSeat, playIdx, fallback: 0); diff --git a/SVSim.BattleNode/Sessions/Engine/EngineSessionGate.cs b/SVSim.BattleNode/Sessions/Engine/EngineSessionGate.cs deleted file mode 100644 index ff25e51..0000000 --- a/SVSim.BattleNode/Sessions/Engine/EngineSessionGate.cs +++ /dev/null @@ -1,17 +0,0 @@ -namespace SVSim.BattleNode.Sessions.Engine; - -/// Process-wide single-active-engine gate. The engine's process-global turn state -/// (ToolboxGame.RealTimeNetworkAgent, GameMgr) cannot safely back two concurrent battles, so exactly -/// one BattleSession may own the engine at a time (AskUserQuestion 2026-06-06: serialize + document). -/// A session that cannot acquire runs WITHOUT the engine and logs it loudly — NOT a silent fallback -/// (the operator sees the limitation). Per-session isolation (removing this gate) is the tracked -/// follow-up. Non-blocking TryAcquire keeps it out of the synchronous dispatch path; in local -/// single-user dev battles are sequential, so contention never arises. -internal static class EngineSessionGate -{ - private static int _owned; // 0 = free, 1 = owned - - public static bool TryAcquire() => System.Threading.Interlocked.CompareExchange(ref _owned, 1, 0) == 0; - - public static void Release() => System.Threading.Interlocked.Exchange(ref _owned, 0); -} diff --git a/SVSim.BattleNode/Sessions/Engine/SessionBattleEngine.cs b/SVSim.BattleNode/Sessions/Engine/SessionBattleEngine.cs index 9a0739b..625526e 100644 --- a/SVSim.BattleNode/Sessions/Engine/SessionBattleEngine.cs +++ b/SVSim.BattleNode/Sessions/Engine/SessionBattleEngine.cs @@ -53,6 +53,15 @@ internal sealed class SessionBattleEngine ViewerId = EngineGlobalInit.ThisViewerId, IsForecast = true, IsRandomDraw = true, + // Per-session BattleRecoveryInfo: the receive-conductor deal path runs under IsRecovery + // (set after mgr construction below) and reads Data.BattleRecoveryInfo.IsMulliganEnd in + // MulliganMgrBase.StartDeal — null reads NRE. Each session owns its own no-op instance with + // IsMulliganEnd=false (the default); GetUninitializedObject skips the JsonData ctor. Each + // SessionBattleEngine carries its own ambient _ctx, so per-session isolation is by construction + // (the EngineGlobalInit fallback only seeded once-per-process and silently fell over for the + // second + later session that entered a fresh ambient — diagnosed Task 7). + RecoveryInfo = (engine::Wizard.BattleRecoveryInfo)FormatterServices + .GetUninitializedObject(typeof(engine::Wizard.BattleRecoveryInfo)), }; private HeadlessNetworkBattleMgr? _mgr; @@ -68,9 +77,11 @@ internal sealed class SessionBattleEngine /// the CardClass int value); they select the leader's class via the all-8-class /// ClassCharacterList EngineGlobalInit installs (chara_id == class_id for 1..8). The 3-arg overload /// behavior is preserved by the defaults (1/2), matching the test-harness charaIds. - /// NOTE: GameMgr (the leader chara ids set below) is a PROCESS GLOBAL. Setting per-session - /// chara ids is therefore only safe while exactly one engine-backed battle exists at a time — the - /// invariant enforces on the caller side. + /// NOTE: GameMgr is now per-session via ; the leader + /// chara ids are set on the SESSION's GameMgr (resolved through the ambient by + /// EngineGlobalInit.WirePerSessionGameMgr), not on a process-wide singleton. This is the Task-7 + /// payoff: concurrent sessions each own their own GameMgr + engine state, so the historical + /// single-active-engine gate (deleted EngineSessionGate) is no longer needed. public void Setup(int masterSeed, IReadOnlyList seatADeck, IReadOnlyList seatBDeck, int seatAClass = 1, int seatBClass = 2) @@ -155,8 +166,8 @@ internal sealed class SessionBattleEngine InstallHeadlessNetworkAgent(); // turn-flow resolve reads ToolboxGame.RealTimeNetworkAgent // Per-session leader class: chara_id == class_id for 1..8 in the all-8-class ClassCharacterList, - // so writing the seats' class ordinals into GameMgr's DataMgr resolves each leader's correct - // class. Process-global — safe only under EngineSessionGate (see method remarks above). + // so writing the seats' class ordinals into the SESSION's GameMgr DataMgr (resolved through the + // ambient — see Setup remarks) resolves each leader's correct class. SetGameMgrCharaIds(seatAClass, seatBClass); SeedDeck(mgr, seatADeck, isPlayer: true); @@ -340,12 +351,12 @@ internal sealed class SessionBattleEngine // // INVARIANT (two accessor bands, different null-engine policy): // • This "oracle" band (down to EvolveWaitTurnCount) goes through Seat(), which THROWS if the - // engine isn't owned/seated for this session. It is TEST-ONLY — called solely from the + // engine isn't seated for this session. It is TEST-ONLY — called solely from the // node-native harness/tests, where the engine is always seated. Do NOT call these from a wire // handler. // • The wire-path band below (PlayedCardCost/Spellboost/Clan/Tribe/Id) DEGRADES to a fallback - // when the engine isn't owned (single-active-engine gate), so a non-engine session never - // crashes. Production handlers read ONLY that band. + // when _mgr is null (Setup failed and the ComputeFrames try/catch swallowed it, ND6), so a + // non-engine session never crashes. Production handlers read ONLY that band. public int LeaderLife(bool playerSeat) { using var _ambient = BattleAmbient.Enter(_ctx); return Seat(playerSeat).Class.Life; } public int Pp(bool playerSeat) { using var _ambient = BattleAmbient.Enter(_ctx); return Seat(playerSeat).Pp; } @@ -449,8 +460,8 @@ internal sealed class SessionBattleEngine /// so is the authoritative play-time discounted cost. We search /// the seat's post-resolution zones (in-play, cemetery) by Index, then fall back to the hand /// (a not-yet-resolved card, e.g. a degenerate test path) reading the live Cost there. - /// Degrades to when the engine is not set up (the single-active-engine - /// gate left this session without an owned engine) or the idx resolves to no card — so a non-engine + /// Degrades to when the engine is not set up (Setup failed and the + /// ComputeFrames try/catch swallowed it, ND6) or the idx resolves to no card — so a non-engine /// session never crashes and a vanilla play simply emits its base cost via the caller's fallback. public int PlayedCardCost(bool playerSeat, int idx, int fallback = 0) { @@ -500,7 +511,7 @@ internal sealed class SessionBattleEngine /// /// Same post-resolution zone search + degrade-to- contract as /// : no engine / no card → , so a non-engine session - /// (the single-active-engine gate left this session without an owned engine) keeps emitting the deck-map id via + /// (Setup failed and the ComputeFrames try/catch swallowed it, ND6) keeps emitting the deck-map id via /// the caller's fallback, never crashing. public long PlayedCardId(bool playerSeat, int idx, long fallback = 0) { @@ -538,10 +549,10 @@ internal sealed class SessionBattleEngine /// : no engine / no card → (default "0", the /// prod no-tribe form — NEVER empty, which is wire-illegal: prod always sends tribe as a non-empty string, /// the client reads it via item.Value.ToString() at NetworkBattleReceiver.cs:2382). The degrade is - /// LIVE, not dead: a second concurrent battle that loses the single-active-engine gate has _mgr is null - /// yet still emits a knownList entry (the handler resolves the identity via the deck-map/mined fallback when - /// the engine read degrades, so BuildPlayedCard still synthesizes an entry), so this path must hand back a - /// legal wire value. + /// LIVE, not dead: a session whose Setup failed (the ComputeFrames try/catch swallowed it, ND6) has + /// _mgr is null yet still emits a knownList entry (the handler resolves the identity via the + /// deck-map/mined fallback when the engine read degrades, so BuildPlayedCard still synthesizes an + /// entry), so this path must hand back a legal wire value. public string PlayedCardTribe(bool playerSeat, int idx, string fallback = "0") { using var _ambient = BattleAmbient.Enter(_ctx); @@ -923,12 +934,11 @@ internal sealed class SessionBattleEngine ToolboxGame.SetRealTimeNetworkBattle(agent); } - // Write the two seats' class ordinals into GameMgr's DataMgr leader chara ids. Mirrors the test - // seam HeadlessFixture.cs:202-204 (SetField(dm, "_playerCharaId"/"_enemyCharaId", ...)). chara_id == - // class_id for 1..8 in EngineGlobalInit's all-8-class ClassCharacterList, so the ordinal selects the - // class. A non-positive ordinal (e.g. CardClass.None == 0) clamps to the default seat (1/2). - // GameMgr is a process global → safe only under EngineSessionGate (one engine-backed battle at a - // time). + // Write the two seats' class ordinals into the SESSION's GameMgr DataMgr leader chara ids. Mirrors + // the test seam HeadlessFixture.cs:202-204 (SetField(dm, "_playerCharaId"/"_enemyCharaId", ...)). + // chara_id == class_id for 1..8 in EngineGlobalInit's all-8-class ClassCharacterList, so the ordinal + // selects the class. A non-positive ordinal (e.g. CardClass.None == 0) clamps to the default seat + // (1/2). GameMgr is per-session (BattleAmbientContext.GameMgr); writes resolve through the ambient. private static void SetGameMgrCharaIds(int a, int b) { var dm = GameMgr.GetIns().GetDataMgr(); diff --git a/SVSim.UnitTests/BattleNode/Integration/NodeNativeBattleHarness.cs b/SVSim.UnitTests/BattleNode/Integration/NodeNativeBattleHarness.cs index 1b4279f..0343d77 100644 --- a/SVSim.UnitTests/BattleNode/Integration/NodeNativeBattleHarness.cs +++ b/SVSim.UnitTests/BattleNode/Integration/NodeNativeBattleHarness.cs @@ -26,9 +26,9 @@ namespace SVSim.UnitTests.BattleNode.Integration; /// Engine globals (CardMaster, GameMgr, Wizard.Data) are primed by /// SessionBattleEngine.Setup itself (it calls EngineGlobalInit.EnsureInitialized(), which /// loads the full cards.json from AppContext.BaseDirectory/Data/cards.json). The harness adds no -/// global init of its own. NOTE: unlike the live session, the harness does NOT acquire -/// EngineSessionGate — driving the engine directly bypasses it. One engine-backed battle at a -/// time is assumed within a test (the engine's process-global statics can't back two concurrently). +/// global init of its own. Per-battle state is isolated via the engine's per-session +/// BattleAmbientContext (Task 7 of multi-instancing migration), so the historical +/// single-active-engine gate is gone — concurrent harnesses + sessions are now safe. /// internal sealed class NodeNativeBattleHarness : IDisposable { diff --git a/SVSim.UnitTests/BattleNode/Sessions/KnownListBuilderTests.cs b/SVSim.UnitTests/BattleNode/Sessions/KnownListBuilderTests.cs index 2787495..7c9ac35 100644 --- a/SVSim.UnitTests/BattleNode/Sessions/KnownListBuilderTests.cs +++ b/SVSim.UnitTests/BattleNode/Sessions/KnownListBuilderTests.cs @@ -139,9 +139,9 @@ public class KnownListBuilderTests [Test] public void BuildPlayedCard_defaults_clan_to_zero_and_tribe_to_string_zero_when_caller_passes_none() { - // A play whose engine read degraded (single-active-engine gate: _mgr null → the accessor fallback) - // emits clan 0 (ClanType.ALL ordinal) and tribe "0" (the prod no-tribe form, NEVER empty — - // empty is wire-illegal). The param defaults match the accessor fallbacks. + // A play whose engine read degraded (Setup failed and the ComputeFrames try/catch swallowed it → + // _mgr null → the accessor fallback) emits clan 0 (ClanType.ALL ordinal) and tribe "0" (the prod + // no-tribe form, NEVER empty — empty is wire-illegal). The param defaults match the accessor fallbacks. var entry = KnownListBuilder.BuildPlayedCard(playIdx: 3, cardId: 101311010L, orderList: OrderListMove(3, 10, 20)); Assert.That(entry, Is.Not.Null); Assert.That(entry!.Clan, Is.EqualTo(0));