From f1cd4789702791794fc43a4ab3374563883e615e Mon Sep 17 00:00:00 2001 From: VariableVince <24507472+VariableVince@users.noreply.github.com> Date: Wed, 18 Feb 2026 22:09:24 +0100 Subject: [PATCH] Cleanup/refactor: Remove some redundant checks (#3235) ## Description: PR 3/x in effort to break up PR https://github.com/openfrontio/OpenFrontIO/pull/3220. Follows on already merged https://github.com/openfrontio/OpenFrontIO/pull/3233 and https://github.com/openfrontio/OpenFrontIO/pull/3234. Please see if these can be merged for v30. - **ClientGameRunner**: removed two redundant myPlayer===null checks since that was already done right above, instead use !. - **BuildMenu**: just like in UnitDisplay, assign public PlayerActions default value of null. So that in canCreateOrBuild, where we already do a === null check on it btw, we can safely skip the assignment to const buildableUnits and just directly loop over this.playerActions.buildableUnits. - **RadialMenuElements**: don't call canBuildOrUpgrade 3x in CreateMenuElements for the .map on flattenedBuildTable, instead do it once and re-use outcome. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] 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 --- src/client/ClientGameRunner.ts | 6 +- src/client/graphics/layers/BuildMenu.ts | 7 +- .../graphics/layers/RadialMenuElements.ts | 84 ++++++++++--------- 3 files changed, 49 insertions(+), 48 deletions(-) diff --git a/src/client/ClientGameRunner.ts b/src/client/ClientGameRunner.ts index ca7dfef7f..fb01edb34 100644 --- a/src/client/ClientGameRunner.ts +++ b/src/client/ClientGameRunner.ts @@ -558,12 +558,11 @@ export class ClientGameRunner { this.myPlayer = myPlayer; } this.myPlayer.actions(tile).then((actions) => { - if (this.myPlayer === null) return; if (actions.canAttack) { this.eventBus.emit( new SendAttackIntentEvent( this.gameView.owner(tile).id(), - this.myPlayer.troops() * this.renderer.uiState.attackRatio, + this.myPlayer!.troops() * this.renderer.uiState.attackRatio, ), ); } else if (this.canAutoBoat(actions, tile)) { @@ -674,12 +673,11 @@ export class ClientGameRunner { } this.myPlayer.actions(tile).then((actions) => { - if (this.myPlayer === null) return; if (actions.canAttack) { this.eventBus.emit( new SendAttackIntentEvent( this.gameView.owner(tile).id(), - this.myPlayer.troops() * this.renderer.uiState.attackRatio, + this.myPlayer!.troops() * this.renderer.uiState.attackRatio, ), ); } diff --git a/src/client/graphics/layers/BuildMenu.ts b/src/client/graphics/layers/BuildMenu.ts index 5ac5fa0e0..62e9c46fc 100644 --- a/src/client/graphics/layers/BuildMenu.ts +++ b/src/client/graphics/layers/BuildMenu.ts @@ -127,7 +127,7 @@ export class BuildMenu extends LitElement implements Layer { public eventBus: EventBus; public uiState: UIState; private clickedTile: TileRef; - public playerActions: PlayerActions | null; + public playerActions: PlayerActions | null = null; private filteredBuildTable: BuildItemDisplay[][] = buildTable; public transformHandler: TransformHandler; @@ -361,8 +361,9 @@ export class BuildMenu extends LitElement implements Layer { if (this.game?.myPlayer() === null || this.playerActions === null) { return false; } - const buildableUnits = this.playerActions?.buildableUnits ?? []; - const unit = buildableUnits.filter((u) => u.type === item.unitType); + const unit = this.playerActions.buildableUnits.filter( + (u) => u.type === item.unitType, + ); if (unit.length === 0) { return false; } diff --git a/src/client/graphics/layers/RadialMenuElements.ts b/src/client/graphics/layers/RadialMenuElements.ts index 67ef1d05a..1eb35d364 100644 --- a/src/client/graphics/layers/RadialMenuElements.ts +++ b/src/client/graphics/layers/RadialMenuElements.ts @@ -388,48 +388,50 @@ function createMenuElements( ? ATTACK_UNIT_TYPES.includes(item.unitType) : !ATTACK_UNIT_TYPES.includes(item.unitType)), ) - .map((item: BuildItemDisplay) => ({ - id: `${elementIdPrefix}_${item.unitType}`, - name: item.key - ? item.key.replace("unit_type.", "") - : item.unitType.toString(), - disabled: (params: MenuElementParams) => - !params.buildMenu.canBuildOrUpgrade(item), - color: params.buildMenu.canBuildOrUpgrade(item) - ? filterType === "attack" - ? COLORS.attack - : COLORS.building - : undefined, - icon: item.icon, - tooltipItems: [ - { text: translateText(item.key ?? ""), className: "title" }, - { - text: translateText(item.description ?? ""), - className: "description", + .map((item: BuildItemDisplay) => { + const canBuildOrUpgrade = params.buildMenu.canBuildOrUpgrade(item); + return { + id: `${elementIdPrefix}_${item.unitType}`, + name: item.key + ? item.key.replace("unit_type.", "") + : item.unitType.toString(), + disabled: () => !canBuildOrUpgrade, + color: canBuildOrUpgrade + ? filterType === "attack" + ? COLORS.attack + : COLORS.building + : undefined, + icon: item.icon, + tooltipItems: [ + { text: translateText(item.key ?? ""), className: "title" }, + { + text: translateText(item.description ?? ""), + className: "description", + }, + { + text: `${renderNumber(params.buildMenu.cost(item))} ${translateText("player_panel.gold")}`, + className: "cost", + }, + item.countable + ? { text: `${params.buildMenu.count(item)}x`, className: "count" } + : null, + ].filter( + (tooltipItem): tooltipItem is TooltipItem => tooltipItem !== null, + ), + action: (params: MenuElementParams) => { + const buildableUnit = params.playerActions.buildableUnits.find( + (bu) => bu.type === item.unitType, + ); + if (buildableUnit === undefined) { + return; + } + if (canBuildOrUpgrade) { + params.buildMenu.sendBuildOrUpgrade(buildableUnit, params.tile); + } + params.closeMenu(); }, - { - text: `${renderNumber(params.buildMenu.cost(item))} ${translateText("player_panel.gold")}`, - className: "cost", - }, - item.countable - ? { text: `${params.buildMenu.count(item)}x`, className: "count" } - : null, - ].filter( - (tooltipItem): tooltipItem is TooltipItem => tooltipItem !== null, - ), - action: (params: MenuElementParams) => { - const buildableUnit = params.playerActions.buildableUnits.find( - (bu) => bu.type === item.unitType, - ); - if (buildableUnit === undefined) { - return; - } - if (params.buildMenu.canBuildOrUpgrade(item)) { - params.buildMenu.sendBuildOrUpgrade(buildableUnit, params.tile); - } - params.closeMenu(); - }, - })); + }; + }); } export const attackMenuElement: MenuElement = {