No dispatch arm has emitted these since the Retire/Kill rewrite to RetireWin=105
/ RetireLose=106. Remove them and the docstring paragraph that explained them.
Test fallout: delete BattleFinishBody_LoseAndConsistency_SerializeAsZeroAndTwo
(its only purpose was locking the dead wire values), and re-point
BattleFinishBody_SerializesResultAndResultCode_AsNumericWireValues at the live
LifeWin=101 so it still guards the JsonNumberEnumConverter numeric-wire behavior.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Code-review follow-up to the dispatch unification (0a8a84b).
1. The RunAsync drop cascade synthesized BattleFinish(Win=1), which the client
renders as RESULT_CODE.NoContest ("battle ended in no contest") instead of a
win. Add DisconnectWin=201 (already in the client enum, routes to WIN UI) and
ship it for involuntary opponent drops. Update PvpMidGameDisconnect_FullCascade.
2. Remove BuildBattleFinishNoContest() — dead since the Retire/Kill arm moved to
RetireWin/RetireLose.
3. Correct the BattleResult docstring: Lose/Win/Consistency are no longer emitted
by any dispatch arm; they survive only as serialization-test constants.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three dispatch arms had Type-based branching that was either wrong or
unnecessary. Unified per the audit doc's recommended order, grounded in
verified facts about each participant's PushAsync.
(1) TurnEndFinal — was branched: PvP broadcast TurnEnd+Judge (wrong on a
game-end signal); Scripted pushed BattleFinish(LifeWin). Unified:
- forward the envelope to other (matches prod TK2 capture
battle-traffic_tk2_regular.ndjson:273 — loser receives TurnEndFinal
from server before BattleFinish)
- push BattleFinish(LifeWin) to from (winner)
- push BattleFinish(LifeLose) to other (loser)
- Phase → Terminal
Requires ScriptedBotParticipant.PushAsync to no longer fire its 3-frame
burst on TurnEndFinal (previously it reacted to both TurnEnd and
TurnEndFinal). The dispatch arm now owns TurnEndFinal's response; the
bot reacting too would race with the BattleFinish push. Bot still
fires on regular TurnEnd as before.
(2) Retire / Kill — was branched: PvP pushed Lose=0 (NotFinish) /
Win=1 (NoContest); Scripted pushed BuildBattleFinishNoContest() (Win=1).
Both shipped wrong RESULT_CODE values; the audit doc's outstanding item
documented this. Unified:
- push BattleFinish(RetireLose=106) to from (the retirer)
- push BattleFinish(RetireWin=105) to other (the survivor)
- Phase → Terminal
Added RetireWin=105 / RetireLose=106 to BattleResult enum with the
same player-perspective convention.
(3) PvP gameplay forwarder (TurnStart / PlayActions / Echo /
TurnEndActions / JudgeResult) — had a redundant `Type == BattleType.Pvp`
guard. Verified that BothAfterReady() is naturally only true when both
participants are RealParticipant (ScriptedBot / NoOpBot don't implement
IHasHandshakePhase per RealParticipant.cs:20-23 / Participants/*.cs grep).
Dropped the redundant guard.
Bot type still has its dedicated InitBattle/Loaded/TurnEnd arms above
the unified ones, so Bot-specific behavior is unchanged.
Tests: 177 battle-node tests passing.
- Updated 9 tests to match the unified dispatch (paired BattleFinish
pushes, correct RESULT_CODE values, forwarded TurnEndFinal envelope).
- ScriptedBotParticipantTests.PushAsync_TurnEndFinal_* rewritten to
assert the bot does NOT fire on TurnEndFinal (was asserting it did).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
End-to-end trace of FinishBattleEffect proved my prior direction was
backwards. The path is:
RESULT_CODE → JudgeResultReceive switch (NetworkBattleManagerBase:1439-1459)
→ SettingResultUI_SpecialResultTypeText
→ _finishEffectType = battleResult
→ eventually FinishBattleEffect(:1267-1316):
bool isPlayer = false;
switch (_finishEffectType) {
case WIN: isPlayer = true; break;
case LOSE: isPlayer = false; break;
}
InitiateGameEndSequence(!isPlayer); // NEGATED
→ BattleManagerBase.InitiateGameEndSequence(hasWon):
hasWon=true → WIN screen; hasWon=false → LOSE screen.
So LifeWin=101 (player perspective: "I won by life") → _finishEffectType=LOSE
→ isPlayer=false → hasWon=true → WIN UI. And LifeLose=102 ("I lost") → LOSE UI.
My prior misread treated the inner switch's BATTLE_RESULT_TYPE param as
the final UI render — but that param only feeds the secondary "by retire
/ by disconnect" text, not the primary WIN/LOSE. The real flip happens at
FinishBattleEffect:1315's !isPlayer negation.
User's live repro (bot HP to 0 → LOSS screen) confirmed the inversion.
The prior prod TK2 capture interpretation was also corrected: line 274
`result:102` was a LOSS capture (player lost to the opponent's attack on
line 271), not a win as I claimed earlier.
Changes:
- BattleResult.cs: docstring rewritten with the full FinishBattleEffect
trace. Members reordered (LifeWin first since it's used by Scripted).
- BattleSession.cs:267: Scripted TurnEndFinal arm pushes LifeWin instead
of LifeLose.
- Test updated to assert LifeWin=101 + describe the inversion lesson so
the next reader sees the prior bug context.
177 battle-node tests passing.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Before: when the player declared their final winning turn (TurnEndFinal),
Scripted mode forwarded it to the bot — which fired a useless 3-frame
TurnStart/TurnEnd/Judge burst as if the game were continuing. No
BattleFinish was ever pushed, so the client's
BattleFinishToOpponentDisConnectChecker (NetworkBattleManagerBase.cs:1640
+ BattleFinishToOpponentDisConnectChecker.cs) parked the player on a
"waiting for opponent" dialog for 128 seconds, eventually falling through
to a synthetic OnDisConnectWin. The user could see "opponent defeated"
animations but couldn't proceed to the post-battle screen.
After: Scripted TurnEndFinal pushes BattleFinish with result=LifeLose=102
to the player (matches the RESULT_CODE the client expects per
NetworkBattleReceiver.cs:963-986; client maps LifeLose → "opponent's life
ran out, PLAYER WIN" UI per NetworkBattleManagerBase.cs:1450-1459). Phase
transitions to Terminal so RunAsync's PvP-disconnect cascade doesn't
synthesize a second BattleFinish on top. No bot burst — the game is
over.
Wire reference: prod TK2 capture battle-traffic_tk2_regular.ndjson:273-274
shows server pushing TurnEndFinal followed immediately by BattleFinish
result:102.
BattleResult enum gets the LifeWin=101 / LifeLose=102 values and a
corrected docstring. The pre-existing Lose=0 / Win=1 / Consistency=2
values stay (Retire/Kill flow ships them today and works as "no contest"
end-of-battle), but their docstring no longer claims they're the WS
shape — they were always the HTTP /finish shape, mislabeled.
TurnEnd (regular, not final) keeps the existing forward-to-bot behavior
in Scripted mode — that's a normal turn boundary, not game end.
PvP TurnEndFinal still broadcasts the same TurnEnd+Judge as regular
TurnEnd; the actual game-end BattleFinish push in PvP rides the loser's
Retire/Kill or the disconnect cascade in RunAsync.
177 battle-node tests passing (was 176; +1 covering the new dispatch arm).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Scripted-bot softlock root cause: client-stocked SELECT_SKILL_URI /
SLIDE_OBJECT_URI hand emits (e.g. target selection on unit play / leader
attack) arrive as SIO BinaryEvent("hand", ...) with an ack-id. Our
DispatchSocketIo only had cases for "msg" and "alive" — "hand" fell to
the default Debug-drop with no SIO ack going back. Client's
stockEmitMessageMgr (RealTimeNetworkAgent.cs:1463) blocks subsequent
emits until the previous one is acked, so all follow-up PlayActions /
TurnEndActions / TurnEnd frames were stocked but never transmitted. The
loader hooks at EmitMsg (intent) not the socket layer, which is why
battle-traffic.ndjson shows the frames as sent while the server never
received them. ~10s later the client gives up and aborts the WS.
Wire-shape proof from data_dumps/captures/logs/websocket_output.txt:
line 619: [sio-in] uri=TurnStart pubSeq=17 ackId=16 ... (T3 start)
line 689: [ws-rx-text] preview=451-26["hand", {...}] ← unhandled
line 691: [ws-rx-bin] binLen=58 pendingFrame=hand
(no further [sio-in] entries — server received nothing else)
line 709: [ws-recv-exit] reason=OperationCanceled wsState=Aborted
New HandleHandEventAsync (RealParticipant.cs):
- Fire-and-forget hand frames (no ack-id; TOUCH_URI / SELECT_OBJECT_URI /
TURN_END_READY_URI) are silently swallowed — no queue-blocking risk
- Stocked hand frames decode the binary attachment via the same
msgpack-string + NodeCrypto.Decrypt pipeline as HandleMsgEventAsync,
parse the JSON, extract top-level "pubSeq", and SendSioAckAsync with
that pubSeq as the ack arg (matches what stockEmitMessageMgr.GetSelectData
expects to look up)
- Body shape is {"StockHandData":[uri_int, viewerId, udid, ...params,
pubSeq], "try":0, "pubSeq":N} — NOT a MsgEnvelope (no top-level "uri"),
so we can't reuse HandleMsgEventAsync as-is
- Missing-pubSeq fallback acks with arg=0 (rare path, logged at Warning)
so we never softlock from a malformed body
WireConstants gets the HandEvent = "hand" constant for the dispatch case.
In scripted/Bot mode the ack-only handler is correct (no opponent to
forward touches to). PvP-side forwarding semantics are unverified — see
docs/audits/battle-node-sio-events-2026-06-02.md (outer repo) for the
full event inventory and remaining gaps.
Tests:
- RealParticipantHandEventTests covers the three paths: stocked-with-ack,
fire-and-forget (no ack expected), missing-pubSeq fallback (arg=0). Each
drives a real hand frame through RunAsync via TestWebSocket and asserts
the SIO ack frame shape (43<ackId>[<arg>]) in outbound sends.
- 175 battle-node tests passing (was 172; +3 new). Full suite green.
Diagnostic logs ([sio-in] / [sio-out] / [ws-rx-text] / [ws-rx-bin] /
[ws-recv-exit] / [ws-loop-exit]) are left in place for one verification
cycle. After a live re-run confirms the fix, they should be stripped per
the audit doc's recommended-order step 2.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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>
ToJson now throws ArgumentException when a Body key collides with a reserved
envelope field (uri/viewerId/uuid/bid/try/cat/pubSeq/playSeq); FromJson reuses
the same shared ReservedEnvelopeKeys HashSet. ReceiveNodeResultCode expanded
from 9 to 31 codes to mirror the full enums.md catalog. Two regression tests
added for the collision guard and PascalCase uri serialization.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
