Live two-client run (data_dumps/captures/battle_test) exposed a turn-handover
stall: ending a turn on client A made BOTH clients show A's turn again; the
opponent never got a turn. Root cause: JudgeHandler routed the {spin:0} Judge to
ctx.Other. The client rule is 'receive opponent TurnEnd -> SendJudge', so the
PASSIVE player (the one taking over the turn) is the Judge sender, and 'receive
Judge -> ControlTurnStartPlayer' starts the RECEIVER's turn. Routing to ctx.Other
delivered the Judge to the player who had just ended their turn, restarting it in
a closed loop while the taker-over sat on 'Opponent's Turn'.
Fix: the PvP Judge {spin} reflects back to ctx.From (the sender / turn taker-over),
matching the Bot arm's existing 'Judge to sender only' handover. The sender then
emits TurnStart, which relays to the opponent as {spin}. Updated the dispatch unit
test and the PvpHandshakeAndGameplay integration test to the real handover order
(passive sends Judge -> receives it back -> sends TurnStart -> opponent sees it).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- PlayActionsHandler doc: drop the phantom 'with a debug log' (handlers are
stateless singletons with no logger); say token plays degrade silently.
- KnownListBuilder.ExtractMoveTo doc: note first-match-wins semantics and the
send-side==recv-side 'to' assumption pending recv-capture confirmation.
- KnownListBuilderTests: add multi-move first-match coverage and the
in-deck-but-no-matching-move null branch for BuildPlayedCard.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The end-to-end PvP gameplay test asserted the pre-translator relay contract
(A's TurnEnd broadcasts TurnEnd+Judge to both sides). Tasks 7/8 replaced that
with the per-URI translator: the active player ends its turn by sending TurnEnd
then Judge, the opponent receives the translated {turnState:0}/{spin:0} frames,
and the sender receives nothing. Rewrote the gameplay section to drive and
assert the new contract. PlayActions remains delivered to the opponent (Uri
preserved, body now synthesized by PlayActionsHandler).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Unions the two legacy TurnStart arms (IsRealForwardableFromScripted case 11 +
BothAfterReady case 12) into TurnStartHandler. Both arms produce (Other, Env, false)
with no extra guards or state mutations — union is behavior-equivalent.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The three PvP BattleNodeFlowTests drove each client's handshake to Ready
independently; the new barrier withholds Ready until both sides swap, so the
single-client helper timed out. Split DriveHandshakeAsync into DriveThroughSwapAsync
(stops at SwapResponse) + DrivePvpHandshakeAsync (drives both, then drains the
barrier-released Ready for each). Scripted/Bot single-client paths are unaffected
(non-IHasHandshakePhase opponent releases Ready immediately).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ready was sent per-side immediately carrying the placeholder opponent hand, so
one client cleared mulligan before the other. The barrier now releases Ready to
every IHasHandshakePhase participant only once all have swapped, each carrying
the opponent's real post-mulligan hand. No Type check — NoOp (Bot/AINetwork)
isn't a phase impl, so that mode still releases immediately.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements IHasHandshakePhase and emits client-shaped InitNetwork/InitBattle/
Loaded/Swap (reacting to the session's pushes) instead of being a passive
TurnEnd-only fixture the session narrates around. This is what lets the
type-agnostic mulligan barrier (next task) work in Scripted mode.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds BuildReady(selfHand, oppoHand) for the mulligan barrier; the single-arg
overload keeps the InitialHand placeholder for non-interactive opponents.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both PvP clients received turnState:0 ('both go first'). BuildBattleStart
now takes turnState; the Loaded arm assigns 0 to A, 1 to B — no Type check,
correct in Scripted (real player = A = first) and PvP (first arriver first).
Updated three existing BuildBattleStart callers in the test suite to pass
turnState:0 (the param is now required).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ISteamServer contract forbids null tickets (prod impl and sole caller both assume non-null),
so the dev bypass no longer needs the ?. / ?? 0 defensive form. Also adds a class-level XML
doc summary to DevAlwaysValidSteamServerTests matching the style of other fixtures in the suite.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace bare `int RewardType` on 12 catalog/reward entities and GrantedReward
with the existing UserGoodsType enum. Verified against the decompiled client:
every wire reward_type decodes through the single Wizard.UserGoods.Type enum, so
one enum is correct across all endpoint families (item_type is a separate
Item.Type axis, left untouched). EF stores the enum as the same int column, so
there is no migration.
- Importers cast seed int -> UserGoodsType at the ingest boundary.
- New GrantedReward.ToRewardList() extension replaces 8 copy-pasted
GrantedReward -> RewardListEntry projections.
- Fix 3 .ToString() sites that would otherwise emit enum names ("Crystal")
instead of the int wire value ("2").
- Wire DTOs keep int; the enum is widened to int at the wire boundary only.
Build green; 962/962 tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
Add CaptureConformanceTests: drive one Scripted lifecycle, harvest all ten
server-authored synchronize frames (InitNetwork/Matched/BattleStart/Deal/Swap/
Ready/TurnStart/TurnEnd/Judge/BattleFinish), re-serialize via MsgEnvelope.ToJson,
and diff each against representative prod TK2 capture frames embedded as a
fixture. Comparison is capture-subset-of-ours on body shape (recursive keys +
value category), so missing/miscased/mistyped fields fail but extra envelope
fields we emit don't; pure sequencing keys are excluded.
Because PvP reuses the same ScriptedLifecycle builders for the handshake/mulligan
frames, this transitively locks the PvP handshake shape -- a regression oracle
that outlives the June-2026 server shutdown.
Also replace the stale v1-only README with a pointer to the canonical
docs/battle-node.md hub (outer repo).
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>
The prior 'hand'-ack fix worked in test but failed in prod because both
the handler and the test used the wrong wire shape. Re-tracing the
client emit path:
RealTimeNetworkAgent.cs:783-786 (msg path):
return MessagePackSerializer.Serialize(
CryptAES.encryptForNode(JsonMapper.ToJson(info))); // ← encrypted
RealTimeNetworkAgent.cs:815-817 (hand path):
return MessagePackSerializer.Serialize(
JsonMapper.ToJson(info)); // ← NOT encrypted
And EmitFrontStockData:717-723 picks "hand" as the SIO event name only
when frontData["StockHandData"] exists; in that branch it passes the
StockHandData list (NOT the dict) to CreatePackEmitHandData. So the
wire body is:
msgpack_string(JsonMapper.ToJson(List<object>))
i.e. a JSON array, unencrypted. EmitMsgUriPack:1456-1458 puts pubSeq at
index 3 of that array (after uri_int / viewerId / udid). The dict's
top-level pubSeq stays client-local for stockEmitMessageMgr.GetSelectData.
Handler now:
- Skips NodeCrypto.DecryptForNode (was throwing FormatException on the
unencrypted bytes — caught and swallowed silently by the existing
outer try/catch, so the bug presented as 'no warning, no ack')
- Parses RootElement.ValueKind:
- Array → arr[3] is the pubSeq
- Object → top-level "pubSeq" (defensive; not used by prod today)
- Falls back to ack arg=0 if neither extraction works (the client's
GetSelectData lookup misses but its OnAck path still fires — same as a
normal cache-miss — so the queue still drains)
Diagnostic [hand-rx] log added (gated by DiagnosticLogging) so we can
see the actual body content per-frame during verification.
Test was also wrong (encrypted dict shape); rewritten to use the real
wire shape (unencrypted JSON array). +1 net new test covering the
dict-shape defensive path.
176 battle-node tests passing.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The temporary [sio-in] / [sio-out] / [ws-rx-text] / [ws-rx-bin] /
[ws-recv-exit] / [ws-loop-exit] logs added during the hand-ack
investigation are useful enough to keep around (PvP testing, future WS
debugging) but too chatty to leave on by default. Promote them from
"strip before merge" to a permanent opt-in.
New BattleNodeOptions.DiagnosticLogging (bool, default false). Wired
through BattleNodeWebSocketHandler to RealParticipant via a new optional
ctor parameter (default false — existing test sites pick up the silent
default with no changes). Every Information/Warning log added during the
investigation is now if-gated; non-diagnostic logs (the decode-failure
warnings, the dispatch-drop debug) stay as-is.
Toggle via appsettings*.json:
"BattleNode": { "DiagnosticLogging": true }
Or live via the singleton:
factory.Services.GetRequiredService<BattleNodeOptions>().DiagnosticLogging = true
175 battle-node tests still passing — existing tests use the constructor
default and emit nothing, so no test changes were required.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
