refactor(battlenode): low-churn §B/§D/§E/§F quality cleanups
Behavior-preserving; 231 BattleNode tests green. - §D: MsgEnvelope.Try -> RetryAttempt (drops keyword-escape; wire key stays "try"); SocketIoFrame.AckResponse arg -> pubSeqEcho. - §B: Gungnir.EmitInterval -> BattleNodeOptions.AliveEmitInterval (unused literal moved to its config home); deck-idx 4L -> InitialHand.Length + 1. - §E: shared Wire.WireJsonOptions.CamelCase replaces the duplicated camelCase JsonSerializerOptions in EngineIoHandshake and MsgEnvelope. - §F: do-NOT-consistency-fix polarity notes on TurnEndFinalHandler (From wins) and RetireKillHandler (From loses). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -16,6 +16,13 @@ public sealed class BattleNodeOptions
|
||||
/// </summary>
|
||||
public TimeSpan WaitingRoomTimeout { get; set; } = TimeSpan.FromSeconds(60);
|
||||
|
||||
/// <summary>
|
||||
/// Cadence of the server→client alive ("Gungnir") keepalive emit. The driving timer/loop
|
||||
/// (to live on <see cref="Sessions.BattleSession"/>) is deferred in v1; this is its future
|
||||
/// home so the interval isn't a magic literal stranded on the <c>Gungnir</c> body factory.
|
||||
/// </summary>
|
||||
public TimeSpan AliveEmitInterval { get; set; } = TimeSpan.FromSeconds(5);
|
||||
|
||||
/// <summary>
|
||||
/// When true, <see cref="Sessions.Participants.RealParticipant"/> emits per-frame
|
||||
/// diagnostic logs at Information level: <c>[sio-in]</c> on every inbound msg/alive/hand
|
||||
|
||||
@@ -90,13 +90,14 @@ public static class ServerBattleFrames
|
||||
|
||||
/// <summary>
|
||||
/// Compute the player's hand after a mulligan. For every idx in <paramref name="swapIndices"/>
|
||||
/// that is currently in the hand, replace it with the next unused deck idx (starting at 4,
|
||||
/// since 1..3 were dealt). Positions of kept cards are preserved.
|
||||
/// that is currently in the hand, replace it with the next unused deck idx (the first idx past
|
||||
/// the opening hand — <see cref="InitialHand"/> is 1-based and contiguous, so that's
|
||||
/// <c>InitialHand.Length + 1</c>). Positions of kept cards are preserved.
|
||||
/// </summary>
|
||||
public static long[] ComputeHandAfterSwap(IReadOnlyList<long> swapIndices)
|
||||
{
|
||||
var hand = InitialHand.ToArray();
|
||||
var nextDeckIdx = 4L;
|
||||
var nextDeckIdx = (long)(InitialHand.Length + 1);
|
||||
for (var pos = 0; pos < hand.Length; pos++)
|
||||
{
|
||||
if (swapIndices.Contains(hand[pos]))
|
||||
@@ -151,7 +152,7 @@ public static class ServerBattleFrames
|
||||
ViewerId: FakeOpponentViewerId,
|
||||
Uuid: WireConstants.ServerUuid,
|
||||
Bid: bid,
|
||||
Try: 0,
|
||||
RetryAttempt: 0,
|
||||
Cat: EmitCategory.Battle,
|
||||
PubSeq: null,
|
||||
PlaySeq: null,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
using System.Text.Json;
|
||||
using System.Text.Json.Nodes;
|
||||
using System.Text.Json.Serialization;
|
||||
using SVSim.BattleNode.Wire;
|
||||
|
||||
namespace SVSim.BattleNode.Protocol;
|
||||
|
||||
@@ -14,32 +14,21 @@ public sealed record MsgEnvelope(
|
||||
long ViewerId,
|
||||
string Uuid,
|
||||
string? Bid,
|
||||
int Try,
|
||||
int RetryAttempt,
|
||||
EmitCategory Cat,
|
||||
long? PubSeq,
|
||||
long? PlaySeq,
|
||||
IMsgBody Body)
|
||||
{
|
||||
private static readonly JsonSerializerOptions Options = CreateOptions();
|
||||
// Bare-camelCase wire serialization, single-sourced in Wire.WireJsonOptions (shared with
|
||||
// EngineIoHandshake). Every wire key here is explicit via the manual ToJson layering below.
|
||||
private static readonly JsonSerializerOptions Options = WireJsonOptions.CamelCase;
|
||||
|
||||
private static readonly HashSet<string> ReservedEnvelopeKeys = new()
|
||||
{
|
||||
"uri", "viewerId", "uuid", "bid", "try", "cat", "pubSeq", "playSeq",
|
||||
};
|
||||
|
||||
private static JsonSerializerOptions CreateOptions()
|
||||
{
|
||||
var opt = new JsonSerializerOptions
|
||||
{
|
||||
// Wire-key casing is bare camelCase via per-field [JsonPropertyName] —
|
||||
// NOT EmulatedEntrypoint's snake_case policy. The naming-policy line
|
||||
// that was here previously was dead code (every wire key is explicit).
|
||||
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
|
||||
};
|
||||
opt.Converters.Add(new JsonStringEnumConverter());
|
||||
return opt;
|
||||
}
|
||||
|
||||
public static string ToJson(MsgEnvelope env)
|
||||
{
|
||||
// Envelope fields MUST come before body fields on the wire. The client's
|
||||
@@ -51,7 +40,7 @@ public sealed record MsgEnvelope(
|
||||
result["uri"] = env.Uri.ToString();
|
||||
result["viewerId"] = env.ViewerId;
|
||||
result["uuid"] = env.Uuid;
|
||||
result["try"] = env.Try;
|
||||
result["try"] = env.RetryAttempt;
|
||||
result["cat"] = (int)env.Cat;
|
||||
if (env.Bid is not null) result["bid"] = env.Bid;
|
||||
if (env.PubSeq.HasValue) result["pubSeq"] = env.PubSeq.Value;
|
||||
@@ -133,7 +122,7 @@ public sealed record MsgEnvelope(
|
||||
var viewerId = root.GetProperty("viewerId").GetInt64();
|
||||
var uuid = root.GetProperty("uuid").GetString()!;
|
||||
var bid = root.TryGetProperty("bid", out var bidEl) ? bidEl.GetString() : null;
|
||||
var @try = root.TryGetProperty("try", out var tryEl) ? tryEl.GetInt32() : 0;
|
||||
var retryAttempt = root.TryGetProperty("try", out var tryEl) ? tryEl.GetInt32() : 0;
|
||||
var cat = root.TryGetProperty("cat", out var catEl) ? (EmitCategory)catEl.GetInt32() : EmitCategory.Battle;
|
||||
var pubSeq = root.TryGetProperty("pubSeq", out var psEl) ? psEl.GetInt64() : (long?)null;
|
||||
var playSeq = root.TryGetProperty("playSeq", out var plsEl) ? plsEl.GetInt64() : (long?)null;
|
||||
@@ -145,7 +134,7 @@ public sealed record MsgEnvelope(
|
||||
bodyDict[prop.Name] = ToObject(prop.Value);
|
||||
}
|
||||
|
||||
return new MsgEnvelope(uri, viewerId, uuid, bid, @try, cat, pubSeq, playSeq, new RawBody(bodyDict));
|
||||
return new MsgEnvelope(uri, viewerId, uuid, bid, retryAttempt, cat, pubSeq, playSeq, new RawBody(bodyDict));
|
||||
}
|
||||
|
||||
private static object? ToObject(JsonElement el) => el.ValueKind switch
|
||||
|
||||
@@ -3,8 +3,9 @@ namespace SVSim.BattleNode.Reliability;
|
||||
/// <summary>
|
||||
/// Body builders for the alive channel ("Gungnir" is the client's codename for the
|
||||
/// keepalive/connection-status channel — see <see cref="Protocol.Bodies.AlivePushBody"/>).
|
||||
/// The timer/loop that drives <see cref="EmitInterval"/> emits lives on
|
||||
/// BattleSession; this class is just the pure body-shape factory.
|
||||
/// The timer/loop that would drive the emit cadence
|
||||
/// (<see cref="Bridge.BattleNodeOptions.AliveEmitInterval"/>) is to live on BattleSession;
|
||||
/// this class is just the pure body-shape factory.
|
||||
/// v1 always reports scs/ocs=ONLINE — real disconnect detection is deferred. The push
|
||||
/// body itself is constructed inline in BattleSession.HandleAliveEventAsync using
|
||||
/// AlivePushBody; only the emit body (sent by us TO the client on the alive channel,
|
||||
@@ -12,8 +13,6 @@ namespace SVSim.BattleNode.Reliability;
|
||||
/// </summary>
|
||||
public static class Gungnir
|
||||
{
|
||||
public static readonly TimeSpan EmitInterval = TimeSpan.FromSeconds(5);
|
||||
|
||||
public static Dictionary<string, object?> BuildAliveEmitBody(InboundTracker tracker) => new()
|
||||
{
|
||||
["currentSeq"] = tracker.HighWaterMark,
|
||||
|
||||
@@ -13,7 +13,7 @@ internal static class BattleFrames
|
||||
ViewerId: ServerBattleFrames.FakeOpponentViewerId,
|
||||
Uuid: WireConstants.ServerUuid,
|
||||
Bid: null,
|
||||
Try: 0,
|
||||
RetryAttempt: 0,
|
||||
Cat: EmitCategory.General,
|
||||
PubSeq: null,
|
||||
PlaySeq: null,
|
||||
@@ -24,7 +24,7 @@ internal static class BattleFrames
|
||||
ViewerId: ServerBattleFrames.FakeOpponentViewerId,
|
||||
Uuid: WireConstants.ServerUuid,
|
||||
Bid: null,
|
||||
Try: 0,
|
||||
RetryAttempt: 0,
|
||||
Cat: EmitCategory.Battle,
|
||||
PubSeq: null,
|
||||
PlaySeq: null,
|
||||
@@ -35,7 +35,7 @@ internal static class BattleFrames
|
||||
ViewerId: ServerBattleFrames.FakeOpponentViewerId,
|
||||
Uuid: WireConstants.ServerUuid,
|
||||
Bid: null,
|
||||
Try: 0,
|
||||
RetryAttempt: 0,
|
||||
Cat: EmitCategory.Battle,
|
||||
PubSeq: null,
|
||||
PlaySeq: null,
|
||||
@@ -46,7 +46,7 @@ internal static class BattleFrames
|
||||
ViewerId: ServerBattleFrames.FakeOpponentViewerId,
|
||||
Uuid: WireConstants.ServerUuid,
|
||||
Bid: null,
|
||||
Try: 0,
|
||||
RetryAttempt: 0,
|
||||
Cat: EmitCategory.Battle,
|
||||
PubSeq: null,
|
||||
PlaySeq: null,
|
||||
|
||||
@@ -7,6 +7,9 @@ internal sealed class RetireKillHandler : IFrameHandler
|
||||
public IReadOnlyList<DispatchRoute> Handle(FrameDispatchContext ctx)
|
||||
{
|
||||
ctx.State.SessionPhase = BattleSessionPhase.Terminal;
|
||||
// Polarity: the SENDER retired, so From LOSES / Other WINS. This is the OPPOSITE of
|
||||
// TurnEndFinalHandler (From WINS there — sender dealt the lethal). Intentional — do NOT
|
||||
// "consistency-fix" the two handlers to match; a swap here silently reverses every retire.
|
||||
return new[]
|
||||
{
|
||||
new DispatchRoute(ctx.From, BattleFrames.BuildBattleFinish(BattleResult.RetireLose), Stock.Bypass),
|
||||
|
||||
@@ -14,6 +14,10 @@ internal sealed class TurnEndFinalHandler : IFrameHandler
|
||||
if (ctx.SenderPhase == BattleSessionPhase.AfterReady)
|
||||
{
|
||||
ctx.State.SessionPhase = BattleSessionPhase.Terminal;
|
||||
// Polarity: the SENDER dealt the lethal, so From WINS / Other LOSES. This is the
|
||||
// OPPOSITE of RetireKillHandler (From LOSES there — retire is self-inflicted).
|
||||
// Intentional — do NOT "consistency-fix" the two handlers to match; a swap here
|
||||
// silently reverses every lethal-turn outcome.
|
||||
return new[]
|
||||
{
|
||||
new DispatchRoute(ctx.Other, ctx.Env, Stock.Normal),
|
||||
|
||||
@@ -404,7 +404,7 @@ public sealed class RealParticipant : IBattleParticipant, IHasHandshakePhase
|
||||
ViewerId: SVSim.BattleNode.Lifecycle.ServerBattleFrames.FakeOpponentViewerId,
|
||||
Uuid: WireConstants.ServerUuid,
|
||||
Bid: null,
|
||||
Try: 0,
|
||||
RetryAttempt: 0,
|
||||
Cat: EmitCategory.General,
|
||||
PubSeq: null,
|
||||
PlaySeq: null,
|
||||
|
||||
@@ -12,11 +12,5 @@ public sealed record EngineIoHandshake(
|
||||
[property: JsonPropertyName("pingInterval")] int PingInterval,
|
||||
[property: JsonPropertyName("pingTimeout")] int PingTimeout)
|
||||
{
|
||||
// Wire-key casing here is bare camelCase — NOT EmulatedEntrypoint's snake_case policy.
|
||||
private static readonly JsonSerializerOptions Options = new()
|
||||
{
|
||||
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
|
||||
};
|
||||
|
||||
public string ToJson() => JsonSerializer.Serialize(this, Options);
|
||||
public string ToJson() => JsonSerializer.Serialize(this, WireJsonOptions.CamelCase);
|
||||
}
|
||||
|
||||
@@ -158,10 +158,11 @@ public sealed class SocketIoFrame
|
||||
binaryAttachments: attachments);
|
||||
}
|
||||
|
||||
/// <summary>Build an ack response with a single int argument (the spec's pubSeq echo).</summary>
|
||||
public static SocketIoFrame AckResponse(int ackId, int arg)
|
||||
/// <summary>Build an ack response whose single argument echoes the inbound frame's pubSeq
|
||||
/// (the client's ordered-delivery cursor — load-bearing, not a placeholder).</summary>
|
||||
public static SocketIoFrame AckResponse(int ackId, int pubSeqEcho)
|
||||
{
|
||||
var args = new JsonArray { arg };
|
||||
var args = new JsonArray { pubSeqEcho };
|
||||
return new SocketIoFrame(
|
||||
SocketIoPacketType.Ack, ackId, 0, null, NodesToElements(args), Array.Empty<byte[]>());
|
||||
}
|
||||
|
||||
24
SVSim.BattleNode/Wire/WireJsonOptions.cs
Normal file
24
SVSim.BattleNode/Wire/WireJsonOptions.cs
Normal file
@@ -0,0 +1,24 @@
|
||||
using System.Text.Json;
|
||||
using System.Text.Json.Serialization;
|
||||
|
||||
namespace SVSim.BattleNode.Wire;
|
||||
|
||||
/// <summary>Shared System.Text.Json options for the bare-camelCase Socket.IO / Engine.IO wire:
|
||||
/// per-field <c>[JsonPropertyName]</c> casing (NOT EmulatedEntrypoint's snake_case policy), null
|
||||
/// fields omitted, and unattributed enums written as their name. Single-sourced here because
|
||||
/// <see cref="EngineIoHandshake"/> and <see cref="Protocol.MsgEnvelope"/> previously each built a
|
||||
/// byte-identical block in their own namespace — a drift hazard.</summary>
|
||||
internal static class WireJsonOptions
|
||||
{
|
||||
public static readonly JsonSerializerOptions CamelCase = Create();
|
||||
|
||||
private static JsonSerializerOptions Create()
|
||||
{
|
||||
var opt = new JsonSerializerOptions
|
||||
{
|
||||
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
|
||||
};
|
||||
opt.Converters.Add(new JsonStringEnumConverter());
|
||||
return opt;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user