Color blind setting #2729
Color blind setting #2729
Conversation
WalkthroughAdds a color‑blind accessibility toggle and translations, a new teammate glow renderer, and integrates a pulsing teammate glow into spawn highlighting and focused-player visuals for team games; also exposes spawnTile on player updates and views. Changes
Sequence DiagramsequenceDiagram
actor User
participant SettingsModal as Settings Modal
participant UserSettings as User Settings
participant TerritoryLayer as Territory Layer
participant Canvas as Canvas Renderer
User->>SettingsModal: Toggle "Color‑blind" setting
SettingsModal->>UserSettings: toggleColorBlind()
UserSettings-->>SettingsModal: state updated
SettingsModal->>TerritoryLayer: request redraw / UI refresh
rect rgba(120,180,240,0.12)
Note over TerritoryLayer: Spawn highlight render pass
TerritoryLayer->>UserSettings: colorBlind()?
alt Color‑blind team mode ON
TerritoryLayer->>TerritoryLayer: compute spawn center from owned tiles
TerritoryLayer->>TerritoryLayer: compute pulse phase (time * PULSE_SPEED)
TerritoryLayer->>Canvas: drawTeammateGlow(ctx, x, y, {outerRadius, pulsePhase})
Canvas-->>TerritoryLayer: glow drawn
else Normal mode
TerritoryLayer->>Canvas: draw standard spawn highlight
end
end
Canvas-->>User: Updated spawn visuals visible
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/graphics/layers/TerritoryLayer.ts (1)
1-1: Fix Prettier formatting.The CI pipeline reports formatting issues. Run
npx prettier --write src/client/graphics/layers/TerritoryLayer.tsto fix.
♻️ Duplicate comments (5)
resources/lang/pl.json (1)
315-316: Remove non-English locale updates from this PR.Same issue as in other locale files - non-English translations should be added in dedicated translation PRs only. Please remove these changes.
Based on learnings.
resources/lang/fr.json (1)
335-336: Remove non-English locale updates from this PR.Same issue - please remove French translation updates and keep only
en.jsonchanges.Based on learnings.
resources/lang/ko.json (1)
262-263: Remove non-English locale updates from this PR.Korean translations should also be removed and added only in dedicated translation PRs.
Based on learnings.
resources/lang/hi.json (1)
188-189: Remove non-English locale updates from this PR.Hindi translations should be removed from this PR per project translation workflow.
Based on learnings.
resources/lang/bg.json (1)
335-336: Remove non-English locale updates from this PR.Bulgarian translations should also be removed. Only
en.jsonshould be updated with new keys in this PR.Based on learnings.
🧹 Nitpick comments (2)
src/client/graphics/TeammateGlow.ts (1)
14-18: Redundant validation check.Line 14 uses
Math.max(1, options.outerRadius)which guaranteesouterRadius >= 1. The check at line 18 forouterRadius <= 0can never be true after that. Consider removing the redundant part.Proposed fix
export function drawTeammateGlow( ctx: CanvasRenderingContext2D, x: number, y: number, options: TeammateGlowOptions, ): void { - const outerRadius = Math.max(1, options.outerRadius); const phase = options.pulsePhase ?? 0; if (!Number.isFinite(x) || !Number.isFinite(y)) return; - if (!Number.isFinite(outerRadius) || outerRadius <= 0) return; + if (!Number.isFinite(options.outerRadius) || options.outerRadius <= 0) return; + + const outerRadius = Math.max(1, options.outerRadius);src/client/graphics/layers/TerritoryLayer.ts (1)
191-193: Consider extracting the color-blind check to avoid duplication.The condition
this.userSettings.colorBlind() && this.game.config().gameConfig().gameMode === GameMode.Teamappears both inspawnHighlight(lines 191-193) anddrawFocusedPlayerHighlight(lines 285-286). A small helper or property could reduce repetition.Proposed helper
+ private isColorBlindTeamMode(): boolean { + return ( + this.userSettings.colorBlind() && + this.game.config().gameConfig().gameMode === GameMode.Team + ); + } + private spawnHighlight() { // ... const myPlayer = this.game.myPlayer(); - const colorBlindEnabled = - this.userSettings.colorBlind() && - this.game.config().gameConfig().gameMode === GameMode.Team; + const colorBlindEnabled = this.isColorBlindTeamMode(); // ... }Also applies to: 283-289
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
resources/lang/ar.jsonresources/lang/bg.jsonresources/lang/bn.jsonresources/lang/cs.jsonresources/lang/da.jsonresources/lang/de.jsonresources/lang/el.jsonresources/lang/en.jsonresources/lang/eo.jsonresources/lang/es.jsonresources/lang/fa.jsonresources/lang/fi.jsonresources/lang/fr.jsonresources/lang/gl.jsonresources/lang/he.jsonresources/lang/hi.jsonresources/lang/hu.jsonresources/lang/it.jsonresources/lang/ja.jsonresources/lang/ko.jsonresources/lang/mk.jsonresources/lang/nl.jsonresources/lang/pl.jsonresources/lang/pt-PT.jsonresources/lang/ru.jsonresources/lang/sh.jsonresources/lang/sk.jsonresources/lang/sl.jsonresources/lang/sv-SE.jsonresources/lang/tp.jsonresources/lang/tr.jsonresources/lang/uk.jsonresources/lang/zh-CN.jsonsrc/client/UserSettingModal.tssrc/client/graphics/TeammateGlow.tssrc/client/graphics/layers/SettingsModal.tssrc/client/graphics/layers/TerritoryLayer.tssrc/core/game/UserSettings.ts
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/Colors.ts:114-330
Timestamp: 2025-06-06T15:36:31.739Z
Learning: In the OpenFrontIO project, color improvements are implemented incrementally. The current focus for player colors is ensuring sufficient unique color availability rather than optimizing for visual distinction or accessibility features.
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 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:
resources/lang/ru.jsonresources/lang/fa.jsonresources/lang/pl.jsonresources/lang/cs.jsonresources/lang/eo.jsonresources/lang/fi.jsonresources/lang/bg.jsonresources/lang/uk.jsonresources/lang/sk.jsonresources/lang/hi.jsonresources/lang/mk.jsonresources/lang/ko.jsonresources/lang/sh.jsonresources/lang/es.jsonresources/lang/pt-PT.jsonresources/lang/tp.json
📚 Learning: 2025-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.
Applied to files:
resources/lang/en.jsonresources/lang/pl.jsonresources/lang/sh.jsonresources/lang/ja.json
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 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/core/game/UserSettings.tssrc/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
Repo: openfrontio/OpenFrontIO PR: 2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/client/graphics/TeammateGlow.tssrc/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-06-22T21:51:14.990Z
Learnt from: devalnor
Repo: openfrontio/OpenFrontIO PR: 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/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-10-18T11:00:57.142Z
Learnt from: NewYearNewPhil
Repo: openfrontio/OpenFrontIO PR: 2230
File: src/client/graphics/GameRenderer.ts:269-277
Timestamp: 2025-10-18T11:00:57.142Z
Learning: In src/client/graphics/GameRenderer.ts, the GameRecapCapture implementation does not use setCaptureRenderEnabled on layers. Instead, it uses RecapCaptureSurface.capture() to render capture layers (TerrainLayer, TerritoryLayer, RailroadLayer, StructureIconsLayer, UnitLayer) directly to an off-screen canvas without requiring layer-level capture mode methods.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-06-22T05:48:19.241Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 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/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-06-06T21:43:15.706Z
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/Colors.ts:398-411
Timestamp: 2025-06-06T21:43:15.706Z
Learning: In ColorAllocator class, the assignPlayerColor method intentionally allows color duplicates when the humanColors palette is exhausted by resetting availableColors to the full humanColors array. This is done on purpose as an acceptable temporary limitation, with potential palette improvements planned for later.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-06-06T15:36:31.739Z
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/Colors.ts:114-330
Timestamp: 2025-06-06T15:36:31.739Z
Learning: In the OpenFrontIO project, color improvements are implemented incrementally. The current focus for player colors is ensuring sufficient unique color availability rather than optimizing for visual distinction or accessibility features.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-12-26T22:21:15.466Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2689
File: src/client/PublicLobby.ts:245-245
Timestamp: 2025-12-26T22:21:15.466Z
Learning: In public lobbies with HumansVsNations mode in src/client/PublicLobby.ts, maxPlayers represents only human player slots (already halved in DefaultConfig.ts). The nation NPCs are added automatically server-side and don't count toward maxPlayers. Therefore, getTeamSize correctly returns maxPlayers directly for HumansVsNations to display the proper team size (e.g., maxPlayers=5 yields "5 Humans vs 5 Nations").
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-11-01T00:24:33.860Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2161
File: src/core/execution/FakeHumanExecution.ts:670-678
Timestamp: 2025-11-01T00:24:33.860Z
Learning: In OpenFrontIO, PlayerType.Bot entities cannot be in teams and do not have friendliness relationships. Only PlayerType.Human and PlayerType.FakeHuman can participate in teams and alliances. Therefore, when targeting bot-owned tiles, friendliness checks like `owner.isFriendly(this.player)` are unnecessary and meaningless for bots.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-12-13T14:58:29.645Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2607
File: src/core/execution/PlayerExecution.ts:271-295
Timestamp: 2025-12-13T14:58:29.645Z
Learning: In src/core/execution/PlayerExecution.ts surroundedBySamePlayer(), the `as Player` cast on `mg.playerBySmallID(scan.enemyId)` is intentional. Since scan.enemyId comes from ownerID() on an owned tile and playerBySmallID() only returns Player or undefined, the cast expresses a known invariant. The maintainers prefer loud failures (runtime errors) over silent masking (early returns with guards) for corrupted game state scenarios at trusted call sites.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
🧬 Code graph analysis (3)
src/client/graphics/layers/SettingsModal.ts (1)
src/client/LangSelector.ts (1)
translateText(258-278)
src/client/UserSettingModal.ts (1)
src/client/LangSelector.ts (1)
translateText(258-278)
src/client/graphics/layers/TerritoryLayer.ts (4)
src/client/graphics/layers/Layer.ts (1)
Layer(1-7)src/core/game/GameView.ts (4)
myPlayer(728-730)tile(110-112)focusedPlayer(914-916)PlayerView(185-582)src/core/game/GameMap.ts (2)
euclDistFN(371-392)TileRef(3-3)src/client/graphics/TeammateGlow.ts (1)
drawTeammateGlow(8-50)
🪛 GitHub Actions: 🧪 CI
src/client/graphics/TeammateGlow.ts
[warning] 1-1: Prettier formatting issues found in file. Run 'npx prettier --write' to fix.
src/client/graphics/layers/TerritoryLayer.ts
[warning] 1-1: Prettier formatting issues found in file. Run 'npx prettier --write' to fix.
🔇 Additional comments (33)
resources/lang/tr.json (1)
283-284: Localization entries look good.The new
color_blind_labelandcolor_blind_desckeys are correctly placed within theuser_settingobject. The Turkish translations align with the feature's purpose of helping identify teammates during spawn phase.resources/lang/zh-CN.json (1)
335-336: Localization entries look good.The Simplified Chinese translations for
color_blind_labelandcolor_blind_descare correctly added. The wording aligns with the accessibility feature's purpose.resources/lang/sl.json (1)
286-287: Localization entries look good.The Slovenian translations for the color-blind accessibility feature are correctly added under
user_setting. The structure matches the pattern used for other settings.resources/lang/he.json (1)
258-259: Localization entries look good.The Hebrew translations for
color_blind_labelandcolor_blind_descare correctly added. The structure is consistent with other user settings in this file.resources/lang/bn.json (1)
188-189: Localization entries look good.The Bengali translations for the color-blind accessibility feature are correctly added under
user_setting. The key naming and placement follow the existing patterns.resources/lang/es.json (1)
227-228: Localization entries look good.The Spanish translations for
color_blind_labelandcolor_blind_descare correctly added. The term "daltonismo" appropriately describes color blindness in Spanish. Based on learnings, community translators handle translation accuracy separately from technical changes.resources/lang/nl.json (1)
335-336: Localization entries look good.The Dutch translations for
color_blind_labelandcolor_blind_descare correctly added. The wording "Kleurenblind hulp" and the description about "spawn-fase" clearly convey the feature's purpose.resources/lang/da.json (1)
269-270: Localization entries look good.The Danish translations for
color_blind_labelandcolor_blind_descare correctly added. The compound word "Farveblindhjælp" follows Danish naming conventions, and the description clearly explains the feature.resources/lang/tp.json (1)
213-214: LGTM - Color-blind accessibility localization added correctly.The new localization keys are properly structured and consistently placed within the
user_settingsection, following the same pattern as other settings in this file.resources/lang/gl.json (1)
269-270: LGTM - Localization keys properly added.The color-blind accessibility strings are correctly integrated into the user settings section with proper JSON structure.
resources/lang/fi.json (1)
283-284: LGTM - Color-blind setting localization added.The additions follow the established pattern and are properly structured within the user settings section.
resources/lang/eo.json (1)
286-287: LGTM - Accessibility localization properly implemented.The new keys are correctly placed and follow the same structure as other user setting entries in this file.
resources/lang/ar.json (1)
213-214: LGTM - Color-blind setting keys added correctly.The localization strings are properly structured and positioned within the user settings section.
resources/lang/uk.json (1)
335-336: LGTM - Localization additions are properly structured.The new color-blind accessibility keys are correctly integrated into the user settings section following the established pattern.
resources/lang/sk.json (1)
269-270: LGTM - Color-blind accessibility keys added correctly.The localization strings are properly positioned and follow the same structure as other user settings in this file.
resources/lang/pt-PT.json (1)
269-270: LGTM - Final locale file properly updated.The color-blind accessibility localization keys are correctly added and follow the established pattern. All 8 language files in this review show consistent, well-structured additions that properly support the new Color Blind Assist feature.
resources/lang/en.json (1)
374-375: LGTM! English locale keys added correctly.The new localization keys follow the existing naming conventions and provide clear, concise text for the color-blind assist feature. The label and description are appropriate and user-friendly.
src/client/graphics/layers/SettingsModal.ts (2)
146-149: LGTM! Handler follows established pattern.The
onToggleColorBlindButtonClickhandler correctly toggles the color-blind setting and refreshes the component. Implementation is consistent with other toggle handlers in this file.
341-364: LGTM! UI button properly implemented.The color-blind toggle button follows the same structure and pattern as other settings buttons. The translation keys are correctly used, and the on/off state is properly displayed. The placement between dark mode and special effects settings is logical.
src/core/game/UserSettings.ts (2)
76-78: LGTM! Clean implementation following existing patterns.The
colorBlind()getter follows the same pattern as other boolean settings in this class, usinglocalStoragewith a sensible default offalse.
135-137: LGTM! Toggle method is consistent with other settings.The
toggleColorBlind()method follows the established pattern used by other toggle methods in this class.src/client/UserSettingModal.ts (2)
110-116: LGTM! Clean handler following established patterns.The
toggleColorBlindhandler is well-implemented with type checking and follows the same pattern as other simple toggle handlers liketoggleEmojis.
293-300: LGTM! UI toggle properly wired and positioned.The color-blind toggle is correctly:
- Using
translateText()for localized label and description- Bound to the current
colorBlind()state- Connected to the toggle handler
- Positioned logically in the settings list
resources/lang/hu.json (1)
286-287: Localization keys added correctly.The new
color_blind_labelandcolor_blind_desckeys follow the existing pattern used by other settings in this file. Placement is consistent.resources/lang/de.json (1)
258-259: Localization keys added correctly.The German translations for
color_blind_labelandcolor_blind_descare consistent with the existing pattern. Placement looks good.resources/lang/cs.json (1)
227-228: Localization keys added correctly.The Czech translations for the color-blind accessibility feature are properly placed and follow the existing pattern.
resources/lang/it.json (1)
227-228: Localization keys added correctly.The Italian translations for the color-blind accessibility feature are properly placed and follow the existing pattern.
src/client/graphics/TeammateGlow.ts (1)
20-49: Clean rendering logic.The pulsing effect between white and gold layers is well implemented. Good use of
ctx.save()/ctx.restore()to isolate canvas state changes.src/client/graphics/layers/TerritoryLayer.ts (5)
29-30: Good use of named constants.Extracting
SPAWN_HIGHLIGHT_RADIUSandPULSE_SPEEDas static constants improves readability and makes future adjustments easier.
298-348: Well-structured helper method.
spawnCenterFromOwnedTilescleanly computes the center position from owned tiles with proper null handling for empty input. The inner radius clamping between 2 andSPAWN_HIGHLIGHT_RADIUS - 1is sensible.
350-379: Efficient tile accumulation.The method collects owned tiles and sums coordinates in a single pass through the BFS results. Good use of composition with
spawnCenterFromOwnedTilesanddrawTeammateGlow.
225-241: Correct teammate glow integration.The conditions properly exclude the player themselves and only show glow for actual teammates. Using
isOnSameTeamensures proper team detection.
282-296: Consistent integration for focused player.The teammate glow logic for the focused player matches the pattern used in
spawnHighlight. The conditional guard ensures glow only appears for teammates in Team mode with color-blind setting enabled.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/client/graphics/layers/TerritoryLayer.ts (1)
298-377: LGTM: Solid geometric calculations.The helper methods correctly compute spawn centers from owned tiles using centroid calculations and distance measurements. Null handling is appropriate, and the pulse animation timing is well-implemented.
Optional: Extract magic number constants
Lines 335 and 373 contain magic numbers for minimum radii:
private static readonly SPAWN_HIGHLIGHT_RADIUS = 9; private static readonly PULSE_SPEED = 0.2; + private static readonly MIN_INNER_RADIUS = 2; + private static readonly MIN_OUTER_RADIUS = 3; private spawnCenterFromOwnedTiles(...) { ... const innerRadius = Math.max( - 2, + TerritoryLayer.MIN_INNER_RADIUS, Math.min(TerritoryLayer.SPAWN_HIGHLIGHT_RADIUS - 1, Math.sqrt(maxDistSq)), ); ... } private drawTeammateGlowForPlayer(...) { ... if (spawnCenter) { drawTeammateGlow(this.highlightContext, spawnCenter.x, spawnCenter.y, { - outerRadius: Math.max(3, spawnCenter.innerRadius), + outerRadius: Math.max(TerritoryLayer.MIN_OUTER_RADIUS, spawnCenter.innerRadius), pulsePhase: this.game.ticks() * TerritoryLayer.PULSE_SPEED, }); } }This is a minor improvement and can be deferred.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/client/graphics/TeammateGlow.tssrc/client/graphics/layers/TerritoryLayer.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/client/graphics/TeammateGlow.ts
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/Colors.ts:114-330
Timestamp: 2025-06-06T15:36:31.739Z
Learning: In the OpenFrontIO project, color improvements are implemented incrementally. The current focus for player colors is ensuring sufficient unique color availability rather than optimizing for visual distinction or accessibility features.
📚 Learning: 2025-06-22T21:51:14.990Z
Learnt from: devalnor
Repo: openfrontio/OpenFrontIO PR: 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/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-10-18T11:00:57.142Z
Learnt from: NewYearNewPhil
Repo: openfrontio/OpenFrontIO PR: 2230
File: src/client/graphics/GameRenderer.ts:269-277
Timestamp: 2025-10-18T11:00:57.142Z
Learning: In src/client/graphics/GameRenderer.ts, the GameRecapCapture implementation does not use setCaptureRenderEnabled on layers. Instead, it uses RecapCaptureSurface.capture() to render capture layers (TerrainLayer, TerritoryLayer, RailroadLayer, StructureIconsLayer, UnitLayer) directly to an off-screen canvas without requiring layer-level capture mode methods.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 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/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
Repo: openfrontio/OpenFrontIO PR: 2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-06-06T21:43:15.706Z
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/Colors.ts:398-411
Timestamp: 2025-06-06T21:43:15.706Z
Learning: In ColorAllocator class, the assignPlayerColor method intentionally allows color duplicates when the humanColors palette is exhausted by resetting availableColors to the full humanColors array. This is done on purpose as an acceptable temporary limitation, with potential palette improvements planned for later.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-06-06T15:36:31.739Z
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/Colors.ts:114-330
Timestamp: 2025-06-06T15:36:31.739Z
Learning: In the OpenFrontIO project, color improvements are implemented incrementally. The current focus for player colors is ensuring sufficient unique color availability rather than optimizing for visual distinction or accessibility features.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-12-26T22:21:15.466Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2689
File: src/client/PublicLobby.ts:245-245
Timestamp: 2025-12-26T22:21:15.466Z
Learning: In public lobbies with HumansVsNations mode in src/client/PublicLobby.ts, maxPlayers represents only human player slots (already halved in DefaultConfig.ts). The nation NPCs are added automatically server-side and don't count toward maxPlayers. Therefore, getTeamSize correctly returns maxPlayers directly for HumansVsNations to display the proper team size (e.g., maxPlayers=5 yields "5 Humans vs 5 Nations").
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-11-01T00:24:33.860Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2161
File: src/core/execution/FakeHumanExecution.ts:670-678
Timestamp: 2025-11-01T00:24:33.860Z
Learning: In OpenFrontIO, PlayerType.Bot entities cannot be in teams and do not have friendliness relationships. Only PlayerType.Human and PlayerType.FakeHuman can participate in teams and alliances. Therefore, when targeting bot-owned tiles, friendliness checks like `owner.isFriendly(this.player)` are unnecessary and meaningless for bots.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
📚 Learning: 2025-12-13T14:58:29.645Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2607
File: src/core/execution/PlayerExecution.ts:271-295
Timestamp: 2025-12-13T14:58:29.645Z
Learning: In src/core/execution/PlayerExecution.ts surroundedBySamePlayer(), the `as Player` cast on `mg.playerBySmallID(scan.enemyId)` is intentional. Since scan.enemyId comes from ownerID() on an owned tile and playerBySmallID() only returns Player or undefined, the cast expresses a known invariant. The maintainers prefer loud failures (runtime errors) over silent masking (early returns with guards) for corrupted game state scenarios at trusted call sites.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
🔇 Additional comments (4)
src/client/graphics/layers/TerritoryLayer.ts (4)
8-8: LGTM: Clean imports and constants.The GameMode import and drawTeammateGlow utility are used appropriately. The new constants eliminate magic numbers and make the spawn highlight radius and pulse animation speed clear and maintainable.
Also applies to: 24-24, 29-30
190-193: LGTM: Efficient and correct feature gating.Moving
myPlayeroutside the loop eliminates redundant lookups, and thecolorBlindEnabledcheck correctly restricts the assist mode to Team games with the setting enabled.
234-241: LGTM with minor performance observation.The teammate glow rendering logic is well-structured with proper null checks and team validation. The conditions correctly exclude self-highlighting and restrict glow to actual teammates.
Note: Each teammate triggers a BFS traversal per frame during spawn phase. While the spawn phase is brief, consider monitoring frame rates in games with many teammates (8+ team games) if performance issues arise.
Also applies to: 283-295
229-231: The spawn highlight condition checkingownerID(tile) === 0appears intentional and correctly highlights unowned (terra nullius) tiles within the spawn radius. This aligns with the spawn feature's purpose of showing expansion potential. The git history does not show evidence of a change fromhuman.smallID()to0at this line. Line 359 inspawnCenterFromOwnedTiles()correctly usesplayer.smallID()for its distinct purpose of finding the center of a player's owned tiles.Likely an incorrect or invalid review comment.
Duwibi
left a comment
Duwibi
left a comment
There was a problem hiding this comment.
This looks awesome! If this gets added, I'd most likely also be using it, as it looks much more helpful than what we have currently in game, although I'm not colour-blind myself :)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/client/graphics/layers/SettingsModal.ts (1)
341-364: Consider using a distinct icon for the color blind setting.The color blind toggle currently uses
settingsIcon, which is identical to the performance overlay icon (line 506). Using a distinct icon would improve visual scanning and help users quickly locate the accessibility option.💡 Potential alternatives
If an accessibility or color-blind specific icon is available in the project, use it. For example:
<img - src=${settingsIcon} + src=${accessibilityIcon} alt="colorBlind" width="20" height="20" />If no dedicated icon exists, consider importing or creating one that visually represents accessibility or color contrast.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
resources/lang/en.jsonsrc/client/graphics/layers/SettingsModal.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- resources/lang/en.json
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/Colors.ts:114-330
Timestamp: 2025-06-06T15:36:31.739Z
Learning: In the OpenFrontIO project, color improvements are implemented incrementally. The current focus for player colors is ensuring sufficient unique color availability rather than optimizing for visual distinction or accessibility features.
🧬 Code graph analysis (1)
src/client/graphics/layers/SettingsModal.ts (1)
src/client/LangSelector.ts (1)
translateText(258-278)
🔇 Additional comments (1)
src/client/graphics/layers/SettingsModal.ts (1)
146-149: No RefreshGraphicsEvent is needed for the color blind toggle. ThecolorBlind()setting is read reactively withinTerritoryLayer.spawnHighlight()during each render frame, not cached at initialization. The effect will be visible immediately on the next frame without requiring an explicit refresh event.
| } | ||
| } | ||
|
|
||
| private spawnCenterFromOwnedTiles( |
There was a problem hiding this comment.
we already have Player.spawnTile(), just need to pass that through GameUpdate and then expose it to PlayerView
There was a problem hiding this comment.
Thanks for that! I should have checked for that myself ;/
Sashazach
dismissed stale reviews from Duwibi and coderabbitai[bot]
via
d9be493
| if ( | ||
| this.userSettings.colorBlind() && | ||
| this.game.config().gameConfig().gameMode === GameMode.Team && | ||
| myPlayer !== null && | ||
| focusedPlayer.smallID() !== myPlayer.smallID() && | ||
| myPlayer.isOnSameTeam(focusedPlayer) | ||
| ) { | ||
| this.drawTeammateGlowForPlayer(focusedPlayer); | ||
| } |
There was a problem hiding this comment.
since this is duplicated, can you move this check into drawTeammateGlowForPlayer, and then rename it "maybeDrawTeammateGlow"
There was a problem hiding this comment.
Yes will do! Thanks for the feedback.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
resources/lang/en.json (1)
380-381: LGTM! Solid accessibility addition.The translation entries follow the established naming convention and provide clear, user-friendly text. The feature adds valuable accessibility support for team games.
Optional: Consider a slightly more descriptive explanation
The current description is good, but you could make it more specific about the visual aid:
- "color_blind_desc": "Adds visual aid to make spotting teammates during spawn phase easier", + "color_blind_desc": "Adds pulsing visual indicators to teammates during spawn phase for easier identification",This is completely optional—the current version is clear and avoids being color-specific, which may actually be preferable for accessibility.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
resources/lang/en.jsonsrc/core/game/PlayerImpl.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/Colors.ts:114-330
Timestamp: 2025-06-06T15:36:31.739Z
Learning: In the OpenFrontIO project, color improvements are implemented incrementally. The current focus for player colors is ensuring sufficient unique color availability rather than optimizing for visual distinction or accessibility features.
📚 Learning: 2025-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 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:
resources/lang/en.json
📚 Learning: 2025-12-13T14:58:29.645Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2607
File: src/core/execution/PlayerExecution.ts:271-295
Timestamp: 2025-12-13T14:58:29.645Z
Learning: In src/core/execution/PlayerExecution.ts surroundedBySamePlayer(), the `as Player` cast on `mg.playerBySmallID(scan.enemyId)` is intentional. Since scan.enemyId comes from ownerID() on an owned tile and playerBySmallID() only returns Player or undefined, the cast expresses a known invariant. The maintainers prefer loud failures (runtime errors) over silent masking (early returns with guards) for corrupted game state scenarios at trusted call sites.
Applied to files:
src/core/game/PlayerImpl.ts
📚 Learning: 2025-11-26T20:49:29.140Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/game/GameView.ts:516-525
Timestamp: 2025-11-26T20:49:29.140Z
Learning: In GameView.ts, when usesSharedTileState is true (SAB mode), packedTileUpdates contains unpacked tile references as BigInt(tileRef) only, because all tile state lives in the shared Uint16Array. In non-SAB mode, packedTileUpdates contains packed TileUpdate bigints in the format (tileRef << 16n | state), which must be decoded via updateTile(tu). Therefore, Number(tu) is correct in SAB mode and shifting right by 16 bits would be wrong.
Applied to files:
src/core/game/PlayerImpl.ts
📚 Learning: 2025-08-12T00:31:50.144Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.
Applied to files:
src/core/game/PlayerImpl.ts
🔇 Additional comments (1)
src/core/game/PlayerImpl.ts (1)
180-180: LGTM! Clean addition of spawn tile state to player updates.The new field exposes the spawn tile to the PlayerUpdate payload, logically placed right after
hasSpawned. The implementation follows existing patterns and maintains type consistency with the PlayerUpdate interface.
4 participants
Description:
This PR introduces a Color Blind Assist mode as a setting. It is designed to make it easier for color-blind people to see where their team is spawning/clustering during team games. When enabled, this setting creates pulsing golden indicators/highlights in the inner circle of the highlights that appear (that show you where people are spawning) during the spawn-in phase for your team. It does not highlight you, only your teammates. As a moderately color-blind person myself, this would be extremely helpful. I have often accidentally spawned in the purple area as a blue team member or the red area as a green team member etc.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
Sashazach#sashazach