fix(battle-node): revert Bot Matched/BattleStart push (corrupts OppoBattleStartInfo)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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_<fmt>_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:
|
||||
|
||||
@@ -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<string, object?> { ["idxList"] = new List<object?>() }), 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));
|
||||
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user