Leaderboard update part 1: configurable columns

[jsshap]

Before opening a PR: discuss new features on Discord first, and file bugs or small improvements as issues. You must be assigned to an approved issue - unsolicited PRs will be auto-closed.

Add approved & assigned issue number here:

Refs #4074

Description:

Adds a settings cog to the in-game leaderboard so players can choose which existing columns to show.

This first PR only makes the existing Owned, Gold, and Max troops columns configurable. The new leaderboard items are split into a follow-up PR.

Testing: npx vitest tests/UserSettings.test.ts --run; npx tsc --noEmit; targeted ESLint/Prettier

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I have added it to the en.json file
• I have added relevant tests to the test directory

Please put your Discord username so you can be contacted if a bug or regression is found:

jsshap

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

Walkthrough

Adds persisted leaderboard column preferences to UserSettings (leaderboardColumns, setLeaderboardColumns, toggleLeaderboardColumn) with allowed-key filtering and default fallback. The Leaderboard HUD is refactored to render columns dynamically based on these preferences, generalizes sorting, adds a column settings panel, and includes matching locale strings and tests.

Changes

Configurable leaderboard columns

Layer / File(s)	Summary
UserSettings leaderboard column persistence src/core/game/UserSettings.ts, tests/UserSettings.test.ts	Adds LEADERBOARD_COLUMNS_KEY, exported LeaderboardColumnKey type, allowed/default column lists, and leaderboardColumns(), setLeaderboardColumns(), toggleLeaderboardColumn() methods with filtering and last-column protection; tests cover invalid JSON, unknown keys, defaults, and toggling.
Leaderboard HUD dynamic columns and settings panel src/client/hud/layers/Leaderboard.ts, resources/lang/en.json	Introduces ColumnDefinition/COLUMN_DEFINITIONS, wires userSettings and visibleColumnKeys state, generalizes _sortKey/setSort, adds visibleColumns(), ensureVisibleSortKey(), toggleColumn(), moves alive-player filtering earlier, refactors render() for dynamic grid columns, adds renderColumnSettings() checkbox UI with stopGameInput() guarding, and adds columns/configure_columns locale strings.

Sequence Diagram(s)

sequenceDiagram
  participant Player
  participant Leaderboard
  participant UserSettings
  participant LocalStorage
  Player->>Leaderboard: click column checkbox
  Leaderboard->>Leaderboard: toggleColumn(key)
  Leaderboard->>UserSettings: toggleLeaderboardColumn(key)
  UserSettings->>UserSettings: filter to allowed keys
  UserSettings->>LocalStorage: persist updated column list
  UserSettings-->>Leaderboard: refreshed leaderboardColumns()
  Leaderboard->>Leaderboard: ensureVisibleSortKey()
  Leaderboard->>Leaderboard: render() with visibleColumns()

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• openfrontio/OpenFrontIO#4379: Both PRs touch Leaderboard.ts's sorting logic for the maxtroops column, with overlapping changes to updateLeaderboard().

Suggested labels

Feature, Translation

Suggested reviewers

• evanpelle

A checkbox here, a column there,
Now your board can show what you care.
Gold or troops, tiles in a row,
Toggle them on, toggle them off—go!
Settings saved, tucked away tight,
Your leaderboard now feels just right. 🏆✨

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The issue asks for server-side gold/min with a 120-second moving average, but these changes only show configurable columns and no matching income logic.	Add the server-side gold/min moving average with 10-second updates, and keep the configurable columns work in a separate PR.
Out of Scope Changes check	⚠️ Warning	The PR adds a column chooser and persistence, which are not part of #4074's gold/min scoreboard requirement.	Limit this PR to the income column work and move the column-visibility UI and storage changes to a separate PR.
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: making leaderboard columns configurable.
Description check	✅ Passed	The description is directly related to the PR and accurately summarizes the configurable leaderboard columns work.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/client/hud/layers/Leaderboard.ts`:
- Around line 186-202: The ranking logic in updateLeaderboard is using the full
playerViews list, so dead players can affect the computed fallback place for the
local player. Update the Leaderboard.updateLeaderboard flow to filter the
game.playerViews() result to alive players before sorting, or compute the place
from an alive-only list, so the local player’s rank is based only on active
players.

In `@tests/PackedPlayerUpdates.test.ts`:
- Around line 51-55: The PackedPlayerUpdates assertions are incorrectly using
Number(alice.gold()) for the new goldPerMinute slot, which can hide a packing
bug in PackedPlayerUpdates. Update the expectations in the affected test cases
to assert goldPerMinute with its known income value directly, using the relevant
PackedPlayerUpdates/record tuple checks instead of deriving that field from
alice.gold().

In `@tests/PlayerUpdateDiff.test.ts`:
- Around line 134-211: The new tests are bypassing the shared core-simulation
harness by constructing players and calling game.addPlayer directly. Update
these cases to use the setup() helper from tests/util/Setup.ts to create the
game instance and obtain players, keeping the assertions around PlayerInfo,
game.player, toUpdate, donateGold, and drainPackedPlayerUpdates intact. If a
special exception applies, make it explicit; otherwise route both tests through
the standard test setup pattern.

🪄 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: ef07d4b5-718a-4069-92f2-e1e7acf75e78

📥 Commits

Reviewing files that changed from the base of the PR and between f4b47ce and 7534539.

📒 Files selected for processing (19)

• resources/lang/en.json
• src/client/hud/layers/Leaderboard.ts
• src/client/render/types/Renderer.ts
• src/client/view/GameView.ts
• src/client/view/PlayerView.ts
• src/core/game/Game.ts
• src/core/game/GameImpl.ts
• src/core/game/GameUpdateUtils.ts
• src/core/game/GameUpdates.ts
• src/core/game/PlayerImpl.ts
• src/core/game/UserSettings.ts
• tests/GameUpdateUtils.test.ts
• tests/PackedPlayerUpdates.test.ts
• tests/PlayerUpdateDiff.test.ts
• tests/UserSettings.test.ts
• tests/client/render/frame/derive/nuke-telegraphs.test.ts
• tests/client/render/frame/derive/player-status.test.ts
• tests/client/view/GameView.test.ts
• tests/util/viewStubs.ts

[ryanbarlow97]

I think this is tackling too many things at once, can you split them into multiple PRs?

[jsshap]

I think this is tackling too many things at once, can you split them into multiple PRs?

What are you referring to specifically?

[ryanbarlow97]

I think this is tackling too many things at once, can you split them into multiple PRs?

What are you referring to specifically?

I mean, you added adjustable columns, and a gold/min, would be good to split into 2 prs

[jsshap]

I think this is tackling too many things at once, can you split them into multiple PRs?

What are you referring to specifically?

I mean, you added adjustable columns, and a gold/min, would be good to split into 2 prs

That was the ask in the issue. Happy to split if you like, but I think adding extra columns without the ability to remove them will add clutter we don't want there

[ryanbarlow97]

I think this is tackling too many things at once, can you split them into multiple PRs?

What are you referring to specifically?

I mean, you added adjustable columns, and a gold/min, would be good to split into 2 prs

That was the ask in the issue. Happy to split if you like, but I think adding extra columns without the ability to remove them will add clutter we don't want there

PR 1 should be configurable columns, PR 2 will be the addition of new items

You can do something like

• leaderboard update part 1
  then
• leaderboard update part 2

[jsshap]

@ryanbarlow97 this PR now only adds the configurable columns. Will provide link to separate PR with the new columns once I have it

[jsshap]

Split this into two PRs as requested: #4453 is now configurable existing columns only, and #4465 adds the new stat columns. Both branches are based on main.

[ryanbarlow97]

Split this into two PRs as requested: #4453 is now configurable existing columns only, and #4465 adds the new stat columns. Both branches are based on main.

cheers, it lets us do a more effective review