fix: reset HUD/input between games and harden conquest/retreat/silo edge cases#4544
fix: reset HUD/input between games and harden conquest/retreat/silo edge cases#4544Ahmet-Dedeler wants to merge 1 commit into
Conversation
…dge cases Page-lifetime HUD elements and InputHandler were leaking listeners and state across games; also fix zombie retreat executions, repeat conquest, conquest gold display, and silo downgrade cooldown. Co-authored-by: Cursor <cursoragent@cursor.com>
WalkthroughThis PR fixes cleanup bugs across client lifecycle code (connection checks, input listeners) and HUD layers (state reset on init, stable event handlers), and hardens game logic with a repeat-conquest guard, an attack-retreat-cancellation fix, and a missile silo cooldown trimming correction, all backed by new tests. ChangesClient Lifecycle and HUD Reset
Estimated code review effort: 3 (Moderate) | ~25 minutes Repeat Conquest and Execution Fixes
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Game
participant AttackExecution
participant Attack
Game->>AttackExecution: tick()
AttackExecution->>Attack: isActive()?
alt attack deleted externally
Attack-->>AttackExecution: false
AttackExecution->>AttackExecution: set active=false, return
else attack still active
Attack-->>AttackExecution: true
AttackExecution->>Attack: retreating()?
AttackExecution->>AttackExecution: continue normal handling
end
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
github-actions
Bot
added
the
auto-closed-needs-issue
|
Hi @Ahmet-Dedeler, thanks for the contribution. This PR was automatically closed because it doesn't fit our contribution workflow:
To contribute to OpenFront:
If you believe this was closed in error, please reach out on our Discord or comment below. See CONTRIBUTING.md for the full contribution process. — Automated PR gate. Source. |
github-project-automation
Bot
moved this from Triage
to Complete
in OpenFront Release Management
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 763ceeb5c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const target: Player = this.target; | ||
| // Island survivors can sit under the 100-tile threshold for many ticks; | ||
| // only conquer once so gold/kills aren't recorded repeatedly. | ||
| if (this.mg.hasConqueredPlayer(target.id())) return; |
There was a problem hiding this comment.
When a defender has already been marked conquered, this early return skips the cleanup loop below that transfers the remaining <100 tiles. In island or narrow-territory cases the first 10-pass sweep can leave disconnected or deep interior tiles behind; any later landing/attack on those leftovers will now only skip conquerPlayer and also skip the sweep, so the defeated player can keep remnants that used to be cleaned up. The repeat-gold guard should avoid only the reward/stat call, not the tile redistribution.
Useful? React with 👍 / 👎.
| const id = pv.id(); | ||
| // fetch only if we don't have it or the player changed | ||
| if (this._profileForPlayerId !== Number(id)) { | ||
| if (this._profileForPlayerId !== id) { |
There was a problem hiding this comment.
This cache never refreshes while the selected PlayerID stays the same, but PlayerProfile contains dynamic relation/alliance data used by renderRelationPillIfNation. If the user keeps or reopens the panel for the same nation after attacks, decay, or alliance changes, tick() skips pv.profile() and the panel keeps showing the stale relation indefinitely; clear the cached id on show/hide or refresh it periodically/on relevant updates.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/core/execution/AttackExecution.ts (1)
389-421: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the mop-up loop running
GameImpl.conquerPlayer()already no-ops on repeat ids, so this early return only blocks the 10-pass cleanup on later ticks. Move the guard to theconquerPlayer()call, or drop it, so island tiles can still be absorbed after the first conquest.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/execution/AttackExecution.ts` around lines 389 - 421, The mop-up cleanup in handleDeadDefender is being skipped on later ticks because the early return prevents the 10-pass island absorption loop from running after the first conquest. Keep the repeat-id guard tied to the mg.conquerPlayer(this._owner, target) call in AttackExecution.handleDeadDefender, but allow the rest of the method to continue so the tile cleanup logic still runs even when the player was already conquered.src/client/hud/layers/Leaderboard.ts (1)
103-104: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard against a zero denominator.
If fallout ever covers every land tile,
numTilesWithoutFalloutbecomes0and the leaderboard percentage becomesInfinity/NaN. The priornumLandTiles()denominator couldn't hit zero, so this is a new edge case introduced by the change.🛠️ Proposed fix
const numTilesWithoutFallout = - this.game.numLandTiles() - this.game.numTilesWithFallout(); + Math.max(1, this.game.numLandTiles() - this.game.numTilesWithFallout());Also applies to: 117-119, 148-150
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/hud/layers/Leaderboard.ts` around lines 103 - 104, Guard the leaderboard percentage calculations against a zero denominator after switching to numTilesWithoutFallout in Leaderboard. Update the affected computations in the Leaderboard logic (including the shared calculations around the referenced percentage blocks) so that when numTilesWithoutFallout is 0 you return a safe fallback value instead of dividing, and ensure the same protection is applied wherever that denominator is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/ClientGameRunner.ts`:
- Around line 987-994: The stop() cleanup in ClientGameRunner currently tears
down transport/input state but leaves the EventBus listeners registered from
start(), so bound handlers will accumulate across runs. Update ClientGameRunner
to retain references to the callbacks registered for MouseUpEvent,
MouseMoveEvent, AutoUpgradeEvent, and the other start() subscriptions, then
remove them with eventBus.off(...) during stop() before or alongside the
existing cleanup. Use the existing start()/stop() lifecycle in ClientGameRunner
to ensure the same bound handlers are unsubscribed every time.
In `@src/client/hud/layers/EventsDisplay.ts`:
- Around line 151-162: Reset the stale visibility state in EventsDisplay.init()
alongside clearing this.events, because this page-lifetime singleton can carry
_isVisible from a previous game; update init() in EventsDisplay to also clear
_isVisible, matching the reset behavior used by ActionableEvents.init(), and
keep the existing eventBus off/on wiring for SendAllianceRequestIntentEvent and
onAllianceRequestSent.
In `@src/client/hud/layers/GameRightSidebar.ts`:
- Around line 89-113: `GameRightSidebar.init()` resets the bar visibility state
but does not refresh the parent offset, so a stale `marginTop` can carry over
between games. Update `init()` to also invoke the same offset-sync logic used by
`onSpawnBarVisible`/`onImmunityBarVisible` (via `updateParentOffset()` or
equivalent) after resetting `spawnBarVisible` and `immunityBarVisible`, so the
parent style is corrected immediately on re-init.
In `@src/client/InputHandler.ts`:
- Around line 1104-1118: Reset all remaining interaction flags in
InputHandler.destroy() so teardown fully clears state across games. In addition
to the existing listenerAbort, eventBus.off, timer cleanup, activeKeys,
keybindAndEvent, pointers, and pointerDown resets, also clear alternateView,
selectionBoxActive, multiSelectionActive, longPressActive, suppressNextTap, and
restore canvas.style.cursor to its default. Update the destroy() method in
InputHandler to make sure no stale selection or tap-suppression state survives
into the next game.
---
Outside diff comments:
In `@src/client/hud/layers/Leaderboard.ts`:
- Around line 103-104: Guard the leaderboard percentage calculations against a
zero denominator after switching to numTilesWithoutFallout in Leaderboard.
Update the affected computations in the Leaderboard logic (including the shared
calculations around the referenced percentage blocks) so that when
numTilesWithoutFallout is 0 you return a safe fallback value instead of
dividing, and ensure the same protection is applied wherever that denominator is
used.
In `@src/core/execution/AttackExecution.ts`:
- Around line 389-421: The mop-up cleanup in handleDeadDefender is being skipped
on later ticks because the early return prevents the 10-pass island absorption
loop from running after the first conquest. Keep the repeat-id guard tied to the
mg.conquerPlayer(this._owner, target) call in
AttackExecution.handleDeadDefender, but allow the rest of the method to continue
so the tile cleanup logic still runs even when the player was already conquered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c906c3f6-36e2-4b16-88e0-5231a6dc0f55
📒 Files selected for processing (15)
src/client/ClientGameRunner.tssrc/client/InputHandler.tssrc/client/hud/layers/ActionableEvents.tssrc/client/hud/layers/EventsDisplay.tssrc/client/hud/layers/GameRightSidebar.tssrc/client/hud/layers/Leaderboard.tssrc/client/hud/layers/PlayerPanel.tssrc/client/hud/layers/WinModal.tssrc/core/execution/AttackExecution.tssrc/core/game/Game.tssrc/core/game/GameImpl.tssrc/core/game/UnitImpl.tstests/AttackRetreatCancel.test.tstests/MissileSiloDowngrade.test.tstests/RepeatConquest.test.ts
| this.isActive = false; | ||
| this.worker.cleanup(); | ||
| this.transport.leaveGame(); | ||
| this.input.destroy(); | ||
| if (this.connectionCheckTimeout) { | ||
| clearTimeout(this.connectionCheckTimeout); | ||
| this.connectionCheckTimeout = null; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if ClientGameRunner and EventBus are reused or recreated per game.
# Search for where ClientGameRunner is instantiated and whether EventBus is passed in.
rg -nC5 'new ClientGameRunner' --type=ts src/
# Check if EventBus is created once or per game.
rg -nC3 'new EventBus' --type=ts src/Repository: openfrontio/OpenFrontIO
Length of output: 1174
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the client lifecycle and game runner event subscriptions.
sed -n '240,320p' src/client/Main.ts
printf '\n--- ClientGameRunner start/stop area ---\n'
sed -n '820,1025p' src/client/ClientGameRunner.ts
printf '\n--- EventBus implementation ---\n'
fd -a 'EventBus.ts' srcRepository: openfrontio/OpenFrontIO
Length of output: 9230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the EventBus API and the listener setup/teardown in ClientGameRunner.
sed -n '1,220p' src/core/EventBus.ts
printf '\n--- start() listener registration ---\n'
rg -nC3 'eventBus\.(on|off)|\.bind\(this\)' src/client/ClientGameRunner.ts
printf '\n--- input destroy path ---\n'
rg -nC4 'class InputHandler|destroy\(\)' src/client -g '*.ts'Repository: openfrontio/OpenFrontIO
Length of output: 6327
Unsubscribe the game runner event handlers in stop()
The client keeps one EventBus for the whole session, so the .bind(this) listeners added in start() will stack across games unless they are removed. Store the bound callbacks (or use arrow fields) and call eventBus.off(...) for MouseUpEvent, MouseMoveEvent, AutoUpgradeEvent, etc.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/ClientGameRunner.ts` around lines 987 - 994, The stop() cleanup in
ClientGameRunner currently tears down transport/input state but leaves the
EventBus listeners registered from start(), so bound handlers will accumulate
across runs. Update ClientGameRunner to retain references to the callbacks
registered for MouseUpEvent, MouseMoveEvent, AutoUpgradeEvent, and the other
start() subscriptions, then remove them with eventBus.off(...) during stop()
before or alongside the existing cleanup. Use the existing start()/stop()
lifecycle in ClientGameRunner to ensure the same bound handlers are unsubscribed
every time.
| init() { | ||
| // Clear carried-over events from a previous game on this page-lifetime singleton. | ||
| this.events = []; | ||
| this.eventBus.off( | ||
| SendAllianceRequestIntentEvent, | ||
| this.onAllianceRequestSent, | ||
| ); | ||
| this.eventBus.on( | ||
| SendAllianceRequestIntentEvent, | ||
| this.onAllianceRequestSentConfirmation.bind(this), | ||
| this.onAllianceRequestSent, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset _isVisible in init() too.
_isVisible isn't cleared here, unlike ActionableEvents.init() in this same cohort. Since this component is a page-lifetime singleton, if the previous game ended while the local player was still alive (team/nation win, spectating), _isVisible stays true and the events containers can render stale-visible during the next game's spawn phase.
🛠️ Proposed fix
init() {
// Clear carried-over events from a previous game on this page-lifetime singleton.
this.events = [];
+ this._isVisible = false;
this.eventBus.off(
SendAllianceRequestIntentEvent,
this.onAllianceRequestSent,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| init() { | |
| // Clear carried-over events from a previous game on this page-lifetime singleton. | |
| this.events = []; | |
| this.eventBus.off( | |
| SendAllianceRequestIntentEvent, | |
| this.onAllianceRequestSent, | |
| ); | |
| this.eventBus.on( | |
| SendAllianceRequestIntentEvent, | |
| this.onAllianceRequestSentConfirmation.bind(this), | |
| this.onAllianceRequestSent, | |
| ); | |
| } | |
| init() { | |
| // Clear carried-over events from a previous game on this page-lifetime singleton. | |
| this.events = []; | |
| this._isVisible = false; | |
| this.eventBus.off( | |
| SendAllianceRequestIntentEvent, | |
| this.onAllianceRequestSent, | |
| ); | |
| this.eventBus.on( | |
| SendAllianceRequestIntentEvent, | |
| this.onAllianceRequestSent, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/hud/layers/EventsDisplay.ts` around lines 151 - 162, Reset the
stale visibility state in EventsDisplay.init() alongside clearing this.events,
because this page-lifetime singleton can carry _isVisible from a previous game;
update init() in EventsDisplay to also clear _isVisible, matching the reset
behavior used by ActionableEvents.init(), and keep the existing eventBus off/on
wiring for SendAllianceRequestIntentEvent and onAllianceRequestSent.
| init() { | ||
| this._isSinglePlayer = | ||
| this.game?.config()?.gameConfig()?.gameType === GameType.Singleplayer || | ||
| this.game.config().isReplay(); | ||
| this._isVisible = true; | ||
|
|
||
| this.eventBus.on(SpawnBarVisibleEvent, (e) => { | ||
| this.spawnBarVisible = e.visible; | ||
| this.updateParentOffset(); | ||
| }); | ||
| this.eventBus.on(ImmunityBarVisibleEvent, (e) => { | ||
| this.immunityBarVisible = e.visible; | ||
| this.updateParentOffset(); | ||
| }); | ||
|
|
||
| this.eventBus.on(SendWinnerEvent, () => { | ||
| this.hasWinner = true; | ||
| this.requestUpdate(); | ||
| }); | ||
|
|
||
| this.eventBus.on(TogglePauseIntentEvent, () => { | ||
| const isReplayOrSingleplayer = | ||
| this._isSinglePlayer || this.game?.config()?.isReplay(); | ||
| if (isReplayOrSingleplayer || this.isLobbyCreator) { | ||
| this.onPauseButtonClick(); | ||
| } | ||
| }); | ||
| // Reset per-game state — this element is a page-lifetime singleton. | ||
| this.hasWinner = false; | ||
| this.isPaused = false; | ||
| this.isLobbyCreator = false; | ||
| this.timer = 0; | ||
| this.spawnBarVisible = false; | ||
| this.immunityBarVisible = false; | ||
|
|
||
| this.eventBus.off(SpawnBarVisibleEvent, this.onSpawnBarVisible); | ||
| this.eventBus.off(ImmunityBarVisibleEvent, this.onImmunityBarVisible); | ||
| this.eventBus.off(SendWinnerEvent, this.onSendWinner); | ||
| this.eventBus.off(TogglePauseIntentEvent, this.onTogglePause); | ||
|
|
||
| this.eventBus.on(SpawnBarVisibleEvent, this.onSpawnBarVisible); | ||
| this.eventBus.on(ImmunityBarVisibleEvent, this.onImmunityBarVisible); | ||
| this.eventBus.on(SendWinnerEvent, this.onSendWinner); | ||
| this.eventBus.on(TogglePauseIntentEvent, this.onTogglePause); | ||
|
|
||
| this.requestUpdate(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Sync marginTop on init() too.
spawnBarVisible/immunityBarVisible are reset here, but updateParentOffset() (which writes parent.style.marginTop) is only invoked from the bar-visibility handlers, not from init(). If the previous game ended with a bar visible, the stale margin persists into the new game until the next visibility event fires.
🛠️ Proposed fix
this.eventBus.on(SpawnBarVisibleEvent, this.onSpawnBarVisible);
this.eventBus.on(ImmunityBarVisibleEvent, this.onImmunityBarVisible);
this.eventBus.on(SendWinnerEvent, this.onSendWinner);
this.eventBus.on(TogglePauseIntentEvent, this.onTogglePause);
+ this.updateParentOffset();
this.requestUpdate();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| init() { | |
| this._isSinglePlayer = | |
| this.game?.config()?.gameConfig()?.gameType === GameType.Singleplayer || | |
| this.game.config().isReplay(); | |
| this._isVisible = true; | |
| this.eventBus.on(SpawnBarVisibleEvent, (e) => { | |
| this.spawnBarVisible = e.visible; | |
| this.updateParentOffset(); | |
| }); | |
| this.eventBus.on(ImmunityBarVisibleEvent, (e) => { | |
| this.immunityBarVisible = e.visible; | |
| this.updateParentOffset(); | |
| }); | |
| this.eventBus.on(SendWinnerEvent, () => { | |
| this.hasWinner = true; | |
| this.requestUpdate(); | |
| }); | |
| this.eventBus.on(TogglePauseIntentEvent, () => { | |
| const isReplayOrSingleplayer = | |
| this._isSinglePlayer || this.game?.config()?.isReplay(); | |
| if (isReplayOrSingleplayer || this.isLobbyCreator) { | |
| this.onPauseButtonClick(); | |
| } | |
| }); | |
| // Reset per-game state — this element is a page-lifetime singleton. | |
| this.hasWinner = false; | |
| this.isPaused = false; | |
| this.isLobbyCreator = false; | |
| this.timer = 0; | |
| this.spawnBarVisible = false; | |
| this.immunityBarVisible = false; | |
| this.eventBus.off(SpawnBarVisibleEvent, this.onSpawnBarVisible); | |
| this.eventBus.off(ImmunityBarVisibleEvent, this.onImmunityBarVisible); | |
| this.eventBus.off(SendWinnerEvent, this.onSendWinner); | |
| this.eventBus.off(TogglePauseIntentEvent, this.onTogglePause); | |
| this.eventBus.on(SpawnBarVisibleEvent, this.onSpawnBarVisible); | |
| this.eventBus.on(ImmunityBarVisibleEvent, this.onImmunityBarVisible); | |
| this.eventBus.on(SendWinnerEvent, this.onSendWinner); | |
| this.eventBus.on(TogglePauseIntentEvent, this.onTogglePause); | |
| this.requestUpdate(); | |
| } | |
| init() { | |
| this._isSinglePlayer = | |
| this.game?.config()?.gameConfig()?.gameType === GameType.Singleplayer || | |
| this.game.config().isReplay(); | |
| this._isVisible = true; | |
| // Reset per-game state — this element is a page-lifetime singleton. | |
| this.hasWinner = false; | |
| this.isPaused = false; | |
| this.isLobbyCreator = false; | |
| this.timer = 0; | |
| this.spawnBarVisible = false; | |
| this.immunityBarVisible = false; | |
| this.eventBus.off(SpawnBarVisibleEvent, this.onSpawnBarVisible); | |
| this.eventBus.off(ImmunityBarVisibleEvent, this.onImmunityBarVisible); | |
| this.eventBus.off(SendWinnerEvent, this.onSendWinner); | |
| this.eventBus.off(TogglePauseIntentEvent, this.onTogglePause); | |
| this.eventBus.on(SpawnBarVisibleEvent, this.onSpawnBarVisible); | |
| this.eventBus.on(ImmunityBarVisibleEvent, this.onImmunityBarVisible); | |
| this.eventBus.on(SendWinnerEvent, this.onSendWinner); | |
| this.eventBus.on(TogglePauseIntentEvent, this.onTogglePause); | |
| this.updateParentOffset(); | |
| this.requestUpdate(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/hud/layers/GameRightSidebar.ts` around lines 89 - 113,
`GameRightSidebar.init()` resets the bar visibility state but does not refresh
the parent offset, so a stale `marginTop` can carry over between games. Update
`init()` to also invoke the same offset-sync logic used by
`onSpawnBarVisible`/`onImmunityBarVisible` (via `updateParentOffset()` or
equivalent) after resetting `spawnBarVisible` and `immunityBarVisible`, so the
parent style is corrected immediately on re-init.
| this.listenerAbort?.abort(); | ||
| this.listenerAbort = null; | ||
| this.eventBus.off(UnitSelectionEvent, this.onUnitSelection); | ||
| if (this.moveInterval !== null) { | ||
| clearInterval(this.moveInterval); | ||
| this.moveInterval = null; | ||
| } | ||
| if (this.longPressTimer !== null) { | ||
| clearTimeout(this.longPressTimer); | ||
| this.longPressTimer = null; | ||
| } | ||
| this.activeKeys.clear(); | ||
| this.keybindAndEvent = []; | ||
| this.pointers.clear(); | ||
| this.pointerDown = false; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset remaining interactive state in destroy() to prevent leaks across games.
destroy() aborts the blur listener, so the blur handler's state resets (lines 447-471) won't fire after teardown. However, destroy() only clears pointers and pointerDown — it leaves alternateView, selectionBoxActive, multiSelectionActive, longPressActive, suppressNextTap, and canvas.style.cursor unset. If the game ends mid-selection or mid-long-press, these stale values persist into the next game. Most critically, suppressNextTap = true would silently swallow the first tap in a new game.
🛡️ Proposed fix: add missing state resets to destroy()
destroy() {
this.listenerAbort?.abort();
this.listenerAbort = null;
this.eventBus.off(UnitSelectionEvent, this.onUnitSelection);
if (this.moveInterval !== null) {
clearInterval(this.moveInterval);
this.moveInterval = null;
}
if (this.longPressTimer !== null) {
clearTimeout(this.longPressTimer);
this.longPressTimer = null;
}
this.activeKeys.clear();
this.keybindAndEvent = [];
this.pointers.clear();
this.pointerDown = false;
+ this.alternateView = false;
+ this.selectionBoxActive = false;
+ this.multiSelectionActive = false;
+ this.longPressActive = false;
+ this.suppressNextTap = false;
+ this.canvas.style.cursor = "";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.listenerAbort?.abort(); | |
| this.listenerAbort = null; | |
| this.eventBus.off(UnitSelectionEvent, this.onUnitSelection); | |
| if (this.moveInterval !== null) { | |
| clearInterval(this.moveInterval); | |
| this.moveInterval = null; | |
| } | |
| if (this.longPressTimer !== null) { | |
| clearTimeout(this.longPressTimer); | |
| this.longPressTimer = null; | |
| } | |
| this.activeKeys.clear(); | |
| this.keybindAndEvent = []; | |
| this.pointers.clear(); | |
| this.pointerDown = false; | |
| this.listenerAbort?.abort(); | |
| this.listenerAbort = null; | |
| this.eventBus.off(UnitSelectionEvent, this.onUnitSelection); | |
| if (this.moveInterval !== null) { | |
| clearInterval(this.moveInterval); | |
| this.moveInterval = null; | |
| } | |
| if (this.longPressTimer !== null) { | |
| clearTimeout(this.longPressTimer); | |
| this.longPressTimer = null; | |
| } | |
| this.activeKeys.clear(); | |
| this.keybindAndEvent = []; | |
| this.pointers.clear(); | |
| this.pointerDown = false; | |
| this.alternateView = false; | |
| this.selectionBoxActive = false; | |
| this.multiSelectionActive = false; | |
| this.longPressActive = false; | |
| this.suppressNextTap = false; | |
| this.canvas.style.cursor = ""; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/InputHandler.ts` around lines 1104 - 1118, Reset all remaining
interaction flags in InputHandler.destroy() so teardown fully clears state
across games. In addition to the existing listenerAbort, eventBus.off, timer
cleanup, activeKeys, keybindAndEvent, pointers, and pointerDown resets, also
clear alternateView, selectionBoxActive, multiSelectionActive, longPressActive,
suppressNextTap, and restore canvas.style.cursor to its default. Update the
destroy() method in InputHandler to make sure no stale selection or
tap-suppression state survives into the next game.
Description:
Fixes several bugs that show up when leaving a game and joining another without a full page reload, plus a few related core edge cases.
Client (page-lifetime HUD / input reuse):
InputHandler.destroy()on game stop and tear down window/canvas listeners viaAbortController, so hotkeys and pan don't stack every game.setTimeout, and guardonConnectionCheckwithisActive, so quitting within 20s doesn't leave a zombie reconnect loop.init(), and stop unknown previous-game small IDs from throwing (which froze later HUD layers).GameRightSidebarwin/pause state and dedupe EventBus subscriptions so the clock and pause hotkey work in the next game.PlayerPanelprofile cache on stringPlayerIDinstead ofNumber(id)(alwaysNaN), which was spamming the worker every tick.Core:
!attack.isActive()before theretreating()early-return so an externally cancelled attack during the retreat delay deactivates instead of spinning forever.conquerPlayerso island survivors under 100 tiles aren't conquered (gold/kills) repeatedly.goldCaptured(half for humans) as the conquest display gold amount so it matches the message text.Added tests for retreat cancel, repeat conquest, and silo downgrade cooldown.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
ahmetd17