Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds a configurable player-name opacity: new translations, user settings getter/setter, slider UI in two modals, SettingSlider zero-label support, and rendering updates that apply per-player label opacity and hide flags when opacity is 0. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client UI (slider)
participant Settings as UserSettings
participant Renderer as NameLayer / DOM
UI->>Settings: setPlayerNameOpacity(value)
Settings-->>UI: persist & acknowledge
UI->>Renderer: request re-render
Renderer->>Settings: playerNameOpacity()
Settings-->>Renderer: current opacity (0-100)
Renderer->>DOM: update nameSpan.style.opacity, troopsDiv.style.opacity
alt opacity == 0
Renderer->>DOM: set flagDiv.style.display = "none"
else
Renderer->>DOM: reset flagDiv.style.display
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/client/graphics/layers/SettingsModal.ts (1)
462-493: Prefer reusing<setting-slider>here to avoid logic drift.This block duplicates slider rendering/value-label logic already implemented in
src/client/components/baseComponents/setting/SettingSlider.ts(including the zero-value translated label). Reusing the shared component will keep behavior consistent between both settings modals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/graphics/layers/SettingsModal.ts` around lines 462 - 493, This block duplicates the slider and zero-value label logic; replace it with the shared SettingSlider component (tag <setting-slider> / component implementation in SettingSlider.ts) to avoid drift: render <setting-slider> bound to this.userSettings.playerNameOpacity(), pass the same min/max (0/100), wire its input/change handler to onPlayerNameOpacityChange (or use the component's change event), and remove the duplicated percent/hidden label logic since SettingSlider already handles zero-value translation via translateText; ensure value source remains this.userSettings.playerNameOpacity() and event handler name matches the existing method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/graphics/layers/NameLayer.ts`:
- Around line 342-345: Replace the unsafe innerHTML assignment for player names
with a safe text assignment: locate the code in NameLayer.ts where
nameDiv.querySelector(".player-name-span") is stored in span and currently calls
span.innerHTML = render.player.name(); and change it to set span.textContent (or
assign to (span as HTMLElement).textContent) while keeping the opacity logic
that uses nameOpacity intact; ensure you use render.player.name() only as text
(no HTML parsing) so player-controlled input cannot inject markup.
In `@src/core/game/UserSettings.ts`:
- Around line 249-252: setPlayerNameOpacity currently allows NaN through into
clampedOpacity and then setFloat which persists and emits "NaN"; guard against
non-finite inputs by validating the incoming opacity with
Number.isFinite(opacity) (or isFinite) before rounding and clamping, and if it's
not finite either return early (no save/emit) or default to a safe value (e.g.,
0), then compute const clampedOpacity = Math.max(0, Math.min(100,
Math.round(opacity))) and call this.setFloat("settings.playerNameOpacity",
clampedOpacity).
---
Nitpick comments:
In `@src/client/graphics/layers/SettingsModal.ts`:
- Around line 462-493: This block duplicates the slider and zero-value label
logic; replace it with the shared SettingSlider component (tag <setting-slider>
/ component implementation in SettingSlider.ts) to avoid drift: render
<setting-slider> bound to this.userSettings.playerNameOpacity(), pass the same
min/max (0/100), wire its input/change handler to onPlayerNameOpacityChange (or
use the component's change event), and remove the duplicated percent/hidden
label logic since SettingSlider already handles zero-value translation via
translateText; ensure value source remains this.userSettings.playerNameOpacity()
and event handler name matches the existing method.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 229ede9c-496a-4d1e-bee2-868e0c5e57d9
📒 Files selected for processing (6)
resources/lang/en.jsonsrc/client/UserSettingModal.tssrc/client/components/baseComponents/setting/SettingSlider.tssrc/client/graphics/layers/NameLayer.tssrc/client/graphics/layers/SettingsModal.tssrc/core/game/UserSettings.ts
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
…tion for player name opacity setting
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/client/graphics/layers/SettingsModal.ts (1)
490-520: UI section looks good, consider a more descriptive icon.The slider control follows the same structure as other settings in this modal. The conditional display showing "Hidden" at 0% is a nice touch for clarity.
One small note: you're using the generic
settingsIconhere. If there's a more specific icon available (like a text/label icon), it might help users visually locate this setting faster. Not a blocker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/graphics/layers/SettingsModal.ts` around lines 490 - 520, Replace the generic settingsIcon used in the player name opacity row with a more specific label/text icon to improve discoverability: locate the SettingsModal component where settingsIcon is used (the block rendering this.userSettings.playerNameOpacity() and the input with `@input`=${this.onPlayerNameOpacityChange}) and swap settingsIcon for a more descriptive symbol (e.g., labelIcon or textIcon) from your existing icon set; if that icon is not yet imported, add the appropriate import and use the specific icon name so the UI shows a text/label glyph instead of the generic settings gear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/client/graphics/layers/SettingsModal.ts`:
- Around line 490-520: Replace the generic settingsIcon used in the player name
opacity row with a more specific label/text icon to improve discoverability:
locate the SettingsModal component where settingsIcon is used (the block
rendering this.userSettings.playerNameOpacity() and the input with
`@input`=${this.onPlayerNameOpacityChange}) and swap settingsIcon for a more
descriptive symbol (e.g., labelIcon or textIcon) from your existing icon set; if
that icon is not yet imported, add the appropriate import and use the specific
icon name so the UI shows a text/label glyph instead of the generic settings
gear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93ff8954-ff31-4d75-9f20-3a8f750533d8
📒 Files selected for processing (4)
resources/lang/en.jsonsrc/client/graphics/layers/NameLayer.tssrc/client/graphics/layers/SettingsModal.tssrc/core/game/UserSettings.ts
✅ Files skipped from review due to trivial changes (1)
- src/client/graphics/layers/NameLayer.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- resources/lang/en.json
1 participant
Description:
It's really hard sometimes to see what buildings or features are under those big text letters of people's nametags and troop counts on the map (also reported by users on the Discord). This PR adds a setting for opacity of the name and troop count in the NameLayer, allowing users to change the opacity via a slider in both lobby settings and in-game settings. Setting the opacity to 0% will also hide the flag, but opacity changes do not change the opacity of a nation/player's flag since it already has transparency applied to it. This does not affect gameplay critical icons such as alliance break, incoming alliance request, alliance notification, nukes, crowns, emojis, etc. (see below)
Video Demo
Recording.2026-03-17.020418.mp4
Setting on Lobby Menu
Screen.Recording.2026-03-17.020514.mp4
Emojis and other icons intact (when hidden)
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
bijx