From 9e8ebd1b2b469ebbc9a45f4d68472ec2474f43a9 Mon Sep 17 00:00:00 2001 From: gamer147 Date: Mon, 1 Jun 2026 08:40:50 -0400 Subject: [PATCH] fix(battle-node): preserve long type on numeric array elements in FromJson MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause for the lingering mulligan failure: the inline conditional expression in MsgEnvelope.ToObject JsonValueKind.Number => el.TryGetInt64(out var l) ? l : el.GetDouble(), unified its branches to the common implicit-convertible type. long→double is implicit, so both branches collapsed to double and the integer value silently widened. Inside an array (idxList:[2]), each element came back as boxed double; OfType in ExtractIdxList then filtered every entry out, so swapIndices arrived empty and BuildSwapResponse echoed the unchanged hand — exactly the diff-against-Deal mismatch the client flagged as "Card swap failed: AbandonCards[2]/DrawCards[]". Extract a ParseNumber helper that returns object explicitly so each branch boxes its own runtime type. Also harden ExtractIdxList to accept any boxed numeric type (long/int/double/decimal/string) so a future JSON-parser drift can't silently regress this path again. Two regression tests: - FromJson_NumericArray_PreservesLongTypeOnEachElement: confirms the fix at the JSON-parse layer with a hardcoded "{\"idxList\":[2,3]}". - Swap_WithIdxListContainingTwo_ProducesHandWithFreshIdxAtPosition1: exercises the dispatch end-to-end with a Body holding a real boxed long; asserts position 1 of the response hand is the fresh deck idx 4. Co-Authored-By: Claude Opus 4.7 --- SVSim.BattleNode/Protocol/MsgEnvelope.cs | 15 ++++++++++++- SVSim.BattleNode/Sessions/BattleSession.cs | 22 +++++++++++++++++-- .../BattleNode/Protocol/MsgEnvelopeTests.cs | 20 +++++++++++++++++ .../Sessions/BattleSessionDispatchTests.cs | 21 ++++++++++++++++++ 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/SVSim.BattleNode/Protocol/MsgEnvelope.cs b/SVSim.BattleNode/Protocol/MsgEnvelope.cs index a4bdc9c..bfe25df 100644 --- a/SVSim.BattleNode/Protocol/MsgEnvelope.cs +++ b/SVSim.BattleNode/Protocol/MsgEnvelope.cs @@ -89,7 +89,14 @@ public sealed record MsgEnvelope( private static object? ToObject(JsonElement el) => el.ValueKind switch { JsonValueKind.String => el.GetString(), - JsonValueKind.Number => el.TryGetInt64(out var l) ? l : el.GetDouble(), + // Extracted to a helper because writing the conditional inline as + // el.TryGetInt64(out var l) ? l : el.GetDouble() + // unifies the conditional's branches to the common implicit-convertible type. long→double + // is implicit; so the result type collapses to double and the long value silently widens. + // Downstream OfType filters then drop the (now boxed-double) entries, which broke + // the mulligan idxList extraction. Separate method returns object explicitly so each + // branch boxes its own runtime type. + JsonValueKind.Number => ParseNumber(el), JsonValueKind.True => true, JsonValueKind.False => false, JsonValueKind.Null => null, @@ -97,4 +104,10 @@ public sealed record MsgEnvelope( JsonValueKind.Object => el.EnumerateObject().ToDictionary(p => p.Name, p => ToObject(p.Value)), _ => el.GetRawText(), }; + + private static object ParseNumber(JsonElement el) + { + if (el.TryGetInt64(out var l)) return l; + return el.GetDouble(); + } } diff --git a/SVSim.BattleNode/Sessions/BattleSession.cs b/SVSim.BattleNode/Sessions/BattleSession.cs index 5ccb61c..4e0c736 100644 --- a/SVSim.BattleNode/Sessions/BattleSession.cs +++ b/SVSim.BattleNode/Sessions/BattleSession.cs @@ -266,8 +266,26 @@ public sealed class BattleSession private static IReadOnlyList ExtractIdxList(MsgEnvelope env) { - if (env.Body.TryGetValue("idxList", out var raw) && raw is List lst) - return lst.OfType().ToList(); + // Defensive: accept any IEnumerable carrying any numeric boxing (long/int/double/decimal/ + // string). MsgEnvelope.FromJson should box small ints as long, but a parser quirk + // anywhere upstream could yield a different boxed type and OfType would silently + // drop the entries — that broke the v1 mulligan during smoke. + if (env.Body.TryGetValue("idxList", out var raw) && raw is System.Collections.IEnumerable seq && raw is not string) + { + var result = new List(); + foreach (var item in seq) + { + switch (item) + { + case long l: result.Add(l); break; + case int i: result.Add(i); break; + case double d: result.Add((long)d); break; + case decimal m: result.Add((long)m); break; + case string s when long.TryParse(s, out var p): result.Add(p); break; + } + } + return result; + } return Array.Empty(); } diff --git a/SVSim.UnitTests/BattleNode/Protocol/MsgEnvelopeTests.cs b/SVSim.UnitTests/BattleNode/Protocol/MsgEnvelopeTests.cs index a1d2212..5e53342 100644 --- a/SVSim.UnitTests/BattleNode/Protocol/MsgEnvelopeTests.cs +++ b/SVSim.UnitTests/BattleNode/Protocol/MsgEnvelopeTests.cs @@ -7,6 +7,26 @@ namespace SVSim.UnitTests.BattleNode.Protocol; [TestFixture] public class MsgEnvelopeTests { + [Test] + public void FromJson_NumericArray_PreservesLongTypeOnEachElement() + { + // Regression for the v1 mulligan bug: idxList:[2] from the wire was decoded as a list + // containing a boxed double (2.0) instead of a boxed long (2L). The conditional expression + // `el.TryGetInt64(out var l) ? l : el.GetDouble()` unified its branches to the common + // implicit type (double) and silently widened the long. Downstream OfType filtered + // every entry out, so swapIndices arrived empty and the server echoed the unchanged hand. + const string json = "{\"uri\":\"Swap\",\"viewerId\":1,\"uuid\":\"u\",\"try\":0,\"cat\":1,\"idxList\":[2,3]}"; + + var env = MsgEnvelope.FromJson(json); + + var idxList = (List)env.Body["idxList"]!; + Assert.That(idxList.Count, Is.EqualTo(2)); + Assert.That(idxList[0], Is.TypeOf(), "idxList[0] must be boxed long, not double."); + Assert.That(idxList[0], Is.EqualTo(2L)); + Assert.That(idxList[1], Is.TypeOf()); + Assert.That(idxList[1], Is.EqualTo(3L)); + } + [Test] public void Roundtrip_PreservesEnvelopeAndBody() { diff --git a/SVSim.UnitTests/BattleNode/Sessions/BattleSessionDispatchTests.cs b/SVSim.UnitTests/BattleNode/Sessions/BattleSessionDispatchTests.cs index a33900c..4fff4eb 100644 --- a/SVSim.UnitTests/BattleNode/Sessions/BattleSessionDispatchTests.cs +++ b/SVSim.UnitTests/BattleNode/Sessions/BattleSessionDispatchTests.cs @@ -51,6 +51,27 @@ public class BattleSessionDispatchTests Assert.That(s.Phase, Is.EqualTo(BattleSessionPhase.AwaitingSwap)); } + [Test] + public void Swap_WithIdxListContainingTwo_ProducesHandWithFreshIdxAtPosition1() + { + var s = NewSession(); + s.ComputeResponses(NewEnvelope(NetworkBattleUri.InitNetwork)); + s.ComputeResponses(NewEnvelope(NetworkBattleUri.InitBattle)); + s.ComputeResponses(NewEnvelope(NetworkBattleUri.Loaded)); + var swapEnv = NewEnvelope(NetworkBattleUri.Swap); + // Simulate the client's Swap{idxList:[2]}: the dict shape produced by MsgEnvelope.FromJson + // (a List of boxed long values). + swapEnv.Body["idxList"] = new List { 2L }; + + var responses = s.ComputeResponses(swapEnv); + + var swapBody = responses[0].Envelope.Body; + var self = (List)swapBody["self"]!; + Assert.That(((Dictionary)self[0]!)["idx"], Is.EqualTo(1)); + Assert.That(((Dictionary)self[1]!)["idx"], Is.EqualTo(4)); // swapped — fresh deck idx + Assert.That(((Dictionary)self[2]!)["idx"], Is.EqualTo(3)); + } + [Test] public void Swap_PushesSwapResponseThenReady_TransitionsToAfterReady() {