diff --git a/SVSim.BattleEngine.Tests/BattleAmbientTests.cs b/SVSim.BattleEngine.Tests/BattleAmbientTests.cs index 12085db..6c01123 100644 --- a/SVSim.BattleEngine.Tests/BattleAmbientTests.cs +++ b/SVSim.BattleEngine.Tests/BattleAmbientTests.cs @@ -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(() => { var _ = BattleManagerBase.IsForecast; }); + Assert.Throws(() => 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(() => { var _ = BattleManagerBase.IsRandomDraw; }); + Assert.Throws(() => 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(() => { 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] diff --git a/SVSim.BattleEngine.Tests/HeadlessFixture.cs b/SVSim.BattleEngine.Tests/HeadlessFixture.cs index c1cdcda..cda2131 100644 --- a/SVSim.BattleEngine.Tests/HeadlessFixture.cs +++ b/SVSim.BattleEngine.Tests/HeadlessFixture.cs @@ -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. diff --git a/SVSim.BattleEngine.Tests/MultiInstanceEngineTests.cs b/SVSim.BattleEngine.Tests/MultiInstanceEngineTests.cs new file mode 100644 index 0000000..a0fbb80 --- /dev/null +++ b/SVSim.BattleEngine.Tests/MultiInstanceEngineTests.cs @@ -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; + +/// The forcing-function tests for the multi-instancing migration (Task 8). Each engine +/// instance carries its OWN 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). +[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(() => GameMgr.GetIns()); + } + + private static void DriveBasicTurns(SessionBattleEngine e) + { + _ = e.LeaderLife(true); + _ = e.Pp(true); + _ = e.HandCount(true); + } +} diff --git a/SVSim.BattleEngine.Tests/NetworkEmitFixtureBase.cs b/SVSim.BattleEngine.Tests/NetworkEmitFixtureBase.cs index ba7e70f..4efbb35 100644 --- a/SVSim.BattleEngine.Tests/NetworkEmitFixtureBase.cs +++ b/SVSim.BattleEngine.Tests/NetworkEmitFixtureBase.cs @@ -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; - } } } diff --git a/SVSim.BattleEngine/COPIED.manifest.tsv b/SVSim.BattleEngine/COPIED.manifest.tsv index 798bd03..6289926 100644 --- a/SVSim.BattleEngine/COPIED.manifest.tsv +++ b/SVSim.BattleEngine/COPIED.manifest.tsv @@ -62,7 +62,7 @@ BattleFinishToOpponentDisConnectChecker.cs BattleFinishToOpponentDisConnectCheck BattleKeywordInfoListMgr.cs BattleKeywordInfoListMgr.cs a014170d0b3f5499635bcc2e29755dc2f3125d5a5a28b1741a4abc74b4abcf86 0 BattleLifeTimeSharedObject.cs BattleLifeTimeSharedObject.cs ab8bc3703d268752a1de56ab5d3e9ebd276980c20076eb0ca300838b3db13d5f 0 BattleLogTextBuilderAttachSkill.cs BattleLogTextBuilderAttachSkill.cs 11c585ae931fa3dc734bb231d6da61df3b51b803516ca2c5d88a0c78bc7c0104 0 -BattleManagerBase.cs BattleManagerBase.cs d12aed345f596e068bfb130a247e280457cf5160a76f8bfef4a1dda1c561d196 1 +BattleManagerBase.cs BattleManagerBase.cs c40684d0f3e3c11e603b374ee9ba64f1830052723ea183d0b90d76d4a646ad5c 1 BattleMenuMgr.cs BattleMenuMgr.cs 7418699063e01641d0df1ed16773a9ac9418f418cc047fc18c5892eb7971d361 0 BattlePlayer.cs BattlePlayer.cs 001409844b46ddaf0a5edbce4e015749ece61053adf725a978987d7063a02632 0 BattlePlayerBase.cs BattlePlayerBase.cs 9d3a665158706460a52900008dcfcdf575dbe08cb6d3cc05e63e718b2885b51b 0 @@ -178,7 +178,7 @@ Cute\AudioManager.cs Cute\AudioManager.cs 5e0cce94bcfad63266cd298aef89bb383e541f Cute\BootApp.cs Cute\BootApp.cs 1a6b3066b6155aba225b9ca4e79655e428fc7b24cb16569717b53600b1f23bb5 0 Cute\BootNetwork.cs Cute\BootNetwork.cs 99769dd6c38b2ee2ef7ad02e14530c658576e5c4a2188ed1cbcdd563f68443f6 0 Cute\BootSystem.cs Cute\BootSystem.cs 61afa7a7c8df705504031629965440d491603dec531b047a36b9294f255ee04e 0 -Cute\Certification.cs Cute\Certification.cs ea018062d8823eb3527e7fd5e07d094a6fd444c1ad5a368bcf9132315cac99ac 1 +Cute\Certification.cs Cute\Certification.cs 0491630cc02f155829e96d2a3624f13a81200b5a00fd253f54cefbe576371d70 1 Cute\CryptAES.cs Cute\CryptAES.cs d3022b9e1be75bc07e57aef61093717a84b60383608eba5daa9b7dc6605b1e75 0 Cute\Cryptographer.cs Cute\Cryptographer.cs f61bc1f4727d323004c443c9db30a4f221db3309be2cb14d6f51fc6a39590908 0 Cute\CustomPreference.cs Cute\CustomPreference.cs ddc46cc13bf2728e4fcee7db550ec093ec3acf651ea48d3dbb5f5099d5a20f89 0 @@ -2624,7 +2624,7 @@ Wizard\CrossoverRewardInfo.cs Wizard\CrossoverRewardInfo.cs bb3763306d0e7d3feefb Wizard\CsvColumns.cs Wizard\CsvColumns.cs d113f92e1f0145adb323c093deca81aab1889e8de34ed78c852b8e5a764c1c4c 0 Wizard\CustomEasing.cs Wizard\CustomEasing.cs c7ac36e40e66f046d42e1d688db22f2acc2567399ce23c0512e3e2c8beefa598 0 Wizard\DamagedTagCollection.cs Wizard\DamagedTagCollection.cs 8e6ecf677b4da8e16a68d3336959cd6d08af4830a0214297d65e96243f777c3f 0 -Wizard\Data.cs Wizard\Data.cs 88529d834ee64a12288ae2a40062824ea594f6d1b6c95d3cab2776f499369e77 1 +Wizard\Data.cs Wizard\Data.cs 46db6a70b06a6ebf89f7210e32385a63ab82a8ae196518a684a3a632ff5dea69 1 Wizard\DeckAttributeType.cs Wizard\DeckAttributeType.cs 006bb4c04d8a60c9caf04873dde6c962366348db03ec40a8bbc0071392f656dd 0 Wizard\DeckAutoCreateTask.cs Wizard\DeckAutoCreateTask.cs e8c21d513114d2c42ad85a28da4adb48642bd36dc012f6029fbc8a8d72b78d6c 0 Wizard\DeckBuildShortageCardView.cs Wizard\DeckBuildShortageCardView.cs 34428e4efb614c4fd59136d1bb87485ce117a97b2c6668f0481fff4b510a31d3 0 @@ -3237,7 +3237,7 @@ Wizard\TargetSelectType.cs Wizard\TargetSelectType.cs 91ab18f9c069784e1140a187ea Wizard\TargetTagCollection.cs Wizard\TargetTagCollection.cs 1bd2fb66e58c9fae3d23fbce351972577da664c2b959ee92e0547e2396c82eb9 0 Wizard\TextLineCreater.cs Wizard\TextLineCreater.cs fdb7f0a918c2f5b92268954b3980724f29cb16a8098ea67c2d99633ae5bd1e92 0 Wizard\TokenPlayPattern.cs Wizard\TokenPlayPattern.cs c14d846afb81b876291013c077cbd503bebd27b651b5b78708e93d68758e2e7b 0 -Wizard\ToolboxGame.cs Wizard\ToolboxGame.cs ac0bde1ef076672c6e19969c5f9ec172aee79e7d5ee5bdf660e89166e8ff07ce 1 +Wizard\ToolboxGame.cs Wizard\ToolboxGame.cs 968903c18a720c29cb410690d8912ff42d11dc7c41e29a61f77f8de112f18414 1 Wizard\TournamentCell.cs Wizard\TournamentCell.cs b07b968c6768b74b9345ab3aa1ae138950de022b08f92a16966b3daef81d379c 0 Wizard\TournamentCellData.cs Wizard\TournamentCellData.cs ecc457bdf2dbaa7db9de893f69a07ac43e589ca140dad29866d658af386e605e 0 Wizard\TournamentController.cs Wizard\TournamentController.cs 236122aa73487c600114c9b0010ab9e9c85a5745a267550088c8f790532647c9 0 diff --git a/SVSim.BattleEngine/Engine/BattleManagerBase.cs b/SVSim.BattleEngine/Engine/BattleManagerBase.cs index be34674..ef2e365 100644 --- a/SVSim.BattleEngine/Engine/BattleManagerBase.cs +++ b/SVSim.BattleEngine/Engine/BattleManagerBase.cs @@ -410,26 +410,14 @@ public class BattleManagerBase private NetworkTouchControl _networkTouchControl; - private static BattleManagerBase _mainFallback; - - private static bool _isRandomDrawFallback = false; public static bool IsRandomDraw { - get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.IsRandomDraw ?? _isRandomDrawFallback; - set { - var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; - if (c != null) c.IsRandomDraw = value; - else _isRandomDrawFallback = value; - } + get => SVSim.BattleEngine.Ambient.BattleAmbient.Require().IsRandomDraw; + set => SVSim.BattleEngine.Ambient.BattleAmbient.Require().IsRandomDraw = value; } - private static bool _isForecastFallback = false; public static bool IsForecast { - get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.IsForecast ?? _isForecastFallback; - set { - var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; - if (c != null) c.IsForecast = value; - else _isForecastFallback = value; - } + get => SVSim.BattleEngine.Ambient.BattleAmbient.Require().IsForecast; + set => SVSim.BattleEngine.Ambient.BattleAmbient.Require().IsForecast = value; } public BattleLifeTimeSharedObject BattleLifeTimeSharedObject; @@ -724,12 +712,15 @@ public class BattleManagerBase public static BattleManagerBase GetIns() { - return SVSim.BattleEngine.Ambient.BattleAmbient.Current?.Mgr ?? _mainFallback; + // Soft read: returns null when no scope is active, preserving engine call sites that pattern + // `GetIns()?.Foo ?? default` (or null-tolerant successors). Inside a scope, returns the + // scope's Mgr (which may itself be null mid-Setup — see SessionBattleEngine.SetupInternal, + // which publishes Mgr before WireMulliganPhase). + return SVSim.BattleEngine.Ambient.BattleAmbient.Current?.Mgr; } protected BattleManagerBase(IBattleMgrContentsCreator contentsCreator) { - _mainFallback = this; BattleLifeTimeSharedObject = new BattleLifeTimeSharedObject(); PublishedSkillList = new List(); _contentsCreator = contentsCreator; @@ -1488,7 +1479,6 @@ public class BattleManagerBase GameMgr.GetIns().GetPrefabMgr().DisposeAllClonedObject(); GameMgr.GetIns().GetGameObjMgr().DisposeUIGameObj(); GameMgr.GetIns().GetPrefabMgr().AllUnLoad(); - _mainFallback = null; } private void DisposeBattleGameObj_DestroyImmediate(GameObject obj) diff --git a/SVSim.BattleEngine/Engine/Cute/Certification.cs b/SVSim.BattleEngine/Engine/Cute/Certification.cs index a86bf22..89cc843 100644 --- a/SVSim.BattleEngine/Engine/Cute/Certification.cs +++ b/SVSim.BattleEngine/Engine/Cute/Certification.cs @@ -16,8 +16,6 @@ public class Certification : MonoBehaviour private static string udid; - private static int _viewerIdFallback; - private static int short_udid; private static string sessionId; @@ -40,28 +38,13 @@ public class Certification : MonoBehaviour public static int ViewerId { - get - { - var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; - if (c != null) return c.ViewerId; - if (_viewerIdFallback == 0) - { - _viewerIdFallback = Toolbox.SavedataManager.GetInt("VIEWER_ID"); - } - return _viewerIdFallback; - } - set - { - var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; - if (c != null) - { - // Inside a scope, ViewerId is fixed at scope entry — swallow the write. - return; - } - Toolbox.SavedataManager.SetInt("VIEWER_ID", value); - Toolbox.SavedataManager.Save(); - _viewerIdFallback = value; - } + // Post-Task-8: strictly ambient. The historical SavedataManager-backed lazy decode was the + // client process's single-viewer-id source; in the headless multi-instance world the viewer + // id MUST come from the per-session ambient context. Setter is a no-op (BattleAmbientContext + // .ViewerId is `init`-only — fixed at scope entry per design — and the historical caller + // (SavedataManager.SetInt + Save) is dead in the server world). + get => SVSim.BattleEngine.Ambient.BattleAmbient.Require().ViewerId; + set { /* ambient ViewerId is init-only; SavedataManager path is dead headless */ } } public static int ShortUdid @@ -152,7 +135,6 @@ public class Certification : MonoBehaviour { sessionId = null; udid = null; - _viewerIdFallback = 0; short_udid = 0; Toolbox.SavedataManager.SetInt("VIEWER_ID", 0); Toolbox.SavedataManager.SetInt("SHORT_UDID", 0); diff --git a/SVSim.BattleEngine/Engine/Wizard/Data.cs b/SVSim.BattleEngine/Engine/Wizard/Data.cs index 6b43f6a..6483d56 100644 --- a/SVSim.BattleEngine/Engine/Wizard/Data.cs +++ b/SVSim.BattleEngine/Engine/Wizard/Data.cs @@ -176,16 +176,16 @@ public static class Data public static ReplayDetailInfo ReplayBattleInfo { get; set; } - private static BattleRecoveryInfo _battleRecoveryInfoFallback; public static BattleRecoveryInfo BattleRecoveryInfo { - get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.RecoveryInfo ?? _battleRecoveryInfoFallback; - set - { - var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; - if (c != null) c.RecoveryInfo = value; - else _battleRecoveryInfoFallback = value; - } + // Soft read: returns null when no scope is active. The MulliganMgrBase.StartDeal call site + // reads this with a null-tolerant ??=-style pattern, so a null degrade is the historical + // fallback. Inside a scope, returns the per-session RecoveryInfo (SessionBattleEngine + // pre-seeds an uninitialized BattleRecoveryInfo on its ctx field initializer). + get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.RecoveryInfo; + // Strict setter: writes must land on the per-session ctx. No historical production caller + // writes this outside a scope; an unwrapped write now fails fast (forcing function). + set => SVSim.BattleEngine.Ambient.BattleAmbient.Require().RecoveryInfo = value; } public static VoteData VoteInfo { get; set; } diff --git a/SVSim.BattleEngine/Engine/Wizard/ToolboxGame.cs b/SVSim.BattleEngine/Engine/Wizard/ToolboxGame.cs index bb2d91b..f7cf948 100644 --- a/SVSim.BattleEngine/Engine/Wizard/ToolboxGame.cs +++ b/SVSim.BattleEngine/Engine/Wizard/ToolboxGame.cs @@ -25,10 +25,12 @@ public static class ToolboxGame private static Transform _gameTransform = null; - private static RealTimeNetworkAgent _realTimeNetworkAgentFallback; public static RealTimeNetworkAgent RealTimeNetworkAgent { - get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.NetworkAgent ?? _realTimeNetworkAgentFallback; + // Soft read: returns null when no scope is active, mirroring GetIns. Engine code reads this + // via `ToolboxGame.RealTimeNetworkAgent?.Foo`-style patterns (network-send paths gated on a + // non-null agent), so a null on the unwrapped path is the correct degrade — not a throw. + get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.NetworkAgent; } public static Transform GameTransform @@ -60,20 +62,19 @@ public static class ToolboxGame public static void SetRealTimeNetworkBattle(RealTimeNetworkAgent agent) { - var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; - if (c != null) c.NetworkAgent = agent; - else _realTimeNetworkAgentFallback = agent; + // Strict: must be inside a scope to set the per-session agent. Forcing-function — any + // historical unwrapped caller (no production callsite remains) now fails fast. + SVSim.BattleEngine.Ambient.BattleAmbient.Require().NetworkAgent = agent; } public static void DestroyNetworkAgent() { var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; - var current = c?.NetworkAgent ?? _realTimeNetworkAgentFallback; + var current = c?.NetworkAgent; if (current != null) { Object.DestroyImmediate(current.gameObject); - if (c != null) c.NetworkAgent = null; - else _realTimeNetworkAgentFallback = null; + c.NetworkAgent = null; } } diff --git a/SVSim.BattleEngine/Patches/BattleManagerBase.ambient-fallback-deletion.patch b/SVSim.BattleEngine/Patches/BattleManagerBase.ambient-fallback-deletion.patch new file mode 100644 index 0000000..ab1410e --- /dev/null +++ b/SVSim.BattleEngine/Patches/BattleManagerBase.ambient-fallback-deletion.patch @@ -0,0 +1,77 @@ +Multi-instancing migration (Step 8 — forcing function): delete the static per-battle fallbacks +the earlier ambient-* patches added as a coexistence shim. With the fallbacks gone, any unwrapped +engine call into IsForecast/IsRandomDraw fails fast (BattleAmbient.Require() throws +InvalidOperationException); GetIns soft-returns null when no scope is active so engine code +that patterns `GetIns()?.Foo ?? default` keeps composing (per design — strict for setters / +load-bearing reads, soft for the GetIns null-tolerant chain). Supersedes the intermediate +.ambient-isforecast-israndomdraw + .ambient-main-getins patches (kept for the audit trail). + +Semantics chosen per accessor: + - IsForecast : get/set both BattleAmbient.Require() — strict (must be in a scope) + - IsRandomDraw : get/set both BattleAmbient.Require() — strict + - GetIns() : Current?.Mgr — soft (null when no scope; preserves engine `?.` patterns) + - ctor : `_mainFallback = this` line deleted entirely + - DisposeManager: `_mainFallback = null` line deleted entirely + +In-file edits: + - line ~413 declaration `private static BattleManagerBase _mainFallback;` DELETED + - lines ~415-432 the four fallback statics + their if-else fallback branches DELETED, + replaced with strict get/set via BattleAmbient.Require() + - line ~727 GetIns() body -> Current?.Mgr (no fallback) + - line ~732 ctor `_mainFallback = this` DELETED (no longer needed; ambient.Mgr is published + explicitly from SessionBattleEngine.SetupInternal after construction) + - line ~1491 dispose `_mainFallback = null` DELETED (the matching scope drop replaces it) + +--- Engine/BattleManagerBase.cs (~lines 413-433) +- private static BattleManagerBase _mainFallback; +- +- private static bool _isRandomDrawFallback = false; +- public static bool IsRandomDraw { +- get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.IsRandomDraw ?? _isRandomDrawFallback; +- set { +- var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; +- if (c != null) c.IsRandomDraw = value; +- else _isRandomDrawFallback = value; +- } +- } +- +- private static bool _isForecastFallback = false; +- public static bool IsForecast { +- get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.IsForecast ?? _isForecastFallback; +- set { +- var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; +- if (c != null) c.IsForecast = value; +- else _isForecastFallback = value; +- } +- } ++ public static bool IsRandomDraw { ++ get => SVSim.BattleEngine.Ambient.BattleAmbient.Require().IsRandomDraw; ++ set => SVSim.BattleEngine.Ambient.BattleAmbient.Require().IsRandomDraw = value; ++ } ++ ++ public static bool IsForecast { ++ get => SVSim.BattleEngine.Ambient.BattleAmbient.Require().IsForecast; ++ set => SVSim.BattleEngine.Ambient.BattleAmbient.Require().IsForecast = value; ++ } + +--- Engine/BattleManagerBase.cs (~line 727) + public static BattleManagerBase GetIns() + { +- return SVSim.BattleEngine.Ambient.BattleAmbient.Current?.Mgr ?? _mainFallback; ++ // Soft read: returns null when no scope is active, preserving engine call sites that ++ // pattern `GetIns()?.Foo ?? default`. Inside a scope, returns the scope's Mgr (which may ++ // itself be null mid-Setup — see SessionBattleEngine.SetupInternal, which publishes Mgr ++ // before WireMulliganPhase). ++ return SVSim.BattleEngine.Ambient.BattleAmbient.Current?.Mgr; + } + +--- Engine/BattleManagerBase.cs (~line 732) + protected BattleManagerBase(IBattleMgrContentsCreator contentsCreator) + { +- _mainFallback = this; + BattleLifeTimeSharedObject = new BattleLifeTimeSharedObject(); + +--- Engine/BattleManagerBase.cs (~line 1491) + GameMgr.GetIns().GetPrefabMgr().AllUnLoad(); +- _mainFallback = null; + } diff --git a/SVSim.BattleEngine/Patches/Certification.ambient-fallback-deletion.patch b/SVSim.BattleEngine/Patches/Certification.ambient-fallback-deletion.patch new file mode 100644 index 0000000..d95145d --- /dev/null +++ b/SVSim.BattleEngine/Patches/Certification.ambient-fallback-deletion.patch @@ -0,0 +1,73 @@ +Multi-instancing migration (Step 8 — forcing function): delete the static viewer-id fallback the +earlier .ambient-viewerid patch added as a coexistence shim. ViewerId is now strictly per-session +ambient (BattleAmbient.Require().ViewerId). Supersedes the intermediate .ambient-viewerid patch +(kept for the audit trail). + +Semantics: + - get : BattleAmbient.Require().ViewerId — strict (throws when no scope is active) + - set : NO-OP. The historical caller (SavedataManager.SetInt + Save) is the client process's + persistent-storage path; in the headless multi-instance world the viewer id flows from the + session ctx (BattleAmbientContext.ViewerId, init-only at scope entry — see + SessionBattleEngine's field initializer, which seeds it from EngineGlobalInit.ThisViewerId). + Making the setter strict (Require()) would also work but degrades to a write that can't land + (init-only) — a comment-documented no-op is clearer about why the setter went dead. + +Related (SVSim.BattleNode/Sessions/Engine/EngineGlobalInit.cs): the reflection write that seeded +`Certification._viewerIdFallback` is now dead — the static is deleted. Replaced with a comment- +only marker so the design intent (ThisViewerId defines the engine's "player" perspective for the +IsRecovery target parse) stays discoverable from the global init path. + +InitializeFileds(): the now-dead `_viewerIdFallback = 0;` line is also deleted (the line above and +below — SavedataManager.SetInt("VIEWER_ID", 0) — stay). + +In-file edits: + - line ~19 declaration `private static int _viewerIdFallback;` DELETED + - lines ~41-65 the ambient+fallback get/set body REPLACED with strict get + no-op set + - line ~155 `_viewerIdFallback = 0;` inside InitializeFileds DELETED + +--- Engine/Cute/Certification.cs (~line 19) + private static string udid; +- +- private static int _viewerIdFallback; +- + private static int short_udid; + +--- Engine/Cute/Certification.cs (~lines 41-65) + public static int ViewerId + { +- get +- { +- var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; +- if (c != null) return c.ViewerId; +- if (_viewerIdFallback == 0) +- { +- _viewerIdFallback = Toolbox.SavedataManager.GetInt("VIEWER_ID"); +- } +- return _viewerIdFallback; +- } +- set +- { +- var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; +- if (c != null) +- { +- // Inside a scope, ViewerId is fixed at scope entry — swallow the write. +- return; +- } +- Toolbox.SavedataManager.SetInt("VIEWER_ID", value); +- Toolbox.SavedataManager.Save(); +- _viewerIdFallback = value; +- } ++ // Post-Task-8: strictly ambient. The historical SavedataManager-backed lazy decode was the ++ // client process's single-viewer-id source; in the headless multi-instance world the viewer ++ // id MUST come from the per-session ambient context. Setter is a no-op (BattleAmbientContext ++ // .ViewerId is `init`-only — fixed at scope entry per design — and the historical caller ++ // (SavedataManager.SetInt + Save) is dead in the server world). ++ get => SVSim.BattleEngine.Ambient.BattleAmbient.Require().ViewerId; ++ set { /* ambient ViewerId is init-only; SavedataManager path is dead headless */ } + } + +--- Engine/Cute/Certification.cs (~line 155) + sessionId = null; + udid = null; +- _viewerIdFallback = 0; + short_udid = 0; diff --git a/SVSim.BattleEngine/Patches/Data.ambient-fallback-deletion.patch b/SVSim.BattleEngine/Patches/Data.ambient-fallback-deletion.patch new file mode 100644 index 0000000..9ba30e0 --- /dev/null +++ b/SVSim.BattleEngine/Patches/Data.ambient-fallback-deletion.patch @@ -0,0 +1,43 @@ +Multi-instancing migration (Step 8 — forcing function): delete the static BattleRecoveryInfo +fallback the earlier .ambient-recoveryinfo patch added as a coexistence shim. BattleRecoveryInfo +is now strictly per-session ambient. Supersedes the intermediate .ambient-recoveryinfo patch +(kept for the audit trail). + +Semantics: + - BattleRecoveryInfo (getter) : Current?.RecoveryInfo — soft (null when no scope). The + MulliganMgrBase.StartDeal call site reads this with a null-tolerant pattern, so a null + degrade is the historical fallback. Inside a scope, returns the per-session RecoveryInfo + (SessionBattleEngine pre-seeds an uninitialized BattleRecoveryInfo on its ctx field + initializer). + - BattleRecoveryInfo (setter) : BattleAmbient.Require().RecoveryInfo = value — strict + (writes must land on the per-session ctx; forcing function). + +Related (SVSim.BattleNode/Sessions/Engine/EngineGlobalInit.cs): the dead `Data.BattleRecoveryInfo += ...` write inside the `_done` lock block is also deleted (flagged by Task 7 reviewer). The +per-session ctx pre-seeds RecoveryInfo, so the process-global write was dead the moment the +ambient seam landed. + +In-file edits: + - line ~179 declaration `private static BattleRecoveryInfo _battleRecoveryInfoFallback;` DELETED + - lines ~180-189 the ambient+fallback get/set body REPLACED with strict semantics + +--- Engine/Wizard/Data.cs (~lines 179-189) +- private static BattleRecoveryInfo _battleRecoveryInfoFallback; + public static BattleRecoveryInfo BattleRecoveryInfo + { +- get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.RecoveryInfo ?? _battleRecoveryInfoFallback; +- set +- { +- var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; +- if (c != null) c.RecoveryInfo = value; +- else _battleRecoveryInfoFallback = value; +- } ++ // Soft read: returns null when no scope is active. The MulliganMgrBase.StartDeal call site ++ // reads this with a null-tolerant ??=-style pattern, so a null degrade is the historical ++ // fallback. Inside a scope, returns the per-session RecoveryInfo (SessionBattleEngine ++ // pre-seeds an uninitialized BattleRecoveryInfo on its ctx field initializer). ++ get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.RecoveryInfo; ++ // Strict setter: writes must land on the per-session ctx. No historical production caller ++ // writes this outside a scope; an unwrapped write now fails fast (forcing function). ++ set => SVSim.BattleEngine.Ambient.BattleAmbient.Require().RecoveryInfo = value; + } diff --git a/SVSim.BattleEngine/Patches/ToolboxGame.ambient-fallback-deletion.patch b/SVSim.BattleEngine/Patches/ToolboxGame.ambient-fallback-deletion.patch new file mode 100644 index 0000000..783d377 --- /dev/null +++ b/SVSim.BattleEngine/Patches/ToolboxGame.ambient-fallback-deletion.patch @@ -0,0 +1,57 @@ +Multi-instancing migration (Step 8 — forcing function): delete the static RealTimeNetworkAgent +fallback the earlier .ambient-rtna patch added as a coexistence shim. RealTimeNetworkAgent is now +strictly per-session ambient. Supersedes the intermediate .ambient-rtna patch (kept for the audit +trail). + +Semantics: + - RealTimeNetworkAgent (getter) : Current?.NetworkAgent — soft (null when no scope). + Engine code reads this via `ToolboxGame.RealTimeNetworkAgent?.Foo`-style patterns (network + send paths gated on a non-null agent), so a null on the unwrapped path is the correct + degrade — not a throw. + - SetRealTimeNetworkBattle : BattleAmbient.Require().NetworkAgent = agent — strict (writes must + land on the per-session ctx; forcing function). + - DestroyNetworkAgent : reads Current, destroys, nulls the ambient NetworkAgent in-place. + Outside a scope is a no-op (nothing to destroy in the ambient world). + +In-file edits: + - line ~28 declaration `private static RealTimeNetworkAgent _realTimeNetworkAgentFallback;` DELETED + - lines ~29-32 getter body no fallback chain + - lines ~61-66 Set body Require() (strict) + - lines ~68-78 Destroy body Current-based; no fallback branch + +--- Engine/Wizard/ToolboxGame.cs (~lines 28-32) +- private static RealTimeNetworkAgent _realTimeNetworkAgentFallback; + public static RealTimeNetworkAgent RealTimeNetworkAgent + { +- get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.NetworkAgent ?? _realTimeNetworkAgentFallback; ++ // Soft read: returns null when no scope is active, mirroring GetIns. Engine code reads ++ // this via `ToolboxGame.RealTimeNetworkAgent?.Foo`-style patterns (network-send paths ++ // gated on a non-null agent), so a null on the unwrapped path is the correct degrade — ++ // not a throw. ++ get => SVSim.BattleEngine.Ambient.BattleAmbient.Current?.NetworkAgent; + } + +--- Engine/Wizard/ToolboxGame.cs (~lines 61-78) + public static void SetRealTimeNetworkBattle(RealTimeNetworkAgent agent) + { +- var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; +- if (c != null) c.NetworkAgent = agent; +- else _realTimeNetworkAgentFallback = agent; ++ // Strict: must be inside a scope to set the per-session agent. Forcing-function — any ++ // historical unwrapped caller (no production callsite remains) now fails fast. ++ SVSim.BattleEngine.Ambient.BattleAmbient.Require().NetworkAgent = agent; + } + + public static void DestroyNetworkAgent() + { + var c = SVSim.BattleEngine.Ambient.BattleAmbient.Current; +- var current = c?.NetworkAgent ?? _realTimeNetworkAgentFallback; ++ var current = c?.NetworkAgent; + if (current != null) + { + Object.DestroyImmediate(current.gameObject); +- if (c != null) c.NetworkAgent = null; +- else _realTimeNetworkAgentFallback = null; ++ c.NetworkAgent = null; + } + } diff --git a/SVSim.BattleNode/Sessions/Engine/EngineGlobalInit.cs b/SVSim.BattleNode/Sessions/Engine/EngineGlobalInit.cs index 656ed2b..09fb77c 100644 --- a/SVSim.BattleNode/Sessions/Engine/EngineGlobalInit.cs +++ b/SVSim.BattleNode/Sessions/Engine/EngineGlobalInit.cs @@ -89,14 +89,11 @@ internal static class EngineGlobalInit // Suppress VFX / take the virtual-battle resolution path (no live view layer). BattleManagerBase.IsForecast = true; - // The receive-conductor deal path runs under IsRecovery (SessionBattleEngine sets it after - // construction) and reads Data.BattleRecoveryInfo.IsMulliganEnd in MulliganMgrBase.StartDeal - // (line 43) — null by default -> NRE. Seed a no-op instance with IsMulliganEnd=false (the - // default) so the deal returns its real parallel VFX rather than the mulligan-end short - // circuit. GetUninitializedObject skips the JsonData ctor. Only when absent (coexistence). - if (Data.BattleRecoveryInfo == null) - Data.BattleRecoveryInfo = - (BattleRecoveryInfo)FormatterServices.GetUninitializedObject(typeof(BattleRecoveryInfo)); + // Post-Task-8: BattleRecoveryInfo lives on the per-session BattleAmbientContext (RecoveryInfo, + // pre-seeded with an uninitialized no-op instance in SessionBattleEngine's field initializer). + // Each session owns its own (IsMulliganEnd=false default), so the MulliganMgrBase.StartDeal + // read resolves per-session — the historical process-global Data.BattleRecoveryInfo write that + // lived here was dead the moment the ambient seam landed (reviewer-flagged in Task 7). // --- static CardMaster (full cards.json) ---------------------------------------------- // ALWAYS rebuild + re-inject the FULL master. We must not defer to a possibly-thin @@ -124,20 +121,14 @@ internal static class EngineGlobalInit udidField.SetValue(null, "headless-udid"); // --- Cute.Certification.viewer_id ------------------------------------------------------ - // The IsRecovery target parse (NetworkBattleReceiver.CreateTargetList, isWatch branch) derives - // a target's owner from `vid != PlayerStaticData.UserViewerID`, where - // UserViewerID => Certification.ViewerId, whose getter LAZILY reads - // Toolbox.SavedataManager.GetInt("VIEWER_ID") when its backing field is 0 — and that savedata - // read throws headless. The exception is SWALLOWED by ConvertReceiveDataToMakeData's blanket - // catch, which (with the node's checkBreakData:false ingest) silently drops the parsed - // targetList, leaving an attack/evolve with an EMPTY target list -> the action throws on - // targetList[0]. Seed the backing field with a stable nonzero id so the getter short-circuits. - // It defines the engine's "player" perspective: a target vid == ThisViewerId resolves on - // BattlePlayer (engine seat A), vid != it on BattleEnemy (seat B). Only set when 0 (coexistence). - var viewerIdField = typeof(Certification).GetField("_viewerIdFallback", - BindingFlags.Static | BindingFlags.NonPublic)!; - if ((int)(viewerIdField.GetValue(null) ?? 0) == 0) - viewerIdField.SetValue(null, ThisViewerId); + // Post-Task-8: viewer id lives on the per-session BattleAmbientContext (ViewerId, init-only). + // SessionBattleEngine seeds it from ThisViewerId in its field initializer and enters the + // ambient on every Setup/Receive — so the engine's Certification.ViewerId getter + // (BattleAmbient.Require().ViewerId) always resolves cleanly when the engine code is + // reached. The reflection write that used to seed `Certification._viewerIdFallback` is now + // dead (the static is deleted). Left as a comment-only marker so the design intent (this + // viewer id defines the engine's "player" perspective for the IsRecovery target parse) + // stays discoverable from the global init path. _done = true; } diff --git a/SVSim.BattleNode/Sessions/Engine/SessionBattleEngine.cs b/SVSim.BattleNode/Sessions/Engine/SessionBattleEngine.cs index 625526e..7589e08 100644 --- a/SVSim.BattleNode/Sessions/Engine/SessionBattleEngine.cs +++ b/SVSim.BattleNode/Sessions/Engine/SessionBattleEngine.cs @@ -173,10 +173,16 @@ internal sealed class SessionBattleEngine SeedDeck(mgr, seatADeck, isPlayer: true); SeedDeck(mgr, seatBDeck, isPlayer: false); - WireMulliganPhase(mgr); // wire OperateReceive.OnReceiveDeal -> StartDeal (deal seats the hand) - + // Publish the mgr on the per-session ambient BEFORE wiring the mulligan phase: that ctor + // chains into MulliganInfoControl.InitMulliganInfo, which reads BattleManagerBase.GetIns() + // (MulliganInfoControl.cs:259). With the fallback gone (Task 8), an unset ambient.Mgr would + // resolve to null and NRE on the very next field read. Set ambient.Mgr here so the wiring + // resolves the per-session mgr cleanly. _mgr = mgr; _ctx.Mgr = _mgr; + + WireMulliganPhase(mgr); // wire OperateReceive.OnReceiveDeal -> StartDeal (deal seats the hand) + // Use the mgr's OWN receiver — the ctor already wired it to the mgr's OperateReceive + // NetworkBattleData (NetworkBattleManagerBase.cs:266, non-recovery branch). This is the same // receiver the engine's RecoveryDataHandler drives when replaying recorded frames.