fix(battle-node): TurnEndFinal pushes LifeWin=101 (player perspective), not LifeLose=102

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>
This commit is contained in:
gamer147
2026-06-02 18:11:24 -04:00
parent ee23985055
commit 1685b509c3
3 changed files with 53 additions and 33 deletions

View File

@@ -4,28 +4,38 @@ namespace SVSim.BattleNode.Protocol;
/// Wire value of <c>result</c> on a WS <c>BattleFinish</c> frame.
/// <para>
/// Maps to the client's <c>NetworkBattleReceiver.RESULT_CODE</c> enum at
/// <c>NetworkBattleReceiver.cs:963-986</c>. The client extracts this via
/// <c>NetworkBattleReceiver.cs:1170-1172</c>:
/// <code>
/// case NetworkParameter.result:
/// _receiveData.result = (RESULT_CODE)ConvertToInt(data.Value);
/// </code>
/// then dispatches through <c>NetworkBattleManagerBase.JudgeResultReceive</c>
/// (lines 1412-1488). The wire codes are categorized by HOW the battle ended
/// (life / deckout / retire / disconnect / timeout) and are named from the
/// OPPONENT'S perspective:
/// <c>NetworkBattleReceiver.cs:963-986</c>. Names are <b>from the player's perspective</b>:
/// <c>LifeWin</c> = "I won by life", <c>LifeLose</c> = "I lost by life". Verified
/// end-to-end via the path
/// <c>RESULT_CODE</c> → <c>JudgeResultReceive</c> switch → <c>_finishEffectType</c> →
/// <c>FinishBattleEffect</c> → <c>InitiateGameEndSequence(hasWon)</c>:
/// </para>
/// <list type="bullet">
/// <item><c>LifeLose</c> = "opponent's life ran out" → PLAYER WIN UI</item>
/// <item><c>LifeWin</c> = "opponent's life win condition met" → PLAYER LOSE UI</item>
/// <item><c>LifeWin = 101</c> → <c>_finishEffectType = LOSE</c> →
/// <c>InitiateGameEndSequence(hasWon: true)</c> → PLAYER WIN UI</item>
/// <item><c>LifeLose = 102</c> → <c>_finishEffectType = WIN</c> →
/// <c>InitiateGameEndSequence(hasWon: false)</c> → PLAYER LOSE UI</item>
/// </list>
/// <para>
/// Prior to 2026-06-02 the docstring here claimed the values were 0/1/2 mapping
/// LOSE/WIN/CONSISTENCY — that was the HTTP <c>/finish</c> response shape, not the
/// WS frame. The previous values are kept for backwards compat with the
/// Retire/Kill scripted flow (which has shipped emitting <c>Win = 1</c>, parsed
/// client-side as <c>RESULT_CODE.NoContest</c> and rendered as "battle ended in
/// no contest" — a working-but-not-quite-right scripted-bot terminator).
/// The <c>SettingResultUI_SpecialResultTypeText</c> switch passes the OPPONENT's
/// outcome to set the secondary "by retire/by disconnect/etc." text — that's why
/// the inner switch direction looks inverted (LifeWin → LOSE param, LifeLose → WIN
/// param). The actual WIN/LOSE rendering happens in <c>FinishBattleEffect</c> via
/// the <c>!isPlayer</c> flip at line 1315.
/// </para>
/// <para>
/// Prior docstrings on this enum had the direction backwards (claimed LifeLose
/// → WIN UI from a misread of the inner switch); see
/// <c>docs/audits/battle-node-sio-events-2026-06-02.md</c> Addendum for the live
/// reproduction that exposed the inversion.
/// </para>
/// <para>
/// The pre-2026-06-02 <c>Lose = 0</c> / <c>Win = 1</c> / <c>Consistency = 2</c>
/// values are kept for the existing Retire/Kill Scripted flow (which ships
/// <c>Win = 1</c>, parsed client-side as <c>RESULT_CODE.NoContest</c> and
/// rendered as "battle ended in no contest" — works in that the battle
/// terminates, but shows the wrong text). To be removed once that flow is
/// rewritten to use the proper retire codes.
/// </para>
/// <para>
/// Always serialize as the int value, not the name; see the
@@ -44,12 +54,15 @@ public enum BattleResult
/// Wire-equivalent of <c>RESULT_CODE.Invalid = 2</c>. Don't use for new code.</summary>
Consistency = 2,
/// <summary>Opponent's life ran out (PLAYER WIN). Pushed by Scripted mode on
/// the player's <c>TurnEndFinal</c> emit. Matches the prod TK2 capture at
/// <c>data_dumps/captures/battle-traffic_tk2_regular.ndjson:274</c>.</summary>
LifeLose = 102,
/// <summary>Opponent's life-win condition met (PLAYER LOSE). Not currently emitted
/// by Scripted mode — the bot can't kill the player — but listed for completeness.</summary>
/// <summary>Player won by reducing opponent's life to 0. Pushed by Scripted mode
/// on the player's <c>TurnEndFinal</c> emit. Routes through the client switch to
/// <c>InitiateGameEndSequence(hasWon: true)</c>.</summary>
LifeWin = 101,
/// <summary>Player lost by their own life dropping to 0. Not currently emitted
/// by Scripted mode — the bot can't kill the player — but listed for completeness
/// and for the prod TK2 capture at
/// <c>data_dumps/captures/battle-traffic_tk2_regular.ndjson:274</c> (a loss the
/// player suffered to a real opponent).</summary>
LifeLose = 102,
}

View File

@@ -260,11 +260,12 @@ public sealed class BattleSession
}
else if (Type == BattleType.Scripted)
{
// Push BattleFinish with LifeLose=102client interprets as "opponent's
// life ran out, player WIN". DON'T forward to bot (no next turn) and DON'T
// re-fire the 3-frame burst — the game is over. Phase → Terminal so the
// Push BattleFinish with LifeWin=101names are from the player's
// perspective: "I won by life". Verified end-to-end via
// FinishBattleEffect:1289-1315 → InitiateGameEndSequence(hasWon: true) →
// WIN UI. Don't forward to bot (no next turn). Phase → Terminal so the
// RunAsync cascade doesn't synthesize a follow-up BattleFinish.
result.Add((from, BuildBattleFinish(BattleResult.LifeLose), true));
result.Add((from, BuildBattleFinish(BattleResult.LifeWin), true));
Phase = BattleSessionPhase.Terminal;
}
// Bot type: no-op (rank-battle finish is HTTP-driven via /ai_*/finish).

View File

@@ -83,12 +83,18 @@ public class BattleSessionDispatchTests
}
[Test]
public void Scripted_TurnEndFinal_pushes_BattleFinish_LifeLose_to_player_and_terminates()
public void Scripted_TurnEndFinal_pushes_BattleFinish_LifeWin_to_player_and_terminates()
{
// Regression for the BattleFinishToOpponentDisConnectChecker softlock — when the
// player declares their final winning turn (TurnEndFinal), the server must push a
// wire BattleFinish so the client doesn't park on "waiting for opponent" for 128s.
// LifeLose=102 = "opponent's life ran out" → client UI shows player WIN.
//
// Wire-result direction: RESULT_CODE names are FROM THE PLAYER'S PERSPECTIVE.
// LifeWin=101 = "I won by life". The client's FinishBattleEffect:1289-1315 then
// routes this through InitiateGameEndSequence(hasWon: true) → WIN UI. (An earlier
// version of this test asserted LifeLose=102 — that pushed LOSE UI in live test,
// matching the inversion documented in
// docs/audits/battle-node-sio-events-2026-06-02.md Addendum.)
var (s, a, b) = NewSession();
s.ComputeFrames(a, NewEnvelope(NetworkBattleUri.InitNetwork));
s.ComputeFrames(a, NewEnvelope(NetworkBattleUri.InitBattle));
@@ -105,8 +111,8 @@ public class BattleSessionDispatchTests
"BattleFinish is a no-stock control push (matches the Retire/Kill arm).");
Assert.That(routes[0].Frame.Body, Is.InstanceOf<SVSim.BattleNode.Protocol.Bodies.BattleFinishBody>());
var body = (SVSim.BattleNode.Protocol.Bodies.BattleFinishBody)routes[0].Frame.Body;
Assert.That(body.Result, Is.EqualTo(BattleResult.LifeLose),
"Wire result must be RESULT_CODE.LifeLose (102); the client maps that to player WIN.");
Assert.That(body.Result, Is.EqualTo(BattleResult.LifeWin),
"Wire result must be RESULT_CODE.LifeWin (101) — names are player-perspective; client routes this to WIN UI.");
Assert.That(s.Phase, Is.EqualTo(BattleSessionPhase.Terminal),
"Session must transition to Terminal so the RunAsync cascade doesn't synthesize a second BattleFinish.");
}