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
This commit is contained in:
VariableVince
2026-02-18 22:09:24 +01:00
committed by GitHub
parent f4f7ae3929
commit f1cd478970
3 changed files with 49 additions and 48 deletions
+2 -4
View File
@@ -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,
),
);
}
+4 -3
View File
@@ -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;
}
@@ -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 = {