Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions resources/lang/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1087,5 +1087,8 @@
"dismiss": "Dismiss",
"go_to_item": "Go to item {num}",
"firefox_warning": "OpenFront.io doesn't perform well with [Firefox-based browsers](https://simple.wikipedia.org/wiki/Web_browsers_based_on_Firefox). We recommend you to use a [Chromium-based browser](https://en.wikipedia.org/wiki/Chromium_(web_browser)#Browsers_based_on_Chromium) for best performance."
},
"control_panel": {
"army_limit_warning": "You're near your army limit! Consider sending troops to teammates."
}
}
35 changes: 31 additions & 4 deletions src/client/graphics/layers/ControlPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { LitElement, html } from "lit";
import { customElement, state } from "lit/decorators.js";
import { assetUrl } from "../../../core/AssetUrls";
import { EventBus } from "../../../core/EventBus";
import { Gold } from "../../../core/game/Game";
import { GameMode, Gold } from "../../../core/game/Game";
import { GameView } from "../../../core/game/GameView";
import { UserSettings } from "../../../core/game/UserSettings";
import { ClientID } from "../../../core/Schemas";
import { AttackRatioEvent } from "../../InputHandler";
import { renderNumber, renderTroops } from "../../Utils";
import { renderNumber, renderTroops, translateText } from "../../Utils";
import { UIState } from "../UIState";
import { Layer } from "./Layer";
const goldCoinIcon = assetUrl("images/GoldCoinIcon.svg");
Expand Down Expand Up @@ -36,6 +36,9 @@ export class ControlPanel extends LitElement implements Layer {
@state()
private _isVisible = false;

@state()
private _showArmyLimitWarning: boolean = false;

@state()
private _gold: Gold;

Expand Down Expand Up @@ -87,14 +90,24 @@ export class ControlPanel extends LitElement implements Layer {

this.updateTroopIncrease();

this._maxTroops = this.game.config().maxTroops(player);
const config = this.game.config();
this._maxTroops = config.maxTroops(player);
this._gold = player.gold();
this._troops = player.troops();
this._attackingTroops = player
.outgoingAttacks()
.map((a) => a.troops)
.reduce((a, b) => a + b, 0);
this.troopRate = this.game.config().troopIncreaseRate(player) * 10;
this.troopRate = config.troopIncreaseRate(player) * 10;
const isTeamGame = config.gameConfig().gameMode === GameMode.Team;
const canDonateTroops = config.donateTroops();
if (isTeamGame && canDonateTroops) {
const ratio = this._troops / Math.max(this._maxTroops, 1);
this._showArmyLimitWarning = ratio >= config.armyLimitWarningThreshold();
} else {
this._showArmyLimitWarning = false;
}
Comment on lines +104 to +109
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Minor: troops() excludes outgoing attacks, so the warning can flicker off during big attacks.

player.troops() does not include troops already committed to outgoing attacks — those live in outgoingAttacks() (you already compute _attackingTroops just above). So a player who is truly near their cap but has launched a large attack will drop below the 80% threshold and the banner will disappear until troops refill. If you want the banner to reflect "effective capacity used", consider using this._troops + this._attackingTroops in the ratio.

-      const ratio = this._troops / Math.max(this._maxTroops, 1);
-      this._showArmyLimitWarning = ratio >= config.armyLimitWarningThreshold();
+      const used = this._troops + this._attackingTroops;
+      const ratio = used / Math.max(this._maxTroops, 1);
+      this._showArmyLimitWarning = ratio >= config.armyLimitWarningThreshold();

Not a bug per se — just noting the behavior so you can decide what feels right for players.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (isTeamGame && canDonateTroops) {
const ratio = this._troops / Math.max(this._maxTroops, 1);
this._showArmyLimitWarning = ratio >= config.armyLimitWarningThreshold();
} else {
this._showArmyLimitWarning = false;
}
if (isTeamGame && canDonateTroops) {
const used = this._troops + this._attackingTroops;
const ratio = used / Math.max(this._maxTroops, 1);
this._showArmyLimitWarning = ratio >= config.armyLimitWarningThreshold();
} else {
this._showArmyLimitWarning = false;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/client/graphics/layers/ControlPanel.ts` around lines 104 - 109, The
army-limit warning currently uses this._troops which excludes troops in
outgoingAttacks and thus can flicker; change the ratio calculation inside the
isTeamGame && canDonateTroops branch to use effective troops (this._troops +
this._attackingTroops) divided by Math.max(this._maxTroops, 1) and keep
assigning the result to this._showArmyLimitWarning using
config.armyLimitWarningThreshold(); ensure you reference the existing symbols
(_troops, _attackingTroops, _maxTroops, _showArmyLimitWarning,
config.armyLimitWarningThreshold()) and preserve the Math.max protection against
zero.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional — the warning targets players with idle troops near cap who should be donating to teammates on the front line. Attacking troops are already being used productively, so including them in the ratio would trigger the warning even when the player is actively engaged in combat, which is the opposite of the behavior we want.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luctrate, that makes total sense — the warning is specifically for players sitting idle near cap, not for those actively fighting. Including attacking troops would penalize exactly the players who are already doing their part, which would be counterproductive. Thanks for the clear explanation!

(づ。◕‿‿◕。)づ


✏️ Learnings added
Learnt from: luctrate
Repo: openfrontio/OpenFrontIO PR: 3743
File: src/client/graphics/layers/ControlPanel.ts:104-109
Timestamp: 2026-04-23T07:19:30.992Z
Learning: In `src/client/graphics/layers/ControlPanel.ts` (OpenFrontIO, PR `#3743`), the army-limit warning ratio intentionally uses only `player.troops()` (idle troops) and excludes `_attackingTroops` (outgoing attack troops). The warning targets players with idle troops near cap who should donate to teammates. Troops already committed to attacks are considered productively used, so including them would incorrectly trigger the warning during active combat. Do not suggest adding `_attackingTroops` to this ratio calculation.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue `#2137` was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.

Learnt from: 1brucben
Repo: openfrontio/OpenFrontIO PR: 977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.

Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 3349
File: src/client/HostLobbyModal.ts:649-660
Timestamp: 2026-03-04T20:14:53.404Z
Learning: In `src/client/HostLobbyModal.ts`, `putGameConfig()` dispatches the entire game config object as a full replacement via `CustomEvent("update-game-config")` on every call (not a partial merge). Fields set to `undefined` in the config (e.g. `startingGold`, `goldMultiplier`, `maxTimerValue`) correctly signal "not set / cleared" to the server. There is no stale-value risk when these fields are `undefined`.

Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2689
File: src/client/PublicLobby.ts:245-245
Timestamp: 2025-12-26T22:21:21.904Z
Learning: In public lobbies with HumansVsNations mode in src/client/PublicLobby.ts, maxPlayers represents only human player slots (already halved in DefaultConfig.ts). The nation NPCs are added automatically server-side and don't count toward maxPlayers. Therefore, getTeamSize correctly returns maxPlayers directly for HumansVsNations to display the proper team size (e.g., maxPlayers=5 yields "5 Humans vs 5 Nations").

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.

Learnt from: ryanbarlow97
Repo: openfrontio/OpenFrontIO PR: 2782
File: src/client/graphics/layers/StructureIconsLayer.ts:616-620
Timestamp: 2026-01-03T19:37:30.357Z
Learning: In `src/client/graphics/layers/StructureIconsLayer.ts`, the confirm button in `ensureGhostControls()` for locked ghost nukes (AtomBomb/HydrogenBomb) intentionally does NOT call `removeGhostStructure()` after emitting the build intent. This allows mobile players to rapidly place multiple nukes by "spamming" the confirm button without needing to reopen the radial menu.

Learnt from: bijx
Repo: openfrontio/OpenFrontIO PR: 2773
File: src/client/graphics/layers/PlayerInfoOverlay.ts:443-478
Timestamp: 2026-01-03T00:01:09.721Z
Learning: In OpenFrontIO, when an attack starts, the attacker's troops are removed from their pool before the `Attack` object is created. This means `player.troops()` does NOT include troops in outgoing attacks—those troops are tracked separately via `player.outgoingAttacks()`. The troop count and attacking troop count represent distinct, non-overlapping groups.

Learnt from: andybellenie
Repo: openfrontio/OpenFrontIO PR: 3509
File: src/server/GameServer.ts:667-680
Timestamp: 2026-03-24T23:21:38.781Z
Learning: In openfrontio/OpenFrontIO, the clan-tag privacy feature in `src/server/GameServer.ts` (`gameInfoForClient()`, PR `#3509`) is intentionally a partial mitigation against tag sniping, not a complete fix. A larger clan system rework is in development. The bypass via rejoin with a different clanTag (identityUpdate path in Worker.ts) is a known, accepted limitation. Do not flag this as a critical issue in future reviews of this file.

Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 3246
File: src/core/execution/nation/NationNukeBehavior.ts:814-832
Timestamp: 2026-02-19T18:07:39.005Z
Learning: In src/core/execution/nation/NationNukeBehavior.ts, the clustering logic in maybeDestroyEnemySam() intentionally checks the arrival time of the first bomb from a silo rather than the last when evaluating if a silo fits within maxTotalArrivalSpread. This slight underestimate is by design: the window is already conservative (half the SAM cooldown = 37 of 75 ticks), staggerInterval is small (1-5 ticks), extra bombs provide margin, and the approach includes borderline silos that work fine in practice. The design should not be "fixed" to use the last bomb's arrival unless SAM timing changes significantly.

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.

Learnt from: BrewedCoffee
Repo: openfrontio/OpenFrontIO PR: 1957
File: src/core/execution/PlayerExecution.ts:40-52
Timestamp: 2025-08-29T16:16:11.309Z
Learning: In OpenFrontIO PlayerExecution.ts, when Defense Posts are captured due to tile ownership changes, the intended behavior is to first call u.decreaseLevel() to downgrade them, then still transfer them to the capturing player via captureUnit(). This is not a bug - Defense Posts should be both downgraded and captured, not destroyed outright.

Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 3540
File: src/client/graphics/layers/NameLayer.ts:126-129
Timestamp: 2026-03-30T19:14:20.158Z
Learning: In `src/client/graphics/layers/NameLayer.ts` (OpenFrontIO), the `emojiTemplate` div in `NameLayer` intentionally uses `position: absolute`, `top: 50%`, and `transform: translateY(-50%)`. Emoji badges are designed to appear behind the name/troops overlay, not as inline items in the `iconsDiv` flex row. Do not flag this absolute positioning as a layout bug in future reviews.

Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 0
File: :0-0
Timestamp: 2026-02-01T16:36:52.217Z
Learning: In this TypeScript game codebase, UI/layer updates should be wall-clock throttled (independent of simulation speed). Do not use tick-modulus gating like `if (game.ticks()%N===0)` inside `layer.tick()` methods. Instead, layers should expose a `getTickIntervalMs()` method and let the renderer schedule ticks based on wall-clock time.

Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2874
File: src/server/MapLandTiles.ts:7-11
Timestamp: 2026-01-12T21:37:01.156Z
Learning: In this repository's OpenFrontIO deployment, inter-service HTTP calls to the master should target http://localhost:3000 (master at port 3000) as the canonical address. Apply this as the standard for all server-side TypeScript code that communicates with the master. Avoid hardcoding non-master URLs; centralize the master address (e.g., via config or env) when possible, and ensure internal service communication uses localhost:3000 in this architecture.

Learnt from: evanpelle
Repo: openfrontio/OpenFrontIO PR: 3114
File: src/client/graphics/layers/AttacksDisplay.ts:64-77
Timestamp: 2026-02-10T05:02:28.715Z
Learning: In the OpenFrontIO codebase, GameUpdateType maps always have an array value for each key (never undefined). When iterating, you can safely use: for (const event of updates[GameUpdateType.X]) without optional chaining or guards. Treat updates as a Record<GameUpdateType, any[]> and rely on its guaranteed array values. If you modify the type, ensure the guarantee remains (e.g., initialize missing keys to []) to preserve safe iteration.

Learnt from: wozniakpl
Repo: openfrontio/OpenFrontIO PR: 3317
File: src/client/InputHandler.ts:344-350
Timestamp: 2026-03-07T11:35:24.071Z
Learning: Ensure ghost preservation logic for special units (AtomBomb and HydrogenBomb) is implemented via shouldPreserveGhostAfterBuild(unitType) and that ghost removal is conditional on !shouldPreserveGhostAfterBuild(unitType) within createStructure. Confirm that both mouse (MouseUpEvent) and keyboard (ConfirmGhostStructureEvent) confirmation paths go through requestConfirmStructure → createStructure, and that Enter/NumpadEnter confirmations do not break the locked-nuke multi-placement since the ghost should be preserved by design. Add tests to cover shouldPreserveGhostAfterBuild returning true for AtomBomb/HydrogenBomb and verify that ghost removal only occurs for other units. Applies to files in the repository OpenFrontIO (src/client/InputHandler.ts and src/client/graphics/layers/StructureIconsLayer.ts).

Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 3540
File: src/client/graphics/layers/NameLayer.ts:84-86
Timestamp: 2026-03-30T18:41:35.016Z
Learning: In graphics layers, `redraw()` should be intentionally a no-op (or have an empty/limited body) when the layer is DOM-based and does not depend on Canvas/WebGL context (e.g., it does not require reinitializing GPU resources after context loss). The `redraw()` hook should only contain reinitialization logic for Canvas/WebGL layers that must restore GPU resources on context restoration.

Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 3549
File: tests/pathfinding/playground/server.ts:246-250
Timestamp: 2026-03-31T17:02:41.798Z
Learning: When migrating/implementing Express 5+ code, use the Express 5 `app.listen(PORT, (error) => { ... })` callback signature: Express 5 passes listen errors (e.g., `EADDRINUSE`) as the first argument to the callback. Handle the error inside that callback instead of relying on Node.js `http.Server`’s `'error'` event semantics.

Learnt from: baculinivan-web
Repo: openfrontio/OpenFrontIO PR: 3688
File: src/client/graphics/layers/GameRightSidebar.ts:235-248
Timestamp: 2026-04-22T08:54:50.345Z
Learning: In the OpenFrontIO/OpenFrontIO repository, do not raise review findings for standard web accessibility (a11y) keyboard/semantic issues that would normally require button semantics (e.g., using clickable `<div>` elements instead of `<button>`, or similar keyboard accessibility/ARIA patterns). This repository intentionally does not enforce those semantic/keyboard-accessibility conventions because gameplay requires touch or mouse input and is not operable via keyboard alone.


this.requestUpdate();
}

Expand Down Expand Up @@ -258,8 +271,21 @@ export class ControlPanel extends LitElement implements Layer {
`;
}

private renderArmyLimitWarning() {
if (!this._showArmyLimitWarning) return html``;
return html`
<div
class="flex items-center gap-1.5 px-1.5 py-1 rounded-md border border-orange-400/60 bg-orange-400/10 text-orange-300 text-xs font-medium mb-1"
>
<span class="shrink-0">⚠</span>
<span>${translateText("control_panel.army_limit_warning")}</span>
</div>
`;
}

private renderDesktop() {
return html`
${this.renderArmyLimitWarning()}
<!-- Row 1: troop rate | troop bar | gold -->
<div class="flex gap-1.5 items-center mb-1">
<!-- Troop rate -->
Expand Down Expand Up @@ -334,6 +360,7 @@ export class ControlPanel extends LitElement implements Layer {

private renderMobile() {
return html`
${this.renderArmyLimitWarning()}
<div class="flex gap-2 items-center">
<!-- Gold -->
<div
Expand Down
1 change: 1 addition & 0 deletions src/core/configuration/Config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export interface Config {
gameConfig(): GameConfig;
theme(): Theme;
percentageTilesOwnedToWin(): number;
armyLimitWarningThreshold(): number;
numBots(): number;
spawnNations(): boolean;
isUnitDisabled(unitType: UnitType): boolean;
Expand Down
3 changes: 3 additions & 0 deletions src/core/configuration/DefaultConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,9 @@ export class DefaultConfig implements Config {
}
return 80;
}
armyLimitWarningThreshold(): number {
return 0.8;
}
boatMaxNumber(): number {
if (this.isUnitDisabled(UnitType.TransportShip)) {
return 0;
Expand Down
Loading