mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-07-07 19:15:59 +00:00
fix: survive host modal close during listed-lobby auto-start (#4514)
Follow-up to the listed-lobby auto-start feature (`2a82b01e3`, on main). ## Bug When a listed lobby auto-started (and in some paths on manual start), the game began server-side but a player got ejected back to the menu before it loaded. Root cause: `BaseModal.close()` navigates via `showPage()`, which **force-closes whichever page-modal is currently visible**. During the game-start transition that is the *other* lobby modal — closing the join modal first cascade-closed the host's modal with `leaveLobbyOnClose` still armed (host disconnected → host-left teardown killed the game); reversing the order just moved the victim to the joiner. No close order fixes both sides. ## Fix - **Disarm both lobby modals before closing either** in the prestart transition (`disarmLeaveOnClose()` on `HostLobbyModal` and `JoinLobbyModal`), so no cascade order can disconnect anyone mid game-start. - Both modals re-arm `leaveLobbyOnClose` in `onOpen` instead of `onClose`'s state reset, so once disarmed no later cascade close can re-arm it until the modal is reopened. - `HostLobbyModal.closeWithoutLeaving()` (pattern `JoinLobbyModal` already used), called explicitly instead of the generic modal-close loop. - **Server hardening:** `handleClientDisconnect` bails on `hasStarted()` (which includes prestart) instead of raw `_hasStarted`, so host socket churn during the lobby → game transition can never tear down a starting game regardless of client behavior. Regression test included. ## Verification Headless end-to-end (two browser contexts, host + joiner, `leave-lobby` stack-trace tripwire armed on both pages): - **Manual start:** host and joiner both receive prestart/start and enter the game; zero `leave-lobby` dispatches. - **Auto-start:** same, with nobody interacting after listing. Also confirmed manually in a real browser. The auto-start deadline was temporarily 30s during testing and is restored to 5 minutes in the final commit. Full suite green (1843 + 161), tsc/eslint/prettier clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -633,6 +633,11 @@ export class HostLobbyModal extends BaseModal {
|
||||
}
|
||||
|
||||
protected onOpen(): void {
|
||||
// Re-armed here (not in onClose's reset) so that once
|
||||
// closeWithoutLeaving() disarms it, no close cascade — e.g. another
|
||||
// modal's close() navigating via showPage, which force-closes this one —
|
||||
// can re-arm it and disconnect the host mid game-start.
|
||||
this.leaveLobbyOnClose = true;
|
||||
this.startLobbyUpdates();
|
||||
void getUserMe().then((userMe) => {
|
||||
// Dev skips the subscription gate (matching the server) so the
|
||||
@@ -695,6 +700,21 @@ export class HostLobbyModal extends BaseModal {
|
||||
);
|
||||
}
|
||||
|
||||
// Close as part of the lobby -> game transition (e.g. auto-start of a
|
||||
// listed lobby): the host is entering the game, not leaving the lobby, so
|
||||
// closing must not disconnect them (the server would tear the lobby down).
|
||||
// disarmLeaveOnClose is separate because closing ANY page-modal navigates
|
||||
// via showPage, which force-closes the currently visible page — so all
|
||||
// lobby modals must be disarmed before any of them is closed.
|
||||
public disarmLeaveOnClose() {
|
||||
this.leaveLobbyOnClose = false;
|
||||
}
|
||||
|
||||
public closeWithoutLeaving() {
|
||||
this.disarmLeaveOnClose();
|
||||
this.close();
|
||||
}
|
||||
|
||||
public confirmBeforeClose(): boolean | Promise<boolean> {
|
||||
return this.confirmClose(translateText("host_modal.leave_confirmation"));
|
||||
}
|
||||
@@ -765,8 +785,6 @@ export class HostLobbyModal extends BaseModal {
|
||||
this.publiclyListed = false;
|
||||
this.showSubscriptionRequired = false;
|
||||
this.autoStartAt = null;
|
||||
|
||||
this.leaveLobbyOnClose = true;
|
||||
}
|
||||
|
||||
private async handleSelectRandomMap() {
|
||||
|
||||
@@ -338,6 +338,10 @@ export class JoinLobbyModal extends BaseModal {
|
||||
}
|
||||
|
||||
protected onOpen(args?: Record<string, unknown>): void {
|
||||
// Re-armed here (not in onClose's reset) so that once
|
||||
// disarmLeaveOnClose() runs, no close cascade can re-arm it and
|
||||
// disconnect the player mid game-start.
|
||||
this.leaveLobbyOnClose = true;
|
||||
void this.hostedLobbySocket.start();
|
||||
const lobbyId = typeof args?.lobbyId === "string" ? args.lobbyId : "";
|
||||
const lobbyInfo = args?.lobbyInfo as GameInfo | PublicGameInfo | undefined;
|
||||
@@ -453,7 +457,6 @@ export class JoinLobbyModal extends BaseModal {
|
||||
this.serverTimeOffset = 0;
|
||||
this.lobbyCreatorClientID = null;
|
||||
this.isConnecting = true;
|
||||
this.leaveLobbyOnClose = true;
|
||||
}
|
||||
|
||||
disconnectedCallback() {
|
||||
@@ -474,8 +477,17 @@ export class JoinLobbyModal extends BaseModal {
|
||||
this.close();
|
||||
}
|
||||
|
||||
public closeWithoutLeaving() {
|
||||
// Closing this modal is part of the game-start transition, not the player
|
||||
// leaving. Kept separate from closeWithoutLeaving because closing ANY
|
||||
// page-modal navigates via showPage, which force-closes the currently
|
||||
// visible page — so all lobby modals must be disarmed before any of them
|
||||
// is closed.
|
||||
public disarmLeaveOnClose() {
|
||||
this.leaveLobbyOnClose = false;
|
||||
}
|
||||
|
||||
public closeWithoutLeaving() {
|
||||
this.disarmLeaveOnClose();
|
||||
this.close();
|
||||
}
|
||||
|
||||
|
||||
+8
-1
@@ -869,10 +869,17 @@ class Client {
|
||||
document
|
||||
.getElementById("username-validation-error")
|
||||
?.classList.add("hidden");
|
||||
// Disarm BOTH lobby modals before closing either: closing any
|
||||
// page-modal navigates via showPage, which force-closes the currently
|
||||
// visible page — the other lobby modal. If that one is still armed,
|
||||
// its onClose leaves the lobby and disconnects the player mid
|
||||
// game-start (host or joiner, depending on close order).
|
||||
this.hostModal?.disarmLeaveOnClose();
|
||||
this.joinModal?.disarmLeaveOnClose();
|
||||
this.hostModal?.closeWithoutLeaving();
|
||||
this.joinModal?.closeWithoutLeaving();
|
||||
[
|
||||
"single-player-modal",
|
||||
"host-lobby-modal",
|
||||
"game-starting-modal",
|
||||
"game-top-bar",
|
||||
"help-modal",
|
||||
|
||||
@@ -762,7 +762,10 @@ export class GameServer {
|
||||
(c) => c.clientID !== client.clientID,
|
||||
);
|
||||
|
||||
if (this._hasStarted) {
|
||||
// hasStarted() includes prestart: during the lobby -> game transition
|
||||
// clients reconnect, and a host socket closing then must not tear the
|
||||
// starting game down.
|
||||
if (this.hasStarted()) {
|
||||
return;
|
||||
}
|
||||
// Remove persistentId if the game has not started to prevent going over max players
|
||||
|
||||
@@ -332,6 +332,23 @@ describe("host-left lobby teardown", () => {
|
||||
(game as any)._hasEnded = true;
|
||||
expect(game.joinClient({} as any)).toBe("rejected");
|
||||
});
|
||||
|
||||
it("does not tear down when the host disconnects during prestart", () => {
|
||||
// During the lobby -> game transition the host modal closes and sockets
|
||||
// churn; a starting game (e.g. listed-lobby auto-start) must survive it.
|
||||
const gm = new GameManager(mockLogger);
|
||||
const game = gm.createGame("g-prestart", undefined, CREATOR)!;
|
||||
game.setListed(true);
|
||||
|
||||
const hostWs = fakeWs();
|
||||
game.joinClient(makeClient("host", CREATOR, hostWs));
|
||||
(game as any)._hasPrestarted = true;
|
||||
|
||||
hostWs.emit("close");
|
||||
|
||||
expect((game as any)._hasEnded).toBe(false);
|
||||
expect(game.phase()).not.toBe(GamePhase.Finished);
|
||||
});
|
||||
});
|
||||
|
||||
describe("listed lobby host powers", () => {
|
||||
|
||||
Reference in New Issue
Block a user