refactor(engine-ambient): wrap residual UnitTests + delete EngineSessionGate
Step 7 of multi-instancing migration. Residual SVSim.UnitTests that touch engine code directly are wrapped in TestBattleScope. EngineSessionGate is deleted along with the _engineOwned bookkeeping in BattleSession; engine setup is unconditional now that per-battle state is isolated on the ambient. Gate-specific fallback branches in BattleSession.ShadowIngest are simplified. Suite fully green (SVSim.UnitTests, SVSim.BattleEngine.Tests). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
gamer147
@@ -52,11 +52,6 @@ public sealed class BattleSession
|
||||
/// was already processed during the Swap feed.</summary>
|
||||
private bool _engineReadyFed;
|
||||
|
||||
/// <summary>True once this session has acquired the process-wide <see cref="Engine.EngineSessionGate"/>
|
||||
/// (and is therefore the single active engine owner). Drives the matching <c>Release</c> at battle
|
||||
/// end so the next session can take the engine.</summary>
|
||||
private bool _engineOwned;
|
||||
|
||||
/// <summary>Serializes dispatch. Both participants' read loops raise FrameEmitted on their own
|
||||
/// threads, and a dispatch (<see cref="ComputeFrames"/> + the relay <c>PushAsync</c> calls) mutates
|
||||
/// shared, non-thread-safe state — the <see cref="BattleSessionState"/> 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
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -1,17 +0,0 @@
|
||||
namespace SVSim.BattleNode.Sessions.Engine;
|
||||
|
||||
/// <summary>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.</summary>
|
||||
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);
|
||||
}
|
||||
@@ -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 <c>CardClass</c> 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.
|
||||
/// <para>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 <see cref="EngineSessionGate"/> enforces on the caller side.</para></summary>
|
||||
/// <para>NOTE: GameMgr is now per-session via <see cref="BattleAmbientContext.GameMgr"/>; the leader
|
||||
/// chara ids are set on the SESSION's GameMgr (resolved through the ambient by
|
||||
/// <c>EngineGlobalInit.WirePerSessionGameMgr</c>), 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.</para></summary>
|
||||
public void Setup(int masterSeed,
|
||||
IReadOnlyList<long> seatADeck, IReadOnlyList<long> 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 <see cref="BattleCardBase.PlayedCost"/> is the authoritative play-time discounted cost. We search
|
||||
/// the seat's post-resolution zones (in-play, cemetery) by <c>Index</c>, then fall back to the hand
|
||||
/// (a not-yet-resolved card, e.g. a degenerate test path) reading the live <c>Cost</c> there.</para>
|
||||
/// <para>Degrades to <paramref name="fallback"/> 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
|
||||
/// <para>Degrades to <paramref name="fallback"/> 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.</para></summary>
|
||||
public int PlayedCardCost(bool playerSeat, int idx, int fallback = 0)
|
||||
{
|
||||
@@ -500,7 +511,7 @@ internal sealed class SessionBattleEngine
|
||||
/// </list>
|
||||
/// Same post-resolution zone search + degrade-to-<paramref name="fallback"/> contract as
|
||||
/// <see cref="PlayedCardCost"/>: no engine / no card → <paramref name="fallback"/>, 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.</summary>
|
||||
public long PlayedCardId(bool playerSeat, int idx, long fallback = 0)
|
||||
{
|
||||
@@ -538,10 +549,10 @@ internal sealed class SessionBattleEngine
|
||||
/// <see cref="PlayedCardClan"/>: no engine / no card → <paramref name="fallback"/> (default <c>"0"</c>, 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 <c>item.Value.ToString()</c> at NetworkBattleReceiver.cs:2382). The degrade is
|
||||
/// LIVE, not dead: a second concurrent battle that loses the single-active-engine gate has <c>_mgr is null</c>
|
||||
/// 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.</para></summary>
|
||||
/// LIVE, not dead: a session whose Setup failed (the ComputeFrames try/catch swallowed it, ND6) has
|
||||
/// <c>_mgr is null</c> 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.</para></summary>
|
||||
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();
|
||||
|
||||
@@ -26,9 +26,9 @@ namespace SVSim.UnitTests.BattleNode.Integration;
|
||||
/// <para>Engine globals (<c>CardMaster</c>, <c>GameMgr</c>, <c>Wizard.Data</c>) are primed by
|
||||
/// <c>SessionBattleEngine.Setup</c> itself (it calls <c>EngineGlobalInit.EnsureInitialized()</c>, which
|
||||
/// loads the full cards.json from <c>AppContext.BaseDirectory/Data/cards.json</c>). The harness adds no
|
||||
/// global init of its own. NOTE: unlike the live session, the harness does NOT acquire
|
||||
/// <c>EngineSessionGate</c> — 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).</para>
|
||||
/// global init of its own. Per-battle state is isolated via the engine's per-session
|
||||
/// <c>BattleAmbientContext</c> (Task 7 of multi-instancing migration), so the historical
|
||||
/// single-active-engine gate is gone — concurrent harnesses + sessions are now safe.</para>
|
||||
/// </summary>
|
||||
internal sealed class NodeNativeBattleHarness : IDisposable
|
||||
{
|
||||
|
||||
@@ -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));
|
||||
|
||||
Reference in New Issue
Block a user