Pull replay files from api #1725
Conversation
WalkthroughAdded a server-config Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Archive
participant Config
participant PublicAPI
Client->>Archive: readGameRecord(gameId)
Archive->>Archive: try read from primary R2 storage
alt Primary returns body
Archive->>Archive: parse & validate (RedactedGameRecordSchema)
Archive-->>Client: return RedactedGameRecord
else Primary missing / invalid / error
Archive->>Config: replayUrl(gameId)
Archive->>PublicAPI: HTTP GET fallback (Accept: application/json, timeout)
alt Public API 200 + JSON
PublicAPI-->>Archive: JSON body
Archive->>Archive: validate (RedactedGameRecordSchema)
Archive-->>Client: return RedactedGameRecord
else 404 / non-JSON / error
Archive-->>Client: return error string ("replay.not_found" / "replay.error" / "replay.invalid")
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
scottanderson
added
Feature - Frontend
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/server/Archive.ts (2)
184-184: Add schema validation for response data.The response JSON is cast to
GameRecordwithout validation, which could cause runtime errors if the API returns unexpected data structure.
184-184: Critical: Missing schema validation for API responseThe response JSON is cast to GameRecord without validation, creating unsafe type assumptions. Based on past comments, GameRecordSchema validation fails with API responses due to missing fields like
persistentIDand differentintent.typestructures.You need to either:
- Fix the API to return data matching GameRecordSchema, or
- Create a separate schema for API responses, or
- Add proper error handling for schema mismatches
- return await response.json(); + const json = await response.json(); + try { + return GameRecordSchema.parse(json); + } catch (parseError) { + log.info(`${gameId}: API response doesn't match expected schema: ${parseError}`); + return null; + }
🧹 Nitpick comments (1)
src/server/Archive.ts (1)
161-202: Consider adding timeout and retry logicThe fallback function lacks timeout protection and retry logic for network requests. Consider adding:
- Request timeout to prevent hanging
- Simple retry logic for transient failures
- More specific error handling for different HTTP status codes
+const FALLBACK_TIMEOUT = 10000; // 10 seconds + export async function readGameRecordFallback( gameId: GameID, ): Promise<GameRecord | null> { try { - const response = await fetch(config.replayFallbackUrl(gameId), { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), FALLBACK_TIMEOUT); + + const response = await fetch(config.replayFallbackUrl(gameId), { headers: { Accept: "application/json", }, + signal: controller.signal, }); + + clearTimeout(timeoutId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/core/configuration/DefaultConfig.ts(1 hunks)src/core/configuration/DevConfig.ts(2 hunks)src/server/Archive.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/configuration/DefaultConfig.ts
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
📚 Learning: in the codebase, playerstats is defined as `z.infer` where playerstatssche...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as `z.infer<typeof PlayerStatsSchema>` where PlayerStatsSchema has `.optional()` applied at the object level, making PlayerStats a union type that already includes undefined (PlayerStats | undefined).
Applied to files:
src/core/configuration/DevConfig.tssrc/server/Archive.ts
📚 Learning: in the codebase, playerstats is defined as a type inferred from a zod schema that is marked as optio...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as a type inferred from a Zod schema that is marked as optional, which means PlayerStats already includes undefined as a possible type (PlayerStats | undefined).
Applied to files:
src/core/configuration/DevConfig.tssrc/server/Archive.ts
📚 Learning: in the openfrontio codebase, usersettings instances are created directly with `new usersettings()` i...
Learnt from: devalnor
PR: openfrontio/OpenFrontIO#1195
File: src/client/graphics/layers/AlertFrame.ts:18-18
Timestamp: 2025-06-20T20:11:00.965Z
Learning: In the OpenFrontIO codebase, UserSettings instances are created directly with `new UserSettings()` in each component that needs them. This pattern is used consistently across at least 12+ files including OptionsMenu, EventsDisplay, DarkModeButton, Main, UserSettingModal, UsernameInput, NameLayer, AlertFrame, UILayer, InputHandler, ClientGameRunner, and GameView. This is the established architectural pattern and should be followed for consistency.
Applied to files:
src/core/configuration/DevConfig.ts
📚 Learning: the playerstats type from archiveschemas already includes undefined in its definition, making explic...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:44-53
Timestamp: 2025-05-21T04:10:59.706Z
Learning: The PlayerStats type from ArchiveSchemas already includes undefined in its definition, making explicit union types with undefined (PlayerStats | undefined) redundant.
Applied to files:
src/core/configuration/DevConfig.tssrc/server/Archive.ts
📚 Learning: when working with zod schemas in the openfrontio project, values can be marked as possibly undefined...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:189-209
Timestamp: 2025-05-18T23:43:38.150Z
Learning: When working with zod schemas in the OpenFrontIO project, values can be marked as possibly undefined by zod even if logic suggests they should always be defined. It's better to preserve the `| undefined` union types in method signatures to align with zod's type inference, even if runtime checks throw errors for invalid cases.
Applied to files:
src/server/Archive.ts
📚 Learning: in zod, when a schema has `.optional()` applied at the object level, the typescript type inferred us...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:44-53
Timestamp: 2025-05-21T04:10:59.706Z
Learning: In Zod, when a schema has `.optional()` applied at the object level, the TypeScript type inferred using `z.infer` already includes `undefined` in the union type. Therefore, when using such a type as a return type annotation, there's no need to explicitly add `| undefined`.
Applied to files:
src/server/Archive.ts
📚 Learning: in zod v4, the correct import is `import { z } from "zod/v4"` and z.prettifyerror(error) is a built-...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1161
File: src/server/jwt.ts:0-0
Timestamp: 2025-06-19T19:31:29.475Z
Learning: In Zod v4, the correct import is `import { z } from "zod/v4"` and z.prettifyError(error) is a built-in utility method for formatting ZodError into readable, multi-line strings. This is different from Zod v3.x which required different error handling approaches.
Applied to files:
src/server/Archive.ts
🔇 Additional comments (9)
src/core/configuration/DevConfig.ts (4)
3-3: LGTM!The
GameIDimport is needed for the new method parameter type.
46-48: LGTM!Clean implementation using template literal. The hardcoded URL approach is appropriate for dev config.
3-3: LGTM: Import addition is correctThe GameID import is properly added for the new method parameter type.
46-48: LGTM: Clean implementation of fallback URL methodThe method correctly implements the interface with a hardcoded URL appropriate for development configuration. The template string interpolation is clean and typed.
src/server/Archive.ts (5)
134-137: LGTM!Good fallback handling when R2 returns empty response body. The warning log helps with debugging.
144-158: LGTM!Smart change to use info level logging since R2 failures are no longer terminal with the fallback mechanism in place.
185-201: LGTM!Consistent error handling pattern with appropriate info level logging for the fallback mechanism.
134-137: Good handling of undefined response bodyThe explicit check for undefined response body with appropriate logging and fallback call improves robustness.
144-158: LGTM: Error handling improvementsThe change to log at
infolevel instead oferroraddresses previous feedback and makes sense for fallback scenarios. The consistent error handling structure is maintained.
|
Thanks for the quick PR for this. Please note the supported keywords at https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue I have manually linked the referenced ticket, so no need to do it on this PR. |
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/server/Archive.ts (1)
218-218: Add schema validation for API response.The function performs unsafe casting of the JSON response to
GameRecord. Based on past review discussions, this should be validated with a proper schema to prevent runtime errors.Consider implementing the schema validation approach discussed in previous reviews, or use a more lenient schema specifically for API responses if the full
GameRecordschema is too strict.
🧹 Nitpick comments (2)
src/server/Archive.ts (2)
36-36: Fix typo in function name.The function name has a typo:
stripPerisistentIdsshould bestripPersistentIds.Apply this diff to fix the typo:
- await archiveFullGameToR2(stripPerisistentIds(gameRecord)); + await archiveFullGameToR2(stripPersistentIds(gameRecord));
178-192: Consider the impact of reducing error logging level.The error logging was changed from
errortoinfolevel. While this reduces noise in logs, it might make debugging R2 connectivity issues harder to spot. Consider ifwarnlevel would be more appropriate for actual failures.The fallback mechanism implementation looks solid and provides good resilience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/Schemas.ts(1 hunks)src/server/Archive.ts(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/core/Schemas.ts
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
📚 Learning: in the codebase, playerstats is defined as `z.infer` where playerstatssche...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as `z.infer<typeof PlayerStatsSchema>` where PlayerStatsSchema has `.optional()` applied at the object level, making PlayerStats a union type that already includes undefined (PlayerStats | undefined).
Applied to files:
src/server/Archive.ts
📚 Learning: in the codebase, playerstats is defined as a type inferred from a zod schema that is marked as optio...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as a type inferred from a Zod schema that is marked as optional, which means PlayerStats already includes undefined as a possible type (PlayerStats | undefined).
Applied to files:
src/server/Archive.ts
📚 Learning: when working with zod schemas in the openfrontio project, values can be marked as possibly undefined...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:189-209
Timestamp: 2025-05-18T23:43:38.150Z
Learning: When working with zod schemas in the OpenFrontIO project, values can be marked as possibly undefined by zod even if logic suggests they should always be defined. It's better to preserve the `| undefined` union types in method signatures to align with zod's type inference, even if runtime checks throw errors for invalid cases.
Applied to files:
src/server/Archive.ts
📚 Learning: in zod, when a schema has `.optional()` applied at the object level, the typescript type inferred us...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:44-53
Timestamp: 2025-05-21T04:10:59.706Z
Learning: In Zod, when a schema has `.optional()` applied at the object level, the TypeScript type inferred using `z.infer` already includes `undefined` in the union type. Therefore, when using such a type as a return type annotation, there's no need to explicitly add `| undefined`.
Applied to files:
src/server/Archive.ts
📚 Learning: the playerstats type from archiveschemas already includes undefined in its definition, making explic...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:44-53
Timestamp: 2025-05-21T04:10:59.706Z
Learning: The PlayerStats type from ArchiveSchemas already includes undefined in its definition, making explicit union types with undefined (PlayerStats | undefined) redundant.
Applied to files:
src/server/Archive.ts
📚 Learning: in zod v4, the correct import is `import { z } from "zod/v4"` and z.prettifyerror(error) is a built-...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1161
File: src/server/jwt.ts:0-0
Timestamp: 2025-06-19T19:31:29.475Z
Learning: In Zod v4, the correct import is `import { z } from "zod/v4"` and z.prettifyError(error) is a built-in utility method for formatting ZodError into readable, multi-line strings. This is different from Zod v3.x which required different error handling approaches.
Applied to files:
src/server/Archive.ts
📚 Learning: in the openfrontio project, the correct path for icons from src/client/index.html is ../../resources...
Learnt from: tnhnblgl
PR: openfrontio/OpenFrontIO#875
File: src/client/index.html:396-402
Timestamp: 2025-05-26T09:52:52.465Z
Learning: In the OpenFrontIO project, the correct path for icons from src/client/index.html is ../../resources/icons/, not /static/icons/ as the build process handles the path mapping differently.
Applied to files:
src/server/Archive.ts
📚 Learning: in the openfrontio codebase, for unit tests it's acceptable to let exceptions (like json parsing err...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1441
File: tests/LangCode.test.ts:19-32
Timestamp: 2025-07-15T17:20:17.831Z
Learning: In the OpenFrontIO codebase, for unit tests it's acceptable to let exceptions (like JSON parsing errors) propagate and fail the test naturally rather than adding explicit error handling with try-catch blocks, as the test framework will catch them and fail the test appropriately.
Applied to files:
src/server/Archive.ts
🧬 Code Graph Analysis (1)
src/server/Archive.ts (2)
src/core/Schemas.ts (3)
GameRecord(560-560)AnalyticsRecord(555-555)GameID(21-21)src/core/Util.ts (1)
replacer(287-289)
🪛 GitHub Actions: 🧪 CI
src/server/Archive.ts
[error] 98-98: TypeScript error TS2322: Type '{ clientID: string; username: string; pattern: string | undefined; flag: string | undefined; }[]' is not assignable to type with required property 'persistentID'. Property 'persistentID' is missing but required.
🔇 Additional comments (3)
src/server/Archive.ts (3)
57-68: LGTM!Clean implementation that extracts analytics data by excluding the
turnsfield. The destructuring approach is readable and type-safe.
108-109: LGTM!Good refactoring - the function now has a clear single responsibility by accepting pre-processed
AnalyticsRecordinstead of extracting analytics data internally.
146-147: LGTM!Good separation of concerns - the archiving function now focuses solely on storage while data sanitization is handled by the caller.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/server/Archive.ts(6 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: the playerstats type from archiveschemas already includes undefined in its definition, making explic...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:44-53
Timestamp: 2025-05-21T04:10:59.706Z
Learning: The PlayerStats type from ArchiveSchemas already includes undefined in its definition, making explicit union types with undefined (PlayerStats | undefined) redundant.
Applied to files:
src/server/Archive.ts
📚 Learning: in the codebase, playerstats is defined as `z.infer` where playerstatssche...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as `z.infer<typeof PlayerStatsSchema>` where PlayerStatsSchema has `.optional()` applied at the object level, making PlayerStats a union type that already includes undefined (PlayerStats | undefined).
Applied to files:
src/server/Archive.ts
📚 Learning: in the codebase, playerstats is defined as a type inferred from a zod schema that is marked as optio...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as a type inferred from a Zod schema that is marked as optional, which means PlayerStats already includes undefined as a possible type (PlayerStats | undefined).
Applied to files:
src/server/Archive.ts
📚 Learning: when working with zod schemas in the openfrontio project, values can be marked as possibly undefined...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:189-209
Timestamp: 2025-05-18T23:43:38.150Z
Learning: When working with zod schemas in the OpenFrontIO project, values can be marked as possibly undefined by zod even if logic suggests they should always be defined. It's better to preserve the `| undefined` union types in method signatures to align with zod's type inference, even if runtime checks throw errors for invalid cases.
Applied to files:
src/server/Archive.ts
📚 Learning: in zod, when a schema has `.optional()` applied at the object level, the typescript type inferred us...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:44-53
Timestamp: 2025-05-21T04:10:59.706Z
Learning: In Zod, when a schema has `.optional()` applied at the object level, the TypeScript type inferred using `z.infer` already includes `undefined` in the union type. Therefore, when using such a type as a return type annotation, there's no need to explicitly add `| undefined`.
Applied to files:
src/server/Archive.ts
📚 Learning: in zod v4, the correct import is `import { z } from "zod/v4"` and z.prettifyerror(error) is a built-...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1161
File: src/server/jwt.ts:0-0
Timestamp: 2025-06-19T19:31:29.475Z
Learning: In Zod v4, the correct import is `import { z } from "zod/v4"` and z.prettifyError(error) is a built-in utility method for formatting ZodError into readable, multi-line strings. This is different from Zod v3.x which required different error handling approaches.
Applied to files:
src/server/Archive.ts
📚 Learning: in the openfrontio project, the correct path for icons from src/client/index.html is ../../resources...
Learnt from: tnhnblgl
PR: openfrontio/OpenFrontIO#875
File: src/client/index.html:396-402
Timestamp: 2025-05-26T09:52:52.465Z
Learning: In the OpenFrontIO project, the correct path for icons from src/client/index.html is ../../resources/icons/, not /static/icons/ as the build process handles the path mapping differently.
Applied to files:
src/server/Archive.ts
📚 Learning: in the openfrontio codebase, for unit tests it's acceptable to let exceptions (like json parsing err...
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1441
File: tests/LangCode.test.ts:19-32
Timestamp: 2025-07-15T17:20:17.831Z
Learning: In the OpenFrontIO codebase, for unit tests it's acceptable to let exceptions (like JSON parsing errors) propagate and fail the test naturally rather than adding explicit error handling with try-catch blocks, as the test framework will catch them and fail the test appropriately.
Applied to files:
src/server/Archive.ts
📚 Learning: in the openfrontio codebase, `checkpermission()` function's return values need to be handled with de...
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#709
File: src/client/Main.ts:276-296
Timestamp: 2025-05-16T12:06:01.732Z
Learning: In the OpenFrontIO codebase, `checkPermission()` function's return values need to be handled with defensive checks using `Array.isArray()`, even though its TypeScript signature indicates it returns arrays. Removing these checks breaks functionality.
Applied to files:
src/server/Archive.ts
🧬 Code Graph Analysis (1)
src/server/Archive.ts (2)
src/core/Schemas.ts (5)
GameRecord(560-560)AnalyticsRecord(555-555)RedactedGameRecord(573-573)GameID(21-21)RedactedGameRecordSchema(562-572)src/core/Util.ts (1)
replacer(287-289)
🔇 Additional comments (8)
src/server/Archive.ts (8)
36-36: LGTM! Proper data redaction before archiving.The preprocessing approach correctly separates analytics data from full game data and removes sensitive information before storage.
Also applies to: 43-43
64-75: LGTM! Clean implementation with proper type safety.The function clearly extracts analytics data while excluding turns, using readable destructuring syntax.
77-113: LGTM! Addresses past review feedback effectively.The function correctly:
- Fixes the typo in function name
- Uses proper RedactedGameRecord return type
- Cleanly removes persistentID fields using functional mapping
- Uses underscore convention for unused destructured variables
115-116: LGTM! Proper type alignment with preprocessing.The function signature correctly accepts AnalyticsRecord, making the archiving process more type-safe and purposeful.
Also applies to: 124-124
149-149: LGTM! Enhanced data privacy with RedactedGameRecord.The function signature change ensures sensitive data cannot be accidentally archived to full game storage.
Also applies to: 153-154
167-167: LGTM! Excellent resilience improvements.The changes provide:
- Consistent RedactedGameRecord return type
- Proper fallback when R2 response is empty
- Appropriate info-level logging for expected failures
- Centralized validation through validateRecord helper
The fallback mechanism ensures users can replay games even when primary storage fails.
Also applies to: 175-200
202-243: LGTM! Well-implemented HTTP fallback mechanism.The function demonstrates good practices:
- Proper HTTP headers and status checking
- Content type validation before parsing
- Appropriate info-level error logging for fallback scenarios
- Consistent schema validation using validateRecord helper
245-258: LGTM! Clean validation helper with proper error handling.The function correctly:
- Centralizes schema validation logic
- Uses safeParse for graceful error handling
- Formats errors with z.prettifyError (consistent with project patterns)
- Provides clear return type semantics
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/client/JoinPrivateLobbyModal.ts (2)
243-254: Localize “Version mismatch” messageThis user-facing string bypasses translateText, which breaks i18n and contradicts the PR goal that all UI text is localized.
Apply this diff:
- this.message = - "This game was created with a different version. Cannot join."; + this.message = translateText("replay.error.version_mismatch");Optionally include details via params if you expose safe fields:
// example if details has { serverHash, clientHash } this.message = translateText("replay.error.version_mismatch", { // serverHash, clientHash });Per team standards, update en.json only; other locales will be handled by translators.
130-141: Bug: lobby id cleared before dispatching "leave-lobby"close() clears lobbyIdInput.value (Line 122). You then dispatch leave-lobby using the cleared value. Capture it first.
- public closeAndLeave() { - this.close(); - this.hasJoined = false; - this.message = ""; - this.dispatchEvent( - new CustomEvent("leave-lobby", { - detail: { lobby: this.lobbyIdInput.value }, - bubbles: true, - composed: true, - }), - ); - } + public closeAndLeave() { + const lobby = this.lobbyIdInput.value; + this.close(); + this.hasJoined = false; + this.message = ""; + this.dispatchEvent( + new CustomEvent("leave-lobby", { + detail: { lobby }, + bubbles: true, + composed: true, + }), + ); + }
♻️ Duplicate comments (1)
src/client/JoinPrivateLobbyModal.ts (1)
269-271: Good: translate error key and short-circuit once handledUsing translateText(archiveData.error) and returning early matches the earlier guidance to send translation keys from the server. This keeps UI strings localized and avoids falling through to “not found.”
🧹 Nitpick comments (2)
src/client/JoinPrivateLobbyModal.ts (2)
270-273: Fallback when translation key is missingIf the server sends a key not yet in en.json, translateText returns the key itself. Show a generic localized message instead of the raw key/server string.
Apply this diff:
- } else if (archiveData.error) { - this.message = translateText(archiveData.error); - return true; - } + } else if (archiveData.error) { + const key = archiveData.error; + const localized = translateText(key); + this.message = + localized !== key ? localized : translateText("replay.error.generic"); + return true; + }Ensure en.json has replay.error.generic.
26-27: Use browser-friendly timer typeUse ReturnType for compatibility in browser builds.
- private playersInterval: NodeJS.Timeout | null = null; + private playersInterval: ReturnType<typeof setInterval> | null = null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/client/JoinPrivateLobbyModal.ts(1 hunks)src/core/Schemas.ts(1 hunks)src/server/Worker.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/server/Worker.ts
- src/core/Schemas.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/JoinPrivateLobbyModal.ts
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
Applied to files:
src/client/JoinPrivateLobbyModal.ts
📚 Learning: 2025-06-02T14:27:23.893Z
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#1007
File: resources/lang/he.json:138-138
Timestamp: 2025-06-02T14:27:23.893Z
Learning: andrewNiziolek's project uses community translation for internationalization. When updating map names or other user-facing text, they update the keys programmatically but wait for community translators to provide accurate translations in each language rather than using machine translations.
Applied to files:
src/client/JoinPrivateLobbyModal.ts
📚 Learning: 2025-07-12T08:41:35.101Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/de.json:523-540
Timestamp: 2025-07-12T08:41:35.101Z
Learning: In OpenFrontIO project localization files, always check the en.json source file before flagging potential spelling errors in other language files, as some keys may intentionally use non-standard spellings that need to be consistent across all translations.
Applied to files:
src/client/JoinPrivateLobbyModal.ts
📚 Learning: 2025-05-30T03:53:52.231Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#949
File: resources/lang/en.json:8-10
Timestamp: 2025-05-30T03:53:52.231Z
Learning: For the OpenFrontIO project, do not suggest updating translation files in resources/lang/*.json except for en.json. The project has a dedicated translation team that handles all other locale files.
Applied to files:
src/client/JoinPrivateLobbyModal.ts
🧬 Code Graph Analysis (1)
src/client/JoinPrivateLobbyModal.ts (1)
src/client/Utils.ts (1)
translateText(82-137)
🔇 Additional comments (1)
src/client/JoinPrivateLobbyModal.ts (1)
241-242: Ignore schema refactor—error is already a strict literal union
TheWorkerApiArchivedGameLobbySchemadefineserroras exactly"Game not found"or"Version mismatch". It does not accept arbitrary strings, so no change to make it a tighter z.enum is needed. Also, there are noreplay.error.*keys in your i18n files, so replacing the schema literals with those keys isn’t applicable.Likely an incorrect or invalid review comment.
github-project-automation
bot
moved this from Development
to Final Review
in OpenFront Release Management
|
I pulled this branch locally, but it looks like the there's
|
|
|
|
Ope, good catch. Here's one from v25.3: https://api.openfront.io/game/uHgqzZBF which fails for different reasons
|
|
@aaa4xu could you please resolve the conflicts with main? |
scottanderson
dismissed stale reviews from coderabbitai[bot] and themself
via
44dc690
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/client/JoinPrivateLobbyModal.ts (1)
275-279: Good: translate server error keys and short-circuit the flowUsing translateText(archiveData.error) and returning early keeps the UI logic simple and localized. This matches the earlier guidance to send/translate keys instead of raw messages.
🧹 Nitpick comments (6)
src/client/JoinPrivateLobbyModal.ts (1)
249-260: Localize the version-mismatch message; avoid hard-coded EnglishRight now the message is a literal string. Please use a translation key for consistency with the rest of this file and the new replay.* keys.
Two options:
- Minimal change (add a key): set message via translateText("replay.version_mismatch") and add that key to en.json.
- Better: remove this special-case and rely on archiveData.error to be a translation key ("replay.version_mismatch"), same as the generic branch below.
Suggested minimal diff:
- this.message = - "This game was created with a different version. Cannot join."; + this.message = translateText("replay.version_mismatch");Also add "replay.version_mismatch" to en.json, e.g. "This replay is from a different game version."
Would you like me to wire the en.json key and open a follow-up to make the server return "replay.version_mismatch" for this case so we can delete this special-case?
src/server/Archive.ts (5)
28-41: Nit: avoid mutating the input gameRecord; prefer a composed copyarchive() sets gameRecord.gitCommit in place. If callers reuse the object (tests, logging, or other sinks), this side effect can surprise. Compose a copy with the updated field and pass that through.
-export async function archive(gameRecord: GameRecord) { +export async function archive(gameRecord: GameRecord) { try { - gameRecord.gitCommit = config.gitCommit(); + const recordWithCommit: GameRecord = { ...gameRecord, gitCommit: config.gitCommit() }; // Archive to R2 - await archiveAnalyticsToR2(stripTurns(gameRecord)); + await archiveAnalyticsToR2(stripTurns(recordWithCommit)); // Archive full game if there are turns - if (gameRecord.turns.length > 0) { + if (recordWithCommit.turns.length > 0) { log.info( - `${gameRecord.info.gameID}: game has more than zero turns, attempting to write to full game to R2`, + `${recordWithCommit.info.gameID}: game has more than zero turns, attempting to write to full game to R2`, ); - await archiveFullGameToR2(stripPersistentIds(gameRecord)); + await archiveFullGameToR2(stripPersistentIds(recordWithCommit)); }
60-71: Confirm PII policy for analytics: players in info may still include persistentIDstripTurns() copies info as-is. If analytics objects are accessible outside trusted scope, we should also remove persistentID from info.players here (same as the full replay path). If analytics are private, ignore this.
If needed, a lightweight redact inline:
function stripTurns(gameRecord: GameRecord): AnalyticsRecord { const { info, version, gitCommit, subdomain, domain } = gameRecord; - const analyticsData: AnalyticsRecord = { - info, + const analyticsData: AnalyticsRecord = { + info: { + ...info, + players: info.players.map(({ persistentID: _omit, ...rest }) => rest), + }, version, gitCommit, subdomain, domain, }; return analyticsData; }
73-109: Solid redaction logic; minor naming clarityThe player mapping cleanly strips persistentID. To avoid confusion with the top-level config variable, consider renaming the destructured info.config to gameConfig.
- info: { - gameID, - config, + info: { + gameID, + config: gameConfig, players: privatePlayers, start, end, duration, num_turns, winner, }, @@ - const replayData: RedactedGameRecord = { - info: { gameID, config, players, start, end, duration, num_turns, winner }, + const replayData: RedactedGameRecord = { + info: { gameID, config: gameConfig, players, start, end, duration, num_turns, winner },
198-247: Timeout on public API fetch: OK given Node ≥20; optionally include the status text in logsAbortSignal.timeout(5000) is fine on supported Node lines (20/22/24). If you want slightly richer ops context, log response.statusText with status when non-OK.
- throw new Error( - `Http error: non-successful http status ${response.status}`, - ); + throw new Error( + `Http error: non-successful http status ${response.status} ${response.statusText ?? ""}`.trim(), + );
249-262: Consider a discriminated union to carry error details as values (no exceptions)Current return type collapses all failures to a key (e.g., "replay.invalid"), so the client cannot surface pretty zod errors as requested in the PR discussion. Keep errors-as-values but lift details into the type.
Example shape:
- Success: { ok: true; data: RedactedGameRecord }
- Failure: { ok: false; error: "replay.not_found" | "replay.invalid" | "replay.error"; details?: string }
This keeps typed unions, avoids throws, and lets the UI optionally display details.
If accepted, I can provide a follow-up patch touching:
- readGameRecord/readGameRecordFallback/validateRecord returns
- Worker endpoint response shape (add details passthrough)
- WorkerApiArchivedGameLobbySchema + client branch that shows details (behind a disclosure)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
resources/lang/en.json(1 hunks)src/client/JoinPrivateLobbyModal.ts(1 hunks)src/core/Schemas.ts(1 hunks)src/core/configuration/Config.ts(1 hunks)src/core/configuration/DefaultConfig.ts(1 hunks)src/core/configuration/DevConfig.ts(2 hunks)src/server/Archive.ts(6 hunks)src/server/Worker.ts(3 hunks)tests/util/TestServerConfig.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/core/Schemas.ts
- tests/util/TestServerConfig.ts
- src/core/configuration/Config.ts
- src/core/configuration/DefaultConfig.ts
- resources/lang/en.json
- src/core/configuration/DevConfig.ts
- src/server/Worker.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/JoinPrivateLobbyModal.ts
📚 Learning: 2025-05-21T04:10:59.706Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:44-53
Timestamp: 2025-05-21T04:10:59.706Z
Learning: The PlayerStats type from ArchiveSchemas already includes undefined in its definition, making explicit union types with undefined (PlayerStats | undefined) redundant.
Applied to files:
src/server/Archive.ts
📚 Learning: 2025-08-09T05:03:18.745Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1761
File: src/core/ApiSchemas.ts:2-2
Timestamp: 2025-08-09T05:03:18.745Z
Learning: After migrating to Zod v4 in the OpenFrontIO project, the standard import statement `import { z } from "zod"` should be used across the entire codebase, not `import { z } from "zod/v4"`.
Applied to files:
src/server/Archive.ts
🧬 Code graph analysis (2)
src/client/JoinPrivateLobbyModal.ts (2)
src/client/Utils.ts (1)
translateText(83-147)src/client/LangSelector.ts (1)
translateText(259-279)
src/server/Archive.ts (2)
src/core/Schemas.ts (5)
GameRecord(565-565)AnalyticsRecord(560-560)RedactedGameRecord(578-578)GameID(22-22)RedactedGameRecordSchema(567-577)src/core/Util.ts (1)
replacer(303-305)
🔇 Additional comments (3)
src/server/Archive.ts (3)
111-143: LGTM: analytics write path with bigint-safe serializationUsing the replacer prevents bigint crashes and the per-gameID key keeps lookups simple.
145-159: LGTM: full replay write pathKeying by folder/gameID (no extension) matches the read path below and avoids a breaking change.
161-196: Good fallback behavior and structured logging
- Empty Body fallback to API is pragmatic.
- Logging non-Error at info level keeps noise down.
No changes needed.
github-project-automation
bot
moved this from Final Review
to Development
in OpenFront Release Management
|
@aaa4xu You mind signing the CLA so this can be merged? :) |
| try { | ||
| // Check if file exists and download in one operation | ||
| const response = await r2.getObject({ | ||
| Bucket: bucket, | ||
| Key: `${gameFolder}/${gameId}`, // Fixed - needed to include gameFolder | ||
| }); | ||
| // Parse the response body | ||
| if (response.Body === undefined) return null; | ||
| if (response.Body === undefined) { | ||
| log.warn(`${gameId}: Received empty response from R2`); |
There was a problem hiding this comment.
Do we need a warning here?
| log.info(`${gameId}: Error reading game record from R2: ${error}`, { | ||
| message, | ||
| stack, | ||
| name, |
There was a problem hiding this comment.
I think it may be redundant to log both error and message
|
|
||
| export async function readGameRecordFallback( | ||
| gameId: GameID, | ||
| ): Promise<RedactedGameRecord | string> { |
There was a problem hiding this comment.
The current return type (RedactedGameRecord | string) works, but it has a couple of drawbacks. It forces callers to do a typeof check, and it limits the amount of information we can pass back on failure.
One option to improve this is to use a discriminated union and to generate a Response from the error handler:
export type Result<T, E> =
| { success: true; value: T; error: never; }
| { success: false; value: never; error: E; };| ): Promise<RedactedGameRecord | string> { | |
| ): Promise<Result<RedactedGameRecord, Response>> { |
return { success: false, error: error(404) };| throw new Error( | ||
| `Http error: non-successful http status ${response.status}`, | ||
| ); |
There was a problem hiding this comment.
nit: Consider being consistent with error handling. Mixing throw with errors as values can be a bit confusing. In this case I think it is better to log/return from this point in the code.
| throw new Error( | ||
| `Http error: unexpected content type "${response.headers.get("Content-Type")}"`, | ||
| ); |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/server/Archive.ts (3)
127-131: Fix undefined variable in analytics error log
gameRecordis not in scope here; this will throw when logging the Non-Error case. Useinfo.gameIDwhich you already have.- if (!(error instanceof Error)) { - log.error( - `${gameRecord.info.gameID}: Error writing game analytics to R2. Non-Error type: ${String(error)}`, - ); - return; - } + if (!(error instanceof Error)) { + log.error( + `${info.gameID}: Error writing game analytics to R2. Non-Error type: ${String(error)}`, + ); + return; + }
153-159: Fix two more out-of-scope references to gameRecordThis function receives
recordCopy. The logs referencegameRecord, which is undefined in this scope.- } catch (error) { - log.error(`error saving game ${gameRecord.info.gameID}`); + } catch (error: unknown) { + log.error(`${recordCopy.info.gameID}: error saving game`); throw error; } - log.info(`${gameRecord.info.gameID}: game record successfully written to R2`); + log.info(`${recordCopy.info.gameID}: game record successfully written to R2`);
111-143: Fix out-of-scopegameRecordreferences in archive functionsBoth
archiveAnalyticsToR2andarchiveFullGameToR2log usinggameRecord.info.gameID, but neither function has agameRecordparameter. This will cause compile or runtime errors. Please update the logs to use the correct in-scope variable in each function:• In archiveAnalyticsToR2 (src/server/Archive.ts around lines 127–131):
- if (!(error instanceof Error)) { - log.error( - `${gameRecord.info.gameID}: Error writing game analytics to R2. Non-Error type: ${String(error)}`, - ); + if (!(error instanceof Error)) { + log.error( + `${info.gameID}: Error writing game analytics to R2. Non-Error type: ${String(error)}`, + ); return; }• In archiveFullGameToR2 (src/server/Archive.ts around lines 153–159):
- } catch (error) { - log.error(`error saving game ${gameRecord.info.gameID}`); - throw error; - } - - log.info(`${gameRecord.info.gameID}: game record successfully written to R2`); + } catch (error) { + log.error(`error saving game ${recordCopy.info.gameID}`); + throw error; + } + + log.info(`${recordCopy.info.gameID}: game record successfully written to R2`);These changes ensure you only reference the parameters actually passed into each function.
♻️ Duplicate comments (2)
src/server/Archive.ts (2)
171-175: Lower this to info to avoid noisy warnings on expected fallbackEmpty bodies from R2 can happen and we already have a graceful fallback. Using
infokeeps logs cleaner. This mirrors earlier reviewer feedback.- if (response.Body === undefined) { - log.warn(`${gameId}: Received empty response from R2`); + if (response.Body === undefined) { + log.info(`${gameId}: Received empty response from R2`); return readGameRecordFallback(gameId); }
161-164: Consider a discriminated Result type instead of string error keysReturning
RedactedGameRecord | stringforcestypeofchecks and makes it easy to miss a new error code. A small, typed union keeps call-sites clean and i18n flexible.Example:
export type ReplayReadResult = | { ok: true; record: RedactedGameRecord } | { ok: false; code: "replay.not_found" | "replay.error" | "replay.invalid" };Then change function signatures to return
Promise<ReplayReadResult>and wrap existing returns accordingly.
🧹 Nitpick comments (4)
src/server/Archive.ts (4)
73-109: Redact by omitting persistentID, not by hand-picking a subset of fieldsRight now you rebuild each player with only four fields. If PlayerRecord gains new required fields (e.g., playerType), they’ll be lost here and fail validation later. Prefer a structured omit that drops only
persistentIDand keeps everything else.- const players = privatePlayers.map( - ({ clientID, persistentID: _, username, pattern, flag }) => ({ - clientID, - username, - pattern, - flag, - }), - ); + const players = privatePlayers.map(({ persistentID: _omit, ...rest }) => rest);This keeps the code simple and future‑proof. TypeScript will infer the shape as “PlayerRecord without persistentID”, which matches
RedactedGameRecord.
198-247: Optional: handle fetch timeout explicitly for clearer ops signalsYou already use
AbortSignal.timeout(5000). If a timeout happens, logging it explicitly helps triage. We can keep the same return key to avoid i18n churn.} catch (error: unknown) { + // Special-case fetch timeouts for clearer logs + if (error && typeof error === "object" && (error as any).name === "AbortError") { + log.info(`${gameId}: Timeout reading game record from public api`); + return "replay.error"; + } // If the error is not an instance of Error, log it as a string if (!(error instanceof Error)) { log.info(
28-40: Avoid mutating the incoming gameRecord (optional)Setting
gameRecord.gitCommitmutates the input. Prefer creating a shallow copy so callers don’t see surprises if they reuse the object.Sketch:
const record = { ...gameRecord, gitCommit: config.gitCommit() }; await archiveAnalyticsToR2(stripTurns(record)); if (record.turns.length > 0) await archiveFullGameToR2(stripPersistentIds(record));Also applies to: 60-71
145-152: Nit: align object key naming and content types are goodUsing
.jsonextension for analytics and no extension for full games is fine as long as readers match the same convention (they do). Just calling out that the Content-Type is correctly set toapplication/jsonin both paths.Also applies to: 118-122
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/server/Archive.ts(6 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-05-21T04:10:59.706Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:44-53
Timestamp: 2025-05-21T04:10:59.706Z
Learning: The PlayerStats type from ArchiveSchemas already includes undefined in its definition, making explicit union types with undefined (PlayerStats | undefined) redundant.
Applied to files:
src/server/Archive.ts
📚 Learning: 2025-08-09T05:03:18.745Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1761
File: src/core/ApiSchemas.ts:2-2
Timestamp: 2025-08-09T05:03:18.745Z
Learning: After migrating to Zod v4 in the OpenFrontIO project, the standard import statement `import { z } from "zod"` should be used across the entire codebase, not `import { z } from "zod/v4"`.
Applied to files:
src/server/Archive.ts
📚 Learning: 2025-08-25T18:30:48.646Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1725
File: src/server/Archive.ts:255-258
Timestamp: 2025-08-25T18:30:48.646Z
Learning: z.prettifyError was added in zod v4 and is part of the official Zod API. It can be safely used in codebases using zod v4+.
Applied to files:
src/server/Archive.ts
🧬 Code graph analysis (1)
src/server/Archive.ts (1)
src/core/Schemas.ts (5)
GameRecord(565-565)AnalyticsRecord(560-560)RedactedGameRecord(578-578)GameID(22-22)RedactedGameRecordSchema(567-577)
🔇 Additional comments (3)
src/server/Archive.ts (3)
255-258: LGTM: use z.prettifyError with Zod v4Nice use of
z.prettifyErrorfor readable diagnostics. This aligns with the project’s Zod v4 usage.
1-7: Imports look consistent with project standardsUsing
{ z }from "zod" matches the codebase convention after the v4 migration. Schema imports are correct.
249-262: Validation flow is solid and simpleCentralizing schema validation into
validateRecordkeeps the read paths clean and ensures consistent logging and error translation.
scottanderson
force-pushed
the
pull-replays-from-api
branch
from
f90e723 to
44dc690
Compare
|
I would like to inform you that I am withdrawing my CLA with OpenFront LLC and will no longer be contributing to the project. Since I have already created a pull request, I understand that the project retains the rights to the code submitted under the CLA. However, I will not be proceeding further with this contribution, and if the pull request is to be merged, someone else will need to carry it through. |
Description:
This PR introduces a graceful fallback path for loading replay files.
If fetching a replay from Cloudflare R2 fails, the server now retries via the public endpoint
https://api.openfront.io/game/{gameId}, so developers can replay games locally.Closes issue #1571
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
aaa4xu