From 8aead621160cd7f9b9ca6fbb6ea6c9ae82a2e69a Mon Sep 17 00:00:00 2001 From: gamer147 Date: Tue, 2 Jun 2026 11:23:13 -0400 Subject: [PATCH] fix(battle-node): revert Bot Matched/BattleStart push (corrupts OppoBattleStartInfo) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous commit 51e9dd2 changed Bot's InitBattle to push Matched and Loaded to push BattleStart+Deal, on the theory that the architecture spec's "no Matched in Bot mode" claim was wrong. That theory was based on misreading Matching.cs:400 (the Matched handler) as a required state-machine trigger. End-to-end trace of the AI client flow shows: 1. _initNetworkSuccess (set when the client receives uri=InitNetwork, i.e., our ack) is the actual trigger — MatchingNetworkConnectChecker phase 3 sees it and calls MatchingInitBattle. 2. MatchingInitBattle (Matching.cs:298) for IsAINetwork IMMEDIATELY calls StartBattleLoad + GotoBattle right after emitting InitBattle. It does NOT wait for any wire envelope. 3. The Matched handler at Matching.cs:400 is gated on status == Connect and is already past Prepared by the time the wire round-trip completes — sending Matched is harmless but unnecessary. 4. The BattleStart handler at Matching.cs:417 runs UNCONDITIONALLY and SetNetworkInfo at RealTimeNetworkAgent.cs:1562 overwrites OppoBattleStartInfo with the wire envelope's oppoInfo. Our oppoInfo comes from NoOpBotParticipant.Context placeholders (classId/emblemId etc. = 0), corrupting the good values the client set from the HTTP AIBattleStart response. The "Waiting for opponent" hang was caused by SBattleLoad.LoadOpponentAssets trying to fetch emblemId=0, degreeId=0, etc. after BattleStart corrupted OppoBattleStartInfo. The asset group load silently hangs on missing assets, no error logged. Restored the spec's original Bot arms: InitBattle ack-only, Loaded silent, TurnEnd Judge-to-sender. ai-passive.md updated with the corrected reasoning and a discovery-history note. Co-Authored-By: Claude Opus 4.7 --- SVSim.BattleNode/Sessions/BattleSession.cs | 50 ++++++++++++++----- .../Integration/BattleNodeFlowTests.cs | 23 ++++----- .../Sessions/BattleSessionDispatchTests.cs | 32 ++++++------ 3 files changed, 63 insertions(+), 42 deletions(-) diff --git a/SVSim.BattleNode/Sessions/BattleSession.cs b/SVSim.BattleNode/Sessions/BattleSession.cs index 02be295..0067345 100644 --- a/SVSim.BattleNode/Sessions/BattleSession.cs +++ b/SVSim.BattleNode/Sessions/BattleSession.cs @@ -135,19 +135,45 @@ public sealed class BattleSession phaseFrom!.Phase = BattleSessionPhase.AwaitingInitBattle; break; - // --- Phase 3 Bot arms — placed BEFORE the existing handshake arms so the - // Bot-specific TurnEnd wins pattern matching on Type == Bot. InitBattle and - // Loaded fall through to the regular handshake arms below (which push Matched - // and BattleStart+Deal) — the architecture spec's "no Matched in Bot mode" - // claim was wrong: the client's Matching.ReactionReceiveUri (Matching.cs:400) - // gates StartBattleLoad on receiving Matched, and BattleStart triggers - // GotoBattle. Without those envelopes the client hangs in matching status - // Connect ("Waiting for opponent"). AIBattleStart HTTP only provides - // opponent cosmetics, not the state-machine triggers. + // --- Phase 3 Bot arms — placed BEFORE the existing handshake arms so they + // win pattern matching on Type == Bot. Bot mode: ack handshake, silent + // Loaded, Judge-to-sender on TurnEnd. The rest reuse Scripted's arms + // (Retire/Kill → BattleFinishNoContest, Swap → per-sender response, + // default → drop). Reference: docs/api-spec/in-battle/ai-passive.md. // - // Bot's Swap arm reuses the existing Scripted Swap arm (per-sender - // SwapResponse + Ready), and Retire/Kill reuses BattleFinishNoContest. - // Reference: docs/api-spec/in-battle/ai-passive.md. + // Critically, do NOT push Matched or BattleStart for Bot mode. The + // architecture spec was right about this: + // 1. The client's MatchingInitBattle (Matching.cs:298) immediately calls + // StartBattleLoad + GotoBattle on the IsAINetwork branch right after + // emitting InitBattle — it does NOT wait for a wire Matched or + // BattleStart envelope. The state-machine trigger is _initNetworkSuccess + // (set when InitNetwork uri is received, i.e., our ack). + // 2. Sending Matched is harmless (gated on status == Connect, which is + // already past by the time the wire round-trip completes). + // 3. Sending BattleStart is ACTIVELY HARMFUL: its handler at + // Matching.cs:417 runs unconditionally and SetNetworkInfo + // (RealTimeNetworkAgent.cs:1553-1564) overwrites OppoBattleStartInfo + // with the wire envelope's oppoInfo. Our oppoInfo comes from + // NoOpBotParticipant.Context placeholders (classId:0, emblemId:0, + // etc.), corrupting the good values the client just set from the + // HTTP /ai__rank_battle/start response — subsequent asset + // loads (LoadOpponentAssets at SBattleLoad.cs:933) then look up + // non-existent assets and silently hang on "Waiting for opponent." + + case NetworkBattleUri.InitBattle + when Type == BattleType.Bot && phaseFrom?.Phase == BattleSessionPhase.AwaitingInitBattle: + // Ack only — NO Matched push. + result.Add((from, BuildAck(NetworkBattleUri.InitBattle), true)); + phaseFrom!.Phase = BattleSessionPhase.AwaitingLoaded; + break; + + case NetworkBattleUri.Loaded + when Type == BattleType.Bot && phaseFrom?.Phase == BattleSessionPhase.AwaitingLoaded: + // Silent — no BattleStart, no Deal. The client's AINetworkBattleManager + // populates opponent state from AIBattleStart HTTP data; pushing + // BattleStart here overwrites that state with zeros. + phaseFrom!.Phase = BattleSessionPhase.AwaitingSwap; + break; case NetworkBattleUri.TurnEnd when Type == BattleType.Bot && phaseFrom?.Phase == BattleSessionPhase.AfterReady: diff --git a/SVSim.UnitTests/BattleNode/Integration/BattleNodeFlowTests.cs b/SVSim.UnitTests/BattleNode/Integration/BattleNodeFlowTests.cs index 1500ab7..01952bd 100644 --- a/SVSim.UnitTests/BattleNode/Integration/BattleNodeFlowTests.cs +++ b/SVSim.UnitTests/BattleNode/Integration/BattleNodeFlowTests.cs @@ -440,25 +440,22 @@ public class BattleNodeFlowTests var ack1 = await client.ReceiveSynchronizeAsync(ct); Assert.That(ack1.Uri, Is.EqualTo(NetworkBattleUri.InitNetwork)); - // InitBattle → Matched. (Same handshake arm as Scripted/PvP; the client's - // matching state machine gates StartBattleLoad on receiving Matched, so the - // envelope MUST be sent. Opponent cosmetics in the body are placeholders; - // the client renders opponent UI from AIBattleStart HTTP data.) + // InitBattle → ack (NOT Matched). The client's AI flow doesn't gate on + // Matched and pushing BattleStart later corrupts OppoBattleStartInfo, so + // Bot mode keeps the handshake silent (just an ack). await client.SendMsgAsync(MakeEnvelopeWith(vid, NetworkBattleUri.InitBattle, pubSeq: 2), key, ct); - var matched = await client.ReceiveSynchronizeAsync(ct); - Assert.That(matched.Uri, Is.EqualTo(NetworkBattleUri.Matched), - "Bot's InitBattle pushes Matched (the state-machine trigger)."); + var ack2 = await client.ReceiveSynchronizeAsync(ct); + Assert.That(ack2.Uri, Is.EqualTo(NetworkBattleUri.InitBattle), + "Bot's InitBattle is ack-only — no Matched envelope."); - // Loaded → BattleStart + Deal (BattleStart triggers GotoBattle on the client). + // Loaded → silent. Send Swap right after; the next inbound must be SwapResponse + // (no orphan BattleStart / Deal in the queue). await client.SendMsgAsync(MakeEnvelopeWith(vid, NetworkBattleUri.Loaded, pubSeq: 3), key, ct); - Assert.That((await client.ReceiveSynchronizeAsync(ct)).Uri, Is.EqualTo(NetworkBattleUri.BattleStart)); - Assert.That((await client.ReceiveSynchronizeAsync(ct)).Uri, Is.EqualTo(NetworkBattleUri.Deal)); - - // Swap → SwapResponse + Ready. await client.SendMsgAsync(MakeEnvelopeWith(vid, NetworkBattleUri.Swap, pubSeq: 4, body: new Dictionary { ["idxList"] = new List() }), key, ct); var swapResp = await client.ReceiveSynchronizeAsync(ct); - Assert.That(swapResp.Uri, Is.EqualTo(NetworkBattleUri.Swap)); + Assert.That(swapResp.Uri, Is.EqualTo(NetworkBattleUri.Swap), + "Expected Swap response (mulligan ack). Got " + swapResp.Uri + " — Loaded may have leaked a frame."); var readyResp = await client.ReceiveSynchronizeAsync(ct); Assert.That(readyResp.Uri, Is.EqualTo(NetworkBattleUri.Ready)); diff --git a/SVSim.UnitTests/BattleNode/Sessions/BattleSessionDispatchTests.cs b/SVSim.UnitTests/BattleNode/Sessions/BattleSessionDispatchTests.cs index 0d9d470..4f327c9 100644 --- a/SVSim.UnitTests/BattleNode/Sessions/BattleSessionDispatchTests.cs +++ b/SVSim.UnitTests/BattleNode/Sessions/BattleSessionDispatchTests.cs @@ -458,40 +458,38 @@ public class BattleSessionDispatchTests } [Test] - public void Bot_InitBattle_pushes_Matched_to_sender() + public void Bot_InitBattle_acks_to_sender_with_no_Matched_push() { var (s, a, _) = NewBotSession(); s.ComputeFrames(a, NewEnvelope(NetworkBattleUri.InitNetwork)); var routes = s.ComputeFrames(a, NewEnvelope(NetworkBattleUri.InitBattle)); - // Bot InitBattle reuses the regular handshake arm — pushes Matched. The - // client's Matching.ReactionReceiveUri gates StartBattleLoad on receiving - // Matched (Matching.cs:400), so without it the client hangs in - // "Waiting for opponent." Cosmetics in the Matched body come from - // NoOpBotParticipant.Context placeholders; the client renders opponent - // cosmetics from AIBattleStart HTTP data, not from Matched, so the - // placeholders are harmless here. + // Bot InitBattle is ack-only — NO Matched push. Matched would be ignored + // by the client anyway (gated on status == Connect, which is already + // past by the time the wire round-trip completes), but the spec is to + // not send it for clarity. Assert.That(routes.Count, Is.EqualTo(1)); Assert.That(routes[0].Target, Is.SameAs(a)); - Assert.That(routes[0].Frame.Uri, Is.EqualTo(NetworkBattleUri.Matched)); + Assert.That(routes[0].Frame.Uri, Is.EqualTo(NetworkBattleUri.InitBattle), + "Expected an ack envelope for InitBattle, NOT a Matched envelope."); Assert.That(a.Phase, Is.EqualTo(BattleSessionPhase.AwaitingLoaded)); } [Test] - public void Bot_Loaded_pushes_BattleStart_then_Deal_to_sender() + public void Bot_Loaded_produces_no_routes_but_advances_phase() { var (s, a, _) = NewBotSession(); s.ComputeFrames(a, NewEnvelope(NetworkBattleUri.InitNetwork)); s.ComputeFrames(a, NewEnvelope(NetworkBattleUri.InitBattle)); var routes = s.ComputeFrames(a, NewEnvelope(NetworkBattleUri.Loaded)); - // Bot Loaded reuses the regular handshake arm — pushes BattleStart + Deal. - // BattleStart triggers GotoBattle on the client (Matching.cs:417); without - // it the client hangs. - Assert.That(routes.Select(r => r.Frame.Uri), - Is.EqualTo(new[] { NetworkBattleUri.BattleStart, NetworkBattleUri.Deal })); - Assert.That(routes.All(r => ReferenceEquals(r.Target, a)), Is.True); - Assert.That(a.Phase, Is.EqualTo(BattleSessionPhase.AwaitingSwap)); + // Bot Loaded is silent — no BattleStart, no Deal. Pushing BattleStart + // would actively CORRUPT OppoBattleStartInfo on the client (the wire + // handler at Matching.cs:417 → SetNetworkInfo overwrites it with our + // placeholder NoOpBotParticipant.Context zeros). + Assert.That(routes, Is.Empty, "Bot Loaded is silent."); + Assert.That(a.Phase, Is.EqualTo(BattleSessionPhase.AwaitingSwap), + "Phase still advances even though there are no outbound routes."); } [Test]