Max timer #1289
Max timer #1289
Conversation
WalkthroughA new optional "max timer" feature was added to the game. This allows users to set a maximum game duration in minutes through the UI, which is enforced in both single-player and host lobbies. The timer is integrated into game logic, server configuration, and visual display, with supporting tests and localization. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (Host/Single Modal)
participant Server
participant Game Logic
User->>UI (Host/Single Modal): Enable Max Timer, input value
UI (Host/Single Modal)->>Server: Send maxTimerValue in config
Server->>Game Logic: Store maxTimerValue
Game Logic->>Game Logic: On spawn phase, set timer = maxTimerValue * 60
Game Logic->>Game Logic: Every 10 ticks, decrement timer
Game Logic->>Game Logic: If timer == 0, trigger win condition
Game Logic->>UI (Options Menu): Update timer display (red if < 60s)
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/core/execution/WinCheckExecution.ts (1)
22-22: Remove the unnecessary empty constructor.Static analysis correctly identifies this constructor as unnecessary since it does nothing.
- constructor() {} -src/client/HostLobbyModal.ts (1)
496-513: Consider extracting shared validation logic.The input validation handlers are identical to SinglePlayerModal. This duplication could be extracted into a shared utility function to follow DRY principles.
Create a shared utility like:
// utils/TimerInputValidation.ts export const handleTimerKeyDown = (e: KeyboardEvent) => { if (["-", "+", "e"].includes(e.key)) { e.preventDefault(); } }; export const handleTimerInput = (e: Event): number | undefined => { const input = e.target as HTMLInputElement; input.value = input.value.replace(/[e\+\-]/gi, ""); const value = parseInt(input.value); return (isNaN(value) || value < 0 || value > 400) ? undefined : value; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
resources/lang/en.json(2 hunks)src/client/HostLobbyModal.ts(4 hunks)src/client/SinglePlayerModal.ts(4 hunks)src/client/graphics/layers/OptionsMenu.ts(2 hunks)src/core/Schemas.ts(1 hunks)src/core/execution/WinCheckExecution.ts(3 hunks)src/server/GameManager.ts(1 hunks)src/server/GameServer.ts(1 hunks)src/server/MapPlaylist.ts(1 hunks)tests/core/execution/WinCheckExecution.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
src/core/Schemas.ts (3)
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#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).
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#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).
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#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.
src/client/graphics/layers/OptionsMenu.ts (1)
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#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.
resources/lang/en.json (2)
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#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.
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#949
File: resources/lang/en.json:8-10
Timestamp: 2025-05-30T03:53:52.231Z
Learning: For the OpenFrontIO project, do not suggest updating translation files in resources/lang/*.json except for en.json. The project has a dedicated translation team that handles all other locale files.
src/client/HostLobbyModal.ts (2)
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#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.
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#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.
src/client/SinglePlayerModal.ts (2)
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#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.
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#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.
🧬 Code Graph Analysis (3)
tests/core/execution/WinCheckExecution.test.ts (2)
src/core/execution/WinCheckExecution.ts (1)
WinCheckExecution(15-111)tests/util/Setup.ts (1)
setup(22-83)
src/client/HostLobbyModal.ts (2)
src/client/LangSelector.ts (1)
translateText(222-242)src/client/Utils.ts (1)
translateText(79-98)
src/client/SinglePlayerModal.ts (2)
src/client/LangSelector.ts (1)
translateText(222-242)src/client/Utils.ts (1)
translateText(79-98)
🪛 GitHub Actions: 🧪 CI
tests/core/execution/WinCheckExecution.test.ts
[error] 45-45: ENOENT: no such file or directory, open '/home/runner/work/OpenFrontIO/OpenFrontIO/tests/testdata/maps/BigPlains/map.bin'. This caused multiple test failures in WinCheckExecution tests.
🪛 Biome (1.9.4)
src/core/execution/WinCheckExecution.ts
[error] 22-23: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🔇 Additional comments (17)
src/server/GameManager.ts (1)
39-39: LGTM: Max timer property added correctly.The addition follows the established pattern for optional configuration properties and aligns with the schema definition.
src/core/Schemas.ts (1)
143-143: Good validation range for timer feature.The 1-120 minute range provides sensible bounds for game timers. The validation correctly enforces integer values and marks the field as optional.
src/server/MapPlaylist.ts (1)
59-59: Consistent implementation across configuration objects.The max timer property is properly included in the map playlist configuration, maintaining consistency with other configuration files.
resources/lang/en.json (1)
126-126: Clear and descriptive localization text.The "Max timer (minutes)" text is simple and clear for users. Good practice including the unit in the label to avoid confusion.
Also applies to: 198-198
src/server/GameServer.ts (1)
93-95: Update logic follows established pattern.The max timer update implementation is consistent with other configuration properties in the method. Clean and predictable code structure.
src/client/graphics/layers/OptionsMenu.ts (2)
151-164: Timer logic implementation looks solid.The countdown timer logic correctly initializes to
maxTimerValue * 60seconds during spawn phase and decrements every 10 ticks. The fallback to original count-up behavior whenmaxTimerValueis undefined maintains backward compatibility.
193-196: Good visual feedback for timer urgency.The red color (
#ff8080) when timer drops below 60 seconds provides clear visual warning to players. The conditional styling is implemented correctly.tests/core/execution/WinCheckExecution.test.ts (1)
21-91: Comprehensive test coverage for timer functionality.The test suite properly covers:
- Timer initialization from maxTimerValue
- Timer decrement behavior every 10 ticks
- Game mode routing to correct win check methods
- Win conditions for both tile percentage and timer expiration
- Edge cases like no players
The mocking approach using
jest.fn()and spies is clean and effective.src/client/SinglePlayerModal.ts (3)
38-39: Clean state management for max timer feature.The reactive state properties
maxTimerandmaxTimerValuefollow the established pattern in the component.
388-404: Good input validation preventing invalid characters.The keydown handler blocks problematic characters (
-,+,e) and the input handler removes them via regex. The range validation is also properly implemented.
479-479: Correct conditional inclusion in game config.The ternary operator properly includes
maxTimerValueonly whenmaxTimeris enabled, otherwiseundefinedto exclude it from the config.src/core/execution/WinCheckExecution.ts (4)
19-20: Good documentation for the timer property.The comment clearly explains the timer represents remaining seconds and is undefined when disabled.
26-29: Timer initialization logic is correct.Properly converts minutes to seconds (
maxTimerValue * 60) and only initializes whenmaxTimerValueis defined.
38-40: Safe timer decrement implementation.Using
Math.max(0, this.timer - 1)prevents the timer from going negative, which is the correct behavior.
62-63: Timer-based win condition properly integrated.Both FFA and Team win check methods correctly include
this.timer === 0as an additional win condition alongside the existing tile percentage logic.Also applies to: 93-96
src/client/HostLobbyModal.ts (2)
37-38: Consistent state management with SinglePlayerModal.The same reactive state properties maintain consistency between single-player and multiplayer hosting experiences.
551-552: Correct conditional config inclusion matches SinglePlayerModal.The ternary logic properly includes
maxTimerValueonly when enabled, maintaining consistency across both modals.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
|
|
|
What is the state of this PR now that v24 is about to be released? |
drillskibo
left a comment
drillskibo
left a comment
There was a problem hiding this comment.
Please remove unrelated commits, make the requested changes above, and fix potential conflicts.
drillskibo
dismissed
coderabbitai[bot]’s stale review
The base branch was changed.
drillskibo
added
the
Feature - Backend
drillskibo
added
the
Feature - Frontend
Co-authored-by: Loymdayddaud <[email protected]>
|
If we want this in v26 it needs to be rebased into that branch. |
| } | ||
| } else { | ||
| if (this.game.inSpawnPhase()) { | ||
| this.timer = 0; |
There was a problem hiding this comment.
instead of a seperate timer field, i think we can just calculate it in render:
const timeLeft = this.game.config().gameConfig().maxTimerValue - this.game.ticks() - this.game.config().spawnDuration()
github-project-automation
bot
moved this from Development
to Final Review
in OpenFront Release Management
github-project-automation
bot
moved this from Final Review
to Complete
in OpenFront Release Management
## Description: Adds a max timer setting The timer starts at max timer and goes down, becoming red if reaching < 1 min The player with the biggest territory wins at the end of the timer  ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced - [x] I understand that submitting code with bugs that could have been caught through manual testing blocks releases and new features for all contributors ## Please put your Discord username so you can be contacted if a bug or regression is found: Vivacious Box --------- Co-authored-by: Loymdayddaud <[email protected]>
6 participants
Description:
Adds a max timer setting
The timer starts at max timer and goes down, becoming red if reaching < 1 min
The player with the biggest territory wins at the end of the timer
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
Vivacious Box