Lobby Gold Options (Starting Gold, Gold Multiplier) #2461
Conversation
WalkthroughAdds three game-config parameters (startingGold, goldMultiplier, spawnImmunityDuration), surfaces them in host and single-player UIs, applies them in server/defaults/schema, updates gameplay (spawn-immunity gating and gold scaling), and adds a CeasefireTimer rendering layer plus translations and test defaults. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client UI
participant API as GameServer
participant Core as Game Loop / Config
participant Player as PlayerImpl
UI->>API: PUT/POST gameConfig (startingGold, goldMultiplier, spawnImmunityDuration)
API->>API: updateGameConfig() apply new fields
API-->>UI: GET poll returns gameConfig with new fields
UI->>Core: Initialize client state from gameConfig
Core->>Player: create player (uses config.startingGold)
Player->>Player: gold ← config.startingGold(playerInfo)
rect rgb(255, 245, 230)
Note over Core,Player: Spawn-Immunity Window active
Core->>Core: ticks ≤ numSpawnPhaseTurns + spawnImmunityDuration
Core->>Core: execution layers check isSpawnImmunityActive()
Core->>Core: Warship/Transport/Attack skip actions targeting Human players
end
rect rgb(235, 245, 255)
Note over Core: Ongoing economy
Core->>Core: train/trade gold computed
Core->>Player: gold += applyGoldMultiplier(baseAmount)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/server/GameManager.ts (1)
48-66: New GameConfig defaults are sensible; consider centralizing them
startingGold: 0,goldMultiplier: 1, andspawnImmunityDuration: 5 * 10fit the intended ranges and match the host UI defaults. To avoid drift later, you might want to pull these from a single place (e.g. a shared default GameConfig / config helper) rather than hard‑coding them here and in other server paths.src/core/game/TransportShipUtils.ts (1)
11-19: Spawn‑immunity guard for transport ships looks correctThe early check cleanly blocks transport‑ship builds onto tiles owned by human players during the spawn + immunity window, while still allowing builds against bots / TN. Logic and placement at the top of
canBuildTransportShiplook good.Since this same formula appears in multiple places (here, AttackExecution, WarshipExecution), you may later want a small helper like
isSpawnImmunityActive(game)to keep it consistent.src/core/execution/AttackExecution.ts (1)
93-105: Immunity check correctly limited to human targetsGating the immunity window to
targetPlayer.type() === PlayerType.Humanis a nice refinement: humans cannot be attacked during spawn + immunity, but attacks against bots / TN are still allowed. The flow (warn → deactivate → return) is clear.If you keep adding similar checks elsewhere, consider using a small shared helper (e.g.
isSpawnImmunityActive(this.mg)or a config helper) to avoid repeating the tick arithmetic.src/core/execution/WarshipExecution.ts (1)
68-82: Warship immunity handling is clear; confirm scope vs other systemsThe warship now fully pauses combat during the spawn + immunity window: it clears any target, keeps patrolling, and only resumes targeting / shooting once
isSpawnImmunityActive()is false. This is straightforward and safe.Note that this is slightly stricter than AttackExecution / TransportShipUtils, which only apply immunity when the target is a human player. Here, warships skip shots against any enemy unit (including bots / TN) while immunity is active. If the design is “global ceasefire” that’s fine; if the goal is “protect human nations only,” you might want to gate on the target unit’s owner type similarly.
isSpawnImmunityActive()itself looks correct and could be shared with other call sites to remove duplicated tick math.Also applies to: 292-298
src/client/HostLobbyModal.ts (2)
336-365: New sliders for bots, spawn immunity, starting gold, and gold multiplier are wired well
- Bots slider now uses
sliderStyleplus a debouncedputGameConfig, which should cut down on noisy PUTs.- Spawn‑immunity slider (0–300s, step 5) maps cleanly to ticks, and
formatSecondsAsClockmakes the value easy to read.- Starting gold / gold multiplier toggles with nested sliders and “off” labels are clear, and using presets +
renderNumbergives readable values.One UX thought: when you turn the starting‑gold or gold‑multiplier toggles off, you reset the value to 0 / 1. That is technically correct but forgets the last non‑default choice. If you want a smoother UX, you could keep the previous value in a separate field and only send 0 / 1 to the server while the toggle is disabled.
Also applies to: 539-639
796-818: Consider debouncing more sliders and sharing slider progress logicRight now only the bots slider is debounced; spawn‑immunity, starting‑gold, and gold‑multiplier sliders call
putGameConfig()on everyinputevent. For hosts who drag the handles a lot, this can send many PUTs in a short time.If this becomes noisy in practice, you could reuse the same 300ms debounce pattern for these sliders as well. Also,
sliderStyleandupdateSliderProgressElementboth compute the same percentage; you might later consolidate them so each slider only needs one call site for progress styling.Also applies to: 825-836, 851-895, 1010-1038
src/client/SinglePlayerModal.ts (1)
647-673: Consider consolidating slider progress logic.Both
sliderStyleandupdateSliderProgressElementcompute the same percentage and set the--progressCSS property, just through different mechanisms (declarative vs imperative). This is duplication.Consider:
- Using only
sliderStylein templates and letting Lit's reactivity handle updates, OR- Using only imperative updates in handlers if immediate feedback is required
If both are needed, add a comment explaining why (e.g., "updateSliderProgressElement provides immediate visual feedback in input handlers before the next render cycle").
+ // Helper to compute slider progress percentage private sliderStyle(value: number, min: number, max: number): string { if (max === min) { return "--progress:0%"; } const percent = ((value - min) / (max - min)) * 100; return `--progress:${Math.max(0, Math.min(100, percent))}%`; } + // Updates slider progress imperatively for immediate visual feedback + // before the next render cycle (used in input handlers) private updateSliderProgressElement(slider: HTMLInputElement): void { const min = Number(slider.min); const max = Number(slider.max);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
resources/lang/debug.json(2 hunks)resources/lang/en.json(2 hunks)src/client/HostLobbyModal.ts(12 hunks)src/client/SinglePlayerModal.ts(9 hunks)src/client/graphics/GameRenderer.ts(3 hunks)src/client/graphics/layers/CeasefireTimer.ts(1 hunks)src/client/index.html(1 hunks)src/client/styles.css(4 hunks)src/client/utilities/StartingGoldPresets.ts(1 hunks)src/core/Schemas.ts(1 hunks)src/core/configuration/Config.ts(1 hunks)src/core/configuration/DefaultConfig.ts(7 hunks)src/core/execution/AttackExecution.ts(1 hunks)src/core/execution/TransportShipExecution.ts(2 hunks)src/core/execution/WarshipExecution.ts(2 hunks)src/core/game/PlayerImpl.ts(4 hunks)src/core/game/TransportShipUtils.ts(1 hunks)src/server/GameManager.ts(1 hunks)src/server/GameServer.ts(1 hunks)src/server/MapPlaylist.ts(1 hunks)tests/util/Setup.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (26)
📓 Common learnings
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: 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.
📚 Learning: 2025-10-18T17:54:01.311Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.311Z
Learning: In src/core/execution/FakeHumanExecution.ts, MIRVs and ground attacks should not be mutually exclusive. The considerMIRV() method should not short-circuit maybeAttack() - both actions can occur in the same tick.
Applied to files:
src/core/execution/AttackExecution.tssrc/core/execution/WarshipExecution.tssrc/core/execution/TransportShipExecution.tssrc/core/game/PlayerImpl.ts
📚 Learning: 2025-10-20T20:15:28.858Z
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.
Applied to files:
src/core/execution/AttackExecution.tssrc/core/execution/WarshipExecution.tstests/util/Setup.tssrc/server/MapPlaylist.tssrc/core/configuration/Config.tssrc/client/SinglePlayerModal.tssrc/core/Schemas.tssrc/core/configuration/DefaultConfig.tssrc/server/GameServer.tssrc/core/execution/TransportShipExecution.tssrc/core/game/PlayerImpl.tssrc/server/GameManager.tssrc/client/HostLobbyModal.ts
📚 Learning: 2025-10-20T11:02:16.969Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.
Applied to files:
src/core/execution/AttackExecution.tssrc/core/execution/TransportShipExecution.tssrc/core/game/PlayerImpl.ts
📚 Learning: 2025-10-27T09:47:26.395Z
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.
Applied to files:
src/core/execution/AttackExecution.ts
📚 Learning: 2025-08-23T08:03:44.914Z
Learnt from: ElMelchizedek
Repo: openfrontio/OpenFrontIO PR: 1876
File: src/core/execution/FakeHumanExecution.ts:547-592
Timestamp: 2025-08-23T08:03:44.914Z
Learning: In OpenFrontIO, FakeHumanExecution is used for AI-controlled nations that simulate human behavior, which is distinct from PlayerType.Bot. FakeHumanExecution players are not PlayerType.Bot - they represent more sophisticated AI that mimics human nation behavior, while PlayerType.Bot represents basic AI players.
Applied to files:
src/core/execution/AttackExecution.ts
📚 Learning: 2025-11-01T00:24:33.860Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2161
File: src/core/execution/FakeHumanExecution.ts:670-678
Timestamp: 2025-11-01T00:24:33.860Z
Learning: In OpenFrontIO, PlayerType.Bot entities cannot be in teams and do not have friendliness relationships. Only PlayerType.Human and PlayerType.FakeHuman can participate in teams and alliances. Therefore, when targeting bot-owned tiles, friendliness checks like `owner.isFriendly(this.player)` are unnecessary and meaningless for bots.
Applied to files:
src/core/execution/AttackExecution.tssrc/core/game/PlayerImpl.ts
📚 Learning: 2025-08-29T16:16:11.309Z
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.
Applied to files:
src/core/execution/AttackExecution.ts
📚 Learning: 2025-08-12T00:31:50.144Z
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.
Applied to files:
src/core/execution/AttackExecution.tssrc/core/game/TransportShipUtils.tstests/util/Setup.tssrc/server/MapPlaylist.tssrc/core/configuration/Config.tssrc/client/SinglePlayerModal.tssrc/core/Schemas.tssrc/server/GameServer.tssrc/core/game/PlayerImpl.tssrc/server/GameManager.tssrc/client/HostLobbyModal.ts
📚 Learning: 2025-08-24T11:13:08.495Z
Learnt from: DevelopingTom
Repo: openfrontio/OpenFrontIO PR: 1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.
Applied to files:
src/core/execution/AttackExecution.tssrc/core/execution/WarshipExecution.ts
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/index.htmlsrc/client/SinglePlayerModal.tssrc/client/graphics/layers/CeasefireTimer.tssrc/client/HostLobbyModal.ts
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/server/MapPlaylist.tssrc/client/SinglePlayerModal.tssrc/client/HostLobbyModal.ts
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
Repo: openfrontio/OpenFrontIO PR: 2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/core/configuration/Config.tssrc/client/SinglePlayerModal.tssrc/server/GameServer.tssrc/core/game/PlayerImpl.tssrc/client/HostLobbyModal.ts
📚 Learning: 2025-10-18T11:00:57.142Z
Learnt from: NewYearNewPhil
Repo: openfrontio/OpenFrontIO PR: 2230
File: src/client/graphics/GameRenderer.ts:269-277
Timestamp: 2025-10-18T11:00:57.142Z
Learning: In src/client/graphics/GameRenderer.ts, the GameRecapCapture implementation does not use setCaptureRenderEnabled on layers. Instead, it uses RecapCaptureSurface.capture() to render capture layers (TerrainLayer, TerritoryLayer, RailroadLayer, StructureIconsLayer, UnitLayer) directly to an off-screen canvas without requiring layer-level capture mode methods.
Applied to files:
src/client/graphics/GameRenderer.tssrc/client/graphics/layers/CeasefireTimer.ts
📚 Learning: 2025-05-21T04:10:33.435Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as `z.infer<typeof PlayerStatsSchema>` where PlayerStatsSchema has `.optional()` applied at the object level, making PlayerStats a union type that already includes undefined (PlayerStats | undefined).
Applied to files:
src/core/Schemas.ts
📚 Learning: 2025-05-21T04:10:33.435Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as a type inferred from a Zod schema that is marked as optional, which means PlayerStats already includes undefined as a possible type (PlayerStats | undefined).
Applied to files:
src/core/Schemas.tssrc/core/execution/TransportShipExecution.ts
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
Applied to files:
resources/lang/debug.jsonresources/lang/en.json
📚 Learning: 2025-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.
Applied to files:
resources/lang/debug.jsonresources/lang/en.json
📚 Learning: 2025-08-21T21:42:45.277Z
Learnt from: plazmaezio
Repo: openfrontio/OpenFrontIO PR: 1883
File: src/client/HostLobbyModal.ts:227-245
Timestamp: 2025-08-21T21:42:45.277Z
Learning: In openfrontio/OpenFrontIO, the en.json file contains difficulty translation keys using the descriptive names ("Relaxed", "Balanced", "Intense", "Impossible") rather than the enum values ("Easy", "Medium", "Hard", "Impossible"). The DifficultyDescription mapping in src/client/components/Difficulties.ts correctly maps enum values to these descriptive translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-27T08:12:19.610Z
Learnt from: mokizzz
Repo: openfrontio/OpenFrontIO PR: 1940
File: resources/lang/en.json:763-766
Timestamp: 2025-08-27T08:12:19.610Z
Learning: In OpenFrontIO, some country entries in src/client/data/countries.json may have similar names but different codes (e.g., "Empire of Japan" vs "Empire of Japan1"). Each unique code requires its own translation key in resources/lang/en.json after normalization. Always verify against countries.json before suggesting removal of translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-11-08T16:57:19.090Z
Learnt from: mepoohsta
Repo: openfrontio/OpenFrontIO PR: 2413
File: src/core/game/RailNetworkImpl.ts:298-311
Timestamp: 2025-11-08T16:57:19.090Z
Learning: In src/core/game/RailNetworkImpl.ts, the getDestinationWeight method calculates route profitability (not just length) for train destination selection. The heavy relationship bonuses (ally +50, team/other +25, self +10) compared to the base +1 per station are intentional, as profitability considers both route length and player relationships. Longer routes are usually more profitable, but shorter routes to allies can be more valuable.
Applied to files:
src/core/configuration/DefaultConfig.ts
📚 Learning: 2025-05-31T18:15:03.445Z
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.
Applied to files:
src/core/configuration/DefaultConfig.tssrc/core/execution/TransportShipExecution.tssrc/core/game/PlayerImpl.ts
📚 Learning: 2025-10-08T17:14:49.369Z
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.
Applied to files:
src/core/configuration/DefaultConfig.tssrc/core/game/PlayerImpl.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from UnitType values (specifically bomb-related values like AtomBomb, HydrogenBomb, MIRV, and MIRVWarhead) rather than a separate enum. This means comparing NukeType values against UnitType values in switch statements is valid and intentional.
Applied to files:
src/core/configuration/DefaultConfig.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from specific UnitType enum values (AtomBomb, HydrogenBomb, MIRV, MIRVWarhead). This means that comparisons in switch statements between a NukeType parameter and UnitType enum values are valid and will work correctly at runtime.
Applied to files:
src/core/configuration/DefaultConfig.ts
📚 Learning: 2025-06-05T02:34:45.899Z
Learnt from: Egraveline
Repo: openfrontio/OpenFrontIO PR: 1012
File: src/core/execution/UpgradeStructureExecution.ts:49-54
Timestamp: 2025-06-05T02:34:45.899Z
Learning: In the upgrade system, gold deduction for structure upgrades is handled internally by the `upgradeUnit` method in PlayerImpl, not in the UpgradeStructureExecution class. The UpgradeStructureExecution only needs to check if the player has sufficient gold before calling `upgradeUnit`.
Applied to files:
src/core/game/PlayerImpl.ts
🧬 Code graph analysis (11)
src/core/execution/AttackExecution.ts (1)
src/core/game/Game.ts (1)
Player(526-662)
src/core/game/TransportShipUtils.ts (2)
src/core/game/Game.ts (2)
Game(664-748)Player(526-662)src/core/game/GameMap.ts (1)
TileRef(3-3)
src/server/MapPlaylist.ts (2)
src/core/configuration/DefaultConfig.ts (1)
playerTeams(343-345)src/core/game/Game.ts (1)
HumansVsNations(56-56)
src/core/configuration/Config.ts (2)
tests/util/Setup.ts (1)
playerInfo(86-88)src/core/game/Game.ts (2)
PlayerInfo(406-420)Gold(24-24)
src/client/SinglePlayerModal.ts (2)
src/client/utilities/StartingGoldPresets.ts (3)
STARTING_GOLD_PRESETS(1-6)startingGoldValueFromIndex(8-14)startingGoldIndexFromValue(16-33)src/client/Utils.ts (1)
renderNumber(19-43)
src/core/configuration/DefaultConfig.ts (3)
src/core/Util.ts (1)
within(37-39)src/core/game/Game.ts (2)
Gold(24-24)PlayerInfo(406-420)tests/util/Setup.ts (1)
playerInfo(86-88)
src/server/GameServer.ts (1)
src/server/MapPlaylist.ts (1)
gameConfig(79-108)
src/client/graphics/layers/CeasefireTimer.ts (2)
src/client/graphics/layers/Layer.ts (1)
Layer(1-7)src/core/game/GameView.ts (1)
GameView(448-774)
src/core/execution/TransportShipExecution.ts (2)
src/core/Schemas.ts (1)
Player(117-117)src/core/game/Game.ts (1)
Player(526-662)
src/core/game/PlayerImpl.ts (2)
tests/util/Setup.ts (1)
playerInfo(86-88)src/core/game/GameImpl.ts (1)
owner(183-185)
src/client/HostLobbyModal.ts (2)
src/client/Utils.ts (1)
renderNumber(19-43)src/client/utilities/StartingGoldPresets.ts (3)
STARTING_GOLD_PRESETS(1-6)startingGoldValueFromIndex(8-14)startingGoldIndexFromValue(16-33)
🔇 Additional comments (41)
resources/lang/debug.json (1)
99-100: LGTM!The debug translation keys are correctly structured and placed within their respective sections. They follow the established pattern for debug translations.
Also applies to: 150-152
resources/lang/en.json (1)
148-149: LGTM!The English translations are clear and user-friendly. The labels accurately describe the new game configuration options without being overly verbose.
Also applies to: 254-256
src/client/index.html (1)
407-407: LGTM!The ceasefire-timer custom element is properly placed in the DOM structure and follows the established naming convention for custom elements in this codebase.
src/core/configuration/Config.ts (1)
97-97: LGTM!The method signature is well-typed and appropriately placed. Using
PlayerInfoas the parameter allows for flexible per-player starting gold configuration.src/client/graphics/GameRenderer.ts (1)
11-11: LGTM!The CeasefireTimer integration follows the exact pattern established for other layers in this file. The DOM query, type checking, game instance assignment, and layer registration are all consistent with existing code.
Also applies to: 236-242, 272-272
src/client/styles.css (1)
236-281: LGTM!Refactoring from ID selectors to a class selector is the right approach for reusable slider styling. This allows the new sliders (starting gold, gold multiplier, spawn immunity) to share the same styles without duplication.
src/server/GameServer.ts (1)
121-129: LGTM!The config update logic follows the established pattern used throughout this method. The undefined checks ensure that the config is only updated when explicitly provided.
src/core/execution/TransportShipExecution.ts (1)
7-7: LGTM!The spawn immunity check for human players is well-structured and type-safe. The logic correctly:
- Validates the target is a player before checking immunity
- Calculates the immunity window accurately
- Only protects human players (not bots or nations)
- Safely casts after the type guard
This implementation aligns with similar immunity checks mentioned in the AI summary for AttackExecution.ts.
Also applies to: 92-103
src/core/execution/AttackExecution.ts (1)
198-205: Local caching in tick/addNeighbors is fineCaching
troopCount,targetIsPlayer/targetPlayer, andtickNowavoids repeated method calls inside hot paths without changing behaviour. The new code still updatesthis.attackwith the cachedtroopCounteach iteration, so state stays in sync.No changes needed here.
Also applies to: 301-337
tests/util/Setup.ts (1)
55-75: Test GameConfig now covers new fieldsSeeding
spawnImmunityDuration: 0,startingGold: 0, andgoldMultiplier: 1in the test setup keepsGameConfigvalid with the new required fields, while still letting individual tests override via_gameConfig. Looks good.src/core/Schemas.ts (1)
157-178: New GameConfig schema fields align with UI and usage
startingGold,goldMultiplier, andspawnImmunityDurationare added with ranges that match the host UI controls (gold presets to 100M, multiplier 0–10, immunity 0–300s → 0–3000 ticks). This keeps client sliders and server validation in sync.Looks good.
src/client/HostLobbyModal.ts (2)
4-5: State and helpers for gold/immunity options are consistentThe new imports, state fields (
spawnImmunityDurationSeconds,startingGold,goldMultiplier, and their enabled flags), andStartingGoldPresetshelpers line up with the server defaults and schema limits. Using seconds in state and converting to ticks at the edge keeps the UI logic simple.Also applies to: 26-37, 45-63
718-747: Client↔server mapping for new config fields looks solid
- On
open()and inpollPlayers(), you snapstartingGoldto the nearest preset and normalizegoldMultiplierinto [0, 10] with one decimal. This keeps the UI stable even if the server ever sends slightly off values.spawnImmunityDurationSecondsis derived from ticks byfloor(duration / 10), andputGameConfig()sendsspawnImmunityDuration: this.spawnImmunityDurationSeconds * 10, which stays within the schema’s 0–3000 range given the 0–300s slider.- The enabled flags are only flipped when values actually change, so polling does not fight with local user input.
All of this makes the new options feel consistent and predictable.
Also applies to: 957-999, 1096-1123
src/client/SinglePlayerModal.ts (9)
4-4: LGTM!The new imports are clean and directly support the starting gold preset functionality. All are properly utilized in the component.
Also applies to: 30-34
36-36: Verify spawn immunity configuration parity with HostLobbyModal.The spawn immunity duration is hardcoded here at 5 seconds, while HostLobbyModal provides a UI slider (0-300s range). Consider whether single-player mode should also allow this to be configured, or document why single-player uses a fixed value.
55-58: LGTM!State properties are initialized with sensible defaults: 0 starting gold, 1x multiplier (neutral), both disabled.
257-261: LGTM!The bots slider now uses
sliderStylefor visual progress feedback and properly binds the value as a string. The changes are consistent with the new slider pattern.
325-361: LGTM!The starting gold UI correctly uses an index-based slider mapped to presets. Conditional rendering of the slider only when enabled is a good UX pattern. The display properly shows formatted values or "off" state.
363-395: LGTM!The gold multiplier UI uses a direct value slider with appropriate range [0, 10] and 0.1 step. Display formatting with
toFixed(1)is consistent. The conditional rendering pattern matches the starting gold control.
550-571: LGTM!The starting gold handlers correctly manage the enabled state and map between slider indices and preset values. Resetting to preset[0] (0 gold) when disabled and auto-enabling on slider change are good UX patterns.
573-590: LGTM!The gold multiplier handlers correctly manage the enabled state. Resetting to 1 (neutral) when disabled and auto-enabling on slider change are appropriate. The normalization ensures values stay within [0, 10] with 0.1 precision.
744-751: LGTM!The startGame payload correctly applies the configured values when enabled and falls back to appropriate defaults when disabled (0 gold, 1x multiplier). The spawn immunity duration uses the constant value defined at the top of the file.
src/core/configuration/DefaultConfig.ts (7)
35-35: LGTM!The
GOLD_MULTIPLIER_PRECISIONconstant of 1000n provides 3 decimal places of precision, which is more than sufficient for the current 0.1 step UI controls and allows for future flexibility.
264-264: LGTM!Spawn immunity duration now correctly reads from game configuration instead of being hardcoded, enabling per-game customization.
282-300: LGTM!The gold multiplier implementation is solid:
- Clamps to [0, 10] matching UI constraints
- Handles both bigint and number inputs correctly
- Uses precise bigint arithmetic with 1000x scaling
- Optimizes for common cases (0 base, 1x multiplier)
382-397: LGTM!Train gold income now correctly applies the global gold multiplier to all base values (ally: 50k, team/other: 25k, self: 10k). The base amounts remain unchanged from the original implementation.
409-418: LGTM!Trade ship gold calculation now applies the global multiplier to the final computed value (base distance calculation × port bonus). The
Math.floorensures integer input toapplyGoldMultiplier.
853-858: LGTM!The
startingGoldmethod correctly provides starting gold only to Human players based on game configuration, with a safe fallback to 0. Bots and FakeHuman players get no starting gold, which is appropriate.
923-926: LGTM!Passive gold addition rate now applies the global multiplier to base values (100 for players, 50 for bots). This completes the gold multiplier implementation across all gold income sources.
src/client/utilities/StartingGoldPresets.ts (3)
1-6: LGTM!The preset array is well-designed with fine granularity at lower values and appropriate jumps at higher values. The progression from 0 to 100M with 25 steps provides good user choice without overwhelming the UI.
8-14: LGTM!The
startingGoldValueFromIndexfunction safely clamps the index to valid bounds and returns the corresponding preset. Simple and correct.
16-33: LGTM!The
startingGoldIndexFromValuefunction correctly finds the nearest preset using absolute delta. The value is safely clamped to the preset range, and the linear search through 25 items is perfectly acceptable for this use case.src/core/game/PlayerImpl.ts (3)
116-116: LGTM!Player gold initialization now correctly uses the configured starting gold value via
mg.config().startingGold(playerInfo), which returns the appropriate value based on player type and game configuration.
995-1001: LGTM!The spawn immunity check correctly prevents nuke spawning during the protected period (spawn phase + immunity duration). This applies to all nuke types and provides consistent spawn protection.
1186-1228: LGTM!The
canAttackrefactoring improves readability by caching the tile owner and adds spawn immunity protection for Human players. The immunity check correctly prevents attacks on newly spawned Human players during the protected period, and the logic flow is clear and correct.src/client/graphics/layers/CeasefireTimer.ts (5)
1-24: LGTM!Component structure is clean. The light DOM rendering and fixed positioning are appropriate for a game overlay. The
pointerEvents: 'none'ensures the timer doesn't interfere with user interactions.
26-28: LGTM!Simple initialization method that enables visibility when the layer is added to the renderer.
30-65: LGTM!The tick method correctly manages the timer visibility and progress calculation:
- Adjusts position based on team ownership bar
- Hides for durations ≤ 5 seconds (too brief to be useful)
- Correctly calculates progress ratio during immunity period
- Properly handles edge cases (before spawn phase, after immunity ends)
67-72: LGTM!The
setInactivemethod properly manages the active state with a guard to prevent unnecessary updates.
74-93: LGTM!The render method efficiently shows an empty template when inactive and renders an orange progress bar when active. The smooth transition animation (100ms) provides good visual feedback. The z-index layering is correct (1000 on root, 999 on inner container).
src/server/MapPlaylist.ts (1)
96-97: LGTM!The default game configuration values for public matchmaking are appropriate:
startingGold: 0(no bonus)goldMultiplier: 1(normal income)spawnImmunityDuration: 50ticks (5 seconds)These provide a balanced default experience without modifying the core game economy.
Also applies to: 105-105
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/core/game/TransportShipUtils.ts (1)
11-19: Consider extracting spawn immunity check to a shared helper.This spawn immunity check pattern appears in multiple files (TransportShipExecution, WarshipExecution, PlayerImpl, etc.). Consider adding a helper method to the Config or Game interface:
isInSpawnImmunity(): boolean { return this.config().numSpawnPhaseTurns() + this.config().spawnImmunityDuration() > this.ticks(); }Then this code could be simplified to:
- if ( - game.config().numSpawnPhaseTurns() + game.config().spawnImmunityDuration() > - game.ticks() - ) { + if (game.isInSpawnImmunity()) { const targetOwner = game.owner(tile); if (targetOwner.isPlayer() && targetOwner.type() === PlayerType.Human) { return false; } }This would improve maintainability and reduce duplication.
src/core/execution/WarshipExecution.ts (1)
288-294: Good pattern, but inconsistent with other execution files.The
isSpawnImmunityActive()helper method is a clean way to encapsulate the spawn immunity check. However, this pattern is not used in TransportShipExecution, TransportShipUtils, or PlayerImpl, which inline the same check.Consider either:
- Extracting this to a shared Game interface method (as suggested in TransportShipUtils review)
- Or adding similar helper methods to other execution classes for consistency
The current approach with a private helper is acceptable, but a shared solution would be better.
src/core/configuration/DefaultConfig.ts (1)
35-35: Consider adding a comment to document the precision level.The constant provides 3 decimal places of precision (0.001) for the gold multiplier. A brief comment would help future maintainers understand this choice.
Apply this diff to add documentation:
+// Provides 0.001 precision for gold multiplier (e.g., 1.5x, 2.3x, 0.567x) const GOLD_MULTIPLIER_PRECISION = 1000n;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
resources/lang/en.json(2 hunks)src/client/graphics/GameRenderer.ts(3 hunks)src/client/index.html(1 hunks)src/core/configuration/Config.ts(1 hunks)src/core/configuration/DefaultConfig.ts(7 hunks)src/core/execution/AttackExecution.ts(1 hunks)src/core/execution/TransportShipExecution.ts(2 hunks)src/core/execution/WarshipExecution.ts(2 hunks)src/core/game/PlayerImpl.ts(4 hunks)src/core/game/TransportShipUtils.ts(1 hunks)tests/util/Setup.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/util/Setup.ts
- src/core/execution/AttackExecution.ts
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
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: 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.
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/index.html
📚 Learning: 2025-08-12T00:31:50.144Z
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.
Applied to files:
src/core/game/TransportShipUtils.tssrc/core/configuration/Config.tssrc/core/configuration/DefaultConfig.tssrc/core/game/PlayerImpl.tssrc/core/execution/TransportShipExecution.ts
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
Repo: openfrontio/OpenFrontIO PR: 2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/core/configuration/Config.tssrc/core/game/PlayerImpl.ts
📚 Learning: 2025-10-20T20:15:28.858Z
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.
Applied to files:
src/core/configuration/Config.tssrc/core/execution/WarshipExecution.tssrc/core/configuration/DefaultConfig.tssrc/core/game/PlayerImpl.tssrc/core/execution/TransportShipExecution.ts
📚 Learning: 2025-10-18T17:54:01.311Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.311Z
Learning: In src/core/execution/FakeHumanExecution.ts, MIRVs and ground attacks should not be mutually exclusive. The considerMIRV() method should not short-circuit maybeAttack() - both actions can occur in the same tick.
Applied to files:
src/core/execution/WarshipExecution.tssrc/core/game/PlayerImpl.tssrc/core/execution/TransportShipExecution.ts
📚 Learning: 2025-08-24T11:13:08.495Z
Learnt from: DevelopingTom
Repo: openfrontio/OpenFrontIO PR: 1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.
Applied to files:
src/core/execution/WarshipExecution.ts
📚 Learning: 2025-11-08T16:57:19.090Z
Learnt from: mepoohsta
Repo: openfrontio/OpenFrontIO PR: 2413
File: src/core/game/RailNetworkImpl.ts:298-311
Timestamp: 2025-11-08T16:57:19.090Z
Learning: In src/core/game/RailNetworkImpl.ts, the getDestinationWeight method calculates route profitability (not just length) for train destination selection. The heavy relationship bonuses (ally +50, team/other +25, self +10) compared to the base +1 per station are intentional, as profitability considers both route length and player relationships. Longer routes are usually more profitable, but shorter routes to allies can be more valuable.
Applied to files:
src/core/configuration/DefaultConfig.ts
📚 Learning: 2025-05-31T18:15:03.445Z
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.
Applied to files:
src/core/configuration/DefaultConfig.tssrc/core/game/PlayerImpl.tssrc/core/execution/TransportShipExecution.ts
📚 Learning: 2025-10-08T17:14:49.369Z
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.
Applied to files:
src/core/configuration/DefaultConfig.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from UnitType values (specifically bomb-related values like AtomBomb, HydrogenBomb, MIRV, and MIRVWarhead) rather than a separate enum. This means comparing NukeType values against UnitType values in switch statements is valid and intentional.
Applied to files:
src/core/configuration/DefaultConfig.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from specific UnitType enum values (AtomBomb, HydrogenBomb, MIRV, MIRVWarhead). This means that comparisons in switch statements between a NukeType parameter and UnitType enum values are valid and will work correctly at runtime.
Applied to files:
src/core/configuration/DefaultConfig.ts
📚 Learning: 2025-10-20T11:02:16.969Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.
Applied to files:
src/core/game/PlayerImpl.tssrc/core/execution/TransportShipExecution.ts
📚 Learning: 2025-11-01T00:24:33.860Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2161
File: src/core/execution/FakeHumanExecution.ts:670-678
Timestamp: 2025-11-01T00:24:33.860Z
Learning: In OpenFrontIO, PlayerType.Bot entities cannot be in teams and do not have friendliness relationships. Only PlayerType.Human and PlayerType.FakeHuman can participate in teams and alliances. Therefore, when targeting bot-owned tiles, friendliness checks like `owner.isFriendly(this.player)` are unnecessary and meaningless for bots.
Applied to files:
src/core/game/PlayerImpl.ts
📚 Learning: 2025-10-18T11:00:57.142Z
Learnt from: NewYearNewPhil
Repo: openfrontio/OpenFrontIO PR: 2230
File: src/client/graphics/GameRenderer.ts:269-277
Timestamp: 2025-10-18T11:00:57.142Z
Learning: In src/client/graphics/GameRenderer.ts, the GameRecapCapture implementation does not use setCaptureRenderEnabled on layers. Instead, it uses RecapCaptureSurface.capture() to render capture layers (TerrainLayer, TerritoryLayer, RailroadLayer, StructureIconsLayer, UnitLayer) directly to an off-screen canvas without requiring layer-level capture mode methods.
Applied to files:
src/client/graphics/GameRenderer.ts
📚 Learning: 2025-10-27T09:47:26.395Z
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.
Applied to files:
src/core/execution/TransportShipExecution.ts
📚 Learning: 2025-05-21T04:10:33.435Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as a type inferred from a Zod schema that is marked as optional, which means PlayerStats already includes undefined as a possible type (PlayerStats | undefined).
Applied to files:
src/core/execution/TransportShipExecution.ts
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.
Applied to files:
resources/lang/en.json
🧬 Code graph analysis (5)
src/core/game/TransportShipUtils.ts (2)
src/core/game/Game.ts (2)
Game(664-748)Player(526-662)src/core/game/GameMap.ts (1)
TileRef(3-3)
src/core/configuration/Config.ts (1)
src/core/game/Game.ts (2)
PlayerInfo(406-420)Gold(24-24)
src/core/configuration/DefaultConfig.ts (2)
src/core/Util.ts (1)
within(37-39)src/core/game/Game.ts (2)
Gold(24-24)PlayerInfo(406-420)
src/core/game/PlayerImpl.ts (3)
src/core/game/GameImpl.ts (1)
owner(183-185)src/core/game/GameView.ts (3)
owner(109-111)owner(647-649)tile(106-108)src/core/game/UnitImpl.ts (2)
owner(179-181)tile(176-178)
src/core/execution/TransportShipExecution.ts (1)
src/core/game/Game.ts (1)
Player(526-662)
🔇 Additional comments (9)
src/core/game/PlayerImpl.ts (1)
1190-1232: Good refactoring with local owner variable.The introduction of the
ownervariable at line 1191 and its use throughout the method improves readability and reduces repeatedthis.mg.owner(tile)calls. This is a clean refactor that makes the code easier to follow.The spawn immunity check correctly gates attacks against human players during the immunity window.
src/core/configuration/Config.ts (1)
97-97: Implementation verified and correct.The
startingGoldmethod is properly implemented in DefaultConfig.ts (line 853) with the exact signature matching the interface. The method correctly returns a Gold value based on player type, following the established pattern.src/core/configuration/DefaultConfig.ts (7)
263-264: LGTM - clean config extraction.Moving from hardcoded value to gameConfig aligns with the PR objective to make spawn immunity duration configurable.
282-284: Good defensive programming with the clamp.The
within()call prevents invalid multiplier values from causing issues. Returning 1 as the default (via??) maintains backward compatibility when the config field is absent.
286-300: Solid fixed-point arithmetic implementation.The early returns optimize for common cases (zero base, no multiplier). The math is correct: scales multiplier by 1000, multiplies, then divides for precise fractional scaling. Integer division truncation is expected behavior.
382-397: Clean refactor to apply the multiplier consistently.Calculating base values then passing through
applyGoldMultiplieris clearer than inline scaling.
409-418: Correct application of multiplier to final trade ship gold.The multiplier applies to the fully calculated value (base gold × bonus), which matches the PR objective to multiply trade ship income.
853-858: Correct implementation - starting gold separate from multiplier.Returns configured starting gold for Human players only, defaulting to 0 if not set. Bots correctly get 0n. This matches the PR description where starting gold is a distinct setting from the income multiplier.
923-926: Consistent multiplier application to passive income.Bot base of 50n and Human base of 100n correctly scaled by the multiplier. This completes the gold multiplier coverage for all income sources mentioned in the PR objectives.
| ${ | ||
| this.startingGoldEnabled | ||
| ? renderNumber(this.startingGold) | ||
| : translateText("user_setting.off") |
There was a problem hiding this comment.
Could we just not show anything instead of "off", like with the game length option?
Same goes for Gold multiplier and Ceasefire timer
| let base: Gold; | ||
| switch (rel) { | ||
| case "ally": | ||
| return 50_000n; | ||
| base = 50_000n; | ||
| break; | ||
| case "team": | ||
| case "other": | ||
| return 25_000n; | ||
| base = 25_000n; | ||
| break; | ||
| case "self": | ||
| return 10_000n; | ||
| base = 10_000n; | ||
| break; | ||
| } | ||
| return this.applyGoldMultiplier(base); |
There was a problem hiding this comment.
Why do we apply multiplier here in bonus when we already apply multiplier in tradeShipGold?
| return; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Should we also add console.warn("cannot attack player during immunity phase"); here as in AttackExecution?
| this.patrol(); | ||
|
|
||
| if (this.warship.targetUnit() !== undefined) { | ||
| if (!spawnImmunityActive && this.warship.targetUnit() !== undefined) { |
There was a problem hiding this comment.
This will also disallow nations' warships from attacking. We should check if the target unit's owner is a human
| if (this.target.isPlayer()) { | ||
| const targetPlayer = this.target as Player; | ||
| if ( | ||
| targetPlayer.type() === PlayerType.Human && |
There was a problem hiding this comment.
This (and also changes in TransportShipExecution, and WarShipExecution) allows humans to attack nations (and bots), but does not allow them to attack back, meaning they are free targets.
We should if check the attacker is a human too
Lavodan
left a comment
Lavodan
left a comment
There was a problem hiding this comment.
Looks good. I left a few comments.
Regarding the ceasefire, you should also disallow launching nukes and MIRVs for completeness. Maybe just check if the nuke would break alliance if the players were allied to see if the nuke should be allowed?
|
Ah I now see this PR is only meant for the gold options, my bad. |
4 participants
Split off from #2458 so its easier to review.
Also includes the lobby creation UI for #2460, as well as the changes from that PR. To see only the changes for the gold options:
NewYearNewPhil/OpenFrontIO@lobby-gold-options~6...lobby-gold-options
Description:
Includes the following additional options when creating lobbies:
HostLobbyModal:
SinglePlayerModal (same as HostLobbyModal, but without spawn immunity setting):
Please complete the following:
I don't think tests are required for these features, I'm happy to implement them if a reviewer disagrees.
Please put your Discord username so you can be contacted if a bug or regression is found:
newyearnewphil / [CYN] super mario