Fix/Perf/Refactor: playerActions and buildableUnits, their callers and related types

[VariableVince]

Description:

TL;DR: it's faster.

buildableUnits is called via PlayerView.actions from UnitDisplay (each tick without TileRef), BuildMenu (each tick when open), MainRadialMenu (each tick when open), PlayerPanel (each tick when open), StructureIconsLayer (when placing a building from build bar), NukeTrajectoryPreviewLayer (when placing nuke, on tick when tile changes), ClientGameRunner (on click to attack/auto-boat or hotkey B or G).

After #3213 got merged, the change with largest impact in #3193 was done in such a different way that a new PR was needed

The idea in 3193 was to not always ask for Transport Ship from buildableUnits. In such a way that very little extra data was send to the worker. This had the biggest impact on performance (the idea was months older btw, see #2295). Now, we do it the other way around, by telling buildableUnits all unit types we want. Or we want them all (undefined). The downside is more data is send in the worker message. The upside is we have more options and can add more in this PR.

This PR implements some of the leftovers in 3193 on top of 3213 and adds further improvements.

(Some unrelated refactor/perf changes where moved out of this PR and into already merged #3233, #3234, #3235, #3236, #3237, #3238, #3239)

• GameRunner, WorkerMessages: playerActions and PlayerActionsMessage . Option to ask for no buildable units (null). It now has 3 modes: get all actions and all buildings (units undefined), get all actions and no buildings (units null), or get all actions and specific building (units contains Unit Types).

• GameRunner: playerActions. fixes wrong assumption in PR 3213: that only if units was undefined, we have to know canAttack. ClientGameRunner wants to know both, in case of a click on non-bordering land, to decide if it should auto-boat using a Transport Ship. So units is not undefined (we only ask for Transport Ship now which has a positive effect on performance for each click/tap) but we need canAttack still.
  Solved by removing the unit === undefined check before canAttack in playerActions.

• GameRunner, GameView, WorkerClient, WorkerMessages, Worker.worker: added playerBuildables / buildables next to existing playerActions / actions. With above solved, there was still no option to only get buildable units when the actions are not needed. While StructureIconsLayer, NukeTrajectoryPreviewLayer, BuildMenu and UnitDisplay need only that. To not make playerActions more convoluted with more params or so, i've added a new function playerBuildables in GameView to only get buildable units (GameRunner playerBuildables). playerBuildables has 2 modes: get all buildings (units undefined) or get specific buildings (units contains Unit Types). Also update some comments that mentioned .actions in NukeTrajectoryPreviewLayer.

• ClientGameRunner, PlayerPanel, BuildMenu, UnitDisplay, StructureIconsLayer and NukeTrajectoryPreviewLayer: Since PR 3213, StructureIconsLayer and NukeTrajectoryPreviewLayer ask for specific types of units from GameView actions (GameRunner playerActions). Now have the other files do the same. For example BuildMenu asks for the new BuildMenuTypes when it calls .buildables and ClientGameRunner asks for UnitType.TransportShip when sending a boat

• ClientGameRunner: canBoatAttack now accepts BuildableUnit[] instead of PlayerActions so we can send it either actions.buildableUnits or just buildables. Have functions call myPlayer.buildables(tileRef, [UnitType.TransportShip]) when we only need a buildable unit and no actions. Or myPlayer.actions(tileRef, null) when we need actions but no buildable units. Or myPlayer.actions(tileRef, [UnitType.TransportShip]) when we need both actions, like canAttack, and a buildable unit. Then if needed send either actions.buildableUnits or buildables to to canAutoBoat / canBoatAttack.

• MainRadialMenu: needs all player buildable unit types including Transport Ship, so the actions call argument for unit types can stay undefined (unchanged) there.

• MainRadialMenu: now that BuildMenu uses playerBuildables instead of playerActions, we must put data in this.buildMenu.playerBuildables. And since we're not putting the (unneeded) full actions in there anymore, we can now put only the needed and expected _actions.buildableUnits in it.

• Game / PlayerImpl: Typesafety and some added perf: new type PlayerBuildableUnitType (see also the below point for how it is formed). So callers of buildableUnits can never ask for the wrong type like e.g. UnitType.Train because it doesn't return data for that type. This type is now used in PlayerImpl, BuildMenu, RadialMenuElements, StructureDrawingUtils and UnitDisplay for that reason. And InputHandler, StructureIconsLayer and UIState (little more on that in point below).

• InputHandler, StructureIconsLayer, UIState: In order to make type safety work for GhostUnit.buildableUnit.type too (line ~217 of StructureIconsLayer), changed type of interface BuildableUnit to PlayerBuildableType. Which is only more accurate. Same for uiState.ghostStructure and with that, renderUnitItem in UnitDisplay and setGhostStructure in InputHandler. All Structures are of PlayerBuildableType (there are even some in PlayerBuildableType that aren't Structures, but it is much more confined than UnitType).

• Game: Typesafety and some added perf: added BuildMenuTypes and BuildableAttackTypes and isBuildableAttackType in the same fashion that the existing StructureTypes and isStructureType were already used. isBuildableAttackType is used in RadialMenuElements. BuildableAttackTypes and existing StructureTypes now make up the BuildMenuTypes. BuildMenuTypes is used in BuildMenu, StructureIconsLayer and UnitDisplay. Then BuildMenuTypes together with UnitType.TransportShip make up the PlayerBuildableTypes. Which is used in PlayerImpl buildableUnits (see point below). And with PlayerBuildableUnits we get the new PlayerBuildableUnitType (see above point on Game / PlayerImpl).

• RadialMenuElements: replace non-central ATTACK_UNIT_TYPES in RadialMenuElements with centralized BuildableAttackTypes too. Use PlayerBuildableUnitType for more type safety (can't by mistake add UnitType.Train to its build menu). Make use of BuildableAttackTypes instead of adding items hardcoded line by line in getAllEnabledUnits, just like we already did since PR 3239 with StructureTypes. And use isBuildableAttackTypes in the same fashion that existing isStructureTypes was already used elsewhere.

• PlayerImpl: buildableUnits
  -- would do Object.values(UnitTypes) on every call. Now for better perf directly loop over player buildable units by using PlayerBuildableTypes (see above point). In this way we also exclude MIRVWarhead, TradeShip, Train, SamMissile and Shell so there are less unit types to loop through by default. Since a player doesn't build those by themselves, they are only build by Executions which use canBuild directly and not buildableUnits.
  -- for more performance, do for loop instead of using .map and .filter, no intermediate array needed nor callback overhead. We just loop over the given units (which if undefined will contain PlayerBuildableTypes). Also pre-allocate the results array to get the most out of it, even if V8 might already be very good at this.
  -- cache config, railNetwork and inSpawnPhase so they can be re-used inside the for loop.
  -- cache cost inside the loop
  -- it would check twice for tile!==null to decide to call findUnitToUpgrade and canBuild. Now once.
  -- eliminated double/triple checks for the same thing. It called findUnitToUpgrade (and with that canUpgradeUnit) and then canBuild which both check if player has enough gold for the cost of the unit type. And they both check if the unit type is disabled. Now we call private functions canBuildUnitType, canUpgradeUnitType to first do checks on unit type level for early returns, and findExistingUnitToUpgrade to find existing unit without doing anything extra. in a specific order to check everything only once. The public functions findUnitToUpgrade and canBuild have an unchanged functionality and we don't call them from buildableUnits anymore.

• PlayerImpl: findUnitToUpgrade: unchanged functionality, but have it call new private function findExistingUnitToUpgrade to find existing unit.

• PlayerImpl: canBuild: unchanged functionality, but have it call new private function canBuildUnitType to do the checks it first did itself. And then new private function canSpawnUnitType for the rest of the checks. This way we can call canBuildUnitType and canSpawnUnitType from buildableUnits in a specific order to prevent double/triple checks.

• PlayerImpl: canBuildUnitType: new private function to be shared by buildableUnits, canBuild and canUpgradeUnit to be able do unit type level checks in a specific order to prevent double/triple checks. Via parameter knownCost, buildableUnits can send it the cost it already fetched so that it doesn't have to be fetched again. For caller canUpgradeUnit, the isAlive() check (which was previously only done in canBuild) is new but harmless, maybe even better to have also check isAlive() on upgrade now that Nations are also upgrading which might prevent some edge case bugs.

• PlayerImpl: canUpgradeUnitType: new private function to be shared by buildableUnits and canUpgradeUnit to be able do unit type level checks in a specific order to prevent double/triple checks.

• PlayerImpl: canSpawnUnitType: new private function to be shared by buildableUnits and canBuildUnit to be able do unit type level checks in a specific order to prevent double/triple checks.

• PlayerImpl: findExistingUnitToUpgrade: new private function to be shared by buildableUnits and findUnitToUpgrade to be able do unit level checks in a specific order to prevent double/triple checks.

• PlayerImpl: isUnitValidToUpgrade: new private function to be shared by buildableUnits and canUpgradeUnit to be able do unit level checks in a specific order to prevent double/triple checks.

• PlayerImpl.test.ts: because of the isAlive() check in which is new for canUpgradeUnit (see above at canBuildUnitType), the tests needed to have the players be alive at the start, in order to pass.

PERFORMANCE
As for calling .buildables instead of unnecessarily getting .actions, there is an obvious win because there's less to send calculate and recieve.

Also asking for only the needed buildings helps a lot (especially if TradeShip isn't needed, see the difference in benchmark in original #3193).

But the real-world impact is hard to measure. gave it a try in #3193 and those results should be even better now.

Now testing only buildableUnits performance in a synthetic benchmark, we get these results. This is after other performance improvments so the base is already better than it was in original #3193:

BEFORE (only buildableUnits itself)

AFTER (only buildableUnits itself)

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I've added it to the en.json file
• I have added relevant tests to the test directory
• I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

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

tryout33

[coderabbitai]

Walkthrough

Adds BuildableUnit and PlayerBuildableUnitType types, a new player_buildables worker RPC and GameRunner.playerBuildables, and shifts many client flows from actions(...) to buildables(...). Updates PlayerImpl build logic, WorkerMessages/WorkerClient, GameView APIs, and multiple UI/drawing layers to use the new buildable-unit typing.

Changes

Cohort / File(s)	Summary
Core public types & worker bridge src/core/game/Game.ts, src/core/game/GameView.ts, src/core/GameRunner.ts, src/core/worker/WorkerMessages.ts, src/core/worker/WorkerClient.ts, src/core/worker/Worker.worker.ts	Introduce BuildableUnit, BuildMenuTypes, PlayerBuildableTypes, PlayerBuildableUnitType; add GameRunner.playerBuildables; add player_buildables/player_buildables_result messages; change actions/RPC units typing to `readonly PlayerBuildableUnitType[]
Player implementation & tests src/core/game/PlayerImpl.ts, tests/PlayerImpl.test.ts	Refactor buildableUnits to evaluate per-type build/upgrade rules with helpers (canBuildUnitType, canSpawnUnitType, etc.); adjust tests setup and tile conquest ordering.
Client runner & input src/client/ClientGameRunner.ts, src/client/InputHandler.ts	Update imports and call sites to pass explicit action-context/unit filters (e.g., actions(tile, StructureTypes)), and change signatures to accept BuildableUnit[] / PlayerBuildableUnitType where applicable.
UI layers: build & radial menus, unit display src/client/graphics/layers/BuildMenu.ts, src/client/graphics/layers/UnitDisplay.ts, src/client/graphics/layers/MainRadialMenu.ts, src/client/graphics/layers/RadialMenuElements.ts	Replace PlayerActions usage with BuildableUnit[] (playerBuildables); switch unit/structure types to PlayerBuildableUnitType; use BuildMenuTypes/isBuildableAttackType for filtering, costs and rendering.
UI state, structure drawing & icons src/client/graphics/UIState.ts, src/client/graphics/layers/StructureDrawingUtils.ts, src/client/graphics/layers/StructureIconsLayer.ts	Change ghostStructure and related parameters to `PlayerBuildableUnitType
Targeting, nuke preview & trajectory src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts	Switch from actions(...) to buildables(...) for spawn-tile lookup and trajectory caching; adjust data shapes from actions.buildableUnits to BuildableUnit[].
Client player panel & miscellany src/client/graphics/layers/PlayerPanel.ts, other small client files	Update calls to myPlayer.actions(...) to include second-argument arity (e.g., null or specific unit filters); wire playerBuildables into BuildMenu and related components.

Sequence Diagram(s)

sequenceDiagram
  participant UI as Client UI
  participant PV as PlayerView
  participant WC as WorkerClient
  participant W as Worker
  participant GR as GameRunner

  UI->>PV: request buildables(tile?, units?)
  PV->>WC: playerBuildables(playerID, x?, y?, units?)
  WC->>W: postMessage "player_buildables" (playerID,x,y,units)
  W->>GR: gameRunner.playerBuildables(playerID,x,y,units)
  GR-->>W: BuildableUnit[] (result)
  W-->>WC: postMessage "player_buildables_result" (result)
  WC-->>PV: resolve BuildableUnit[]
  PV-->>UI: return BuildableUnit[]

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🌱 Small typed lists cross wire and wall,
Menus ask buildables, workers heed the call,
Runner counts tiles, impls check each type,
UI paints ghosts and options ripe,
A tidy mesh of types, both short and tall.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% 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 summarizes the main changes: performance optimization and refactoring of playerActions, buildableUnits, and their related types across the codebase.
Description check	✅ Passed	The description thoroughly explains the purpose, changes, performance improvements, and implementation details of the refactoring across multiple files and systems.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

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 and usage tips.}

[evanpelle]

do you have before/after performance data?

[VariableVince]

do you have before/after performance data?

Added a section on Performance to the PR description

[evanpelle]

Any flame charts to show CPU savings?

[VariableVince]

Any flame charts to show CPU savings?

Hard to test i a repeatable/comparable way since it depends on human input. Can't just run a replay and it depends on what Nations do too, since we also need them for real-life like test.

So i don't know how trustworthy numbers coming out of it would be..