Fix/Perf/Refactor: buildableUnits and many related code

[VariableVince]

Description:

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 more.

• GameRunner: Adds option to ask for no buildable untis (null) in playerActions.

• GameRunner: Fixes wrong assumption in Add units filter on playeractions for performance #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. playerActions 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: But with above solved, there was still no option to only get buildable units. While StructureIconsLayer, NukeTrajectoryPreviewLayer, BuildMenu and UnitsDisplay need only that. To not make playerActions more convoluted with more params or so, i've added a new function playerBuildables to only get buildable units. playerBuildables has 2 modes: get all buildings (units undefined) or get specific buildings (units contains Unit Types).

• Have all callers of GameView .actions or .buildables, ultimately GameRunner playerActions and playerBuildables, ask for their type of units. Not only StructureIconsLayer and NukeTrajectoryPreviewLayer but also ClientGameRunner, BuildMenu, UnitsDisplay and PlayerPanel. Only MainRadialMenu needs all player buildable unit types including Transport Ship, so it can stay undefined there.

• PlayerImpl: validStructureSpawnTiles did a filter on unit types to get isTerroritoryBound units, on every call again. It read this from unit info in DefaultConfig so while that's good for maintainability, other code already uses hardcoded StructureTypes and isisStructureType from Game.ts. Which has the same purpose and thus contains the same unit types. StructureTypes and isisStructureType do need manual maintainance outside of DefaultConfig. And are more bug prone/less type safe. Still, using them gives more speed compared to putting it in some get function in GameImpl for example (tested with buildableUnits and MIRVPerf.ts. So went with StructureTypes in validStructureSpawnTiles.

• PlayerImpl: Early return for non-upgradable unit types in findUnitToUpgrade. If it is upgradable, it will again check in canUpgradeUnit if the unit type is upgradable alas. Since canUpgradeUnit is also used by other code, this double check cannot be removed in a simple way. Still the early return makes the code faster as there are a number of units that can't be upgraded so we can skip all that happens in findUnitToUpgrade.

• PlayerImpl: buildableUnits would do Object.values(UnitTypes) on every call. Now only get player buildable units, exclude MIRVWarhead, TradeShip, Train, SamMissile and Shell. Since a player doesn't build those by themselves, they are only build by Executions which use canBuild directly instead of buildableUnits. Added PlayerBuildableTypes to Game.ts in the same fashion as existing StructureTypes and isStructureType.

• PlayerImpl: buildableUnits would check twice for tile!==null to decide to call findUnitToUpgrade and canBuild. Now once.

• PlayerImpl: buildableUnits would call canBuild which checks Cost, but then it fetches the Cost itself too. Now just do this once with canBuild accepting it as optional param. This does not affect other code that calls canBuild.

• Game / PlayerImpl: Typesafety: adds new type PlayerBuildableUnitType so callers of buidableUnits can never ask for the wrong type like e.g. UnitType.Train because it doesn't return data for that type.

• StructureIconsLayer: In order to make type safety work in StructureIconsLayer for GhostUnit.buildableUnit.type too, 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).

• 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 StructureTypes and BuildableAttackTypes instead of adding items hardcoded line by line in getAllEnabledUnits. Don't call canBuildOrUpgrade 3x in CreateMenuElements for the .map on flattenedBuildTable, instead do it once.

• BuildMenu: Fix some comments. Make playerBuildables have default value of null (null was already set as one of the possible values). Now we can remove the "?? []" check in canBuildOrUpgrade, since we already checked if playerBuildables is null when we get there.

Some related (perf) changes:

• NationStructureBehavior: removed canUpgradeUnit check from maybeUpgradeStructure. Since it already checked this right before in findBestStructureToUpgrade, so only upgradable units are returned. Furthermore, UpgradeStructureExecution is called right after which also again checks canUpgradeUnit. So we're going from 3 times to 2 times canUpgradeUnit, small perf win on its own.

• NationStructureBehavior: instead of hardcoded ruling out Defense Post for upgrade check in maybeSpawnStructure, check dynamically if type is upgradable. That way if defense posts ever do become upgradable, we don't run into a bug right away.

• NationsStructureBehavior: in getTotalStructureDensity, we can pass an array of types as argument so we only have to get an array length once. It needs to ignore levels so we can't make use of other pre-defined functions in PlayerImpl (which were created to avoid array length calls), but at least this saves a few.

• GameImpl/GameView nearbyUnits: it accepted "UnitType | UnitType[]" for tiles, but called UnitGrid nearbyUnits which mandates "UnitType | readonly UnitType[]". Made the requirement the same for GameImpl/GameView nearbyUnits. This way, we don't have make a shallow copy of the StructureTypes array everytime we want to send it as an argument, from validStructureSpawnTiles and Util.ts. Other callers aren't affected.

• ClientGameRunner: removed two redundant myPlayer===null checks since that was already done right above.

• PlayerExecution: now validStructureSpawnTiles no longer needs isTerritoryBound, PlayerExecution is the last place where it was used. Replaced it for isStructureType here too (since it has the same meaning and outcome).

• Game.ts and DefaultConfig unitInfo: removed canBuildTrainStation and expirimental properties, as they already weren't used anywhere anymore. Removed isTerritoryBound too as it was only used in validStructureSpawnTiles and PlayerExecution and has been replaced in both.

• PlayerActionHandler: remove unused getPlayerActions, the only potential caller MainRadialMenu already just calls myPlayer.actions via GameView directly.

• StructureIconsLayer: remove already unused PlayerActions

• Worker.worker: correct some existing error messages

• And more

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 + BuildMenuTypes; changes player.actions to accept readonly PlayerBuildableUnitType[] | null; adds playerBuildables RPC and GameRunner.playerBuildables; removes several UnitInfo flags; replaces territoryBound checks with isStructureType/StructureTypes; updates many client call sites and UI types to use buildables.

Changes

Cohort / File(s)	Summary
Core public API & types src/core/game/Game.ts, src/core/game/GameView.ts, src/core/game/GameImpl.ts	Introduce BuildableUnit, PlayerBuildableUnitType, BuildMenuTypes, PlayerBuildableTypes, StructureTypes, helpers (isBuildableAttackType/isPlayerBuildableType); remove territoryBound/canBuildTrainStation/experimental; tighten nearbyUnits readonly typing.
GameRunner & worker RPCs src/core/GameRunner.ts, src/core/worker/Worker.worker.ts, src/core/worker/WorkerClient.ts, src/core/worker/WorkerMessages.ts	Add playerBuildables API and worker message flow (player_buildables/player_buildables_result); change worker/client messages and method signatures to use `readonly PlayerBuildableUnitType[]
Player impl & execution logic src/core/game/PlayerImpl.ts, src/core/execution/PlayerExecution.ts, src/core/execution/nation/NationStructureBehavior.ts, src/core/execution/Util.ts	buildableUnits returns BuildableUnit[] and accepts PlayerBuildableUnitType[]; replace territoryBound checks with isStructureType; use StructureTypes for nearby queries; simplify upgrade gating and density calculation.
Client runner & call sites src/client/ClientGameRunner.ts, src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts, src/client/graphics/layers/MainRadialMenu.ts, src/client/graphics/layers/PlayerPanel.ts	Call sites updated to use actions(tile, units?), actions(tile, null), or buildables(tile, units?); pass BuildMenuTypes/StructureTypes or explicit lists; added some non-null assertions for myPlayer.
Client UI types & build menus src/client/InputHandler.ts, src/client/graphics/UIState.ts, src/client/graphics/layers/BuildMenu.ts, src/client/graphics/layers/UnitDisplay.ts, src/client/graphics/layers/StructureDrawingUtils.ts, src/client/graphics/layers/StructureIconsLayer.ts	Switch build-related types from UnitType to PlayerBuildableUnitType; replace PlayerActions usage with BuildableUnit[]; fetch buildables via BuildMenuTypes/buildables; rename members (e.g., playerActions → playerBuildables).
Client menus & element logic src/client/graphics/layers/RadialMenuElements.ts, src/client/graphics/layers/PlayerActionHandler.ts, src/client/graphics/layers/MainRadialMenu.ts	Generate enabled unit sets as PlayerBuildableUnitType; remove getPlayerActions; wire menus to playerBuildables; update filtering to use isBuildableAttackType.
Configuration src/core/configuration/DefaultConfig.ts	Remove territoryBound and related flags from UnitInfo entries; simplify UnitInfo shape.
Misc signatures & imports multiple files (src/core/worker/WorkerMessages.ts, src/core/worker/WorkerClient.ts, src/core/game/GameView.ts, etc.)	Adjust imports and method signatures to prefer PlayerBuildableUnitType, readonly unit arrays, nullable unit lists, and BuildableUnit return types; update many call sites to pass explicit unit filters or null.

Sequence Diagram(s)

sequenceDiagram
  participant UI as Client UI
  participant CGR as ClientGameRunner
  participant WC as WorkerClient
  participant WK as Worker
  participant GR as GameRunner
  participant P as PlayerImpl

  UI->>CGR: request buildables(tile, units?)
  CGR->>WC: worker.playerBuildables(playerID, x, y, units)
  WC->>WK: post "player_buildables"
  WK->>GR: gameRunner.playerBuildables(playerID, x, y, units)
  GR->>P: player.buildableUnits(tile, units?)
  P-->>GR: BuildableUnit[] (type, cost, canBuild/canUpgrade)
  GR-->>WK: player_buildables_result
  WK-->>WC: reply
  WC-->>CGR: resolve BuildableUnit[]
  CGR-->>UI: provide buildables to UI

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Add units filter on playeractions for performance #3213 — similar change adding optional unit-type filters to player actions and updating call sites to pass unit lists.

Suggested reviewers

• evanpelle

Poem

🌱 Types rearranged, menus learn new names,
Ghosts wear buildable crowns in calmer games.
Readonly lists keep choices small and true,
Structures counted by a clearer view,
Build menu hums — a tidy world renewed.

🚥 Pre-merge checks | ✅ 3 | ❌ 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 (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: refactoring and optimizing buildableUnits and related code, which is the core focus of the PR.
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, explaining the motivations, implementations, and affected components.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

src/core/game/PlayerImpl.ts (1)

983-991: Cost is still recomputed inside canUpgradeUnit (line 953).

You precompute cost at line 983 and pass it into canBuild — nice. But findUnitToUpgrade calls canUpgradeUnit, which independently recomputes this.mg.config().unitInfo(unit.type()).cost(this.mg, this) at line 953 to check gold. That is the same value as cost here.

Consider threading knownCost through findUnitToUpgrade → canUpgradeUnit the same way you did for canBuild, so you compute cost exactly once per unit type.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/core/game/PlayerImpl.ts` around lines 983 - 991, Precompute cost once and
thread it into the upgrade checks: change findUnitToUpgrade to accept a
knownCost (or pass the already computed cost variable) and forward that into
canUpgradeUnit, then modify canUpgradeUnit to accept an optional knownCost
parameter and use it instead of calling this.mg.config().unitInfo(...).cost(...)
internally; update all call sites of findUnitToUpgrade and canUpgradeUnit to
pass the cost where available and fall back to computing cost only when
knownCost is not provided to preserve existing behavior.

🤖 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/core/game/PlayerImpl.ts`:
- Around line 983-991: Precompute cost once and thread it into the upgrade
checks: change findUnitToUpgrade to accept a knownCost (or pass the already
computed cost variable) and forward that into canUpgradeUnit, then modify
canUpgradeUnit to accept an optional knownCost parameter and use it instead of
calling this.mg.config().unitInfo(...).cost(...) internally; update all call
sites of findUnitToUpgrade and canUpgradeUnit to pass the cost where available
and fall back to computing cost only when knownCost is not provided to preserve
existing behavior.

[evanpelle]

I'm having trouble understanding this PR since it contains unrelated fixes, would it be possible to break this up into multiple PRs?

[VariableVince]

I'm having trouble understanding this PR since it contains unrelated fixes, would it be possible to break this up into multiple PRs?

Not that easy with my lacking git skills. Without those it will cost a lot of time to split up (or fix mistakes when i do try git magic and mess up) so may either take awhile or not happen alas