profile #1758
Open
profile #1758
Conversation
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
WalkthroughAdds a Player Info feature: new PlayerInfoModal UI and button, multiple player-stats UI components, new player-related API schemas and client fetch helper, translation strings, and Main.ts wiring to propagate login state to the modal. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Button as player-info-button
participant Main as Main.ts
participant Modal as PlayerInfoModal
participant JWT as fetchPlayerById
participant API as /player/{id}
participant UI1 as DiscordUserHeader
participant UI2 as PlayerStatsTreeView
participant UI3 as GameList
User->>Button: Click
Button->>Main: click event
Main->>Modal: open()
Note over Modal: clear errors, show modal
Main->>Modal: onUserMe(userMeResponse)
alt user has publicId
Modal->>JWT: fetchPlayerById(publicId)
JWT->>API: GET /player/{id} (Auth)
API-->>JWT: 200 JSON
JWT-->>Modal: PlayerIdResponse
Modal->>UI1: set data(user)
Modal->>UI2: set statsTree(stats)
Modal->>UI3: set games(recent)
else not logged in / no id
Modal->>UI1: set data(null)
Modal->>UI2: clear stats
Modal->>UI3: clear list
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/client/Main.ts (1)
325-336: GuardpiModalbefore invoking methods
piModalmay benull; wrap calls with optional chaining to avoid crashes:-territoryModal.onUserMe(null); -piModal.onLoggedOut(); +territoryModal.onUserMe(null); +piModal?.onLoggedOut(); … -territoryModal.onUserMe(userMeResponse); -piModal.onUserMe(userMeResponse); +territoryModal.onUserMe(userMeResponse); +piModal?.onUserMe(userMeResponse);
🧹 Nitpick comments (2)
src/client/Main.ts (1)
24-24: Side-effect import can be cleaner
PlayerInfoModalis imported for its side-effect (custom-element registration) but the value itself is unused, so TS will warn.-import { PlayerInfoModal } from "./PlayerInfoModal"; +import "./PlayerInfoModal"; // keep side-effect +import type { PlayerInfoModal } from "./PlayerInfoModal"; // type-onlyThis silences the “value is never read” warning while still providing the type.
src/client/PlayerInfoModal.ts (1)
677-699: Logout handler doesn’t clear Discord name
onLoggedOut()resets local name/avatar but forgetsdiscordName, leaving the last user visible.onLoggedOut() { this.playerName = ""; this.playerAvatarUrl = ""; + this.discordName = ""; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/client/Main.ts(4 hunks)src/client/PlayerInfoModal.ts(1 hunks)src/client/index.html(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 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.
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1655
File: resources/maps/giantworldmap/manifest.json:165-174
Timestamp: 2025-07-31T12:03:08.052Z
Learning: In OpenFrontIO draft PRs, flag names in map manifest files may be placeholders that will be updated to match the actual SVG flag file names when the assets are received. The final naming depends on the actual flag SVG files provided, not code naming conventions.
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.
📚 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/Main.tssrc/client/index.htmlsrc/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.ts
🧬 Code Graph Analysis (1)
src/client/PlayerInfoModal.ts (2)
src/server/GameManager.ts (1)
game(18-20)src/core/ApiSchemas.ts (1)
UserMeResponse(50-50)
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/client/PlayerInfoModal.ts (2)
434-436: Guard against divide-by-zero when gamesPlayed is 0
(Same issue flagged earlier.)Add a check so win-rate never shows
NaN%.-${((this.wins / this.gamesPlayed) * 100).toFixed(1)}% +${this.gamesPlayed === 0 + ? "0.0" + : ((this.wins / this.gamesPlayed) * 100).toFixed(1)}%
358-418: Route all visible text through the translation helper
Literal English strings (“Player Info”, “Wins”, “Public” …) still bypass localisation, contrary to project policy noted before.Wrap each with
translateText('key')(or equivalent) and add keys toen.json.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/client/PlayerInfoModal.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
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.
📚 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/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
src/client/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-31T06:39:29.144Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1536
File: src/core/execution/utils/BotBehavior.ts:41-54
Timestamp: 2025-07-31T06:39:29.144Z
Learning: In the OpenFrontIO codebase, the `PseudoRandom.chance(odds: number)` method takes inverse odds as a parameter, not a percentage. The probability is calculated as `1.0 / odds`. So `chance(1.5)` gives 66.67% probability, `chance(2)` gives 50% probability, etc. The implementation returns `Math.floor(this._nextFloat() * odds) === 0`.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.
Applied to files:
src/client/PlayerInfoModal.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/client/PlayerInfoModal.ts (1)
357-417: Hard-coded English strings still bypass localisation – see earlier reviewAll visible text (“Player Info”, “All / Public / Private”, table headings, etc.) is still literal English.
Route through the translation helper as already requested.
🧹 Nitpick comments (3)
src/client/PlayerInfoModal.ts (3)
42-48: Replace hard-coded placeholder stats with neutral defaultsUsing real-looking numbers (“57” wins, “62%” progress, etc.) before the API loads can confuse users and tests.
Set these initial values to zero/empty and rely onapplyBackendStatsor the API call to populate the real data.
207-214:requestUpdate()call is redundant hereAssigning reactive
@statefields (wins,losses,gamesPlayed) already schedules an update.
Removing the explicitrequestUpdate()will avoid an unnecessary extra render.
696-700: Reset all player-specific state on logout
onLoggedOut()clearsplayerNameandplayerAvatarUrlbut leaves wins, losses, games, recent games, etc. from the previous user.
Blank these reactive fields to avoid stale data being shown after a logout.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/client/PlayerInfoModal.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
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.
📚 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/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
src/client/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-31T06:39:29.144Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1536
File: src/core/execution/utils/BotBehavior.ts:41-54
Timestamp: 2025-07-31T06:39:29.144Z
Learning: In the OpenFrontIO codebase, the `PseudoRandom.chance(odds: number)` method takes inverse odds as a parameter, not a percentage. The probability is calculated as `1.0 / odds`. So `chance(1.5)` gives 66.67% probability, `chance(2)` gives 50% probability, etc. The implementation returns `Math.floor(this._nextFloat() * odds) === 0`.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-22T05:48:19.241Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#786
File: src/client/TerritoryPatternsModal.ts:337-338
Timestamp: 2025-06-22T05:48:19.241Z
Learning: In src/client/TerritoryPatternsModal.ts, the bit shifting operators (<<) used in coordinate calculations with decoder.getScale() are intentional and should not be changed to multiplication. The user scottanderson confirmed this is functioning as intended.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
PR: openfrontio/OpenFrontIO#1063
File: src/core/configuration/PastelThemeDark.ts:53-53
Timestamp: 2025-06-10T09:56:44.473Z
Learning: In ColorAllocator class in src/core/configuration/Colors.ts, the correct method names are assignColor(id: string): Colord for general color assignment and assignTeamColor(team: Team): Colord for team-specific colors. There are no assignPlayerColor() or assignBotColor() methods.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from UnitType values (specifically bomb-related values like AtomBomb, HydrogenBomb, MIRV, and MIRVWarhead) rather than a separate enum. This means comparing NukeType values against UnitType values in switch statements is valid and intentional.
Applied to files:
src/client/PlayerInfoModal.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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from specific UnitType enum values (AtomBomb, HydrogenBomb, MIRV, MIRVWarhead). This means that comparisons in switch statements between a NukeType parameter and UnitType enum values are valid and will work correctly at runtime.
Applied to files:
src/client/PlayerInfoModal.ts
🧬 Code Graph Analysis (1)
src/client/PlayerInfoModal.ts (2)
src/core/ApiSchemas.ts (1)
UserMeResponse(50-50)src/core/configuration/ConfigLoader.ts (1)
getServerConfigFromClient(28-45)
scottanderson
requested changes
Aug 9, 2025
There was a problem hiding this comment.
We need to get better about putting everything into @State variables, this is not how these are intended to be used. Please put the API response objects directly into state objects - do not unpack them.
src/client/PlayerInfoModal.ts
Outdated
| @state() private losses: number = 62; | ||
| @state() private lastActive: string = "1992/4/27"; | ||
|
|
||
| @state() private buildingStatsPublic: AllBuildingStats = { |
There was a problem hiding this comment.
I think we should have a reusable component that can display the PlayerStatsSchema grid, since we need a component that we can reuse for the end screen.
There was a problem hiding this comment.
Does this refer to both the Unit table and the part that shows wins, losses, etc., or just the Unit table?
There was a problem hiding this comment.
Please split these out so each UI component is in its own class and file. Having 700+ lines in a single UI class makes it hard to maintain and reuse. The modal, controls, and any other distinct sections should each live in their own file, and be combined using composition.
There was a problem hiding this comment.
@Aotumuri each UI component should live in its own file. Right now, only the stats grid has been broken out, but the others are still combined.
To keep our codebase maintainable, please separate these components. Files shouldn’t be approaching 1000 lines, let’s aim for ~250–300 lines per component as a guideline.
src/client/PlayerInfoModal.ts
Outdated
Comment on lines
678
to
113
| const { user, player } = userMeResponse; | ||
| const { id, avatar, username, discriminator } = user; | ||
|
|
||
| this.discordName = username; | ||
| this.playerAvatarUrl = avatar | ||
| ? `https://cdn.discordapp.com/avatars/${id}/${avatar}.${avatar.startsWith("a_") ? "gif" : "png"}` | ||
| : `https://cdn.discordapp.com/embed/avatars/${Number(discriminator) % 5}.png`; | ||
|
|
||
| if (player.publicId) { | ||
| this.publicId = player.publicId; | ||
| } |
There was a problem hiding this comment.
There is too much state in this class, which adds a lot of unnecessary overhead. Let the rendering function extract the information it needs from this object.
Suggested change
| const { user, player } = userMeResponse; | |
| const { id, avatar, username, discriminator } = user; | |
| this.discordName = username; | |
| this.playerAvatarUrl = avatar | |
| ? `https://cdn.discordapp.com/avatars/${id}/${avatar}.${avatar.startsWith("a_") ? "gif" : "png"}` | |
| : `https://cdn.discordapp.com/embed/avatars/${Number(discriminator) % 5}.png`; | |
| if (player.publicId) { | |
| this.publicId = player.publicId; | |
| } | |
| this.userMeResponse = userMeResponse; |
There was a problem hiding this comment.
@Aotumuri please respond to or resolve all comments
src/client/PlayerInfoModal.ts
Outdated
Comment on lines
217
to
351
| add3(bombs[k], arr as string[]); | ||
| }); | ||
| } | ||
| return { units, boats, bombs, wins, losses, total }; | ||
| }; | ||
|
|
||
| const pubAgg = aggregateFromBranch(pub); | ||
| const prvAgg = aggregateFromBranch(prv); | ||
|
|
||
| const sumAgg = (a: any, b: any) => { | ||
| const outUnits: Record<string, number[]> = {}; | ||
| const outBoats: Record<string, number[]> = {}; | ||
| const outBombs: Record<string, number[]> = {}; | ||
| const keys = new Set([...Object.keys(a.units), ...Object.keys(b.units)]); | ||
| keys.forEach((k) => { | ||
| outUnits[k] = [0, 0, 0, 0]; | ||
| add4(outUnits[k], a.units[k] as any); | ||
| add4(outUnits[k], b.units[k] as any); | ||
| }); | ||
| const keysB = new Set([...Object.keys(a.boats), ...Object.keys(b.boats)]); | ||
| keysB.forEach((k) => { | ||
| outBoats[k] = [0, 0, 0]; | ||
| add3(outBoats[k], a.boats[k] as any); | ||
| add3(outBoats[k], b.boats[k] as any); | ||
| }); | ||
| const keysC = new Set([...Object.keys(a.bombs), ...Object.keys(b.bombs)]); | ||
| keysC.forEach((k) => { | ||
| outBombs[k] = [0, 0, 0]; | ||
| add3(outBombs[k], a.bombs[k] as any); | ||
| add3(outBombs[k], b.bombs[k] as any); | ||
| }); | ||
| return { | ||
| units: outUnits, | ||
| boats: outBoats, | ||
| bombs: outBombs, | ||
| wins: a.wins + b.wins, | ||
| losses: a.losses + b.losses, | ||
| total: a.total + b.total, | ||
| }; | ||
| }; | ||
|
|
||
| const allAgg = sumAgg(pubAgg, prvAgg); | ||
|
|
||
| const shapeFromAgg = (agg: any): AllBuildingStats => { | ||
| const get4 = (k: string) => | ||
| (agg.units[k] ?? [0, 0, 0, 0]) as [number, number, number, number]; | ||
| const get3 = (k: string) => | ||
| (agg.bombs[k] ?? [0, 0, 0]) as [number, number, number]; | ||
| const getShip = (k: string) => | ||
| (agg.boats[k] ?? [0, 0, 0]) as [number, number, number]; | ||
|
|
||
| const [cityB, cityD, cityL, cityC] = get4("city"); | ||
| const [defB, defD, defL, defC] = get4("defp"); | ||
| const [portB, portD, portL, portC] = get4("port"); | ||
| const [samB, samD, samL, samC] = get4("saml"); | ||
| const [siloB, siloD, siloL, siloC] = get4("silo"); | ||
|
|
||
| const [atomB, atomD, atomH] = get3("abomb"); | ||
| const [hydB, hydD, hydH] = get3("hbomb"); | ||
| const [mirvB, mirvD, mirvH] = get3("mirv"); | ||
|
|
||
| const [transS, transD, transA] = getShip("trans"); | ||
| const [tradeS, tradeD, tradeA] = getShip("trade"); | ||
| const [warS, warD, warA] = getShip("wshp"); | ||
|
|
||
| return { | ||
| city: { built: cityB, destroyed: cityD, captured: cityC, lost: cityL }, | ||
| port: { built: portB, destroyed: portD, captured: portC, lost: portL }, | ||
| defense: { built: defB, destroyed: defD, captured: defC, lost: defL }, | ||
| sam: { built: samB, destroyed: samD, captured: samC, lost: samL }, | ||
| silo: { built: siloB, destroyed: siloD, captured: siloC, lost: siloL }, | ||
| warship: { sent: warS, destroyed: warD, arrived: warA }, | ||
| transportShip: { sent: transS, destroyed: transD, arrived: transA }, | ||
| tradeShip: { sent: tradeS, destroyed: tradeD, arrived: tradeA }, | ||
| atom: { built: atomB, destroyed: atomD, hits: atomH }, | ||
| hydrogen: { built: hydB, destroyed: hydD, hits: hydH }, | ||
| mirv: { built: mirvB, destroyed: mirvD, hits: mirvH }, | ||
| }; | ||
| }; | ||
|
|
||
| this.buildingStatsPublic = shapeFromAgg(pubAgg); | ||
| this.buildingStatsPrivate = shapeFromAgg(prvAgg); | ||
| this.buildingStatsAll = shapeFromAgg(allAgg); | ||
|
|
||
| this.totalsByVisibility = { | ||
| all: { wins: allAgg.wins, losses: allAgg.losses, total: allAgg.total }, | ||
| public: { wins: pubAgg.wins, losses: pubAgg.losses, total: pubAgg.total }, | ||
| private: { | ||
| wins: prvAgg.wins, | ||
| losses: prvAgg.losses, | ||
| total: prvAgg.total, | ||
| }, | ||
| }; | ||
|
|
||
| const t = this.totalsByVisibility[this.visibility]; | ||
| this.wins = t.wins; | ||
| this.losses = t.losses; | ||
| this.gamesPlayed = t.total; | ||
|
|
||
| this.requestUpdate(); |
There was a problem hiding this comment.
Remove all of the state mutations done by this function, and directly render the stats object
There was a problem hiding this comment.
Since you said, “Remove all of the state mutations done by this function, and directly render the stats object”, I changed it so that the stats object is passed to after summing up the values for all difficulties in each mode.
If we removed this calculation entirely, we would have to render 8 separate tables (mode: FFA/TEAM × difficulty: 4), which would be excessive. That’s why I kept the calculation logic in place.
I just want to clarify that I’m not trying to ignore your opinion or stubbornly insist on my own approach — I only kept it because it seemed necessary in this case.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/client/PlayerInfoModal.ts (3)
36-44: Reduce state: derive displayed totals fromtotalsByVisibility[visibility]instead of duplicating inwins/losses/gamesPlayed.There’s redundant state that’s recalculated on visibility changes. Prefer deriving these values at render-time or a computed getter. This keeps the component simpler and avoids accidental divergence.
Example:
- Drop
wins,losses,gamesPlayedfrom state.- In render, compute:
const t = this.totalsByVisibility[this.visibility];and uset.wins,t.losses,t.total.This follows composition and keeps data flow clean.
Also applies to: 84-92
219-259: Replaceany/loose records with Zod-validated types for backend stats.This function uses
anyand untypedRecord<string, number[]>. Prefer a Zod schema mirroring the backend JSON so we have strong types and safer transforms.Follow-up plan:
- Introduce a
PlayerStatsSchema(or import the existing one if present).- Parse
data.statsvia Zod inloadFromApi, then changeapplyBackendStats(rawStats: unknown)to accept the parsed type.- Remove
as any/Record<string, number[]>casts.If you want, I can generate a full schema based on the observed shape (units: 4-tuple strings, boats: 3-4 tuple strings trimmed to 3, bombs: 3-tuple strings) or wire in the existing
PlayerStatsSchemaif available.#!/bin/bash # Find existing PlayerStatsSchema to reuse instead of redefining one. fd -t f '.*\.(ts|tsx)' | xargs rg -n --no-heading "PlayerStatsSchema|PlayerStats"Also applies to: 261-297, 298-355
175-190: Localise all user-facing text (titles, headers, labels, buttons).Strings are hard-coded in English (“Player Info”, “Wins”, “Victory”, “View”, etc.). Per project guidelines, route through the translation helper and add keys only to en.json; other locales will be handled by the translation team.
Example patch (pattern only, apply similarly across the template and getBuildingName):
- private getBuildingName(building: string): string { - const buildingNames: Record<string, string> = { - city: "City", - port: "Port", - defense: "Defense", - warship: "Warship", - atom: "Atom Bomb", - hydrogen: "Hydrogen Bomb", - mirv: "MIRV", - silo: "Missile Silo", - sam: "SAM", - transportShip: "Transport Ship", - tradeShip: "Trade Ship", - }; - return buildingNames[building] || building; - } + private getBuildingName(building: string): string { + // Use translation keys so en.json can be the source of truth. + // Example keys: player_info.buildings.city, .port, .defense, ... + return translateText(`player_info.buildings.${building}`); + } @@ - <o-modal id="playerInfoModal" title="Player Info" alwaysMaximized> + <o-modal id="playerInfoModal" title="${translateText('player_info.title')}" alwaysMaximized> @@ - All + ${translateText('player_info.visibility.all')} @@ - Public + ${translateText('player_info.visibility.public')} @@ - Private + ${translateText('player_info.visibility.private')} @@ - <div class="text-gray-400">Wins</div> + <div class="text-gray-400">${translateText('player_info.stats.wins')}</div> @@ - <div class="text-gray-400">Losses</div> + <div class="text-gray-400">${translateText('player_info.stats.losses')}</div> @@ - <div class="text-gray-400">Win Rate</div> + <div class="text-gray-400">${translateText('player_info.stats.win_rate')}</div> @@ - <div class="text-gray-400">Games Played</div> + <div class="text-gray-400">${translateText('player_info.stats.games_played')}</div> @@ - <div class="text-gray-400">Play Time</div> + <div class="text-gray-400">${translateText('player_info.stats.play_time')}</div> @@ - <div class="text-gray-400">Last Active</div> + <div class="text-gray-400">${translateText('player_info.stats.last_active')}</div> @@ - 🏗️ Building Statistics + ${translateText('player_info.sections.building_stats')} @@ - <th class="text-left w-1/5">Building</th> + <th class="text-left w-1/5">${translateText('player_info.table.building')}</th> @@ - <th class="text-center w-1/5">Built</th> + <th class="text-center w-1/5">${translateText('player_info.table.built')}</th> @@ - <th class="text-center w-1/5">Destroyed</th> + <th class="text-center w-1/5">${translateText('player_info.table.destroyed')}</th> @@ - <th class="text-center w-1/5">Captured</th> + <th class="text-center w-1/5">${translateText('player_info.table.captured')}</th> @@ - <th class="text-center w-1/5">Lost</th> + <th class="text-center w-1/5">${translateText('player_info.table.lost')}</th> @@ - 🚢 Ship Arrivals + ${translateText('player_info.sections.ship_arrivals')} @@ - <th class="text-left w-2/5">Ship Type</th> + <th class="text-left w-2/5">${translateText('player_info.table.ship_type')}</th> @@ - <th class="text-center w-1/5">Arrived</th> + <th class="text-center w-1/5">${translateText('player_info.table.arrived')}</th> @@ - ☢️ Nuke Statistics + ${translateText('player_info.sections.nuke_stats')} @@ - <th class="text-left w-2/5">Weapon</th> + <th class="text-left w-2/5">${translateText('player_info.table.weapon')}</th> @@ - 🎮 Recent Games + ${translateText('player_info.sections.recent_games')} @@ - ${game.gameMode === "ffa" ? "Free-for-All" : html`Team (${game.teamCount} teams)`} + ${game.gameMode === "ffa" + ? translateText('player_info.games.mode.ffa') + : html`${translateText('player_info.games.mode.team')} (${game.teamCount} ${translateText('player_info.games.mode.teams')})`} @@ - Player Team Color: ${game.teamColor} + ${translateText('player_info.games.team_color')}: ${game.teamColor} @@ - ${game.won ? "Victory" : "Defeat"} + ${game.won ? translateText('player_info.games.victory') : translateText('player_info.games.defeat')} @@ - View + ${translateText('player_info.actions.view')} @@ - Details + ${translateText('player_info.actions.details')} @@ - <span class="font-semibold">Started:</span> + <span class="font-semibold">${translateText('player_info.games.started')}:</span> @@ - <span class="font-semibold">Mode:</span> + <span class="font-semibold">${translateText('player_info.games.mode.label')}:</span> @@ - <span class="font-semibold">Map:</span> + <span class="font-semibold">${translateText('player_info.games.map')}:</span> @@ - <span class="font-semibold">Difficulty:</span> + <span class="font-semibold">${translateText('player_info.games.difficulty')}:</span> @@ - <span class="font-semibold">Type:</span> + <span class="font-semibold">${translateText('player_info.games.type')}:</span>Note: only update en.json keys in this PR; other locales are handled by the translation team.
Also applies to: 369-676
🧹 Nitpick comments (5)
src/client/PlayerInfoModal.ts (5)
706-706: Remove debug log.The
console.log(url);is noisy for users and CI logs.- console.log(url);
697-699: Clear UI state on logout for a consistent blank slate.After logout, stale player stats and recent games remain until next open/fetch. Reset to default empty values.
onLoggedOut() { this.userMeResponse = null; + this.buildingStatsPublic = this.buildingStatsPrivate = this.buildingStatsAll = { + city: { built: 0, destroyed: 0, lost: 0, captured: 0 }, + port: { built: 0, destroyed: 0, lost: 0, captured: 0 }, + defense: { built: 0, destroyed: 0, lost: 0, captured: 0 }, + sam: { built: 0, destroyed: 0, lost: 0, captured: 0 }, + silo: { built: 0, destroyed: 0, lost: 0, captured: 0 }, + warship: { sent: 0, destroyed: 0, arrived: 0 }, + transportShip: { sent: 0, destroyed: 0, arrived: 0 }, + tradeShip: { sent: 0, destroyed: 0, arrived: 0 }, + atom: { built: 0, destroyed: 0, hits: 0 }, + hydrogen: { built: 0, destroyed: 0, hits: 0 }, + mirv: { built: 0, destroyed: 0, hits: 0 }, + }; + this.totalsByVisibility = { + all: { wins: 0, losses: 0, total: 0 }, + public: { wins: 0, losses: 0, total: 0 }, + private: { wins: 0, losses: 0, total: 0 }, + }; + this.recentGames = []; + this.visibility = "all"; + this.requestUpdate(); }
469-472: Remove duplicate horizontal rule.Two adjacent
<hr>lines add extra spacing for no content.- <hr class="w-2/3 border-gray-600 my-2" /> - <hr class="w-2/3 border-gray-600 my-2" />
38-44: Default placeholder numbers to zero to avoid misleading UI before load.These initial values look like demo data. Use zero defaults or derive from totals.
- @state() private wins: number = 57; - @state() private playTimeSeconds: number = 5 * 3600 + 33 * 60; - @state() private progressPercent: number = 62; - @state() private gamesPlayed: number = 119; - @state() private losses: number = 62; - @state() private lastActive: string = "1992/4/27"; + @state() private wins: number = 0; + @state() private playTimeSeconds: number = 0; + @state() private progressPercent: number = 0; + @state() private gamesPlayed: number = 0; + @state() private losses: number = 0; + @state() private lastActive: string = "";
29-35: Prefer composition helpers over overridingcreateRenderRoot()unless required.Light DOM may be intentional due to
<o-modal>. If not strictly required, keep Shadow DOM and compose via slots. This helps encapsulation.Please confirm that
<o-modal>needs light DOM. If not, removecreateRenderRoot()override.Also applies to: 161-164
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/client/PlayerInfoModal.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
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.
📚 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/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
src/client/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-31T06:39:29.144Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1536
File: src/core/execution/utils/BotBehavior.ts:41-54
Timestamp: 2025-07-31T06:39:29.144Z
Learning: In the OpenFrontIO codebase, the `PseudoRandom.chance(odds: number)` method takes inverse odds as a parameter, not a percentage. The probability is calculated as `1.0 / odds`. So `chance(1.5)` gives 66.67% probability, `chance(2)` gives 50% probability, etc. The implementation returns `Math.floor(this._nextFloat() * odds) === 0`.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-22T05:48:19.241Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#786
File: src/client/TerritoryPatternsModal.ts:337-338
Timestamp: 2025-06-22T05:48:19.241Z
Learning: In src/client/TerritoryPatternsModal.ts, the bit shifting operators (<<) used in coordinate calculations with decoder.getScale() are intentional and should not be changed to multiplication. The user scottanderson confirmed this is functioning as intended.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
PR: openfrontio/OpenFrontIO#1063
File: src/core/configuration/PastelThemeDark.ts:53-53
Timestamp: 2025-06-10T09:56:44.473Z
Learning: In ColorAllocator class in src/core/configuration/Colors.ts, the correct method names are assignColor(id: string): Colord for general color assignment and assignTeamColor(team: Team): Colord for team-specific colors. There are no assignPlayerColor() or assignBotColor() methods.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from UnitType values (specifically bomb-related values like AtomBomb, HydrogenBomb, MIRV, and MIRVWarhead) rather than a separate enum. This means comparing NukeType values against UnitType values in switch statements is valid and intentional.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from specific UnitType enum values (AtomBomb, HydrogenBomb, MIRV, MIRVWarhead). This means that comparisons in switch statements between a NukeType parameter and UnitType enum values are valid and will work correctly at runtime.
Applied to files:
src/client/PlayerInfoModal.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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:43:38.150Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-06-13T20:57:49.599Z
Learnt from: El-Magico777
PR: openfrontio/OpenFrontIO#1166
File: src/core/Schemas.ts:50-56
Timestamp: 2025-06-13T20:57:49.599Z
Learning: In the OpenFrontIO codebase, player identification primarily uses numeric smallIDs throughout the system (e.g., playerBySmallID). When adding new intents or schemas, maintain consistency with this existing pattern rather than introducing mixed ID types, even if string IDs might be more standard elsewhere.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:29.540Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:61-63
Timestamp: 2025-05-19T06:00:29.540Z
Learning: The maintainer prefers to prioritize performance over defensive programming for the stats() method in StatsImpl, trusting callers not to directly modify the returned data structure.
Applied to files:
src/client/PlayerInfoModal.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: 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/client/PlayerInfoModal.ts
🪛 GitHub Actions: 🧪 CI
src/client/PlayerInfoModal.ts
[error] 714-714: TS2339: Property 'stats' does not exist on type '{}'.
[error] 715-715: TS2339: Property 'stats' does not exist on type '{}'.
[error] 718-718: TS2339: Property 'games' does not exist on type '{}'.
[error] 719-719: TS2339: Property 'games' does not exist on type '{}'.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
src/client/PlayerInfoModal.ts (1)
247-247: Hard-coded English strings bypass localisationRoute all user-facing text through the translation helper and language files.
Example for modal title and grid:
- <o-modal id="playerInfoModal" title="Player Info" alwaysMaximized> + <o-modal id="playerInfoModal" title="${t('player_info.title')}" alwaysMaximized> - .titles=${["Wins","Losses","Win Rate","Games Played","Play Time","Last Active"]} + .titles=${[ + t('player_info.wins'), + t('player_info.losses'), + t('player_info.win_rate'), + t('player_info.games_played'), + t('player_info.play_time'), + t('player_info.last_active'), + ]}Apply similar changes for table headers, buttons (“All/Public/Private”), and Recent Games labels/buttons.
Also applies to: 286-287, 295-296, 304-305, 311-318, 339-349, 380-389, 419-428, 461-462, 472-475, 476-481, 483-486, 489-494, 501-508, 522-541
🧹 Nitpick comments (4)
src/client/components/baseComponents/PlayerStatsGrid.ts (1)
38-41: Render count: derive from provided data lengthYou hard-code 6 items. If
titles/valuesis longer, items are dropped; if shorter, you show empty cells. Consider deriving count from inputs while keeping a minimum of 6.- ${Array(6) + ${Array(Math.max(6, this.titles.length, this.values.length)) .fill(0) .map(src/client/PlayerInfoModal.ts (3)
580-581: Remove stray debug loggingAvoid leaking server URLs into console in production.
- console.log(url);
575-611: Defer manualrequestUpdate()calls; Lit will schedule updates on state changesYou call
requestUpdate()after multiple@stateupdates and also inopen(). Lit batches updates; most of these calls are unnecessary.Consider removing the extra
requestUpdate()at the end ofapplyBackendStats,setVisibility,onUserMe, andopen().
310-331: PlayerStatsGrid margin via Tailwind won’t apply inside its Shadow DOMYou pass
mb-2into<player-stats-grid>, but that class is inside the child’s Shadow DOM and won’t pick up Tailwind styles. After adding internal spacing in the grid component (see separate comment), you can dropmb-2here.- <player-stats-grid - .titles=${[ ... ]} - .values=${[ ... ]} - ></player-stats-grid> + <player-stats-grid + .titles=${[ /* … */ ]} + .values=${[ /* … */ ]} + ></player-stats-grid>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/client/PlayerInfoModal.ts(1 hunks)src/client/components/baseComponents/PlayerStatsGrid.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (23)
📓 Common learnings
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.
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/components/baseComponents/PlayerStatsGrid.tssrc/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/components/baseComponents/PlayerStatsGrid.tssrc/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/client/components/baseComponents/PlayerStatsGrid.tssrc/client/PlayerInfoModal.ts
📚 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/components/baseComponents/PlayerStatsGrid.tssrc/client/PlayerInfoModal.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/client/components/baseComponents/PlayerStatsGrid.tssrc/client/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
src/client/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-31T06:39:29.144Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1536
File: src/core/execution/utils/BotBehavior.ts:41-54
Timestamp: 2025-07-31T06:39:29.144Z
Learning: In the OpenFrontIO codebase, the `PseudoRandom.chance(odds: number)` method takes inverse odds as a parameter, not a percentage. The probability is calculated as `1.0 / odds`. So `chance(1.5)` gives 66.67% probability, `chance(2)` gives 50% probability, etc. The implementation returns `Math.floor(this._nextFloat() * odds) === 0`.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-22T05:48:19.241Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#786
File: src/client/TerritoryPatternsModal.ts:337-338
Timestamp: 2025-06-22T05:48:19.241Z
Learning: In src/client/TerritoryPatternsModal.ts, the bit shifting operators (<<) used in coordinate calculations with decoder.getScale() are intentional and should not be changed to multiplication. The user scottanderson confirmed this is functioning as intended.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
PR: openfrontio/OpenFrontIO#1063
File: src/core/configuration/PastelThemeDark.ts:53-53
Timestamp: 2025-06-10T09:56:44.473Z
Learning: In ColorAllocator class in src/core/configuration/Colors.ts, the correct method names are assignColor(id: string): Colord for general color assignment and assignTeamColor(team: Team): Colord for team-specific colors. There are no assignPlayerColor() or assignBotColor() methods.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from UnitType values (specifically bomb-related values like AtomBomb, HydrogenBomb, MIRV, and MIRVWarhead) rather than a separate enum. This means comparing NukeType values against UnitType values in switch statements is valid and intentional.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from specific UnitType enum values (AtomBomb, HydrogenBomb, MIRV, MIRVWarhead). This means that comparisons in switch statements between a NukeType parameter and UnitType enum values are valid and will work correctly at runtime.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:43:38.150Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-06-13T20:57:49.599Z
Learnt from: El-Magico777
PR: openfrontio/OpenFrontIO#1166
File: src/core/Schemas.ts:50-56
Timestamp: 2025-06-13T20:57:49.599Z
Learning: In the OpenFrontIO codebase, player identification primarily uses numeric smallIDs throughout the system (e.g., playerBySmallID). When adding new intents or schemas, maintain consistency with this existing pattern rather than introducing mixed ID types, even if string IDs might be more standard elsewhere.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:29.540Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:61-63
Timestamp: 2025-05-19T06:00:29.540Z
Learning: The maintainer prefers to prioritize performance over defensive programming for the stats() method in StatsImpl, trusting callers not to directly modify the returned data structure.
Applied to files:
src/client/PlayerInfoModal.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: 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/client/PlayerInfoModal.ts
📚 Learning: 2025-06-07T13:15:55.439Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#786
File: src/core/Util.ts:4-4
Timestamp: 2025-06-07T13:15:55.439Z
Learning: In the OpenFrontIO codebase, JSON files should be imported using standard import syntax without import attributes, as the TypeScript configuration supports resolveJsonModule and the codebase already uses this pattern successfully in files like src/client/Cosmetic.ts.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-20T20:11:00.965Z
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/client/PlayerInfoModal.ts
🔇 Additional comments (2)
src/client/PlayerInfoModal.ts (2)
322-325: Win-rate zero guard looks goodYou avoid
NaN%by guarding division withgamesPlayed === 0. Good.
8-19: Verify and treatwonas optional in PlayerInfoModal.tsThe API type currently lacks a
wonflag, yet the UI forceswon: falsefor all games—this hides real win/loss data if it exists. If the backend starts returningwon, we should update types and render logic; otherwise we risk incorrect UI.Please:
• Confirm with the backend team or API docs that each game object may include a
won: booleanfield.
• Ifwonis returned, update insrc/client/PlayerInfoModal.ts:
- Add
won?: booleanto thePlayerApiResponse.gamesitem type.- Change the local state type for
recentGamestowon?: boolean.- Render Victory/Defeat only when
typeof game.won === "boolean".- In the
mapcall, spread{ won: g.won }only ifg.wonis boolean.Example diffs:
// PlayerApiResponse type type PlayerApiResponse = { stats?: unknown; games?: Array<{ gameId: string; start: string; map: string; difficulty: string; type: string; mode?: string; + won?: boolean; }>; };// Component state @state() private recentGames: { gameId: string; start: string; map: string; difficulty: string; type: string; - won: boolean; + won?: boolean; gameMode: "ffa" | "team"; teamCount?: number; teamColor?: string; }[] = [];// Conditional render - ${game.won ? html`<div class="text-xs text-green-400">Victory</div>` : html`<div class="text-xs text-red-400">Defeat</div>`} + ${typeof game.won === "boolean" + ? html`<div class="text-xs ${game.won ? "text-green-400" : "text-red-400"}"> + ${game.won ? "Victory" : "Defeat"} + </div>` + : null}// Mapping API data - this.recentGames = data.games.map(g => ({ - …, - won: false, - })); + this.recentGames = data.games.map(g => ({ + …, + ...(typeof g.won === "boolean" ? { won: g.won } : {}), + }));
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
src/client/PlayerInfoModal.ts (7)
30-35: Remove placeholder defaults for top-level statsThese values will show fake data before API load. Initialize to neutral values to avoid user confusion.
- @state() private wins: number = 57; - @state() private playTimeSeconds: number = 5 * 3600 + 33 * 60; - @state() private gamesPlayed: number = 119; - @state() private losses: number = 62; - @state() private lastActive: string = "1992/4/27"; + @state() private wins: number = 0; + @state() private playTimeSeconds: number = 0; + @state() private gamesPlayed: number = 0; + @state() private losses: number = 0; + @state() private lastActive: string = "";
36-38: Align PlayerStats optionality with Zod (prefer undefined over null)Project learning: PlayerStats already includes undefined. Use
PlayerStats | undefinedfor state and return type.- @state() private statsPublic: PlayerStats | null = null; - @state() private statsPrivate: PlayerStats | null = null; - @state() private statsAll: PlayerStats | null = null; + @state() private statsPublic: PlayerStats | undefined = undefined; + @state() private statsPrivate: PlayerStats | undefined = undefined; + @state() private statsAll: PlayerStats | undefined = undefined;- private getDisplayedStats(): PlayerStats | null { + private getDisplayedStats(): PlayerStats | undefined {Also applies to: 151-160
126-141: Fix key mapping in getBuildingName to match schema codesKeys don’t match schema (defp/saml/abomb/hbomb/wshp/trans/trade). This renders raw codes. Update labels.
private getBuildingName(building: string): string { const buildingNames: Record<string, string> = { - city: "City", - port: "Port", - defense: "Defense", - warship: "Warship", - atom: "Atom Bomb", - hydrogen: "Hydrogen Bomb", - mirv: "MIRV", - silo: "Missile Silo", - sam: "SAM", - transportShip: "Transport Ship", - tradeShip: "Trade Ship", + // units/buildings + city: "City", + port: "Port", + defp: "Defense", + saml: "SAM", + silo: "Missile Silo", + // boats + wshp: "Warship", + trans: "Transport Ship", + trade: "Trade Ship", + // bombs + abomb: "Atom Bomb", + hbomb: "Hydrogen Bomb", + mirv: "MIRV", }; return buildingNames[building] ?? building; }
212-229: Bug: mergePlayerStats overwrites nested records and mixes bigint/numberSpreading units/boats/bombs loses data;
0nBigInt will crash when added to number. Sum arrays per key as numbers.- private mergePlayerStats(a: PlayerStats, b: PlayerStats): PlayerStats { - const safeA = a ?? {}; - const safeB = b ?? {}; - const mergeArrays = (arr1?: any[], arr2?: any[]) => { - if (!arr1 && !arr2) return undefined; - if (!arr1) return arr2; - if (!arr2) return arr1; - return arr1.map((v, i) => Number(v ?? 0) + Number(arr2[i] ?? 0)); - }; - return { - attacks: mergeArrays(safeA.attacks, safeB.attacks), - betrayals: (safeA.betrayals ?? 0n) + (safeB.betrayals ?? 0n), - boats: { ...(safeA.boats ?? {}), ...(safeB.boats ?? {}) }, - bombs: { ...(safeA.bombs ?? {}), ...(safeB.bombs ?? {}) }, - gold: mergeArrays(safeA.gold, safeB.gold), - units: { ...(safeA.units ?? {}), ...(safeB.units ?? {}) }, - }; - } + private mergePlayerStats(a: PlayerStats, b: PlayerStats): PlayerStats { + const sumTuple = (t1?: unknown[], t2?: unknown[]) => { + if (!t1 && !t2) return undefined; + if (!t1) return Array.isArray(t2) ? t2.map((x) => Number(x ?? 0)) : undefined; + if (!t2) return Array.isArray(t1) ? t1.map((x) => Number(x ?? 0)) : undefined; + const len = Math.max(t1.length, t2.length); + const out: number[] = []; + for (let i = 0; i < len; i++) { + out[i] = Number((t1 as any)[i] ?? 0) + Number((t2 as any)[i] ?? 0); + } + return out; + }; + const mergeRecord = ( + ra?: Record<string, unknown[]>, + rb?: Record<string, unknown[]>, + ): Record<string, number[]> | undefined => { + if (!ra && !rb) return undefined; + const keys = new Set<string>([ + ...Object.keys(ra ?? {}), + ...Object.keys(rb ?? {}), + ]); + const out: Record<string, number[]> = {}; + for (const k of keys) { + out[k] = sumTuple(ra?.[k], rb?.[k]) ?? []; + } + return out; + }; + return { + attacks: sumTuple(a?.attacks, b?.attacks), + betrayals: Number(a?.betrayals ?? 0) + Number(b?.betrayals ?? 0), + boats: mergeRecord(a?.boats as any, b?.boats as any), + bombs: mergeRecord(a?.bombs as any, b?.bombs as any), + gold: sumTuple(a?.gold, b?.gold), + units: mergeRecord(a?.units as any, b?.units as any), + }; + }
418-421: Fix bombs table headers to match data tuple orderBombs data is [launched, landed, intercepted]. Update headers accordingly.
- <th class="text-center w-1/5">Built</th> - <th class="text-center w-1/5">Destroyed</th> - <th class="text-center w-1/5">Hits</th> + <th class="text-center w-1/5">Launched</th> + <th class="text-center w-1/5">Landed</th> + <th class="text-center w-1/5">Intercepted</th>
243-243: Localize all user-facing stringsHard-coded English bypasses localization. Route through the translation helper and add keys to en.json only.
Example edits:
-<o-modal id="playerInfoModal" title="Player Info" alwaysMaximized> +<o-modal id="playerInfoModal" title="${translateText('player_info.title')}" alwaysMaximized>-<button>All</button> +<button>${translateText('player_info.visibility.all')}</button>-["Wins","Losses","Win Rate","Games Played","Play Time","Last Active"] +[ + translateText('player_info.wins'), + translateText('player_info.losses'), + translateText('player_info.win_rate'), + translateText('player_info.games_played'), + translateText('player_info.play_time'), + translateText('player_info.last_active'), +]Apply similar changes to section titles, table headers, and action buttons (“View”, “Details”, “Started”, “Mode”, “Map”, “Difficulty”, “Type”, “Free‑for‑All”, “Team”). I can draft the full key list if helpful.
Also applies to: 282-302, 306-314, 333-346, 376-386, 412-421, 451-455, 463-469, 474-475, 484-491, 505-525
231-534: Split UI into composable componentsThis class mixes modal shell, identity header, visibility toggle, three stats tables, and recent games list. Split into small components and compose.
Suggested components:
- PlayerInfoHeader (flag, names, avatar)
- VisibilityToggle (all/public/private)
- PlayerStatsGrid wrapper
- BuildingStatsTable
- ShipArrivalsTable
- NukeStatsTable
- RecentGamesList
Keep data flow via typed props; avoid internal mutable state where possible.
🧹 Nitpick comments (3)
src/client/PlayerInfoModal.ts (3)
316-326: Consistency: include “%” for zero win rateCurrently renders “0.0” (no percent). Keep the unit consistent.
- this.gamesPlayed === 0 - ? "0.0" - : ((this.wins / this.gamesPlayed) * 100).toFixed(1) + "%", + this.gamesPlayed === 0 + ? "0.0%" + : ((this.wins / this.gamesPlayed) * 100).toFixed(1) + "%",
563-563: Remove stray console.logLeftover debug logging.
- console.log(url);
161-168: Reduce state mutations; derive on renderThese assignments duplicate state that can be derived from
totalsByVisibilityat render time. Prefer computing inside render to keep state minimal.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/client/PlayerInfoModal.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (27)
📓 Common learnings
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.
📚 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/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
src/client/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-31T06:39:29.144Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1536
File: src/core/execution/utils/BotBehavior.ts:41-54
Timestamp: 2025-07-31T06:39:29.144Z
Learning: In the OpenFrontIO codebase, the `PseudoRandom.chance(odds: number)` method takes inverse odds as a parameter, not a percentage. The probability is calculated as `1.0 / odds`. So `chance(1.5)` gives 66.67% probability, `chance(2)` gives 50% probability, etc. The implementation returns `Math.floor(this._nextFloat() * odds) === 0`.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-22T05:48:19.241Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#786
File: src/client/TerritoryPatternsModal.ts:337-338
Timestamp: 2025-06-22T05:48:19.241Z
Learning: In src/client/TerritoryPatternsModal.ts, the bit shifting operators (<<) used in coordinate calculations with decoder.getScale() are intentional and should not be changed to multiplication. The user scottanderson confirmed this is functioning as intended.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
PR: openfrontio/OpenFrontIO#1063
File: src/core/configuration/PastelThemeDark.ts:53-53
Timestamp: 2025-06-10T09:56:44.473Z
Learning: In ColorAllocator class in src/core/configuration/Colors.ts, the correct method names are assignColor(id: string): Colord for general color assignment and assignTeamColor(team: Team): Colord for team-specific colors. There are no assignPlayerColor() or assignBotColor() methods.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from UnitType values (specifically bomb-related values like AtomBomb, HydrogenBomb, MIRV, and MIRVWarhead) rather than a separate enum. This means comparing NukeType values against UnitType values in switch statements is valid and intentional.
Applied to files:
src/client/PlayerInfoModal.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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from specific UnitType enum values (AtomBomb, HydrogenBomb, MIRV, MIRVWarhead). This means that comparisons in switch statements between a NukeType parameter and UnitType enum values are valid and will work correctly at runtime.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:43:38.150Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-06-13T20:57:49.599Z
Learnt from: El-Magico777
PR: openfrontio/OpenFrontIO#1166
File: src/core/Schemas.ts:50-56
Timestamp: 2025-06-13T20:57:49.599Z
Learning: In the OpenFrontIO codebase, player identification primarily uses numeric smallIDs throughout the system (e.g., playerBySmallID). When adding new intents or schemas, maintain consistency with this existing pattern rather than introducing mixed ID types, even if string IDs might be more standard elsewhere.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:29.540Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:61-63
Timestamp: 2025-05-19T06:00:29.540Z
Learning: The maintainer prefers to prioritize performance over defensive programming for the stats() method in StatsImpl, trusting callers not to directly modify the returned data structure.
Applied to files:
src/client/PlayerInfoModal.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: 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/client/PlayerInfoModal.ts
📚 Learning: 2025-06-07T13:15:55.439Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#786
File: src/core/Util.ts:4-4
Timestamp: 2025-06-07T13:15:55.439Z
Learning: In the OpenFrontIO codebase, JSON files should be imported using standard import syntax without import attributes, as the TypeScript configuration supports resolveJsonModule and the codebase already uses this pattern successfully in files like src/client/Cosmetic.ts.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-20T20:11:00.965Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-06-12T23:22:55.328Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#786
File: src/server/Client.ts:16-18
Timestamp: 2025-06-12T23:22:55.328Z
Learning: Undefined values for `roles`, `flares`, and `pattern` are intentionally used to mark an unauthenticated client, whereas empty arrays/strings indicate an authenticated user with no roles or flares. Avoid suggesting non-optional types for these fields.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-22T21:51:14.990Z
Learnt from: devalnor
PR: openfrontio/OpenFrontIO#1248
File: src/client/graphics/layers/TerritoryInfoLayer.ts:20-20
Timestamp: 2025-06-22T21:51:14.990Z
Learning: In TerritoryInfoLayer.ts, the highlightedTerritory field uses both null and undefined intentionally: undefined represents initial state or inactive layer (Ctrl released), while null represents active layer with no territory being highlighted at cursor position. This distinction is important for proper state change detection.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-16T03:03:59.778Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1192
File: src/client/graphics/layers/RadialMenuElements.ts:312-314
Timestamp: 2025-06-16T03:03:59.778Z
Learning: In RadialMenuElements.ts, when fixing undefined params errors in subMenu functions, use explicit checks like `if (params === undefined || params.selected === null)` rather than optional chaining, as it makes the intent clearer and matches the specific error scenarios where params can be undefined during spawn phase operations.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-07-31T12:03:08.052Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1655
File: resources/maps/giantworldmap/manifest.json:165-174
Timestamp: 2025-07-31T12:03:08.052Z
Learning: In OpenFrontIO draft PRs, flag names in map manifest files may be placeholders that will be updated to match the actual SVG flag file names when the assets are received. The final naming depends on the actual flag SVG files provided, not code naming conventions.
Applied to files:
src/client/PlayerInfoModal.ts
🔇 Additional comments (2)
src/client/PlayerInfoModal.ts (2)
427-441: LGTM: silo correctly excluded from nukesFiltering nukes to ["abomb", "hbomb", "mirv"] avoids undefined fields for silo.
170-210: Good: Zod-guarded parse of nested PlayerStatsSafe parsing from rawStats with PlayerStatsSchema is a solid approach.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (8)
src/client/PlayerInfoModal.ts (8)
36-74: Drop placeholder recent games; start empty and fill from APISample games will render in production. Initialize with an empty list.
- }[] = [ - { - gameId: "tGadjhgg", - start: "2025-08-08T10:00:00Z", - map: "Australia", - difficulty: "Medium", - type: "Public", - gameMode: "ffa", - }, - { - gameId: "I7XQ63rt", - start: "2025-08-07T09:00:00Z", - map: "Baikal", - difficulty: "Medium", - type: "Public", - gameMode: "team", - teamCount: 2, - teamColor: "blue", - }, - { - gameId: "Chocolat", - start: "2025-08-06T11:30:00Z", - map: "World", - difficulty: "Medium", - type: "Private", - gameMode: "team", - teamCount: 3, - teamColor: "red", - }, - ]; + }[] = [];
113-128: Fix name mapping keys to match stats schema codes (defp,saml,abomb,hbomb,wshp,trans,trade)Current keys (
defense,sam,warship,atom,hydrogen,transportShip,tradeShip) don’t match the data, so codes render instead of friendly names.private getBuildingName(building: string): string { const buildingNames: Record<string, string> = { - city: "City", - port: "Port", - defense: "Defense", - warship: "Warship", - atom: "Atom Bomb", - hydrogen: "Hydrogen Bomb", - mirv: "MIRV", - silo: "Missile Silo", - sam: "SAM", - transportShip: "Transport Ship", - tradeShip: "Trade Ship", + // units + city: "City", + port: "Port", + defp: "Defense", + saml: "SAM", + silo: "Missile Silo", + // boats + wshp: "Warship", + trans: "Transport Ship", + trade: "Trade Ship", + // bombs + abomb: "Atom Bomb", + hbomb: "Hydrogen Bomb", + mirv: "MIRV", }; return buildingNames[building] ?? building; }
23-26: Align with Zod typing: useundefinedinstead ofnullfor PlayerStats statePer project learnings,
PlayerStatsalready includesundefined. Avoidnullto reduce checks and stay idiomatic.- @state() private statsPublic: PlayerStats | null = null; - @state() private statsPrivate: PlayerStats | null = null; - @state() private statsAll: PlayerStats | null = null; + @state() private statsPublic: PlayerStats | undefined = undefined; + @state() private statsPrivate: PlayerStats | undefined = undefined; + @state() private statsAll: PlayerStats | undefined = undefined; @@ - private getDisplayedStats(): PlayerStats | null { + private getDisplayedStats(): PlayerStats | undefined { switch (this.visibility) { case "public": return this.statsPublic; case "private": return this.statsPrivate; default: return this.statsAll; } }Also applies to: 138-147
199-216: Bug:mergePlayerStatsoverwrites nested records; sum arrays per key and avoid bigint mixingSpreading
boats/bombs/unitsloses data when keys overlap. Also(0n + number)mixes bigint/number. Sum tuples per key and use numbers.- private mergePlayerStats(a: PlayerStats, b: PlayerStats): PlayerStats { - const safeA = a ?? {}; - const safeB = b ?? {}; - const mergeArrays = (arr1?: any[], arr2?: any[]) => { - if (!arr1 && !arr2) return undefined; - if (!arr1) return arr2; - if (!arr2) return arr1; - return arr1.map((v, i) => Number(v ?? 0) + Number(arr2[i] ?? 0)); - }; - return { - attacks: mergeArrays(safeA.attacks, safeB.attacks), - betrayals: (safeA.betrayals ?? 0n) + (safeB.betrayals ?? 0n), - boats: { ...(safeA.boats ?? {}), ...(safeB.boats ?? {}) }, - bombs: { ...(safeA.bombs ?? {}), ...(safeB.bombs ?? {}) }, - gold: mergeArrays(safeA.gold, safeB.gold), - units: { ...(safeA.units ?? {}), ...(safeB.units ?? {}) }, - }; - } + private mergePlayerStats(a: PlayerStats, b: PlayerStats): PlayerStats { + const sumTuple = (t1?: unknown[], t2?: unknown[]) => { + if (!t1 && !t2) return undefined; + if (!t1) return Array.isArray(t2) ? t2.map((x) => Number(x ?? 0)) : undefined; + if (!t2) return Array.isArray(t1) ? t1.map((x) => Number(x ?? 0)) : undefined; + const len = Math.max(t1.length, t2.length); + const out: number[] = []; + for (let i = 0; i < len; i++) { + out[i] = Number((t1 as any)[i] ?? 0) + Number((t2 as any)[i] ?? 0); + } + return out; + }; + const mergeRecord = ( + ra?: Record<string, unknown[]>, + rb?: Record<string, unknown[]>, + ): Record<string, number[]> | undefined => { + if (!ra && !rb) return undefined; + const keys = new Set<string>([ + ...Object.keys(ra ?? {}), + ...Object.keys(rb ?? {}), + ]); + const out: Record<string, number[]> = {}; + for (const k of keys) { + out[k] = sumTuple(ra?.[k], rb?.[k]) ?? []; + } + return out; + }; + return { + attacks: sumTuple(a?.attacks, b?.attacks), + betrayals: Number(a?.betrayals ?? 0) + Number(b?.betrayals ?? 0), + boats: mergeRecord(a?.boats as any, b?.boats as any), + bombs: mergeRecord(a?.bombs as any, b?.bombs as any), + gold: sumTuple(a?.gold, b?.gold), + units: mergeRecord(a?.units as any, b?.units as any), + }; + }
293-314: Win-rate zero case missing “%”Keep formatting consistent.
- this.gamesPlayed === 0 - ? "0.0" - : ((this.wins / this.gamesPlayed) * 100).toFixed(1) + "%", + this.gamesPlayed === 0 + ? "0.0%" + : ((this.wins / this.gamesPlayed) * 100).toFixed(1) + "%",
230-289: Localize all user-facing strings (modal title, labels, buttons, headers)Hard-coded English bypasses i18n. Wrap with the translation helper and add keys to en.json. Example:
- <o-modal id="playerInfoModal" title="Player Info" alwaysMaximized> + <o-modal id="playerInfoModal" .title=${translateText('player_info.title')} alwaysMaximized> @@ - All + ${translateText('player_info.visibility.all')} @@ - Public + ${translateText('player_info.visibility.public')} @@ - Private + ${translateText('player_info.visibility.private')} @@ - .titles=${[ - "Wins", - "Losses", - "Win Rate", - "Games Played", - "Play Time", - "Last Active", - ]} + .titles=${[ + translateText('player_info.wins'), + translateText('player_info.losses'), + translateText('player_info.win_rate'), + translateText('player_info.games_played'), + translateText('player_info.play_time'), + translateText('player_info.last_active'), + ]} @@ - 🏗️ Building Statistics + ${translateText('player_info.section.buildings')} @@ - <th class="text-left w-1/5">Building</th> + <th class="text-left w-1/5">${translateText('player_info.building')}</th>Apply similarly for “Ship Arrivals”, “Nuke Statistics”, “Recent Games”, column headers, “View”, “Details”, and inline labels like “Game ID”, “Started”, “Map”, “Difficulty”, “Type”.
Note: follow project policy — add keys only to en.json; other locales are handled by translators.
Also applies to: 318-359, 362-396, 398-430, 436-517
541-543: Reset all player info state on logout to prevent stale UIClear stats, totals, recents, and view state.
onLoggedOut() { this.userMeResponse = null; + this.statsPublic = undefined; + this.statsPrivate = undefined; + this.statsAll = undefined; + this.visibility = "all"; + this.totalsByVisibility = { + all: { wins: 0, losses: 0, total: 0 }, + public: { wins: 0, losses: 0, total: 0 }, + private: { wins: 0, losses: 0, total: 0 }, + }; + this.wins = 0; + this.losses = 0; + this.gamesPlayed = 0; + this.playTimeSeconds = 0; + this.lastActive = ""; + this.recentGames = []; + this.expandedGameId = null; }
17-21: Remove placeholder counters; initialize to neutral defaultsHard-coded numbers will leak to users before API load and during logout. Start from zero/empty.
- @state() private wins: number = 57; - @state() private playTimeSeconds: number = 5 * 3600 + 33 * 60; - @state() private gamesPlayed: number = 119; - @state() private losses: number = 62; - @state() private lastActive: string = "1992/4/27"; + @state() private wins: number = 0; + @state() private playTimeSeconds: number = 0; + @state() private gamesPlayed: number = 0; + @state() private losses: number = 0; + @state() private lastActive: string = "";
🧹 Nitpick comments (1)
src/client/PlayerInfoModal.ts (1)
570-583: Guard recent games mapping in casegamesis empty/missingSchema requires
games, but defensive checks keep UI resilient.- this.recentGames = data.games.map((g) => ({ + this.recentGames = Array.isArray(data.games) ? data.games.map((g) => ({ gameId: g.gameId, start: g.start, map: g.map, difficulty: g.difficulty, type: g.type, gameMode: g.mode && String(g.mode).toLowerCase().includes("team") ? "team" : "ffa", - })); + })) : [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/client/PlayerInfoModal.ts(1 hunks)src/core/ApiSchemas.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (27)
📓 Common learnings
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.
📚 Learning: 2025-05-21T04:10:33.435Z
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/ApiSchemas.tssrc/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/ApiSchemas.tssrc/client/PlayerInfoModal.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/core/ApiSchemas.tssrc/client/PlayerInfoModal.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: 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/core/ApiSchemas.tssrc/client/PlayerInfoModal.ts
📚 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/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/client/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
src/client/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-31T06:39:29.144Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1536
File: src/core/execution/utils/BotBehavior.ts:41-54
Timestamp: 2025-07-31T06:39:29.144Z
Learning: In the OpenFrontIO codebase, the `PseudoRandom.chance(odds: number)` method takes inverse odds as a parameter, not a percentage. The probability is calculated as `1.0 / odds`. So `chance(1.5)` gives 66.67% probability, `chance(2)` gives 50% probability, etc. The implementation returns `Math.floor(this._nextFloat() * odds) === 0`.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-22T05:48:19.241Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#786
File: src/client/TerritoryPatternsModal.ts:337-338
Timestamp: 2025-06-22T05:48:19.241Z
Learning: In src/client/TerritoryPatternsModal.ts, the bit shifting operators (<<) used in coordinate calculations with decoder.getScale() are intentional and should not be changed to multiplication. The user scottanderson confirmed this is functioning as intended.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
PR: openfrontio/OpenFrontIO#1063
File: src/core/configuration/PastelThemeDark.ts:53-53
Timestamp: 2025-06-10T09:56:44.473Z
Learning: In ColorAllocator class in src/core/configuration/Colors.ts, the correct method names are assignColor(id: string): Colord for general color assignment and assignTeamColor(team: Team): Colord for team-specific colors. There are no assignPlayerColor() or assignBotColor() methods.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from UnitType values (specifically bomb-related values like AtomBomb, HydrogenBomb, MIRV, and MIRVWarhead) rather than a separate enum. This means comparing NukeType values against UnitType values in switch statements is valid and intentional.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from specific UnitType enum values (AtomBomb, HydrogenBomb, MIRV, MIRVWarhead). This means that comparisons in switch statements between a NukeType parameter and UnitType enum values are valid and will work correctly at runtime.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:43:38.150Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-06-13T20:57:49.599Z
Learnt from: El-Magico777
PR: openfrontio/OpenFrontIO#1166
File: src/core/Schemas.ts:50-56
Timestamp: 2025-06-13T20:57:49.599Z
Learning: In the OpenFrontIO codebase, player identification primarily uses numeric smallIDs throughout the system (e.g., playerBySmallID). When adding new intents or schemas, maintain consistency with this existing pattern rather than introducing mixed ID types, even if string IDs might be more standard elsewhere.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:29.540Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:61-63
Timestamp: 2025-05-19T06:00:29.540Z
Learning: The maintainer prefers to prioritize performance over defensive programming for the stats() method in StatsImpl, trusting callers not to directly modify the returned data structure.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-07T13:15:55.439Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#786
File: src/core/Util.ts:4-4
Timestamp: 2025-06-07T13:15:55.439Z
Learning: In the OpenFrontIO codebase, JSON files should be imported using standard import syntax without import attributes, as the TypeScript configuration supports resolveJsonModule and the codebase already uses this pattern successfully in files like src/client/Cosmetic.ts.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-20T20:11:00.965Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-06-12T23:22:55.328Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#786
File: src/server/Client.ts:16-18
Timestamp: 2025-06-12T23:22:55.328Z
Learning: Undefined values for `roles`, `flares`, and `pattern` are intentionally used to mark an unauthenticated client, whereas empty arrays/strings indicate an authenticated user with no roles or flares. Avoid suggesting non-optional types for these fields.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-22T21:51:14.990Z
Learnt from: devalnor
PR: openfrontio/OpenFrontIO#1248
File: src/client/graphics/layers/TerritoryInfoLayer.ts:20-20
Timestamp: 2025-06-22T21:51:14.990Z
Learning: In TerritoryInfoLayer.ts, the highlightedTerritory field uses both null and undefined intentionally: undefined represents initial state or inactive layer (Ctrl released), while null represents active layer with no territory being highlighted at cursor position. This distinction is important for proper state change detection.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-16T03:03:59.778Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1192
File: src/client/graphics/layers/RadialMenuElements.ts:312-314
Timestamp: 2025-06-16T03:03:59.778Z
Learning: In RadialMenuElements.ts, when fixing undefined params errors in subMenu functions, use explicit checks like `if (params === undefined || params.selected === null)` rather than optional chaining, as it makes the intent clearer and matches the specific error scenarios where params can be undefined during spawn phase operations.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-07-31T12:03:08.052Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1655
File: resources/maps/giantworldmap/manifest.json:165-174
Timestamp: 2025-07-31T12:03:08.052Z
Learning: In OpenFrontIO draft PRs, flag names in map manifest files may be placeholders that will be updated to match the actual SVG flag file names when the assets are received. The final naming depends on the actual flag SVG files provided, not code naming conventions.
Applied to files:
src/client/PlayerInfoModal.ts
🧬 Code Graph Analysis (1)
src/core/ApiSchemas.ts (1)
src/core/StatsSchemas.ts (1)
PlayerStatsSchema(100-109)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (10)
src/client/PlayerInfoModal.ts (10)
218-229: Split into small, reusable componentsThis class is large and holds too much state. Extract header, stats grid, each table, and recent-games into their own components; compose here.
- Less state, more derived render
- Reusable for end screen
- Matches maintainer guidance
Also applies to: 293-316, 320-436, 442-525
17-21: Remove test placeholders; default to neutral valuesThese seeded numbers will leak to UI before API loads.
- @state() private wins: number = 57; - @state() private playTimeSeconds: number = 5 * 3600 + 33 * 60; - @state() private gamesPlayed: number = 119; - @state() private losses: number = 62; - @state() private lastActive: string = "1992/4/27"; + @state() private wins: number = 0; + @state() private playTimeSeconds: number = 0; + @state() private gamesPlayed: number = 0; + @state() private losses: number = 0; + @state() private lastActive: string = "";
23-26: Prefer undefined over null for PlayerStats (matches Zod optional)In this codebase PlayerStats already includes undefined. Use undefined to simplify checks.
- @state() private statsPublic: PlayerStats | null = null; - @state() private statsPrivate: PlayerStats | null = null; - @state() private statsAll: PlayerStats | null = null; + @state() private statsPublic: PlayerStats | undefined = undefined; + @state() private statsPrivate: PlayerStats | undefined = undefined; + @state() private statsAll: PlayerStats | undefined = undefined;- private getDisplayedStats(): PlayerStats | null { + private getDisplayedStats(): PlayerStats | undefined { switch (this.visibility) { case "public": return this.statsPublic; case "private": return this.statsPrivate; default: return this.statsAll; } }Also applies to: 138-147
36-74: Drop placeholder recent games; start with empty listShow nothing until API response arrives.
- }[] = [ - { - gameId: "tGadjhgg", - start: "2025-08-08T10:00:00Z", - map: "Australia", - difficulty: "Medium", - type: "Public", - gameMode: "ffa", - }, - { - gameId: "I7XQ63rt", - start: "2025-08-07T09:00:00Z", - map: "Baikal", - difficulty: "Medium", - type: "Public", - gameMode: "team", - teamCount: 2, - teamColor: "blue", - }, - { - gameId: "Chocolat", - start: "2025-08-06T11:30:00Z", - map: "World", - difficulty: "Medium", - type: "Private", - gameMode: "team", - teamCount: 3, - teamColor: "red", - }, - ]; + }[] = [];
113-128: Fix key-to-label mapping to match stats schema codesYour filters use defp/saml/abomb/hbomb/wshp/trans/trade, but names map uses different keys. This renders raw codes.
private getBuildingName(building: string): string { const buildingNames: Record<string, string> = { - city: "City", - port: "Port", - defense: "Defense", - warship: "Warship", - atom: "Atom Bomb", - hydrogen: "Hydrogen Bomb", - mirv: "MIRV", - silo: "Missile Silo", - sam: "SAM", - transportShip: "Transport Ship", - tradeShip: "Trade Ship", + // units + city: "City", + port: "Port", + defp: "Defense", + saml: "SAM", + silo: "Missile Silo", + // boats + wshp: "Warship", + trans: "Transport Ship", + trade: "Trade Ship", + // bombs + abomb: "Atom Bomb", + hbomb: "Hydrogen Bomb", + mirv: "MIRV", }; return buildingNames[building] ?? building; }
157-197: Type input and set stats to undefined; avoid anyKeep runtime validation with Zod, but avoid any and use undefined for state.
- private applyBackendStats(rawStats: any): void { + private applyBackendStats(rawStats: unknown): void { const pubStats = PlayerStatsSchema.safeParse( rawStats?.Public?.["Free For All"]?.Medium?.stats ?? {}, ); const prvStats = PlayerStatsSchema.safeParse( rawStats?.Private?.["Free For All"]?.Medium?.stats ?? {}, ); - this.statsPublic = pubStats.success ? pubStats.data : null; - this.statsPrivate = prvStats.success ? prvStats.data : null; + this.statsPublic = pubStats.success ? pubStats.data : undefined; + this.statsPrivate = prvStats.success ? prvStats.data : undefined; if (this.statsPublic && this.statsPrivate) { this.statsAll = this.mergePlayerStats( this.statsPublic, this.statsPrivate, ); } else { - this.statsAll = this.statsPublic ?? this.statsPrivate; + this.statsAll = this.statsPublic ?? this.statsPrivate; }
199-216: Fix merge: sum nested arrays per key; don’t spread-overwrite or mix bigintCurrent code overwrites duplicate keys and mixes bigint with number.
private mergePlayerStats(a: PlayerStats, b: PlayerStats): PlayerStats { - const safeA = a ?? {}; - const safeB = b ?? {}; - const mergeArrays = (arr1?: any[], arr2?: any[]) => { - if (!arr1 && !arr2) return undefined; - if (!arr1) return arr2; - if (!arr2) return arr1; - return arr1.map((v, i) => Number(v ?? 0) + Number(arr2[i] ?? 0)); - }; - return { - attacks: mergeArrays(safeA.attacks, safeB.attacks), - betrayals: (safeA.betrayals ?? 0n) + (safeB.betrayals ?? 0n), - boats: { ...(safeA.boats ?? {}), ...(safeB.boats ?? {}) }, - bombs: { ...(safeA.bombs ?? {}), ...(safeB.bombs ?? {}) }, - gold: mergeArrays(safeA.gold, safeB.gold), - units: { ...(safeA.units ?? {}), ...(safeB.units ?? {}) }, - }; + const sumTuple = (t1?: unknown[], t2?: unknown[]) => { + if (!t1 && !t2) return undefined; + if (!t1) return Array.isArray(t2) ? t2.map((x) => Number(x ?? 0)) : undefined; + if (!t2) return Array.isArray(t1) ? t1.map((x) => Number(x ?? 0)) : undefined; + const len = Math.max(t1.length, t2.length); + const out: number[] = []; + for (let i = 0; i < len; i++) { + out[i] = Number((t1 as any)[i] ?? 0) + Number((t2 as any)[i] ?? 0); + } + return out; + }; + const mergeRecord = ( + ra?: Record<string, unknown[]>, + rb?: Record<string, unknown[]>, + ): Record<string, number[]> | undefined => { + if (!ra && !rb) return undefined; + const keys = new Set<string>([ + ...Object.keys(ra ?? {}), + ...Object.keys(rb ?? {}), + ]); + const out: Record<string, number[]> = {}; + for (const k of keys) { + out[k] = sumTuple(ra?.[k], rb?.[k]) ?? []; + } + return out; + }; + return { + attacks: sumTuple(a?.attacks, b?.attacks), + betrayals: Number(a?.betrayals ?? 0) + Number(b?.betrayals ?? 0), + boats: mergeRecord(a?.boats as any, b?.boats as any), + bombs: mergeRecord(a?.bombs as any, b?.bombs as any), + gold: sumTuple(a?.gold, b?.gold), + units: mergeRecord(a?.units as any, b?.units as any), + }; }
230-231: Route all user-facing strings through i18nLiteral English strings bypass localisation. Use translation helper and keys.
Example:
-<o-modal id="playerInfoModal" title="Player Info" alwaysMaximized> +<o-modal id="playerInfoModal" title="${translateText('player_info.title')}" alwaysMaximized>Apply similarly for:
- visibility buttons (“All”, “Public”, “Private”)
- stats grid titles (“Wins”, “Losses”, “Win Rate”, “Games Played”, “Play Time”, “Last Active”)
- section headers (“Building Statistics”, “Ship Arrivals”, “Nuke Statistics”, “Recent Games”)
- table headers
- labels in Recent Games (“Game ID”, “Mode”, “Free‑for‑All”, “Team”, “Started”, “Map”, “Difficulty”, “Type”)
- buttons (“View”, “Details”)
Also applies to: 269-289, 294-301, 321-323, 327-332, 361-364, 368-372, 401-403, 407-410, 444-445, 455-468, 476-478, 482-484, 498-517
407-410: Bomb table headers don’t match data tupleBombs are [launched, landed, intercepted]. Fix header order.
- <th class="text-center w-1/5">Built</th> - <th class="text-center w-1/5">Destroyed</th> - <th class="text-center w-1/5">Hits</th> + <th class="text-center w-1/5">Launched</th> + <th class="text-center w-1/5">Landed</th> + <th class="text-center w-1/5">Intercepted</th>
547-549: Reset all player state on logout to avoid stale UIClear stats, totals, and recentGames, and reset visibility.
onLoggedOut() { - this.userMeResponse = null; + this.userMeResponse = null; + this.statsPublic = undefined; + this.statsPrivate = undefined; + this.statsAll = undefined; + this.visibility = "all"; + this.totalsByVisibility = { + all: { wins: 0, losses: 0, total: 0 }, + public: { wins: 0, losses: 0, total: 0 }, + private: { wins: 0, losses: 0, total: 0 }, + }; + this.wins = 0; + this.losses = 0; + this.gamesPlayed = 0; + this.playTimeSeconds = 0; + this.lastActive = ""; + this.recentGames = []; + this.expandedGameId = null; }
🧹 Nitpick comments (3)
src/client/PlayerInfoModal.ts (3)
303-307: Add “%” for zero win-rateZero branch returns "0.0" without the percent sign.
- this.gamesPlayed === 0 - ? "0.0" + this.gamesPlayed === 0 + ? "0.0%" : ((this.wins / this.gamesPlayed) * 100).toFixed(1) + "%",
555-555: Encode playerId when building URL pathAvoid accidental path traversal if id contains slashes.
-url.pathname = "/player/" + playerId; +url.pathname = "/player/" + encodeURIComponent(playerId);
538-546: Reduce state; derive in render from validated dataStore the validated API payload and render from it. Avoid copying totals into fields and mutating them in setters. Simpler and less bug-prone.
- Keep userMeResponse and parsed player payload
- Compute totals and tables on the fly in render or small helpers
- Remove merge function if backend provides aggregated “all”, or compute via pure helpers without mutating component state
Also applies to: 157-197, 199-216
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/client/PlayerInfoModal.ts(1 hunks)src/core/ApiSchemas.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/ApiSchemas.ts
🧰 Additional context used
🧠 Learnings (27)
📓 Common learnings
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.
📚 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/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
src/client/PlayerInfoModal.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/PlayerInfoModal.ts
📚 Learning: 2025-07-31T06:39:29.144Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1536
File: src/core/execution/utils/BotBehavior.ts:41-54
Timestamp: 2025-07-31T06:39:29.144Z
Learning: In the OpenFrontIO codebase, the `PseudoRandom.chance(odds: number)` method takes inverse odds as a parameter, not a percentage. The probability is calculated as `1.0 / odds`. So `chance(1.5)` gives 66.67% probability, `chance(2)` gives 50% probability, etc. The implementation returns `Math.floor(this._nextFloat() * odds) === 0`.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-22T05:48:19.241Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#786
File: src/client/TerritoryPatternsModal.ts:337-338
Timestamp: 2025-06-22T05:48:19.241Z
Learning: In src/client/TerritoryPatternsModal.ts, the bit shifting operators (<<) used in coordinate calculations with decoder.getScale() are intentional and should not be changed to multiplication. The user scottanderson confirmed this is functioning as intended.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
PR: openfrontio/OpenFrontIO#1063
File: src/core/configuration/PastelThemeDark.ts:53-53
Timestamp: 2025-06-10T09:56:44.473Z
Learning: In ColorAllocator class in src/core/configuration/Colors.ts, the correct method names are assignColor(id: string): Colord for general color assignment and assignTeamColor(team: Team): Colord for team-specific colors. There are no assignPlayerColor() or assignBotColor() methods.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from UnitType values (specifically bomb-related values like AtomBomb, HydrogenBomb, MIRV, and MIRVWarhead) rather than a separate enum. This means comparing NukeType values against UnitType values in switch statements is valid and intentional.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from specific UnitType enum values (AtomBomb, HydrogenBomb, MIRV, MIRVWarhead). This means that comparisons in switch statements between a NukeType parameter and UnitType enum values are valid and will work correctly at runtime.
Applied to files:
src/client/PlayerInfoModal.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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:43:38.150Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-06-13T20:57:49.599Z
Learnt from: El-Magico777
PR: openfrontio/OpenFrontIO#1166
File: src/core/Schemas.ts:50-56
Timestamp: 2025-06-13T20:57:49.599Z
Learning: In the OpenFrontIO codebase, player identification primarily uses numeric smallIDs throughout the system (e.g., playerBySmallID). When adding new intents or schemas, maintain consistency with this existing pattern rather than introducing mixed ID types, even if string IDs might be more standard elsewhere.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:29.540Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:61-63
Timestamp: 2025-05-19T06:00:29.540Z
Learning: The maintainer prefers to prioritize performance over defensive programming for the stats() method in StatsImpl, trusting callers not to directly modify the returned data structure.
Applied to files:
src/client/PlayerInfoModal.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: 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/client/PlayerInfoModal.ts
📚 Learning: 2025-06-07T13:15:55.439Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#786
File: src/core/Util.ts:4-4
Timestamp: 2025-06-07T13:15:55.439Z
Learning: In the OpenFrontIO codebase, JSON files should be imported using standard import syntax without import attributes, as the TypeScript configuration supports resolveJsonModule and the codebase already uses this pattern successfully in files like src/client/Cosmetic.ts.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-20T20:11:00.965Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-06-12T23:22:55.328Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#786
File: src/server/Client.ts:16-18
Timestamp: 2025-06-12T23:22:55.328Z
Learning: Undefined values for `roles`, `flares`, and `pattern` are intentionally used to mark an unauthenticated client, whereas empty arrays/strings indicate an authenticated user with no roles or flares. Avoid suggesting non-optional types for these fields.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-22T21:51:14.990Z
Learnt from: devalnor
PR: openfrontio/OpenFrontIO#1248
File: src/client/graphics/layers/TerritoryInfoLayer.ts:20-20
Timestamp: 2025-06-22T21:51:14.990Z
Learning: In TerritoryInfoLayer.ts, the highlightedTerritory field uses both null and undefined intentionally: undefined represents initial state or inactive layer (Ctrl released), while null represents active layer with no territory being highlighted at cursor position. This distinction is important for proper state change detection.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-06-16T03:03:59.778Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1192
File: src/client/graphics/layers/RadialMenuElements.ts:312-314
Timestamp: 2025-06-16T03:03:59.778Z
Learning: In RadialMenuElements.ts, when fixing undefined params errors in subMenu functions, use explicit checks like `if (params === undefined || params.selected === null)` rather than optional chaining, as it makes the intent clearer and matches the specific error scenarios where params can be undefined during spawn phase operations.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-07-31T12:03:08.052Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1655
File: resources/maps/giantworldmap/manifest.json:165-174
Timestamp: 2025-07-31T12:03:08.052Z
Learning: In OpenFrontIO draft PRs, flag names in map manifest files may be placeholders that will be updated to match the actual SVG flag file names when the assets are received. The final naming depends on the actual flag SVG files provided, not code naming conventions.
Applied to files:
src/client/PlayerInfoModal.ts
🔇 Additional comments (1)
src/client/PlayerInfoModal.ts (1)
578-589: Guard optional games; avoid runtime errorIf games is missing or not an array, this throws.
- this.recentGames = data.games.map((g) => ({ + this.recentGames = Array.isArray(data.games) ? data.games.map((g) => ({ gameId: g.gameId, start: g.start, map: g.map, difficulty: g.difficulty, type: g.type, gameMode: g.mode && String(g.mode).toLowerCase().includes("team") ? "team" : "ffa", - })); + })) : [];
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
src/client/PlayerInfoModal.ts (5)
114-129: Fix label mapping keys or delete this unused helper.Keys here (defense/sam/atom/warship/…) don’t match schema (defp/saml/abomb/wshp/…). Also, this method appears unused in this class.
Preferred: remove the method to avoid drift and rely on player-stats-table (or a shared util):
- private getBuildingName(building: string): string { - const buildingNames: Record<string, string> = { - city: "City", - port: "Port", - defense: "Defense", - warship: "Warship", - atom: "Atom Bomb", - hydrogen: "Hydrogen Bomb", - mirv: "MIRV", - silo: "Missile Silo", - sam: "SAM", - transportShip: "Transport Ship", - tradeShip: "Trade Ship", - }; - return buildingNames[building] ?? building; - } + // Removed: label mapping moved to shared component/util
37-75: Remove placeholder recent games; start with empty list.Sample data will show to users before API load.
- }[] = [ - { - gameId: "tGadjhgg", - start: "2025-08-08T10:00:00Z", - map: "Australia", - difficulty: "Medium", - type: "Public", - gameMode: "ffa", - }, - { - gameId: "I7XQ63rt", - start: "2025-08-07T09:00:00Z", - map: "Baikal", - difficulty: "Medium", - type: "Public", - gameMode: "team", - teamCount: 2, - teamColor: "blue", - }, - { - gameId: "Chocolat", - start: "2025-08-06T11:30:00Z", - map: "World", - difficulty: "Medium", - type: "Private", - gameMode: "team", - teamCount: 3, - teamColor: "red", - }, - ]; + }[] = [];
24-27: Align PlayerStats state with zod optional typing; avoid null and any.Use undefined instead of null, and type the raw stats param as unknown.
- @state() private statsPublic: PlayerStats | null = null; - @state() private statsPrivate: PlayerStats | null = null; - @state() private statsAll: PlayerStats | null = null; + @state() private statsPublic: PlayerStats | undefined = undefined; + @state() private statsPrivate: PlayerStats | undefined = undefined; + @state() private statsAll: PlayerStats | undefined = undefined;- private getDisplayedStats(): PlayerStats | null { + private getDisplayedStats(): PlayerStats | undefined {- onLoggedOut() { - this.userMeResponse = null; - } + onLoggedOut() { + this.userMeResponse = null; + this.statsPublic = undefined; + this.statsPrivate = undefined; + this.statsAll = undefined; + this.visibility = "all"; + this.totalsByVisibility = { + all: { wins: 0, losses: 0, total: 0 }, + public: { wins: 0, losses: 0, total: 0 }, + private: { wins: 0, losses: 0, total: 0 }, + }; + this.wins = 0; + this.losses = 0; + this.gamesPlayed = 0; + this.playTimeSeconds = 0; + this.lastActive = ""; + this.recentGames = []; + this.expandedGameId = null; + }- private applyBackendStats(rawStats: any): void { + private applyBackendStats(rawStats: unknown): void { @@ - this.statsPublic = pubStats.success ? pubStats.data : null; - this.statsPrivate = prvStats.success ? prvStats.data : null; + this.statsPublic = pubStats.success ? pubStats.data : undefined; + this.statsPrivate = prvStats.success ? prvStats.data : undefined;Also applies to: 139-148, 435-437, 159-166, 166-175
18-22: Remove placeholder numbers; initialize to neutral defaults.These test values will leak into UI before API loads.
- @state() private wins: number = 57; - @state() private playTimeSeconds: number = 5 * 3600 + 33 * 60; - @state() private gamesPlayed: number = 119; - @state() private losses: number = 62; - @state() private lastActive: string = "1992/4/27"; + @state() private wins: number = 0; + @state() private playTimeSeconds: number = 0; + @state() private gamesPlayed: number = 0; + @state() private losses: number = 0; + @state() private lastActive: string = "";
201-218: Fix mergePlayerStats: spreading nested records overwrites counts; BigInt mixing.Sum per-key tuples and keep numbers. Avoid BigInt unless schema requires it.
- private mergePlayerStats(a: PlayerStats, b: PlayerStats): PlayerStats { - const safeA = a ?? {}; - const safeB = b ?? {}; - const mergeArrays = (arr1?: any[], arr2?: any[]) => { - if (!arr1 && !arr2) return undefined; - if (!arr1) return arr2; - if (!arr2) return arr1; - return arr1.map((v, i) => Number(v ?? 0) + Number(arr2[i] ?? 0)); - }; - return { - attacks: mergeArrays(safeA.attacks, safeB.attacks), - betrayals: (safeA.betrayals ?? 0n) + (safeB.betrayals ?? 0n), - boats: { ...(safeA.boats ?? {}), ...(safeB.boats ?? {}) }, - bombs: { ...(safeA.bombs ?? {}), ...(safeB.bombs ?? {}) }, - gold: mergeArrays(safeA.gold, safeB.gold), - units: { ...(safeA.units ?? {}), ...(safeB.units ?? {}) }, - }; - } + private mergePlayerStats(a: PlayerStats, b: PlayerStats): PlayerStats { + const sumTuple = (t1?: unknown[], t2?: unknown[]) => { + if (!t1 && !t2) return undefined; + if (!t1) return Array.isArray(t2) ? t2.map((x) => Number(x ?? 0)) : undefined; + if (!t2) return Array.isArray(t1) ? t1.map((x) => Number(x ?? 0)) : undefined; + const len = Math.max(t1.length, t2.length); + const out: number[] = []; + for (let i = 0; i < len; i++) { + out[i] = Number((t1 as any)[i] ?? 0) + Number((t2 as any)[i] ?? 0); + } + return out; + }; + const mergeRec = (ra?: Record<string, unknown[]>, rb?: Record<string, unknown[]>) => { + if (!ra && !rb) return undefined; + const keys = new Set<string>([...Object.keys(ra ?? {}), ...Object.keys(rb ?? {})]); + const out: Record<string, number[]> = {}; + for (const k of keys) out[k] = sumTuple(ra?.[k], rb?.[k]) ?? []; + return out; + }; + return { + attacks: sumTuple(a?.attacks, b?.attacks), + betrayals: Number(a?.betrayals ?? 0) + Number(b?.betrayals ?? 0), + boats: mergeRec(a?.boats as any, b?.boats as any), + bombs: mergeRec(a?.bombs as any, b?.bombs as any), + gold: sumTuple(a?.gold, b?.gold), + units: mergeRec(a?.units as any, b?.units as any), + }; + }
🧹 Nitpick comments (5)
src/client/components/baseComponents/PlayerStatsTable.ts (2)
36-36: Explicitly disable attribute reflection for object prop.Lit won’t serialize objects to attributes reliably. Make it explicit.
- @property({ type: Object }) stats: PlayerStats; + @property({ type: Object, attribute: false }) stats: PlayerStats;
38-56: Avoid duplicated label mapping; centralize keys-to-names in a shared util.You already have a similar map elsewhere. Extract a small typed helper (e.g., playerLabelForKey(key: UnitKey|BoatKey|BombKey)) and reuse across components to keep names consistent.
src/client/PlayerInfoModal.ts (3)
307-314: Add “%” when gamesPlayed is 0.Currently “0.0” renders without the percent sign.
- this.gamesPlayed === 0 - ? "0.0" - : ((this.wins / this.gamesPlayed) * 100).toFixed(1) + "%", + this.gamesPlayed === 0 + ? "0.0%" + : ((this.wins / this.gamesPlayed) * 100).toFixed(1) + "%",
318-320: Remove duplicate horizontal rules.There are back-to-back
elements with the same styling.- <hr class="w-2/3 border-gray-600 my-2" /> - <hr class="w-2/3 border-gray-600 my-2" />- <hr class="w-2/3 border-gray-600 my-2" /> - <hr class="w-2/3 border-gray-600 my-2" />Also applies to: 326-328
295-325: Prefer composition: keep this class lean.Good move using and . Continue extracting any remaining chunked UI (e.g., header/visibility toggle) into small components to reduce state here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/client/PlayerInfoModal.ts(1 hunks)src/client/components/baseComponents/PlayerStatsTable.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
📚 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/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-18T23:43:38.150Z
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/client/PlayerInfoModal.ts
🔇 Additional comments (1)
src/client/components/baseComponents/PlayerStatsTable.ts (1)
108-116: Boat stats ordering is correct
Verified that StatsSchemas defines boats as [sent, arrived, captured, destroyed] (indexes 0–3). In PlayerStatsTable you destructure[sent, arrived, _captured, destroyed]and render cells in the order Sent, Destroyed, Arrived—matching the<th>labels. No change needed.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/client/components/baseComponents/stats/PlayerStatsTree.ts (1)
1-9: Use DifficultyDescription mapping + i18n keys ("Relaxed", "Balanced", ...).
Follow existing convention from Difficulties.ts; avoid showing enum literals.import "./PlayerStatsGrid"; import "./PlayerStatsTable"; import { Difficulty, GameMode, GameType } from "../../../../core/game/Game"; import { LitElement, html } from "lit"; import { PlayerStatsLeaf, PlayerStatsTree } from "../../../../core/ApiSchemas"; import { customElement, property, state } from "lit/decorators.js"; import { renderNumber, translateText } from "../../../Utils"; import { PlayerStats } from "../../../../core/StatsSchemas"; +import { DifficultyDescription } from "../../Difficulties";- ${d} + ${translateText(`difficulty.${DifficultyDescription[d]}`)}Also applies to: 154-171
🧹 Nitpick comments (3)
src/client/components/baseComponents/stats/PlayerStatsTree.ts (3)
12-12: Make statsTree accept null (aligns with caller and avoids type mismatch).
PlayerInfoModal passes null; this prop is typed as optional but not null. Make it explicit.- @property({ type: Object }) statsTree?: PlayerStatsTree; + @property({ type: Object }) statsTree: PlayerStatsTree | null = null;
61-73: Drop manual requestUpdate() in setters; Lit reactivity already updates.
Removes extra work and avoids redundant renders.private setGameType(t: GameType) { if (this.selectedType === t) return; this.selectedType = t; const modes = this.availableModes; if (!modes.includes(this.selectedMode)) { this.selectedMode = modes[0] ?? this.selectedMode; } const diffs = this.availableDifficulties; if (!diffs.includes(this.selectedDifficulty)) { this.selectedDifficulty = diffs[0] ?? this.selectedDifficulty; } - this.requestUpdate(); } private setMode(m: GameMode) { if (this.selectedMode === m) return; this.selectedMode = m; const diffs = this.availableDifficulties; if (!diffs.includes(this.selectedDifficulty)) { this.selectedDifficulty = diffs[0] ?? this.selectedDifficulty; } - this.requestUpdate(); } private setDifficulty(d: Difficulty) { if (this.selectedDifficulty === d) return; this.selectedDifficulty = d; - this.requestUpdate(); }Also applies to: 75-83, 85-89
196-200: Avoid passing null to .stats (typed as non-null).
Guard the render or widen the prop type in PlayerStatsTable.- <player-stats-table - .stats=${this.getDisplayedStats()} - ></player-stats-table> + ${this.getDisplayedStats() + ? html`<player-stats-table .stats=${this.getDisplayedStats()!}></player-stats-table>` + : html``}
📜 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 (2)
resources/lang/en.json(1 hunks)src/client/components/baseComponents/stats/PlayerStatsTree.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- resources/lang/en.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T21:42:45.277Z
Learnt from: plazmaezio
PR: openfrontio/OpenFrontIO#1883
File: src/client/HostLobbyModal.ts:227-245
Timestamp: 2025-08-21T21:42:45.277Z
Learning: In openfrontio/OpenFrontIO, the en.json file contains difficulty translation keys using the descriptive names ("Relaxed", "Balanced", "Intense", "Impossible") rather than the enum values ("Easy", "Medium", "Hard", "Impossible"). The DifficultyDescription mapping in src/client/components/Difficulties.ts correctly maps enum values to these descriptive translation keys.
Applied to files:
src/client/components/baseComponents/stats/PlayerStatsTree.ts
🧬 Code graph analysis (1)
src/client/components/baseComponents/stats/PlayerStatsTree.ts (5)
src/client/PlayerInfoModal.ts (1)
customElement(16-143)src/client/components/baseComponents/stats/PlayerStatsTable.ts (1)
customElement(6-194)src/core/ApiSchemas.ts (2)
PlayerStatsTree(73-73)PlayerStatsLeaf(61-61)src/core/StatsSchemas.ts (1)
PlayerStats(108-108)src/client/Utils.ts (1)
renderNumber(9-33)
🔇 Additional comments (1)
src/client/components/baseComponents/stats/PlayerStatsTree.ts (1)
189-191: Good: “not_applicable” translations wired for missing time fields.
This matches the project’s translation pattern.
Aotumuri
commented
Aug 31, 2025
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/client/components/baseComponents/stats/PlayerStatsGrid.ts (5)
37-38: Tailwind class won’t apply inside this component’s shadow DOM. Move margin into local CSS (or render in light DOM).Because Lit uses shadow DOM by default here,
.mb-2from global utilities will not style the internal<div>. Easiest fix: drop the class and style the host.Apply:
@@ - <div class="grid mb-2"> + <div class="grid">And in
static styles:static styles = css` + :host { + display: block; + margin-bottom: 0.5rem; /* replaces tailwind mb-2 */ + }
12-16: Redundant media rule; set 1-col base, 2-col at ≥640px.Current CSS defines 2 columns for both cases. This makes the breakpoint a no-op. Suggest:
@@ - .grid { - display: grid; - grid-template-columns: repeat(2, 1fr); - gap: 1rem; - } + .grid { + display: grid; + grid-template-columns: 1fr; + gap: 1rem; + } @@ @media (min-width: 640px) { .grid { - grid-template-columns: repeat(2, 1fr); + grid-template-columns: repeat(2, 1fr); } }
18-25: Avoid hard-coded colors; inherit or use theme tokens.White text and #bbb can clash with themes. Let text inherit and soften the title via opacity (or project tokens if available).
@@ - .stat { - text-align: center; - color: white; - font-size: 1rem; - } + .stat { + text-align: center; + color: inherit; + font-size: 1rem; + } @@ - .stat-title { - color: #bbb; - font-size: 0.9rem; - } + .stat-title { + opacity: 0.75; /* or var(--text-muted) if you have one */ + font-size: 0.9rem; + }
38-47: Prefer a simple indices array overArray(4).fill(0)for readability.Both work; indices are a bit clearer and avoid a fill allocation.
- ${Array(4) - .fill(0) - .map( - (_, i) => html` + ${[0, 1, 2, 3].map( + (i) => html` <div class="stat"> <div class="stat-value">${this.values[i] ?? ""}</div> <div class="stat-title">${this.titles[i] ?? ""}</div> </div> - `, - )} + `)}
35-45: Minor a11y: mark the group and values for assistive tech.If these numbers update live, this helps screen readers without being chatty.
- <div class="grid"> + <div class="grid" role="group" aria-describedby="stats-desc"> ${[0, 1, 2, 3].map( (i) => html` <div class="stat"> - <div class="stat-value">${this.values[i] ?? ""}</div> + <div class="stat-value" aria-live="polite">${this.values[i] ?? ""}</div> <div class="stat-title">${this.titles[i] ?? ""}</div> </div> `, )} </div> + <span id="stats-desc" class="sr-only">Key player statistics</span>If you avoid extra text nodes, you can hide the description with your project’s “sr-only” utility or replicate it in component CSS.
📜 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 (2)
src/client/components/baseComponents/stats/PlayerStatsGrid.ts(1 hunks)src/client/components/baseComponents/stats/PlayerStatsTree.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/client/components/baseComponents/stats/PlayerStatsTree.ts
evanpelle
dismissed
coderabbitai[bot]’s stale review
The merge-base changed after approval.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/client/components/baseComponents/stats/PlaterStatsTree.ts (3)
55-59: Prefer undefined over null for PlayerStats (aligns with schema optional)Project convention:
PlayerStatsalready includes undefined. Return/propagateundefined.- private getDisplayedStats(): PlayerStats | null { + private getDisplayedStats(): PlayerStats | undefined { const leaf = this.getSelectedLeaf(); - if (!leaf || !leaf.stats) return null; + if (!leaf || !leaf.stats) return undefined; return leaf.stats; } @@ - <player-stats-table - .stats=${this.getDisplayedStats()} - ></player-stats-table> + <player-stats-table .stats=${this.getDisplayedStats()}></player-stats-table>Also applies to: 188-190
106-110: Match leaderboard’s win:loss ratio logicUse the repo-standard formula: wins=0 → 0; else if losses=0 → wins; else wins/losses.
- const wlr = leaf - ? leaf.losses === 0n - ? Number(leaf.wins) - : Number(leaf.wins) / Number(leaf.losses) - : 0; + const wlr = leaf + ? leaf.wins === 0n + ? 0 + : leaf.losses === 0n + ? Number(leaf.wins) + : Number(leaf.wins) / Number(leaf.losses) + : 0;
61-73: Redundant requestUpdate callsSetting @State fields already schedules an update in Lit. Safe to drop the extra
requestUpdate()calls.Also applies to: 75-83, 85-89
src/client/PlayerInfoModal.ts (2)
69-72: ESLint max-len: split inline style onto multiple linesThis line likely trips the 120-char rule. Break it up (or move to a CSS class).
- style="background: rgba(202,138,4,0.15); border-color: rgba(253,224,71,0.6); color: rgb(253,224,71);" + style=" + background: rgba(202,138,4,0.15); + border-color: rgba(253,224,71,0.6); + color: rgb(253,224,71); + "
26-28: MakestatsTreeandrecentGamesreactiveMark as
@state()so UI updates automatically on changes without needing manualrequestUpdate().- private statsTree: PlayerStatsTree | null = null; - private recentGames: PlayerGame[] = []; + @state() private statsTree: PlayerStatsTree | null = null; + @state() private recentGames: PlayerGame[] = [];
📜 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 (14)
resources/lang/en.json(1 hunks)src/client/LangSelector.ts(1 hunks)src/client/Main.ts(3 hunks)src/client/PlayerInfoModal.ts(1 hunks)src/client/components/baseComponents/stats/DiscordUserHeader.ts(1 hunks)src/client/components/baseComponents/stats/GameList.ts(1 hunks)src/client/components/baseComponents/stats/PlaterStatsTree.ts(1 hunks)src/client/components/baseComponents/stats/PlayerStatsGrid.ts(1 hunks)src/client/components/baseComponents/stats/PlayerStatsTable.ts(1 hunks)src/client/index.html(2 hunks)src/client/jwt.ts(2 hunks)src/core/ApiSchemas.ts(3 hunks)src/core/StatsSchemas.ts(4 hunks)src/core/game/Game.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- src/client/LangSelector.ts
- src/core/game/Game.ts
- src/client/index.html
- src/client/components/baseComponents/stats/GameList.ts
- src/client/jwt.ts
- src/client/components/baseComponents/stats/PlayerStatsTable.ts
- src/client/components/baseComponents/stats/PlayerStatsGrid.ts
- src/client/components/baseComponents/stats/DiscordUserHeader.ts
- src/core/StatsSchemas.ts
- src/client/Main.ts
- resources/lang/en.json
- src/core/ApiSchemas.ts
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
📚 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/PlayerInfoModal.ts
📚 Learning: 2025-08-29T16:16:11.309Z
Learnt from: BrewedCoffee
PR: openfrontio/OpenFrontIO#1957
File: src/core/execution/PlayerExecution.ts:40-52
Timestamp: 2025-08-29T16:16:11.309Z
Learning: In OpenFrontIO PlayerExecution.ts, when Defense Posts are captured due to tile ownership changes, the intended behavior is to first call u.decreaseLevel() to downgrade them, then still transfer them to the capturing player via captureUnit(). This is not a bug - Defense Posts should be both downgraded and captured, not destroyed outright.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.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/client/PlayerInfoModal.ts
📚 Learning: 2025-05-21T04:10:33.435Z
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/client/PlayerInfoModal.ts
📚 Learning: 2025-07-31T12:03:08.052Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1655
File: resources/maps/giantworldmap/manifest.json:165-174
Timestamp: 2025-07-31T12:03:08.052Z
Learning: In OpenFrontIO draft PRs, flag names in map manifest files may be placeholders that will be updated to match the actual SVG flag file names when the assets are received. The final naming depends on the actual flag SVG files provided, not code naming conventions.
Applied to files:
src/client/PlayerInfoModal.ts
🧬 Code graph analysis (1)
src/client/components/baseComponents/stats/PlaterStatsTree.ts (5)
src/client/components/baseComponents/stats/PlayerStatsGrid.ts (1)
customElement(4-51)src/client/components/baseComponents/stats/PlayerStatsTable.ts (1)
customElement(11-196)src/core/ApiSchemas.ts (2)
PlayerStatsTree(73-73)PlayerStatsLeaf(64-64)src/client/Utils.ts (2)
translateText(82-137)renderNumber(9-33)src/core/StatsSchemas.ts (1)
PlayerStats(108-108)
🪛 GitHub Actions: 🧪 CI
src/client/PlayerInfoModal.ts
[error] 12-12: Module not found: Can't resolve './components/baseComponents/stats/PlayerStatsTree' in '/home/runner/work/OpenFrontIO/OpenFrontIO/src/client'
🔇 Additional comments (1)
src/client/PlayerInfoModal.ts (1)
93-101: Modal open/close helpers look goodSimple, readable, and consistent with other components.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/client/components/baseComponents/stats/PlayerStatsTree.ts (2)
91-104: Stop mutating reactive state in render(); clamp in willUpdate and drop manual requestUpdate()Writing to reactive fields inside render() causes extra update cycles. Normalize selections in willUpdate() and simplify setters.
Apply:
@@ createRenderRoot() { return this; } + + protected willUpdate(changed: Map<string, unknown>) { + if (changed.has("statsTree") || changed.has("selectedType") || changed.has("selectedMode")) { + const types = this.availableTypes; + if (types.length && !types.includes(this.selectedType)) this.selectedType = types[0]; + const modes = this.availableModes; + if (modes.length && !modes.includes(this.selectedMode)) this.selectedMode = modes[0]; + const diffs = this.availableDifficulties; + if (diffs.length && !diffs.includes(this.selectedDifficulty)) this.selectedDifficulty = diffs[0]; + } + } @@ - private setGameType(t: GameType) { - if (this.selectedType === t) return; - this.selectedType = t; - const modes = this.availableModes; - if (!modes.includes(this.selectedMode)) { - this.selectedMode = modes[0] ?? this.selectedMode; - } - const diffs = this.availableDifficulties; - if (!diffs.includes(this.selectedDifficulty)) { - this.selectedDifficulty = diffs[0] ?? this.selectedDifficulty; - } - this.requestUpdate(); - } + private setGameType(t: GameType) { + if (this.selectedType !== t) this.selectedType = t; + } @@ - private setMode(m: GameMode) { - if (this.selectedMode === m) return; - this.selectedMode = m; - const diffs = this.availableDifficulties; - if (!diffs.includes(this.selectedDifficulty)) { - this.selectedDifficulty = diffs[0] ?? this.selectedDifficulty; - } - this.requestUpdate(); - } + private setMode(m: GameMode) { + if (this.selectedMode !== m) this.selectedMode = m; + } @@ - private setDifficulty(d: Difficulty) { - if (this.selectedDifficulty === d) return; - this.selectedDifficulty = d; - this.requestUpdate(); - } + private setDifficulty(d: Difficulty) { + if (this.selectedDifficulty !== d) this.selectedDifficulty = d; + } @@ render() { - const types = this.availableTypes; - if (types.length && !types.includes(this.selectedType)) { - this.selectedType = types[0]; - } - const modes = this.availableModes; - if (modes.length && !modes.includes(this.selectedMode)) { - this.selectedMode = modes[0]; - } - const diffs = this.availableDifficulties; - if (diffs.length && !diffs.includes(this.selectedDifficulty)) { - this.selectedDifficulty = diffs[0]; - } + const types = this.availableTypes; + const modes = this.availableModes; + const diffs = this.availableDifficulties;Also applies to: 61-73, 75-83, 85-89, 41-44
105-111: Fix WLR display: keep wins-as-WLR when losses = 0; show ratio with 2 decimals only when losses > 0Right now wlr.toFixed(2) formats wins (e.g., “123.00”) when losses = 0. Show wins as-is; decimals only for ratios.
@@ - const leaf = this.getSelectedLeaf(); - const wlr = leaf - ? leaf.losses === 0n - ? Number(leaf.wins) - : Number(leaf.wins) / Number(leaf.losses) - : 0; + const leaf = this.getSelectedLeaf(); + const wlrDisplay = + leaf + ? leaf.losses === 0n + ? renderNumber(leaf.wins) + : (Number(leaf.wins) / Number(leaf.losses)).toFixed(2) + : "0"; @@ - renderNumber(leaf.wins), - renderNumber(leaf.losses), - wlr.toFixed(2), - renderNumber(leaf.total), + renderNumber(leaf.wins), + renderNumber(leaf.losses), + wlrDisplay, + renderNumber(leaf.total),Also applies to: 181-185
🧹 Nitpick comments (2)
src/client/components/baseComponents/stats/PlayerStatsTree.ts (2)
12-12: Don’t reflect complex objects to attributesPrefer attribute: false for object props to avoid useless attribute serialization and warnings in dev tools.
- @property({ type: Object }) statsTree?: PlayerStatsTree; + @property({ attribute: false }) statsTree?: PlayerStatsTree;
17-20: Optional: stable ordering for selectorsObject.keys order from API may vary. Consider deterministic display order (typed union arrays), e.g., types = [Public, Private, Singleplayer]. Same idea for modes/difficulties.
Example:
const typeOrder: readonly GameType[] = [GameType.Public, GameType.Private, GameType.Singleplayer]; private get availableTypes(): GameType[] { return typeOrder.filter(t => !!this.statsTree?.[t]); }Also applies to: 22-26, 28-33
📜 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/client/components/baseComponents/stats/PlayerStatsTree.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T21:42:45.277Z
Learnt from: plazmaezio
PR: openfrontio/OpenFrontIO#1883
File: src/client/HostLobbyModal.ts:227-245
Timestamp: 2025-08-21T21:42:45.277Z
Learning: In openfrontio/OpenFrontIO, the en.json file contains difficulty translation keys using the descriptive names ("Relaxed", "Balanced", "Intense", "Impossible") rather than the enum values ("Easy", "Medium", "Hard", "Impossible"). The DifficultyDescription mapping in src/client/components/Difficulties.ts correctly maps enum values to these descriptive translation keys.
Applied to files:
src/client/components/baseComponents/stats/PlayerStatsTree.ts
🧬 Code graph analysis (1)
src/client/components/baseComponents/stats/PlayerStatsTree.ts (3)
src/core/ApiSchemas.ts (2)
PlayerStatsTree(73-73)PlayerStatsLeaf(64-64)src/core/StatsSchemas.ts (1)
PlayerStats(108-108)src/client/Utils.ts (1)
renderNumber(9-33)
🔇 Additional comments (1)
src/client/components/baseComponents/stats/PlayerStatsTree.ts (1)
41-43: Confirm light DOM choiceRendering in light DOM is fine; just confirm it matches the rest of stats components (styling and encapsulation expectations).
preview
|
evanpelle
reviewed
Sep 4, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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:
Added PlayerInfoModal
For ease of maintenance, we have separated them into individual schemas(modal).
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
aotumuri