Reduce lobby broadcast bandwidth via counts-only deltas#4116
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughServer fingerprints and caches full lobby snapshots, broadcasting either full snapshots or counts-only deltas; client caches a full snapshot, applies incoming counts deltas to that cache, clears the cache on reconnect/stop, and tests verify message handling and immutability. ChangesLobby WebSocket Delta Update Protocol
Sequence DiagramsequenceDiagram
participant Server as WorkerLobbyService
participant Client as PublicLobbySocket
participant Cache as lastFull
rect rgba(100,150,200,0.5)
Note over Server,Cache: Initial Connection
Server->>Server: check lastPublicGames
Server->>Client: send {type:full, games:{...}}
Client->>Cache: store snapshot
end
rect rgba(150,200,100,0.5)
Note over Server,Cache: Subsequent Updates (no structural change)
Server->>Server: fingerprint unchanged
Server->>Client: send {type:counts, counts:{gameID:numClients}}
Client->>Cache: patch numClients only
end
rect rgba(200,150,100,0.5)
Note over Server,Cache: State Changed
Server->>Server: fingerprint changed
Server->>Client: send {type:full, games:{...}}
Client->>Cache: reset snapshot
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/Schemas.ts`:
- Around line 186-206: Add unit tests that validate the new lobby wire contract
by using the PublicLobbyFullSchema, PublicLobbyCountsSchema and
PublicLobbyMessageSchema to parse/validate messages: write tests that assert
valid "full" payloads (with serverTime, games conforming to PublicGameTypeSchema
and PublicGameInfoSchema) and valid "counts" payloads (with serverTime and
counts map) successfully parse, and tests that mixed/invalid payloads (e.g., a
"full" object containing a counts field or wrong types) are rejected by
PublicLobbyMessageSchema; ensure tests also use the exported type
PublicLobbyMessage where applicable to assert runtime schema matches the
intended TypeScript shape.
In `@src/server/WorkerLobbyService.ts`:
- Around line 17-35: The fingerprintLobbies function should build a stable
fingerprint from all lobby fields except numClients and ensure a deterministic
order; update fingerprintLobbies to sort each games[type] array (e.g., by
lobby.gameID then other stable keys) and include a serialization of every
non-numClients field (for example include lobby.gameID, lobby.startsAt,
JSON.stringify(lobby.gameConfig || {}), and any other public fields on
PublicGameInfo) rather than only a config-existence flag; this ensures
config-only changes and reorderings produce different fingerprints and prevent
stale client state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 760e0217-2575-4edf-95a5-02254794c62d
📒 Files selected for processing (3)
src/client/LobbySocket.tssrc/core/Schemas.tssrc/server/WorkerLobbyService.ts
Comment on lines
+17
to
+35
| const FULL_BROADCAST_INTERVAL_MS = 5000; | ||
|
|
||
| // Stable summary of everything in the lobby snapshot except numClients. | ||
| // When the fingerprint matches the last full we sent, a counts-only delta | ||
| // is enough; otherwise we need a fresh full snapshot. | ||
| function fingerprintLobbies( | ||
| games: Record<PublicGameType, PublicGameInfo[]>, | ||
| ): string { | ||
| const parts: string[] = []; | ||
| for (const type of Object.keys(games).sort() as PublicGameType[]) { | ||
| parts.push(type); | ||
| for (const lobby of games[type]) { | ||
| parts.push( | ||
| `${lobby.gameID}|${lobby.startsAt ?? ""}|${lobby.gameConfig ? 1 : 0}`, | ||
| ); | ||
| } | ||
| } | ||
| return parts.join(","); | ||
| } |
Contributor
There was a problem hiding this comment.
Make the lobby fingerprint complete and order-stable.
Right now the fingerprint only includes gameID, startsAt, and whether gameConfig exists. A config-only change will keep sending counts, so clients stay stale until the 5s full refresh. Also, the lobby arrays are not sorted, so reorder-only broadcasts can force unnecessary full snapshots. Build the fingerprint from a sorted view of all non-numClients fields instead.
Possible fix
function fingerprintLobbies(
games: Record<PublicGameType, PublicGameInfo[]>,
): string {
- const parts: string[] = [];
- for (const type of Object.keys(games).sort() as PublicGameType[]) {
- parts.push(type);
- for (const lobby of games[type]) {
- parts.push(
- `${lobby.gameID}|${lobby.startsAt ?? ""}|${lobby.gameConfig ? 1 : 0}`,
- );
- }
- }
- return parts.join(",");
+ const normalized = (Object.keys(games).sort() as PublicGameType[]).flatMap(
+ (type) =>
+ [...games[type]]
+ .sort((a, b) => a.gameID.localeCompare(b.gameID))
+ .map((lobby) => ({
+ type,
+ gameID: lobby.gameID,
+ startsAt: lobby.startsAt ?? null,
+ publicGameType: lobby.publicGameType,
+ gameConfig: lobby.gameConfig ?? null,
+ })),
+ );
+ return JSON.stringify(normalized);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/WorkerLobbyService.ts` around lines 17 - 35, The
fingerprintLobbies function should build a stable fingerprint from all lobby
fields except numClients and ensure a deterministic order; update
fingerprintLobbies to sort each games[type] array (e.g., by lobby.gameID then
other stable keys) and include a serialization of every non-numClients field
(for example include lobby.gameID, lobby.startsAt,
JSON.stringify(lobby.gameConfig || {}), and any other public fields on
PublicGameInfo) rather than only a config-existence flag; this ensures
config-only changes and reorderings produce different fingerprints and prevent
stale client state.
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
evanpelle
force-pushed
the
reduce-lobby-bandwidth
branch
from
June 2, 2026 19:53
fb1f0be to
0337ae2
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/server/WorkerLobbyService.ts (1)
22-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
gameID-only fingerprint misses real lobby updates.Line 86 updates
startsAton an existing game. The fingerprint at Lines 163-170 only uses sortedgameIDs, so that change still goes out as"counts"and clients keep stalestartsAt/gameConfig/ other lobby metadata. Fingerprint a stable, sorted view of all non-numClientslobby fields instead.Also applies to: 78-87, 163-193
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/WorkerLobbyService.ts` around lines 22 - 27, The current fingerprint stored in lastFullGameIds (a string) only includes sorted gameIDs and misses updates to startsAt/gameConfig/etc; update the fingerprint logic (the code that currently builds the "counts"/full decision — the block that reads/sets lastFullGameIds) to produce a deterministic fingerprint over each lobby's stable, sorted representation of all fields except numClients: for each lobby, build an object excluding numClients, produce a JSON string with keys sorted deterministically, sort the resulting per-lobby strings by gameID (or the existing stable key), join them into one string and use a hash or that joined string for lastFullGameIds; encapsulate this in a helper like computeLobbyFingerprint(lobbies) and use it wherever the old gameID-only fingerprint was used so changes to startsAt/gameConfig will trigger a full update.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/server/WorkerLobbyService.ts`:
- Around line 19-21: The cached connect snapshot lastFullJson is only updated on
the "full" branch so new clients can receive stale numClients after a "counts"
broadcast; modify the "counts" handling to refresh lastFullJson (or recreate the
connect payload) using the current publicGames/numClients state before it's sent
to new connections. Concretely, in the method that processes master broadcasts
(the branch handling the "counts" message), regenerate the same serialized
connect-time payload you build for the "full" path (or call the helper that
builds it from publicGames) and assign it to lastFullJson so subsequent
on-connect sends the up-to-date snapshot. Ensure the same change is applied to
all places noted (the counts path and the places around the existing
full-snapshot send logic).
- Around line 21-27: The broadcast logic treats an empty lobby fingerprint ("")
as equal to the initialized lastFullGameIds, so the first broadcast sends a
counts-only delta with no base; change lastFullGameIds from string = "" to
string | null = null (or another unambiguous sentinel) and update the comparison
in the broadcast/emit logic (the code that decides between "full" and "counts",
referencing lastFullGameIds and lastFullJson) to treat null as "no previous full
sent" and force sending a full snapshot for the first broadcast (also ensure
lastFullJson is set after sending the full snapshot).
---
Duplicate comments:
In `@src/server/WorkerLobbyService.ts`:
- Around line 22-27: The current fingerprint stored in lastFullGameIds (a
string) only includes sorted gameIDs and misses updates to
startsAt/gameConfig/etc; update the fingerprint logic (the code that currently
builds the "counts"/full decision — the block that reads/sets lastFullGameIds)
to produce a deterministic fingerprint over each lobby's stable, sorted
representation of all fields except numClients: for each lobby, build an object
excluding numClients, produce a JSON string with keys sorted deterministically,
sort the resulting per-lobby strings by gameID (or the existing stable key),
join them into one string and use a hash or that joined string for
lastFullGameIds; encapsulate this in a helper like
computeLobbyFingerprint(lobbies) and use it wherever the old gameID-only
fingerprint was used so changes to startsAt/gameConfig will trigger a full
update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9ce65a22-fc5b-4f54-b959-99d4c0160a80
📒 Files selected for processing (4)
src/client/LobbySocket.tssrc/core/Schemas.tssrc/server/WorkerLobbyService.tstests/LobbySocket.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/Schemas.ts
evanpelle
changed the title
evanpelle
changed the title
evanpelle
marked this pull request as ready for review
June 2, 2026 22:32
evanpelle
force-pushed
the
reduce-lobby-bandwidth
branch
from
June 2, 2026 22:47
0337ae2 to
0bdb117
Compare
github-project-automation
Bot
moved this from Development
to Complete
in OpenFront Release Management
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.
Description:
/lobbies) was re-sending the fullPublicGamessnapshot — including each lobby'sgameConfig— to every connected client every 500ms. Almost nothing in that payload changes tick-to-tick; onlynumClientsmoves.WorkerLobbyServicenow tracks the sorted set ofgameIDs it last sent as a full snapshot. On each incoming broadcast it sends afullonly when that set changes; otherwise it sends acountsdelta carrying just{gameID → numClients}.startsAt, it both setsstartsAtAND schedules a fresh lobby on the same tick, so the gameID change brings thestartsAt(andgameConfig) along with it.fullso late joiners don't have to wait for the next structural change.LobbySocketparses the new discriminated union (PublicLobbyMessage = full | counts), keeps the last full snapshot in memory, and merges counts into it before invoking the existing callback.GameModeSelectoris unchanged.Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
evan