Skip to content

Fix client reconnection after page refresh#3117

Open
ryanbarlow97 wants to merge 6 commits intomainfrom
rejoingame
Open

Fix client reconnection after page refresh#3117
ryanbarlow97 wants to merge 6 commits intomainfrom
rejoingame

Conversation

@ryanbarlow97
Copy link
Contributor

Description:

Fix client reconnection after page refresh

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory
  • I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

w.o.n

@ryanbarlow97 ryanbarlow97 requested a review from a team as a code owner February 4, 2026 11:35
Copilot AI review requested due to automatic review settings February 4, 2026 11:35
@ryanbarlow97 ryanbarlow97 added the Bugfix Fixes a bug label Feb 4, 2026
@ryanbarlow97 ryanbarlow97 added this to the v30 milestone Feb 4, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 4, 2026

Walkthrough

Add persistent-ID–based reconnection: server maps persistentIDs to clientIDs, orchestrates socket/listener swap on reconnect, records kicked persistentIDs, and assigns clientIDs in lobby_info; clients stop sending clientID and host creation uses JWT (Authorization) for creator identity.

Changes

Cohort / File(s) Summary
Server: reconnection & identity
src/server/GameServer.ts, src/server/GameManager.ts, src/server/Worker.ts
Introduce persistentId↔clientId map and kickedPersistentIds; new APIs getClientIdForPersistentId, isReconnectingClient, getOrCreateClientId; centralize reconnect flow (swap sockets, reattach listeners, update lastPing, rebroadcast lobby_info); use creatorPersistentID from token.
Client: clientID removal & token-based host
src/client/ClientGameRunner.ts, src/client/HostLobbyModal.ts, src/client/JoinLobbyModal.ts, src/client/Matchmaking.ts, src/client/Transport.ts
Clients no longer send clientID on join/rejoin; host createLobby uses getPlayToken() and Authorization header; clients accept server-assigned yourClientID from lobby_info and store it.
Client: removed helper
src/client/Auth.ts
Removed getClientIDForGame and its sessionStorage-based caching; persistent-ID helpers unchanged.
Schemas / messages
src/core/Schemas.ts
Add yourClientID to ServerLobbyInfoMessageSchema; remove clientID from ClientJoinMessageSchema and ClientRejoinMessageSchema (server derives clientID from token/persistentID).

Sequence Diagram

sequenceDiagram
    participant Client
    participant Worker
    participant GameServer
    participant GameState

    Client->>Worker: WS CONNECT (includes token / persistentID)
    Worker->>GameServer: getOrCreateClientId(gameID, persistentID)
    alt persistentID mapped (reconnect)
        Worker->>GameServer: reconnectExistingClient(persistentID, ws)
        GameServer->>GameServer: close old ws, swap socket, update lastPing, mark connected
        GameServer->>GameServer: reattach listeners
        GameServer->>Client: send lobby_info (yourClientID)
        alt game already started
            GameServer->>GameState: request last turn/state
            GameState-->>GameServer: state
            GameServer->>Client: send start-game + state
        end
    else new client
        GameServer->>GameServer: create new clientID, map persistentID→clientID
        GameServer->>Client: send lobby_info (yourClientID)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

A token taps, "I'm still the same,"
Old sockets close, new hands reclaim.
IDs reborn in lobby light,
Listeners return, the game takes flight. 🎮✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main objective of the changeset: fixing client reconnection after page refresh.
Description check ✅ Passed The description is related to the changeset, explaining the fix for client reconnection after page refresh and confirming completion of testing and documentation requirements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/server/GameServer.ts`:
- Around line 162-186: The reconnect branch in joinClient skips the persistentID
check that rejoinClient enforces, allowing session hijack; update the reconnect
path in joinClient to validate that incoming client.persistentID matches
existingClient.persistentID, and if it does not, log a warning and close/reject
the new socket (do not replace existingClient.ws or add to activeClients); if it
matches, continue with updating existingClient.ws, lastPing,
markClientDisconnected(..., false), addListeners(existingClient) and use
existingClient.ws when calling sendStartGameMsg.
- Around line 169-171: When replacing an existing client's WebSocket (the code
that sets existingClient.ws = client.ws and updates existingClient.lastPing),
first grab the previous WebSocket (e.g., const previousWs = existingClient.ws),
and if it exists and is not already CLOSED/closing, call previousWs.close()
(optionally with a normal close code) to avoid leaking the old connection; do
NOT call this.websockets.delete(previousWs) — leave Set cleanup to the class
end() method. Ensure you then assign existingClient.ws = client.ws and update
lastPing as before.
🧹 Nitpick comments (1)
src/server/GameServer.ts (1)

182-184: Use existingClient.ws for clarity.

After line 170, both client.ws and existingClient.ws point to the same WebSocket. Using existingClient.ws here would be more consistent and easier to follow since existingClient is what's added to activeClients.

✏️ Suggested change
   // Send start message if game has already started
   if (this._hasStarted) {
-    this.sendStartGameMsg(client.ws, 0);
+    this.sendStartGameMsg(existingClient.ws, 0);
   }

@github-project-automation github-project-automation bot moved this from Triage to Development in OpenFront Release Management Feb 4, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements client reconnection functionality after page refresh by modifying the joinClient method in GameServer.ts. Previously, when a client attempted to rejoin with the same clientID, a warning was logged and the client was rejected. The new code now handles reconnection by updating the WebSocket reference, restoring the client's connected status, and re-sending game state if needed.

Changes:

  • Modified joinClient to detect and handle reconnection scenarios when a client with the same clientID already exists
  • Added logic to update WebSocket references, mark client as connected, restore to activeClients, and resend game start information

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ryanbarlow97 ryanbarlow97 marked this pull request as draft February 4, 2026 11:50
@ryanbarlow97 ryanbarlow97 marked this pull request as ready for review February 4, 2026 12:15
coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 4, 2026
@VariableVince VariableVince linked an issue Feb 4, 2026 that may be closed by this pull request
@ryanbarlow97 ryanbarlow97 marked this pull request as draft February 4, 2026 18:04
@ryanbarlow97 ryanbarlow97 marked this pull request as ready for review February 4, 2026 19:13
coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 4, 2026
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic looks pretty simiilar to rejoin. maybe instead we have the client call rejoin if it has already joined the server once?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/client/HostLobbyModal.ts (1)

638-666: ⚠️ Potential issue | 🟡 Minor

Set lobbyCreatorClientID after joining.

It now stays empty, so lobby-player-view cannot show the host or “you”. Consider setting it from the first client returned in pollPlayers (same as JoinLobbyModal) or from lobby_info.

✅ Suggested fix (pollPlayers)
-      .then((data: GameInfo) => {
-        this.clients = data.clients ?? [];
-      });
+      .then((data: GameInfo) => {
+        this.clients = data.clients ?? [];
+        if (!this.lobbyCreatorClientID && this.clients.length > 0) {
+          this.lobbyCreatorClientID = this.clients[0].clientID;
+        }
+      });
src/server/Worker.ts (1)

1-1: ⚠️ Potential issue | 🟡 Minor

Fix Prettier formatting before merge.

The CI pipeline reports formatting issues in this file. Run npx prettier --write src/server/Worker.ts to fix.

🤖 Fix all issues with AI agents
In `@src/client/ClientGameRunner.ts`:
- Around line 85-98: The file fails Prettier formatting; run the project's
formatter and commit the formatted changes to satisfy CI. Open
src/client/ClientGameRunner.ts (look for symbols like onconnect, onmessage,
terrainLoad, lobbyConfig) and apply the project's Prettier configuration (or run
npm/yarn script such as "npm run format" or "yarn format") to reformat the file,
then stage and push the updated file so the prettier check passes.

In `@src/client/JoinLobbyModal.ts`:
- Around line 346-350: startTrackingLobby currently clears this.currentClientID
and nothing sets it afterward; locate the handler that processes the server's
lobby_info message (or wherever yourClientID is received) and assign that value
into this.currentClientID (e.g., set this.currentClientID = message.yourClientID
or the equivalent field) so the lobby-player-view can identify the local player;
ensure the assignment happens after startTrackingLobby is called and consider
emitting/updating any dependent state or view update methods so the UI
re-renders with the new client id.

Comment on lines 85 to +98
const onconnect = () => {
if (hasJoined) {
console.log("rejoining game");
transport.rejoinGame(0);
} else {
hasJoined = true;
console.log(`Joining game lobby ${lobbyConfig.gameID}`);
transport.joinGame();
}
// Always send join - server will detect reconnection via persistentID
console.log(`Joining game lobby ${lobbyConfig.gameID}`);
transport.joinGame();
};
let terrainLoad: Promise<TerrainMapData> | null = null;

const onmessage = (message: ServerMessage) => {
if (message.type === "lobby_info") {
// Server tells us our assigned clientID
if (message.yourClientID) {
lobbyConfig.clientID = message.yourClientID;
console.log(`Received server-assigned clientID: ${message.yourClientID}`);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Prettier check failed for this file.

Please run the formatter so CI passes.

🤖 Prompt for AI Agents
In `@src/client/ClientGameRunner.ts` around lines 85 - 98, The file fails Prettier
formatting; run the project's formatter and commit the formatted changes to
satisfy CI. Open src/client/ClientGameRunner.ts (look for symbols like
onconnect, onmessage, terrainLoad, lobbyConfig) and apply the project's Prettier
configuration (or run npm/yarn script such as "npm run format" or "yarn format")
to reformat the file, then stage and push the updated file so the prettier check
passes.

Comment on lines 346 to 350
private startTrackingLobby(lobbyId: string, lobbyInfo?: GameInfo) {
this.currentLobbyId = lobbyId;
this.currentClientID = getClientIDForGame(lobbyId);
// clientID will be assigned by server via lobby_info message
this.currentClientID = "";
this.gameConfig = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update currentClientID once the server assigns it.

currentClientID is reset to empty and never updated, so lobby-player-view cannot mark the local player. Please wire yourClientID from lobby_info (or another shared state) into currentClientID.

🤖 Prompt for AI Agents
In `@src/client/JoinLobbyModal.ts` around lines 346 - 350, startTrackingLobby
currently clears this.currentClientID and nothing sets it afterward; locate the
handler that processes the server's lobby_info message (or wherever yourClientID
is received) and assign that value into this.currentClientID (e.g., set
this.currentClientID = message.yourClientID or the equivalent field) so the
lobby-player-view can identify the local player; ensure the assignment happens
after startTrackingLobby is called and consider emitting/updating any dependent
state or view update methods so the UI re-renders with the new client id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugfix Fixes a bug

Projects

Status: Development

Development

Successfully merging this pull request may close these issues.

Feature for coming back in game after being disconnected

2 participants