fix(battlenode): EngineGlobalInit guarantees full-master postcondition (Phase 2 N2 review)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,7 @@
|
|||||||
using NUnit.Framework;
|
using NUnit.Framework;
|
||||||
using SVSim.BattleNode.Sessions.Engine;
|
using SVSim.BattleNode.Sessions.Engine;
|
||||||
using System.Linq;
|
using System.Linq;
|
||||||
|
using SVSim.BattleEngine.Tests;
|
||||||
|
|
||||||
namespace SVSim.BattleEngine.Tests.SessionEngine;
|
namespace SVSim.BattleEngine.Tests.SessionEngine;
|
||||||
|
|
||||||
@@ -15,6 +16,10 @@ public class SessionEngineSpellboostTests
|
|||||||
var cl2 = CaptureReplay.Load("battle_test_cl2.ndjson");
|
var cl2 = CaptureReplay.Load("battle_test_cl2.ndjson");
|
||||||
var deckA = CaptureReplay.SelfDeckFrom(cl1);
|
var deckA = CaptureReplay.SelfDeckFrom(cl1);
|
||||||
var deckB = CaptureReplay.SelfDeckFrom(cl2);
|
var deckB = CaptureReplay.SelfDeckFrom(cl2);
|
||||||
|
// Belt-and-suspenders (matches the sibling tests): load the decks into the harness master so
|
||||||
|
// this test never depends on global card-master contents. EnsureInitialized() above still
|
||||||
|
// proves EngineGlobalInit's own path works.
|
||||||
|
foreach (var id in deckA.Concat(deckB).Distinct()) HeadlessCardMaster.Load((int)id);
|
||||||
var engine = new SessionBattleEngine();
|
var engine = new SessionBattleEngine();
|
||||||
Assert.DoesNotThrow(() => engine.Setup(masterSeed: 12345, seatADeck: deckA, seatBDeck: deckB));
|
Assert.DoesNotThrow(() => engine.Setup(masterSeed: 12345, seatADeck: deckA, seatBDeck: deckB));
|
||||||
Assert.That(engine.IsReady, Is.True, "engine must be ready after EngineGlobalInit (carried-risk fix)");
|
Assert.That(engine.IsReady, Is.True, "engine must be ready after EngineGlobalInit (carried-risk fix)");
|
||||||
|
|||||||
@@ -34,8 +34,11 @@ namespace SVSim.BattleNode.Sessions.Engine;
|
|||||||
/// <c>HeadlessCardMaster</c> + <c>HeadlessMasterData</c>; the reflection seams are transcribed verbatim.
|
/// <c>HeadlessCardMaster</c> + <c>HeadlessMasterData</c>; the reflection seams are transcribed verbatim.
|
||||||
/// It differs in exactly three ways: (1) it loads the FULL cards.json (every row, no id filter) since
|
/// It differs in exactly three ways: (1) it loads the FULL cards.json (every row, no id filter) since
|
||||||
/// the live host serves arbitrary decks; (2) it installs ALL 8 classes in ClassCharacterList; and
|
/// the live host serves arbitrary decks; (2) it installs ALL 8 classes in ClassCharacterList; and
|
||||||
/// (3) every set is guarded so the call is idempotent AND does not fight the test
|
/// (3) the call is idempotent (process-once via <c>_done</c>). Coexistence: it does not OVERWRITE
|
||||||
/// <c>HeadlessEngineEnv</c> if both run in one NUnit process.</summary>
|
/// harmless globals another initializer set (<c>Data.Load</c>, <c>Crossover</c>, leader chara ids,
|
||||||
|
/// netUser, udid are all set-only-if-absent), but it ALWAYS guarantees the postcondition — the FULL
|
||||||
|
/// card master and the all-8-class Master — by rebuilding/re-injecting both unconditionally rather
|
||||||
|
/// than deferring to a possibly-thinner existing global (e.g. a test harness's single-card master).</summary>
|
||||||
internal static class EngineGlobalInit
|
internal static class EngineGlobalInit
|
||||||
{
|
{
|
||||||
private static readonly object _gate = new();
|
private static readonly object _gate = new();
|
||||||
@@ -74,16 +77,19 @@ internal static class EngineGlobalInit
|
|||||||
BattleManagerBase.IsForecast = true;
|
BattleManagerBase.IsForecast = true;
|
||||||
|
|
||||||
// --- static CardMaster (full cards.json) ----------------------------------------------
|
// --- static CardMaster (full cards.json) ----------------------------------------------
|
||||||
// Skip the (expensive) full load if a CardMaster is already registered as Default — a
|
// ALWAYS rebuild + re-inject the FULL master. We must not defer to a possibly-thin
|
||||||
// prior EngineGlobalInit call OR a HeadlessEngineEnv/HeadlessCardMaster load in the same
|
// existing Default (e.g. a HeadlessCardMaster.Load(singleCard) from an earlier test in
|
||||||
// NUnit process already populated it.
|
// the same NUnit process): the postcondition is the COMPLETE card master, and a thin
|
||||||
if (!IsCardMasterPopulated())
|
// master makes Setup→SeedDeck NRE in SkillCreator.CreateBuildInfo. EnsureInitialized is
|
||||||
LoadFullCardMaster();
|
// process-once via _done, so an unconditional load runs at most once — the skip-
|
||||||
|
// optimization bought nothing and traded correctness for a coexistence hazard.
|
||||||
|
// LoadFullCardMaster rebuilds + re-injects Default, so it's idempotent.
|
||||||
|
LoadFullCardMaster();
|
||||||
|
|
||||||
// --- Master reference data (all 8 classes' chara list) ---------------------------------
|
// --- Master reference data (all 8 classes' chara list) ---------------------------------
|
||||||
// Skip if Data.Master is already non-null with a non-empty ClassCharacterList.
|
// ALWAYS install all 8 classes. Don't defer to a prior (e.g. 2-class) harness master:
|
||||||
if (!IsMasterPopulated())
|
// the postcondition is all-8-class ClassCharacterList. Idempotent (replaces Data.Master).
|
||||||
InstallMaster();
|
InstallMaster();
|
||||||
|
|
||||||
// --- GameMgr DataMgr leader chara ids --------------------------------------------------
|
// --- GameMgr DataMgr leader chara ids --------------------------------------------------
|
||||||
// Set the backing fields directly: the public SetPlayerCharaId() also pulls MyRotation/
|
// Set the backing fields directly: the public SetPlayerCharaId() also pulls MyRotation/
|
||||||
@@ -111,11 +117,12 @@ internal static class EngineGlobalInit
|
|||||||
// --- Cute.Certification.udid -----------------------------------------------------------
|
// --- Cute.Certification.udid -----------------------------------------------------------
|
||||||
// The emit-path payload builder reads Certification.Udid, whose getter lazily decodes from
|
// The emit-path payload builder reads Certification.Udid, whose getter lazily decodes from
|
||||||
// Toolbox.SavedataManager (null headless). Seed the private static backing field with a
|
// Toolbox.SavedataManager (null headless). Seed the private static backing field with a
|
||||||
// non-empty placeholder so the getter short-circuits. Only set when empty (coexistence).
|
// non-empty placeholder (opaque — only echoed, never matched) so the getter short-
|
||||||
|
// circuits. Matches the harness value. Only set when empty (coexistence).
|
||||||
var udidField = typeof(Certification).GetField("udid",
|
var udidField = typeof(Certification).GetField("udid",
|
||||||
BindingFlags.Static | BindingFlags.NonPublic)!;
|
BindingFlags.Static | BindingFlags.NonPublic)!;
|
||||||
if (string.IsNullOrEmpty(udidField.GetValue(null) as string))
|
if (string.IsNullOrEmpty(udidField.GetValue(null) as string))
|
||||||
udidField.SetValue(null, "host-udid");
|
udidField.SetValue(null, "headless-udid");
|
||||||
|
|
||||||
_done = true;
|
_done = true;
|
||||||
}
|
}
|
||||||
@@ -123,16 +130,6 @@ internal static class EngineGlobalInit
|
|||||||
|
|
||||||
// --- CardMaster (full load) ----------------------------------------------------------------------
|
// --- CardMaster (full load) ----------------------------------------------------------------------
|
||||||
|
|
||||||
private static bool IsCardMasterPopulated()
|
|
||||||
{
|
|
||||||
var idType = typeof(CardMaster).GetNestedType("CardMasterId")!;
|
|
||||||
var defaultId = Enum.Parse(idType, "Default");
|
|
||||||
var fld = typeof(CardMaster).GetField("_dictCardMaster",
|
|
||||||
BindingFlags.Static | BindingFlags.NonPublic)!;
|
|
||||||
if (fld.GetValue(null) is not IDictionary dict) return false;
|
|
||||||
return dict.Contains(defaultId) && dict[defaultId] != null;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Production difference (1): enumerate EVERY card row — no want.Contains(id) filter.
|
// Production difference (1): enumerate EVERY card row — no want.Contains(id) filter.
|
||||||
private static void LoadFullCardMaster()
|
private static void LoadFullCardMaster()
|
||||||
{
|
{
|
||||||
@@ -201,14 +198,6 @@ internal static class EngineGlobalInit
|
|||||||
|
|
||||||
// --- Master reference data (all 8 classes) -------------------------------------------------------
|
// --- Master reference data (all 8 classes) -------------------------------------------------------
|
||||||
|
|
||||||
private static bool IsMasterPopulated()
|
|
||||||
{
|
|
||||||
if (Data.Master is not Master m) return false;
|
|
||||||
var p = typeof(Master).GetProperty("ClassCharacterList",
|
|
||||||
BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
|
|
||||||
return p?.GetValue(m) is ICollection { Count: > 0 };
|
|
||||||
}
|
|
||||||
|
|
||||||
// Transcribed from HeadlessMasterData.Install. Production difference (2): install ALL 8 classes.
|
// Transcribed from HeadlessMasterData.Install. Production difference (2): install ALL 8 classes.
|
||||||
private static void InstallMaster()
|
private static void InstallMaster()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user