From 505cbde45420358b9258e37a31fd69a3b521e39a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Oct 2025 17:55:53 +0000 Subject: [PATCH 1/4] Initial plan From 064f6cc9a88bd5459f42f38ea07a5469c6b3cafc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Oct 2025 18:02:41 +0000 Subject: [PATCH 2/4] Refactor joinLobby to avoid top-level currentGameRunner variable - Changed joinLobby to return a LobbyControl object with cleanup and shouldPreventUnload methods - Moved currentGameRunner from module-level to local closure within joinLobby - Updated Main.ts to use the new LobbyControl API instead of separate gameStop and shouldPreventUnload functions - All tests pass and build is successful Co-authored-by: Foorack <5008081+Foorack@users.noreply.github.com> --- src/client/ClientGameRunner.ts | 32 +++++++++++++++++++------------- src/client/Main.ts | 24 ++++++++++++------------ 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/client/ClientGameRunner.ts b/src/client/ClientGameRunner.ts index 590bb58e5e..859f17f1ed 100644 --- a/src/client/ClientGameRunner.ts +++ b/src/client/ClientGameRunner.ts @@ -63,14 +63,9 @@ export interface LobbyConfig { gameRecord?: GameRecord; } -// Module-level reference to the current game runner -let currentGameRunner: ClientGameRunner | null = null; - -export function shouldPreventUnload(): boolean { - if (currentGameRunner) { - return currentGameRunner.shouldPreventWindowClose(); - } - return false; +export interface LobbyControl { + cleanup: () => void; + shouldPreventUnload: () => boolean; } export function joinLobby( @@ -78,7 +73,7 @@ export function joinLobby( lobbyConfig: LobbyConfig, onPrestart: () => void, onJoin: () => void, -): () => void { +): LobbyControl { console.log( `joining lobby: gameID: ${lobbyConfig.gameID}, clientID: ${lobbyConfig.clientID}`, ); @@ -88,6 +83,9 @@ export function joinLobby( const transport = new Transport(lobbyConfig, eventBus); + // Track the current game runner locally within this closure + let currentGameRunner: ClientGameRunner | null = null; + const onconnect = () => { console.log(`Joined game lobby ${lobbyConfig.gameID}`); transport.joinGame(0); @@ -140,10 +138,18 @@ export function joinLobby( } }; transport.connect(onconnect, onmessage); - return () => { - console.log("leaving game"); - currentGameRunner = null; - transport.leaveGame(); + return { + cleanup: () => { + console.log("leaving game"); + currentGameRunner = null; + transport.leaveGame(); + }, + shouldPreventUnload: () => { + if (currentGameRunner) { + return currentGameRunner.shouldPreventWindowClose(); + } + return false; + }, }; } diff --git a/src/client/Main.ts b/src/client/Main.ts index c468227572..320a60ad0c 100644 --- a/src/client/Main.ts +++ b/src/client/Main.ts @@ -6,7 +6,7 @@ import { ServerConfig } from "../core/configuration/Config"; import { getServerConfigFromClient } from "../core/configuration/ConfigLoader"; import { UserSettings } from "../core/game/UserSettings"; import "./AccountModal"; -import { joinLobby, shouldPreventUnload } from "./ClientGameRunner"; +import { joinLobby, LobbyControl } from "./ClientGameRunner"; import { fetchCosmetics } from "./Cosmetics"; import "./DarkModeButton"; import { DarkModeButton } from "./DarkModeButton"; @@ -80,7 +80,7 @@ export interface JoinLobbyEvent { } class Client { - private gameStop: (() => void) | null = null; + private lobbyControl: LobbyControl | null = null; private eventBus: EventBus = new EventBus(); private usernameInput: UsernameInput | null = null; @@ -155,14 +155,14 @@ class Client { window.addEventListener("beforeunload", (e) => { // Check if we should prevent unload (player is alive) - if (shouldPreventUnload()) { + if (this.lobbyControl && this.lobbyControl.shouldPreventUnload()) { e.preventDefault(); return ""; } // Otherwise, cleanup the game normally console.log("Browser is closing"); - if (this.gameStop !== null) { - this.gameStop(); + if (this.lobbyControl !== null) { + this.lobbyControl.cleanup(); } }); @@ -383,7 +383,7 @@ class Client { const onHashUpdate = () => { // Reset the UI to its initial state this.joinModal.close(); - if (this.gameStop !== null) { + if (this.lobbyControl !== null) { this.handleLeaveLobby(); } @@ -504,13 +504,13 @@ class Client { private async handleJoinLobby(event: CustomEvent) { const lobby = event.detail; console.log(`joining lobby ${lobby.gameID}`); - if (this.gameStop !== null) { + if (this.lobbyControl !== null) { console.log("joining lobby, stopping existing game"); - this.gameStop(); + this.lobbyControl.cleanup(); } const config = await getServerConfigFromClient(); - this.gameStop = joinLobby( + this.lobbyControl = joinLobby( this.eventBus, { gameID: lobby.gameID, @@ -596,12 +596,12 @@ class Client { } private async handleLeaveLobby(/* event: CustomEvent */) { - if (this.gameStop === null) { + if (this.lobbyControl === null) { return; } console.log("leaving lobby, cancelling game"); - this.gameStop(); - this.gameStop = null; + this.lobbyControl.cleanup(); + this.lobbyControl = null; this.publicLobby.leaveLobby(); } From ec67300e44ca8a9aa45caafb56bf92b24bc0c112 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Oct 2025 18:52:50 +0000 Subject: [PATCH 3/4] Simplify joinLobby return type to single function Instead of LobbyControl interface, joinLobby now returns a function that: - Checks if it should prevent closing (player alive in game) - Returns false if close was prevented, true if successful - Eliminates the need for separate cleanup and shouldPreventUnload methods This is a simpler approach as suggested in the code review. Co-authored-by: Foorack <5008081+Foorack@users.noreply.github.com> --- src/client/ClientGameRunner.ts | 28 ++++++++++------------------ src/client/Main.ts | 28 +++++++++++++--------------- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/src/client/ClientGameRunner.ts b/src/client/ClientGameRunner.ts index 859f17f1ed..2fc0ec5b31 100644 --- a/src/client/ClientGameRunner.ts +++ b/src/client/ClientGameRunner.ts @@ -63,17 +63,12 @@ export interface LobbyConfig { gameRecord?: GameRecord; } -export interface LobbyControl { - cleanup: () => void; - shouldPreventUnload: () => boolean; -} - export function joinLobby( eventBus: EventBus, lobbyConfig: LobbyConfig, onPrestart: () => void, onJoin: () => void, -): LobbyControl { +): () => boolean { console.log( `joining lobby: gameID: ${lobbyConfig.gameID}, clientID: ${lobbyConfig.clientID}`, ); @@ -138,18 +133,15 @@ export function joinLobby( } }; transport.connect(onconnect, onmessage); - return { - cleanup: () => { - console.log("leaving game"); - currentGameRunner = null; - transport.leaveGame(); - }, - shouldPreventUnload: () => { - if (currentGameRunner) { - return currentGameRunner.shouldPreventWindowClose(); - } - return false; - }, + return () => { + // Check if we should prevent closing (player is still alive in game) + if (currentGameRunner && currentGameRunner.shouldPreventWindowClose()) { + return false; // Don't close, return false to indicate prevented + } + console.log("leaving game"); + currentGameRunner = null; + transport.leaveGame(); + return true; // Successfully closed, return true }; } diff --git a/src/client/Main.ts b/src/client/Main.ts index 320a60ad0c..6427a2e3df 100644 --- a/src/client/Main.ts +++ b/src/client/Main.ts @@ -6,7 +6,7 @@ import { ServerConfig } from "../core/configuration/Config"; import { getServerConfigFromClient } from "../core/configuration/ConfigLoader"; import { UserSettings } from "../core/game/UserSettings"; import "./AccountModal"; -import { joinLobby, LobbyControl } from "./ClientGameRunner"; +import { joinLobby } from "./ClientGameRunner"; import { fetchCosmetics } from "./Cosmetics"; import "./DarkModeButton"; import { DarkModeButton } from "./DarkModeButton"; @@ -80,7 +80,7 @@ export interface JoinLobbyEvent { } class Client { - private lobbyControl: LobbyControl | null = null; + private leaveLobby: (() => boolean) | null = null; private eventBus: EventBus = new EventBus(); private usernameInput: UsernameInput | null = null; @@ -154,16 +154,14 @@ class Client { this.publicLobby = document.querySelector("public-lobby") as PublicLobby; window.addEventListener("beforeunload", (e) => { - // Check if we should prevent unload (player is alive) - if (this.lobbyControl && this.lobbyControl.shouldPreventUnload()) { + // Try to leave the lobby/game + if (this.leaveLobby && !this.leaveLobby()) { + // If leaveLobby returns false, it means we should prevent unload e.preventDefault(); return ""; } - // Otherwise, cleanup the game normally + // Successfully closed or no game running console.log("Browser is closing"); - if (this.lobbyControl !== null) { - this.lobbyControl.cleanup(); - } }); document.addEventListener("join-lobby", this.handleJoinLobby.bind(this)); @@ -383,7 +381,7 @@ class Client { const onHashUpdate = () => { // Reset the UI to its initial state this.joinModal.close(); - if (this.lobbyControl !== null) { + if (this.leaveLobby !== null) { this.handleLeaveLobby(); } @@ -504,13 +502,13 @@ class Client { private async handleJoinLobby(event: CustomEvent) { const lobby = event.detail; console.log(`joining lobby ${lobby.gameID}`); - if (this.lobbyControl !== null) { + if (this.leaveLobby !== null) { console.log("joining lobby, stopping existing game"); - this.lobbyControl.cleanup(); + this.leaveLobby(); } const config = await getServerConfigFromClient(); - this.lobbyControl = joinLobby( + this.leaveLobby = joinLobby( this.eventBus, { gameID: lobby.gameID, @@ -596,12 +594,12 @@ class Client { } private async handleLeaveLobby(/* event: CustomEvent */) { - if (this.lobbyControl === null) { + if (this.leaveLobby === null) { return; } console.log("leaving lobby, cancelling game"); - this.lobbyControl.cleanup(); - this.lobbyControl = null; + this.leaveLobby(); + this.leaveLobby = null; this.publicLobby.leaveLobby(); } From 2509030983e7a29574a14284d1dc85df71cf7b34 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Oct 2025 16:32:34 +0000 Subject: [PATCH 4/4] Rename leaveLobby to gameStop and remove non-essential comments - Renamed property from leaveLobby to gameStop to minimize diff from original - Removed verbose comments to keep only essential ones - Maintains same functionality with cleaner code Co-authored-by: Foorack <5008081+Foorack@users.noreply.github.com> --- src/client/ClientGameRunner.ts | 6 ++---- src/client/Main.ts | 21 +++++++++------------ 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/client/ClientGameRunner.ts b/src/client/ClientGameRunner.ts index 2fc0ec5b31..f2ded071e3 100644 --- a/src/client/ClientGameRunner.ts +++ b/src/client/ClientGameRunner.ts @@ -78,7 +78,6 @@ export function joinLobby( const transport = new Transport(lobbyConfig, eventBus); - // Track the current game runner locally within this closure let currentGameRunner: ClientGameRunner | null = null; const onconnect = () => { @@ -134,14 +133,13 @@ export function joinLobby( }; transport.connect(onconnect, onmessage); return () => { - // Check if we should prevent closing (player is still alive in game) if (currentGameRunner && currentGameRunner.shouldPreventWindowClose()) { - return false; // Don't close, return false to indicate prevented + return false; } console.log("leaving game"); currentGameRunner = null; transport.leaveGame(); - return true; // Successfully closed, return true + return true; }; } diff --git a/src/client/Main.ts b/src/client/Main.ts index 6427a2e3df..e9df5666e7 100644 --- a/src/client/Main.ts +++ b/src/client/Main.ts @@ -80,7 +80,7 @@ export interface JoinLobbyEvent { } class Client { - private leaveLobby: (() => boolean) | null = null; + private gameStop: (() => boolean) | null = null; private eventBus: EventBus = new EventBus(); private usernameInput: UsernameInput | null = null; @@ -154,13 +154,10 @@ class Client { this.publicLobby = document.querySelector("public-lobby") as PublicLobby; window.addEventListener("beforeunload", (e) => { - // Try to leave the lobby/game - if (this.leaveLobby && !this.leaveLobby()) { - // If leaveLobby returns false, it means we should prevent unload + if (this.gameStop && !this.gameStop()) { e.preventDefault(); return ""; } - // Successfully closed or no game running console.log("Browser is closing"); }); @@ -381,7 +378,7 @@ class Client { const onHashUpdate = () => { // Reset the UI to its initial state this.joinModal.close(); - if (this.leaveLobby !== null) { + if (this.gameStop !== null) { this.handleLeaveLobby(); } @@ -502,13 +499,13 @@ class Client { private async handleJoinLobby(event: CustomEvent) { const lobby = event.detail; console.log(`joining lobby ${lobby.gameID}`); - if (this.leaveLobby !== null) { + if (this.gameStop !== null) { console.log("joining lobby, stopping existing game"); - this.leaveLobby(); + this.gameStop(); } const config = await getServerConfigFromClient(); - this.leaveLobby = joinLobby( + this.gameStop = joinLobby( this.eventBus, { gameID: lobby.gameID, @@ -594,12 +591,12 @@ class Client { } private async handleLeaveLobby(/* event: CustomEvent */) { - if (this.leaveLobby === null) { + if (this.gameStop === null) { return; } console.log("leaving lobby, cancelling game"); - this.leaveLobby(); - this.leaveLobby = null; + this.gameStop(); + this.gameStop = null; this.publicLobby.leaveLobby(); }