Configurable ceasefire time #2460
Conversation
WalkthroughThis PR adds a configurable spawn-immunity feature: UI inputs for hosts and single-player, a new ceasefire timer UI component, schema and default config updates, and enforcement of spawn-immunity across attack, transport, and warship execution logic and related utilities/tests. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as Host / SinglePlayer UI
participant Server as GameServer / GameManager
participant Config as GameConfig
participant Exec as Execution Logic (Attack / Transport / Warship)
participant Render as CeasefireTimer
User->>UI: set spawnImmunityDuration (s)
UI->>Server: send updated config (spawnImmunityDuration in tenths)
Server->>Config: persist update
par Game loop
Exec->>Config: read numSpawnPhaseTurns & spawnImmunityDuration
Exec->>Exec: compute immunityWindow = numSpawnPhaseTurns + spawnImmunityDuration
alt ticks < immunityWindow (immunity active)
Exec->>Exec: block attacks / clear targets / prevent transports
Render->>Config: read ticks & immunityWindow
Render->>Render: compute progressRatio
Render->>User: display progress bar
else immunity expired
Exec->>Exec: allow normal targeting/actions
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (3 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 (5)
src/client/graphics/layers/CeasefireTimer.ts (2)
9-9: Use definite assignment assertion for the game property.The
gameproperty is assigned byGameRendererafter construction but TypeScript doesn't know this. Use the definite assignment assertion operator to clarify the initialization pattern:- public game: GameView; + public game!: GameView;
44-46: Extract minimum duration threshold to a named constant.The check
ceasefireDuration <= 5 * 10uses a magic number. Extract this to a named constant for clarity:+const MIN_CEASEFIRE_DURATION_TO_SHOW = 5 * 10; // 5 seconds in ticks + @customElement("ceasefire-timer") export class CeasefireTimer extends LitElement implements Layer { // ... tick() { // ... - if (ceasefireDuration <= 5 * 10 || this.game.inSpawnPhase()) { + if (ceasefireDuration <= MIN_CEASEFIRE_DURATION_TO_SHOW || this.game.inSpawnPhase()) { this.setInactive(); return; }src/client/SinglePlayerModal.ts (1)
588-588: Consider adding UI control for spawn immunity duration.The spawn immunity duration is hardcoded to
5 * 10(5 seconds) for single player games. Since the PR description mentions "Integration into HostLobbyModal and SingleplayerModal will be changed later," this is likely intentional for now.However, for consistency with other configurable options in this modal (bots, max timer, etc.), consider adding a UI input for this setting in a follow-up change.
src/core/game/TransportShipUtils.ts (1)
11-19: Consider centralizing spawn immunity logic.The spawn immunity check is now spread across multiple files:
AttackExecution.ts(lines 93-105)TransportShipExecution.ts(lines 92-103)TransportShipUtils.ts(lines 11-19, this file)This creates maintenance risk—if the spawn immunity rules change, you'll need to update multiple locations.
Consider extracting spawn immunity logic into a centralized helper:
// src/core/game/SpawnImmunityUtils.ts export function isProtectedBySpawnImmunity( game: Game, target: Player | TerraNullius ): boolean { if (!target.isPlayer()) return false; const targetPlayer = target as Player; if (targetPlayer.type() !== PlayerType.Human) return false; return ( game.config().numSpawnPhaseTurns() + game.config().spawnImmunityDuration() > game.ticks() ); }Then use it consistently:
if (isProtectedBySpawnImmunity(game, targetOwner)) { return false; }This ensures the rules stay consistent and are easier to test and maintain.
src/client/HostLobbyModal.ts (1)
528-546: Add blur handler and consider debouncing.The spawn immunity input implementation is solid, but consider two improvements:
Missing blur handler: The max timer input (lines 519-521) has both
@inputand@blurhandlers, but the spawn immunity input only has@input. Add a blur handler for consistency and to ensure validation happens when the user tabs out.Inconsistent debouncing: The bots input (lines 685-704) uses debouncing to avoid excessive API calls. The spawn immunity input triggers
putGameConfig()on every input event, which could be inefficient if the user is typing multiple digits.Apply this diff to add blur handling and optional debouncing:
<label for="spawn-immunity-duration" class="option-card" > <input type="number" id="spawn-immunity-duration" min="0" max="300" step="1" .value=${String(this.spawnImmunityDurationSeconds)} style="width: 60px; color: black; text-align: right; border-radius: 8px;" @input=${this.handleSpawnImmunityDurationInput} + @blur=${this.handleSpawnImmunityDurationBlur} @keydown=${this.handleSpawnImmunityDurationKeyDown} /> <div class="option-card-title"> <span>${translateText("host_modal.spawn_immunity_duration")}</span> </div> </label>And add the blur handler:
private handleSpawnImmunityDurationBlur(e: Event) { const input = e.target as HTMLInputElement; const value = parseInt(input.value, 10); if (Number.isNaN(value) || value < 0) { input.value = "0"; this.spawnImmunityDurationSeconds = 0; } else if (value > 300) { input.value = "300"; this.spawnImmunityDurationSeconds = 300; } this.putGameConfig(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
resources/lang/debug.json(1 hunks)resources/lang/en.json(1 hunks)src/client/HostLobbyModal.ts(5 hunks)src/client/SinglePlayerModal.ts(1 hunks)src/client/graphics/GameRenderer.ts(3 hunks)src/client/graphics/layers/CeasefireTimer.ts(1 hunks)src/client/index.html(1 hunks)src/core/Schemas.ts(1 hunks)src/core/configuration/DefaultConfig.ts(1 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(3 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 (19)
📚 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/server/GameManager.tssrc/core/execution/AttackExecution.tssrc/core/execution/TransportShipExecution.tssrc/core/execution/WarshipExecution.tssrc/client/HostLobbyModal.tssrc/core/Schemas.tssrc/core/configuration/DefaultConfig.tstests/util/Setup.tssrc/core/game/PlayerImpl.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/server/GameServer.tssrc/core/game/TransportShipUtils.tssrc/core/Schemas.tssrc/client/SinglePlayerModal.tssrc/core/game/PlayerImpl.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/AttackExecution.tssrc/core/execution/TransportShipExecution.tssrc/core/execution/WarshipExecution.tssrc/core/game/PlayerImpl.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.tssrc/core/game/PlayerImpl.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-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/game/PlayerImpl.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-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.tssrc/core/Schemas.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/execution/TransportShipExecution.tssrc/core/game/PlayerImpl.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.json
📚 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-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/HostLobbyModal.tssrc/client/SinglePlayerModal.tssrc/client/graphics/layers/CeasefireTimer.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/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/client/HostLobbyModal.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-08-23T07:48:19.060Z
Learnt from: ElMelchizedek
Repo: openfrontio/OpenFrontIO PR: 1876
File: src/core/execution/FakeHumanExecution.ts:470-473
Timestamp: 2025-08-23T07:48:19.060Z
Learning: In FakeHumanExecution.ts DefensePost placement logic, returning -Infinity from structureSpawnTileValue when no sampled border tiles neighbor enemies is intentional. The logic samples up to 50 border tiles as a heuristic - if none are adjacent to enemies, it assumes DefensePost placement is unnecessary and aborts the entire placement attempt rather than continuing to evaluate individual tiles.
Applied to files:
src/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/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/game/PlayerImpl.ts
🧬 Code graph analysis (7)
src/server/GameServer.ts (2)
src/core/configuration/DefaultConfig.ts (1)
gameConfig(266-268)src/server/MapPlaylist.ts (1)
gameConfig(79-106)
src/core/execution/AttackExecution.ts (2)
src/core/Schemas.ts (1)
Player(117-117)src/core/game/Game.ts (1)
Player(526-662)
src/core/execution/TransportShipExecution.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/client/HostLobbyModal.ts (1)
src/client/Utils.ts (1)
translateText(92-147)
src/core/game/PlayerImpl.ts (2)
src/core/game/GameImpl.ts (1)
owner(183-185)src/core/game/GameView.ts (3)
owner(109-111)owner(636-638)tile(106-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)
🔇 Additional comments (21)
resources/lang/en.json (1)
252-252: LGTM!The localization key is clear and accurately describes the feature. The "(seconds)" suffix helps users understand the unit of measurement.
resources/lang/debug.json (1)
148-148: LGTM!The debug localization entry correctly follows the pattern of returning the key path as the translation value.
src/client/graphics/layers/CeasefireTimer.ts (2)
30-64: Well-structured timer logic.The tick method correctly handles the ceasefire timing window:
- Hides during spawn phase (before ceasefire starts)
- Shows only when ceasefire duration is meaningful (> 5 seconds)
- Calculates progress ratio based on elapsed ticks
- Adjusts position to avoid overlapping with team ownership bar
77-92: Clean render implementation.The render method efficiently returns an empty template when inactive and renders a smooth progress bar with CSS transitions when active.
src/client/graphics/GameRenderer.ts (2)
236-242: LGTM! Consistent layer integration pattern.The ceasefire timer initialization follows the same pattern as other layers like
spawnTimer, with proper type checking and error logging.
272-272: LGTM! Appropriate layer positioning.The ceasefire timer is correctly positioned in the layers array after
spawnTimerand beforeleaderboard, ensuring proper render order for the UI elements.src/server/GameManager.ts (1)
62-62: LGTM! Consistent default configuration.The default spawn immunity duration of
5 * 10(5 seconds) matches the values inSinglePlayerModal.tsandMapPlaylist.ts, providing consistent defaults across game modes.src/server/GameServer.ts (1)
121-123: LGTM! Standard config update pattern.The spawn immunity duration update follows the established pattern for partial config updates, checking for
undefinedbefore applying the change.src/server/MapPlaylist.ts (1)
103-103: LGTM! Consistent public game configuration.The spawn immunity duration default for public games matches the values in
GameManager.tsandSinglePlayerModal.ts, ensuring consistent behavior across all game modes.src/client/index.html (1)
407-407: LGTM!The custom element follows the existing pattern of declarative custom elements in the HTML. Placement alongside
<spawn-timer>is logical for a timer-related UI component.tests/util/Setup.ts (1)
67-67: LGTM!Setting
spawnImmunityDuration: 0as the default for tests is appropriate. This ensures tests don't have unintended spawn immunity behavior unless explicitly configured.src/client/HostLobbyModal.ts (3)
45-45: LGTM!The default of 5 seconds matches the previously hardcoded value in
DefaultConfig.ts(50 ticks = 5 seconds), ensuring backward compatibility.
711-726: LGTM!The input validation is thorough:
- Keydown handler prevents typing invalid characters at input time
- Input handler strips invalid characters and validates the range (0-300)
- Conversion to ticks (*10) correctly aligns with the schema's "10 per second" comment
817-817: LGTM!The conversion from seconds to ticks (
* 10) correctly aligns with the schema documentation stating ticks are at "10 per second".src/core/configuration/DefaultConfig.ts (1)
262-264: LGTM!Making spawn immunity duration configurable via
_gameConfigfollows the pattern used for other config values in this class. The field is required in the schema (no.optional()marker), so it is always defined and safe to access.src/core/execution/AttackExecution.ts (1)
93-105: Code is correct—spawn immunity protection is intentionally Human-only.The existing pattern across the codebase confirms spawn immunity protection applies only to
PlayerType.Human. This is by design:
- AttackExecution (line 96): Human-only guard
- TransportShipExecution (line 99): Human-only guard
- PlayerImpl.nukeSpawn (line 995): No guard—blocks ALL players from nuking (global game rule, not protection)
FakeHuman does not receive spawn immunity protection because it represents AI, not a real player. Spawn immunity exists to give real players (Human) time to learn during the spawn phase. AI players—whether FakeHuman or Bot—do not need this learning protection.
src/core/execution/TransportShipExecution.ts (1)
92-103: No changes needed. Code follows established codebase pattern.The spawn immunity check in
TransportShipExecution.ts(lines 99) correctly protects onlyPlayerType.Human, exactly matching the pattern used throughout the codebase:
AttackExecution.ts(lines 96-99): checkstargetPlayer.type() === PlayerType.HumanTransportShipUtils.ts(line 16): checkstargetOwner.type() === PlayerType.HumanPlayerImpl.tscanAttack() (line 1190): checksowner.type() === PlayerType.HumanFakeHuman players are intentionally excluded from spawn immunity protection. This is consistent game design—the immunity shields human players while bots remain vulnerable during early game.
src/core/Schemas.ts (1)
175-175: No changes required. The spawnImmunityDuration field is already provided at all GameConfig construction sites.All verified GameConfig construction locations include
spawnImmunityDuration:
- tests/util/Setup.ts sets it to 0
- src/server/GameManager.ts defaults to 50 ticks (5 * 10)
- src/server/MapPlaylist.ts sets it to 50 ticks
- src/client/SinglePlayerModal.ts sets it to 50 ticks
- src/client/HostLobbyModal.ts dynamically converts from user input
The original review incorrectly identified a breaking change based on incomplete output. The schema field is not optional, but all code that creates GameConfig objects already provides it, so validation succeeds.
Likely an incorrect or invalid review comment.
src/core/execution/WarshipExecution.ts (1)
68-85: Ceasefire guard for warships looks correct.Thanks for clearing targets and skipping the trade-ship hunt while the ceasefire window lasts; this keeps warships passive without disturbing patrol flow.
src/core/game/PlayerImpl.ts (2)
994-1001: Nuke spawn guard covers the missile ban cleanly.The early return keeps silos idle during the ceasefire, so we stay in line with the no-missile rule without touching later launch logic.
1187-1212: Guarding human targets matches the ceasefire rule.Caching the owner and gating only Human targets blocks PvP until the timer expires while still letting folks push into neutral land.
| ${translateText("host_modal.max_timer")} | ||
| </div> | ||
| </label> | ||
There was a problem hiding this comment.
can this have a checkbox, like game timer?
also i think minutes is a better unit than seconds
There was a problem hiding this comment.
yes, the issue with that is that it would be checked by default, and might be confusing to users a little bit because the 5 sec spawn immunity was not exposed in any UI previously
in the follow up PR for the other options, the card in the host modal is changed to have a slider
if the slider is kept, a max value or individiual step sizes are required. or i revert to number input and go for minutes, your call
| if ( | ||
| game.config().numSpawnPhaseTurns() + game.config().spawnImmunityDuration() > | ||
| game.ticks() | ||
| ) { | ||
| const targetOwner = game.owner(tile); | ||
| if (targetOwner.isPlayer() && targetOwner.type() === PlayerType.Human) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
same here, can we use canAttack?
| // startingGold: z.number().int().min(0).max(10*1000*1000), // maybe in steps instead? good way of setting max? default 0? | ||
| // incomeMultiplier: z.number().min(0).max(10), |
There was a problem hiding this comment.
please remove these comments
| maxTimerValue: z.number().int().min(1).max(120).optional(), | ||
| // startingGold: z.number().int().min(0).max(10*1000*1000), // maybe in steps instead? good way of setting max? default 0? | ||
| // incomeMultiplier: z.number().min(0).max(10), | ||
| spawnImmunityDuration: z.number().int().min(0).max(3000), // In ticks (10 per second) |
There was a problem hiding this comment.
make optional, do we need a max?
| randomSpawn: false, | ||
| gameMode: GameMode.FFA, | ||
| bots: 400, | ||
| spawnImmunityDuration: 5 * 10, |
There was a problem hiding this comment.
let's remove this option, in Config we can use a default if not set.
|
This pull request is stale because it has been open for fourteen days with no activity. If you want to keep this pull request open, add a comment or update the branch. |
github-actions
bot
added
the
Stale
iiamlewis
added
UI/UX
|
@NewYearNewPhil As other posts, can we resolve conflicts and we can look to review and merge |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
resources/lang/en.jsonsrc/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- resources/lang/en.json
🧰 Additional context used
🧠 Learnings (5)
📚 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/SinglePlayerModal.tssrc/client/HostLobbyModal.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/client/SinglePlayerModal.tssrc/client/HostLobbyModal.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/client/SinglePlayerModal.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/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/client/HostLobbyModal.ts
🧬 Code graph analysis (1)
src/client/HostLobbyModal.ts (2)
src/client/LangSelector.ts (1)
translateText(258-278)src/client/Utils.ts (1)
translateText(92-147)
🔇 Additional comments (5)
src/client/SinglePlayerModal.ts (1)
585-585: Hardcoded spawn immunity duration looks correct for now.The value
5 * 10(50 tenths = 5 seconds) matches the default in HostLobbyModal. Per PR description, making this configurable in SinglePlayerModal is planned for PR #2461.Consider adding a brief comment or extracting a constant (e.g.,
DEFAULT_SPAWN_IMMUNITY_TICKS) to clarify the unit conversion for future readers.src/client/HostLobbyModal.ts (4)
49-49: Good naming for unit clarity.The property name
spawnImmunityDurationSecondsclearly indicates the unit, which helps avoid confusion when converting to tenths for the server.
525-545: UI implementation looks good.The input is consistent with the max timer pattern. The 0-300 second range (0-5 minutes) provides reasonable flexibility for hosts.
713-728: Input validation is consistent with existing patterns.Good use of
Number.isNaN()instead ofisNaN()for stricter type checking. The keydown prevention and input sanitization follow the same approach ashandleMaxTimerValueChanges.
819-819: Conversion to tenths is correct.Multiplying by 10 converts seconds to tenths (the schema unit). The max input of 300 seconds yields 3000 tenths, which aligns with the schema bounds.
| TeamCountConfig, | ||
| } from "../core/Schemas"; | ||
| import { generateID } from "../core/Util"; | ||
| import "./components/baseComponents/Button"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n '<o-button' src/client/HostLobbyModal.tsRepository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Also check for any references to Button or o-button in different contexts
rg -n 'o-button|<o-button|Button' src/client/HostLobbyModal.ts | head -30Repository: openfrontio/OpenFrontIO
Length of output: 214
Remove the unused Button import.
Line 26 imports the Button component as a side-effect to register the <o-button> custom element, but the element is not used anywhere in this file. The template uses a native <button> element instead. Remove this import to keep the file clean.
🤖 Prompt for AI Agents
In src/client/HostLobbyModal.ts around line 26, the side-effect import `import
"./components/baseComponents/Button";` is unused because this file's template
uses a native <button>, so remove that import line to clean up the file; simply
delete the import statement (or comment it out) and ensure no other code in this
file depends on the custom element before committing.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
6 participants
Split off from #2458 so its easier to review.
The integration into the
HostLobbyModalandSingleplayerModalare changed later (in #2461)Description:
Includes the following additional option when creating lobbies:
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