fix(battle-node): preserve long type on numeric array elements in FromJson
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<long> 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 <noreply@anthropic.com>
This commit is contained in:
@@ -89,7 +89,14 @@ public sealed record MsgEnvelope(
|
|||||||
private static object? ToObject(JsonElement el) => el.ValueKind switch
|
private static object? ToObject(JsonElement el) => el.ValueKind switch
|
||||||
{
|
{
|
||||||
JsonValueKind.String => el.GetString(),
|
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<long> 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.True => true,
|
||||||
JsonValueKind.False => false,
|
JsonValueKind.False => false,
|
||||||
JsonValueKind.Null => null,
|
JsonValueKind.Null => null,
|
||||||
@@ -97,4 +104,10 @@ public sealed record MsgEnvelope(
|
|||||||
JsonValueKind.Object => el.EnumerateObject().ToDictionary(p => p.Name, p => ToObject(p.Value)),
|
JsonValueKind.Object => el.EnumerateObject().ToDictionary(p => p.Name, p => ToObject(p.Value)),
|
||||||
_ => el.GetRawText(),
|
_ => el.GetRawText(),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
private static object ParseNumber(JsonElement el)
|
||||||
|
{
|
||||||
|
if (el.TryGetInt64(out var l)) return l;
|
||||||
|
return el.GetDouble();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -266,8 +266,26 @@ public sealed class BattleSession
|
|||||||
|
|
||||||
private static IReadOnlyList<long> ExtractIdxList(MsgEnvelope env)
|
private static IReadOnlyList<long> ExtractIdxList(MsgEnvelope env)
|
||||||
{
|
{
|
||||||
if (env.Body.TryGetValue("idxList", out var raw) && raw is List<object?> lst)
|
// Defensive: accept any IEnumerable carrying any numeric boxing (long/int/double/decimal/
|
||||||
return lst.OfType<long>().ToList();
|
// string). MsgEnvelope.FromJson should box small ints as long, but a parser quirk
|
||||||
|
// anywhere upstream could yield a different boxed type and OfType<long> 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<long>();
|
||||||
|
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<long>();
|
return Array.Empty<long>();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -7,6 +7,26 @@ namespace SVSim.UnitTests.BattleNode.Protocol;
|
|||||||
[TestFixture]
|
[TestFixture]
|
||||||
public class MsgEnvelopeTests
|
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<long> 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<object?>)env.Body["idxList"]!;
|
||||||
|
Assert.That(idxList.Count, Is.EqualTo(2));
|
||||||
|
Assert.That(idxList[0], Is.TypeOf<long>(), "idxList[0] must be boxed long, not double.");
|
||||||
|
Assert.That(idxList[0], Is.EqualTo(2L));
|
||||||
|
Assert.That(idxList[1], Is.TypeOf<long>());
|
||||||
|
Assert.That(idxList[1], Is.EqualTo(3L));
|
||||||
|
}
|
||||||
|
|
||||||
[Test]
|
[Test]
|
||||||
public void Roundtrip_PreservesEnvelopeAndBody()
|
public void Roundtrip_PreservesEnvelopeAndBody()
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -51,6 +51,27 @@ public class BattleSessionDispatchTests
|
|||||||
Assert.That(s.Phase, Is.EqualTo(BattleSessionPhase.AwaitingSwap));
|
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<object?> of boxed long values).
|
||||||
|
swapEnv.Body["idxList"] = new List<object?> { 2L };
|
||||||
|
|
||||||
|
var responses = s.ComputeResponses(swapEnv);
|
||||||
|
|
||||||
|
var swapBody = responses[0].Envelope.Body;
|
||||||
|
var self = (List<object?>)swapBody["self"]!;
|
||||||
|
Assert.That(((Dictionary<string, object?>)self[0]!)["idx"], Is.EqualTo(1));
|
||||||
|
Assert.That(((Dictionary<string, object?>)self[1]!)["idx"], Is.EqualTo(4)); // swapped — fresh deck idx
|
||||||
|
Assert.That(((Dictionary<string, object?>)self[2]!)["idx"], Is.EqualTo(3));
|
||||||
|
}
|
||||||
|
|
||||||
[Test]
|
[Test]
|
||||||
public void Swap_PushesSwapResponseThenReady_TransitionsToAfterReady()
|
public void Swap_PushesSwapResponseThenReady_TransitionsToAfterReady()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user