fix(coop): serve the client on a mid-battle campaign resume - #120
Merged
Conversation
Loading a mid-battle campaign save leaves a BattlescapeState on the stack (the host's own battle) before the session is live, so the resume lobby's ctor set _resumeToGame=true. RESUME then ran returnToRunningGame() - popping the host into its own battle and serving nobody - instead of resumeCampaign(). The second player was stranded on the lobby while the host played on alone. The battleRunning term had no sessionLocked guard (the geoscape term does), on the assumption that a battle cannot exist before a session starts - which is false when a mid-battle save is loaded. Gate _resumeToGame on lobbyMode!=2: a resume-from-save lobby (the only mode-2 lobby, set by adoptResumeSave) must always serve the client. The in-game coop menu and a mid-battle reconnect are never mode 2, so they still return straight to the running game. The existing resume-battle tests missed this because session.resume_campaign_battle pressed RESUME via the lobby_resume_campaign command, which calls LobbyMenu::resumeCampaign() directly and bypasses the _resumeToGame branch. Press the real button (lobby_action -> btnCancelClick) instead, so test_coop_resume_battle_control and test_shared_resume_battle_control both reproduce the bug without the fix and pass with it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The mid-battle resume bug hid from the suite because the resume helper pressed RESUME via the lobby_resume_campaign command, which calls LobbyMenu::resumeCampaign() directly and bypasses btnCancelClick (where the _resumeToGame branch lives). The same bypass existed for campaign start (lobby_start_campaign -> startCampaign()) and geoscape resume, so any btnCancelClick-level bug on those paths would hide too. Audit of every TestServer command: the only ones that call a leaf method below the real button were resumeCampaign() and startCampaign(); all others click the real btn*Click, or use a harness* wrapper that itself calls btnOkClick/viewClick (the recruit-stores-full bug proves those hit the real validation). Route session.new_campaign and session.resume_campaign through the real button (lobby_action -> btnCancelClick; start goes via ConfirmStartCampaignState + lobby_confirm_ok). Validated end to end: SEPARATE + SHARED start, geoscape resume, and mid-battle resume all still pass through the real UI path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to the harness-fidelity work: no test should reach a lobby action by a path a player cannot take. Added session.start_campaign_via_button (lobby_action -> ConfirmStartCampaignState -> lobby_confirm_ok) and session.resume_campaign_via_button (lobby_action after the host sees the peer), and routed new_campaign / resume_campaign / resume_campaign_battle plus every inline caller through them: - test_lobby_gating: lone-host rejection now presses the real START button and asserts it no-ops (no confirm dialog, no base placement, still in the lobby); the confirm-dialog case opens via btnCancelClick, not openStartConfirmDialog. - test_lobby_dialogs, test_reconnect_dialog, scenario_dialog_shot: start via the real button. - test_resume_flow, test_coop_basedef_temp_ufo_uaf: resume via the real button. The lobby_start_campaign / lobby_resume_campaign commands remain in TestServer (unused by tests now) - harmless, and kept so a future gate-rejection test could assert on their error path directly. Validated GREEN: test_lobby_gating, test_lobby_dialogs, test_resume_flow, test_reconnect_dialog (plus the earlier SEPARATE/SHARED start + mid-battle resume). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
NonPolynomialTim
enabled auto-merge (squash)
August 1, 2026 16:57
This was referenced Aug 1, 2026
SHARED: host validators don't re-check craft capacity (craftRearmValidate/soldierArmorValidate)
#121
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Loading a mid-battle campaign save, having the second player rejoin, and pressing RESUME CAMPAIGN left the client stranded on the lobby while the host dropped into the battle alone. Reported by a player; reproduced live. SEPARATE and SHARED both affected; geoscape (non-battle) resume was fine.
Root cause
LobbyMenu's ctor sets_resumeToGame = battleRunning || (geoRunning && sessionLocked). Loading a mid-battle save makesLoadGameStatebuild[GeoscapeState, BattlescapeState]before the session is live, sobattleRunningis true in the resume lobby — thebattleRunningterm had nosessionLockedguard (the comment assumed a battle can't exist before a session starts, which a save-load violates). RESUME then ranreturnToRunningGame()— popping the host into its own battle and serving nobody — instead ofresumeCampaign(). Geoscape resume escaped because its term requiressessionLocked.Fix
Gate
_resumeToGameonlobbyMode != 2.lobbyMode == 2is set only byCoopSession::adoptResumeSave()— the resume-from-save lobby, which must always serve the client viaresumeCampaign(). The in-game coop menu (playtest B7) and a mid-battle reconnect are never mode 2, so they still return straight to the running game.Tests
The existing
test_coop_resume_battle_control/test_shared_resume_battle_controlmissed this because their helper pressed RESUME via thelobby_resume_campaigncommand, which callsLobbyMenu::resumeCampaign()directly and bypasses the_resumeToGamebranch where the bug lives. Changedsession.resume_campaign_battleto press the real button (lobby_action→btnCancelClick). Both tests now reproduce the bug without the fix and pass with it.Validated RED → GREEN:
test_resume_game_in_battle(in-game coop menu resume-to-running-game + [Bug] Host returns to Geoscape when clicking the "Resume Game" button #93 reconnect) ✅ no regression🤖 Generated with Claude Code