From e4691d616b74a256c056e227d4ee5d621392a88c Mon Sep 17 00:00:00 2001 From: gamer147 Date: Mon, 1 Jun 2026 10:53:51 -0400 Subject: [PATCH] fix(battle-node): emit envelope keys before body keys in MsgEnvelope.ToJson MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Client RealTimeNetworkAgent.SetNetworkInfo iterates the synchronize-data dict in insertion order. The "uri" key, when recognized as Matched, calls GameMgr.InitializeSelfInfo which sets _selfDeck = null. Any "selfDeck" processed before "uri" gets wiped; Matching.StartBattleLoad then crashes on null.Select(...). Pre-refactor ToJson built a Dictionary envelope-first then appended body keys, so the bug never surfaced. The typed-body rewrite inverted the order — restoring envelope-first matches the prod wire. Regression test BuildMatched_KeyOrder_PutsUriBeforeSelfDeckAndSelfInfo locks the contract. Co-Authored-By: Claude Opus 4.7 --- SVSim.BattleNode/Protocol/MsgEnvelope.cs | 40 ++++++++++++------- .../Lifecycle/TypedBodyWireShapeTests.cs | 25 ++++++++++++ 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/SVSim.BattleNode/Protocol/MsgEnvelope.cs b/SVSim.BattleNode/Protocol/MsgEnvelope.cs index d5398b4..3e89738 100644 --- a/SVSim.BattleNode/Protocol/MsgEnvelope.cs +++ b/SVSim.BattleNode/Protocol/MsgEnvelope.cs @@ -42,12 +42,24 @@ public sealed record MsgEnvelope( public static string ToJson(MsgEnvelope env) { - JsonObject result; + // Envelope fields MUST come before body fields on the wire. The client's + // RealTimeNetworkAgent.SetNetworkInfo iterates the dict in insertion order and + // clears _selfDeck on the "uri" key (via GameMgr.InitializeSelfInfo). Any body + // field processed before "uri" is wiped before Matching.StartBattleLoad reads + // it back. The prod wire emits envelope keys first; we must too. + var result = new JsonObject(); + result["uri"] = env.Uri.ToString(); + result["viewerId"] = env.ViewerId; + result["uuid"] = env.Uuid; + result["try"] = env.Try; + result["cat"] = (int)env.Cat; + if (env.Bid is not null) result["bid"] = env.Bid; + if (env.PubSeq.HasValue) result["pubSeq"] = env.PubSeq.Value; + if (env.PlaySeq.HasValue) result["playSeq"] = env.PlaySeq.Value; + if (env.Body is RawBody raw) { - // Inbound-echo path: flatten Entries to top-level keys, same as before - // the typed-body refactor. - result = new JsonObject(); + // Inbound-echo path: flatten Entries to top-level keys. foreach (var (k, v) in raw.Entries) { if (ReservedEnvelopeKeys.Contains(k)) @@ -60,19 +72,17 @@ public sealed record MsgEnvelope( } else { - // Typed body: serialize via [JsonPropertyName] attributes on the record. - result = (JsonObject)JsonSerializer.SerializeToNode(env.Body, env.Body.GetType(), Options)!; + // Typed body: serialize via [JsonPropertyName] attributes on the record, + // then layer each field onto `result` after the envelope keys. DeepClone + // because S.T.Json JsonNodes can only have one parent; reassigning a node + // owned by `bodyNode` to `result` would throw without the clone. + var bodyNode = (JsonObject)JsonSerializer.SerializeToNode(env.Body, env.Body.GetType(), Options)!; + foreach (var prop in bodyNode) + { + result[prop.Key] = prop.Value?.DeepClone(); + } } - result["uri"] = env.Uri.ToString(); - result["viewerId"] = env.ViewerId; - result["uuid"] = env.Uuid; - result["try"] = env.Try; - result["cat"] = (int)env.Cat; - if (env.Bid is not null) result["bid"] = env.Bid; - if (env.PubSeq.HasValue) result["pubSeq"] = env.PubSeq.Value; - if (env.PlaySeq.HasValue) result["playSeq"] = env.PlaySeq.Value; - return result.ToJsonString(Options); } diff --git a/SVSim.UnitTests/BattleNode/Lifecycle/TypedBodyWireShapeTests.cs b/SVSim.UnitTests/BattleNode/Lifecycle/TypedBodyWireShapeTests.cs index 3a63a51..e4fa8fc 100644 --- a/SVSim.UnitTests/BattleNode/Lifecycle/TypedBodyWireShapeTests.cs +++ b/SVSim.UnitTests/BattleNode/Lifecycle/TypedBodyWireShapeTests.cs @@ -16,6 +16,31 @@ namespace SVSim.UnitTests.BattleNode.Lifecycle; [TestFixture] public class TypedBodyWireShapeTests { + [Test] + public void BuildMatched_KeyOrder_PutsUriBeforeSelfDeckAndSelfInfo() + { + // Regression: the client's RealTimeNetworkAgent.SetNetworkInfo iterates the + // synchronize-data dict in insertion order. When it hits "uri" and recognizes + // "Matched", it calls GameMgr.InitializeSelfInfo() which sets _selfDeck = null. + // Any "selfDeck" / "selfInfo" key processed BEFORE "uri" is wiped before + // Matching.StartBattleLoad reads it back, and GetSelfDeck().Select(...) crashes + // with "Value cannot be null. Parameter name: source". The prod wire format + // emits envelope keys (uri first) before body keys; we must too. + var env = ScriptedLifecycle.BuildMatched( + playerViewerId: 1, opponentViewerId: 2, battleId: "b"); + var json = MsgEnvelope.ToJson(env); + + var uriIdx = json.IndexOf("\"uri\":", StringComparison.Ordinal); + var selfDeckIdx = json.IndexOf("\"selfDeck\":", StringComparison.Ordinal); + var selfInfoIdx = json.IndexOf("\"selfInfo\":", StringComparison.Ordinal); + + Assert.That(uriIdx, Is.GreaterThan(-1), "uri must be present"); + Assert.That(selfDeckIdx, Is.GreaterThan(-1), "selfDeck must be present"); + Assert.That(selfInfoIdx, Is.GreaterThan(-1), "selfInfo must be present"); + Assert.That(uriIdx, Is.LessThan(selfDeckIdx), "uri must appear BEFORE selfDeck"); + Assert.That(uriIdx, Is.LessThan(selfInfoIdx), "uri must appear BEFORE selfInfo"); + } + [Test] public void BuildMatched_SerializesAllWireKeysExpectedByTheClient() {