From 588f682db87e76083383cf99b41658db95c6cc95 Mon Sep 17 00:00:00 2001 From: Josh Harris Date: Sat, 12 Sep 2026 11:10:27 +0100 Subject: [PATCH 1/6] fix(client): detach InputHandler listeners and keybinds on destroy (OPE-411) InputHandler.initialize() registered its pointer/keyboard listeners on window and the canvas with anonymous callbacks, so destroy() could not remove them. A handler from a finished game kept translating keys into events on that game's dead EventBus for the rest of the page's life, and because InputHandler is constructed once per game in ClientGameRunner, every game played without a reload added another live handler. Register every listener added in initializePointerAndKeyboardEvents() with a single AbortController's signal and abort it in destroy(). Also clear this.keybinds (the dispatch table was already cleared) and null out moveInterval after clearing it. Co-Authored-By: Claude Fable 5.1 --- src/client/InputHandler.ts | 389 ++++++++++++++++++++----------------- tests/InputHandler.test.ts | 90 +++++++++ 2 files changed, 302 insertions(+), 177 deletions(-) diff --git a/src/client/InputHandler.ts b/src/client/InputHandler.ts index 09852b58ba..78f500a642 100644 --- a/src/client/InputHandler.ts +++ b/src/client/InputHandler.ts @@ -252,6 +252,9 @@ export class InputHandler { private readonly LONG_PRESS_MS = 800; private moveInterval: NodeJS.Timeout | null = null; + /** Aborts every window/document/canvas listener added in + * `initializePointerAndKeyboardEvents()`. */ + private listenerAbort: AbortController | null = null; private activeKeys = new Set(); private keybinds: Record = {}; private keybindAndEvent: Array<[string, KeybindEntry]> = []; @@ -474,9 +477,17 @@ export class InputHandler { } private initializePointerAndKeyboardEvents() { - this.canvas.addEventListener("pointerdown", (e) => this.onPointerDown(e)); - window.addEventListener("pointerup", (e) => this.onPointerUp(e)); - window.addEventListener("pointercancel", (e) => this.onPointerUp(e)); + this.listenerAbort = new AbortController(); + const { signal } = this.listenerAbort; + this.canvas.addEventListener("pointerdown", (e) => this.onPointerDown(e), { + signal, + }); + window.addEventListener("pointerup", (e) => this.onPointerUp(e), { + signal, + }); + window.addEventListener("pointercancel", (e) => this.onPointerUp(e), { + signal, + }); this.canvas.addEventListener( "wheel", (e) => { @@ -484,7 +495,7 @@ export class InputHandler { this.onShiftScroll(e); e.preventDefault(); }, - { passive: false }, + { passive: false, signal }, ); // Safari trackpad pinch, which fires no ctrl+wheel event. this.canvas.addEventListener( @@ -493,7 +504,7 @@ export class InputHandler { e.preventDefault(); this.lastGestureScale = (e as WebKitGestureEvent).scale; }, - { passive: false }, + { passive: false, signal }, ); this.canvas.addEventListener( "gesturechange", @@ -501,7 +512,7 @@ export class InputHandler { e.preventDefault(); this.onGestureChange(e as WebKitGestureEvent); }, - { passive: false }, + { passive: false, signal }, ); this.canvas.addEventListener( "gestureend", @@ -509,41 +520,53 @@ export class InputHandler { e.preventDefault(); this.lastGestureScale = null; }, - { passive: false }, + { passive: false, signal }, ); - window.addEventListener("pointermove", this.onPointerMove.bind(this)); - this.canvas.addEventListener("contextmenu", (e) => this.onContextMenu(e)); - window.addEventListener("mousemove", (e) => { - if (e.movementX || e.movementY) { - this.eventBus.emit(new MouseMoveEvent(e.clientX, e.clientY)); - } + window.addEventListener("pointermove", this.onPointerMove.bind(this), { + signal, }); + this.canvas.addEventListener("contextmenu", (e) => this.onContextMenu(e), { + signal, + }); + window.addEventListener( + "mousemove", + (e) => { + if (e.movementX || e.movementY) { + this.eventBus.emit(new MouseMoveEvent(e.clientX, e.clientY)); + } + }, + { signal }, + ); // Clear all tracked keys when the window loses focus so keys that had // their keyup swallowed by the browser (e.g. cmd+zoom) don't stay stuck. // Also release the hold-to-view state and any active pointer/drag state // so the alternate view and drags aren't left latched when focus returns. - window.addEventListener("blur", () => { - this.activeKeys.clear(); - if (this.alternateView) { - this.alternateView = false; - this.eventBus.emit(new AlternateViewEvent(false)); - } - this.pointerDown = false; - this.pointers.clear(); - this.lastGestureScale = null; - if (this.longPressTimer !== null) { - clearTimeout(this.longPressTimer); - this.longPressTimer = null; - } - this.longPressActive = false; - this.suppressNextTap = false; - if (this.selectionBoxActive || this.multiSelectionActive) { - this.selectionBoxActive = false; - this.multiSelectionActive = false; - this.eventBus.emit(new WarshipSelectionBoxCancelEvent()); - } - this.canvas.style.cursor = ""; - }); + window.addEventListener( + "blur", + () => { + this.activeKeys.clear(); + if (this.alternateView) { + this.alternateView = false; + this.eventBus.emit(new AlternateViewEvent(false)); + } + this.pointerDown = false; + this.pointers.clear(); + this.lastGestureScale = null; + if (this.longPressTimer !== null) { + clearTimeout(this.longPressTimer); + this.longPressTimer = null; + } + this.longPressActive = false; + this.suppressNextTap = false; + if (this.selectionBoxActive || this.multiSelectionActive) { + this.selectionBoxActive = false; + this.multiSelectionActive = false; + this.eventBus.emit(new WarshipSelectionBoxCancelEvent()); + } + this.canvas.style.cursor = ""; + }, + { signal }, + ); this.pointers.clear(); this.moveInterval = setInterval(() => { @@ -599,169 +622,177 @@ export class InputHandler { } }, 1); - window.addEventListener("keydown", (e) => { - const isTextInput = this.isTextInputTarget(e.target); - if (isTextInput && e.code !== "Escape") { - return; - } + window.addEventListener( + "keydown", + (e) => { + const isTextInput = this.isTextInputTarget(e.target); + if (isTextInput && e.code !== "Escape") { + return; + } - if (this.keybindMatchesEvent(e, this.keybinds.toggleView)) { - e.preventDefault(); - if (!this.alternateView) { - this.alternateView = true; - this.eventBus.emit(new AlternateViewEvent(true)); + if (this.keybindMatchesEvent(e, this.keybinds.toggleView)) { + e.preventDefault(); + if (!this.alternateView) { + this.alternateView = true; + this.eventBus.emit(new AlternateViewEvent(true)); + } } - } - if ( - this.keybindMatchesEvent(e, this.keybinds.coordinateGrid) && - !e.repeat - ) { - e.preventDefault(); - this.coordinateGridEnabled = !this.coordinateGridEnabled; - this.eventBus.emit( - new ToggleCoordinateGridEvent(this.coordinateGridEnabled), - ); - } + if ( + this.keybindMatchesEvent(e, this.keybinds.coordinateGrid) && + !e.repeat + ) { + e.preventDefault(); + this.coordinateGridEnabled = !this.coordinateGridEnabled; + this.eventBus.emit( + new ToggleCoordinateGridEvent(this.coordinateGridEnabled), + ); + } - if (e.code === "Escape") { - e.preventDefault(); - let closedUI = false; + if (e.code === "Escape") { + e.preventDefault(); + let closedUI = false; - if (this.uiState.ghostStructure !== null) { - this.setGhostStructure(null); - closedUI = true; - } + if (this.uiState.ghostStructure !== null) { + this.setGhostStructure(null); + closedUI = true; + } - if (this.selectionBoxActive) { - this.selectionBoxActive = false; - this.eventBus.emit(new WarshipSelectionBoxCancelEvent()); - closedUI = true; - } + if (this.selectionBoxActive) { + this.selectionBoxActive = false; + this.eventBus.emit(new WarshipSelectionBoxCancelEvent()); + closedUI = true; + } - this.eventBus.emit(new CloseViewEvent()); + this.eventBus.emit(new CloseViewEvent()); + + if ( + !closedUI && + (this.unitSelectionActive || this.multiSelectionActive) + ) { + this.eventBus.emit(new UnitSelectionEvent(null, false)); + } + } if ( - !closedUI && - (this.unitSelectionActive || this.multiSelectionActive) + (e.code === "Enter" || e.code === "NumpadEnter") && + this.uiState.ghostStructure !== null ) { - this.eventBus.emit(new UnitSelectionEvent(null, false)); + e.preventDefault(); + this.eventBus.emit(new ConfirmGhostStructureEvent()); } - } - if ( - (e.code === "Enter" || e.code === "NumpadEnter") && - this.uiState.ghostStructure !== null - ) { - e.preventDefault(); - this.eventBus.emit(new ConfirmGhostStructureEvent()); - } - - // Don't track zoom keys when a meta/ctrl modifier is held — that means - // the browser is handling its own zoom (cmd+/cmd-) and the keyup will - // never fire, which would leave the key stuck in activeKeys forever. - // Also covers numpad zoom shortcuts (Ctrl+NumpadAdd/NumpadSubtract). - const isBrowserZoomCombo = - (e.metaKey || e.ctrlKey) && - (e.code === "Minus" || - e.code === "Equal" || - e.code === "NumpadAdd" || - e.code === "NumpadSubtract"); - - const isConfiguredKeybind = - Object.values(this.keybinds).includes(e.code) || - this.keybindAndEvent.some(([k]) => this.keybindMatchesEvent(e, k)); - - if (isConfiguredKeybind && !isBrowserZoomCombo) { - e.preventDefault(); - } + // Don't track zoom keys when a meta/ctrl modifier is held — that means + // the browser is handling its own zoom (cmd+/cmd-) and the keyup will + // never fire, which would leave the key stuck in activeKeys forever. + // Also covers numpad zoom shortcuts (Ctrl+NumpadAdd/NumpadSubtract). + const isBrowserZoomCombo = + (e.metaKey || e.ctrlKey) && + (e.code === "Minus" || + e.code === "Equal" || + e.code === "NumpadAdd" || + e.code === "NumpadSubtract"); + + const isConfiguredKeybind = + Object.values(this.keybinds).includes(e.code) || + this.keybindAndEvent.some(([k]) => this.keybindMatchesEvent(e, k)); + + if (isConfiguredKeybind && !isBrowserZoomCombo) { + e.preventDefault(); + } - if ( - !isBrowserZoomCombo && - [ - this.keybinds.moveUp, - this.keybinds.moveDown, - this.keybinds.moveLeft, - this.keybinds.moveRight, - this.keybinds.zoomOut, - this.keybinds.zoomIn, - "ArrowUp", - "ArrowLeft", - "ArrowDown", - "ArrowRight", - "Minus", - "Equal", - "NumpadAdd", - "NumpadSubtract", - this.keybinds.attackRatioDown, - this.keybinds.attackRatioUp, - this.keybinds.centerCamera, - "ControlLeft", - "ControlRight", - this.keybinds.boxSelectWarships, - this.keybinds.emojiMenuModifier, - this.keybinds.buildMenuModifier, - this.keybinds.altKey, - ].includes(e.code) - ) { - this.activeKeys.add(e.code); - } + if ( + !isBrowserZoomCombo && + [ + this.keybinds.moveUp, + this.keybinds.moveDown, + this.keybinds.moveLeft, + this.keybinds.moveRight, + this.keybinds.zoomOut, + this.keybinds.zoomIn, + "ArrowUp", + "ArrowLeft", + "ArrowDown", + "ArrowRight", + "Minus", + "Equal", + "NumpadAdd", + "NumpadSubtract", + this.keybinds.attackRatioDown, + this.keybinds.attackRatioUp, + this.keybinds.centerCamera, + "ControlLeft", + "ControlRight", + this.keybinds.boxSelectWarships, + this.keybinds.emojiMenuModifier, + this.keybinds.buildMenuModifier, + this.keybinds.altKey, + ].includes(e.code) + ) { + this.activeKeys.add(e.code); + } - // warship box selection mode. - // If a ghost structure is active, discard it first. - if (e.code === this.keybinds.boxSelectWarships) { - if (this.uiState.ghostStructure !== null) { - this.setGhostStructure(null); + // warship box selection mode. + // If a ghost structure is active, discard it first. + if (e.code === this.keybinds.boxSelectWarships) { + if (this.uiState.ghostStructure !== null) { + this.setGhostStructure(null); + } + this.canvas.style.cursor = "crosshair"; + } + }, + { signal }, + ); + window.addEventListener( + "keyup", + (e) => { + const isTextInput = this.isTextInputTarget(e.target); + if (isTextInput && !this.activeKeys.has(e.code)) { + return; } - this.canvas.style.cursor = "crosshair"; - } - }); - window.addEventListener("keyup", (e) => { - const isTextInput = this.isTextInputTarget(e.target); - if (isTextInput && !this.activeKeys.has(e.code)) { - return; - } - // When the meta (cmd) or ctrl key is released, any keys that were held - // simultaneously will have had their keyup swallowed by the browser - // (e.g. cmd+Plus for browser zoom). Clear zoom-related keys to - // prevent them staying stuck in activeKeys. - if ( - e.code === "MetaLeft" || - e.code === "MetaRight" || - e.code === "ControlLeft" || - e.code === "ControlRight" - ) { - this.activeKeys.delete("Minus"); - this.activeKeys.delete("Equal"); - this.activeKeys.delete("NumpadAdd"); - this.activeKeys.delete("NumpadSubtract"); - this.activeKeys.delete(this.keybinds.zoomIn); - this.activeKeys.delete(this.keybinds.zoomOut); - } + // When the meta (cmd) or ctrl key is released, any keys that were held + // simultaneously will have had their keyup swallowed by the browser + // (e.g. cmd+Plus for browser zoom). Clear zoom-related keys to + // prevent them staying stuck in activeKeys. + if ( + e.code === "MetaLeft" || + e.code === "MetaRight" || + e.code === "ControlLeft" || + e.code === "ControlRight" + ) { + this.activeKeys.delete("Minus"); + this.activeKeys.delete("Equal"); + this.activeKeys.delete("NumpadAdd"); + this.activeKeys.delete("NumpadSubtract"); + this.activeKeys.delete(this.keybinds.zoomIn); + this.activeKeys.delete(this.keybinds.zoomOut); + } - outerLoop: for (const item of this.keybindAndEvent) { - if (this.keybindMatchesEvent(e, item[0])) { - for (const i of item[1].conditions) { - if (!i(e)) { - continue outerLoop; + outerLoop: for (const item of this.keybindAndEvent) { + if (this.keybindMatchesEvent(e, item[0])) { + for (const i of item[1].conditions) { + if (!i(e)) { + continue outerLoop; + } } + e.preventDefault(); + item[1].handler(e); } - e.preventDefault(); - item[1].handler(e); } - } - this.activeKeys.delete(e.code); + this.activeKeys.delete(e.code); - // Reset crosshair when Shift is released (unless selection box or multi-selection still active) - if ( - e.code === this.keybinds.boxSelectWarships && - !this.selectionBoxActive && - !this.multiSelectionActive - ) { - this.canvas.style.cursor = ""; - } - }); + // Reset crosshair when Shift is released (unless selection box or multi-selection still active) + if ( + e.code === this.keybinds.boxSelectWarships && + !this.selectionBoxActive && + !this.multiSelectionActive + ) { + this.canvas.style.cursor = ""; + } + }, + { signal }, + ); } private onPointerDown(event: PointerEvent) { @@ -1226,13 +1257,17 @@ export class InputHandler { destroy() { if (this.moveInterval !== null) { clearInterval(this.moveInterval); + this.moveInterval = null; } globalThis.removeEventListener( `${USER_SETTINGS_CHANGED_EVENT}:${KEYBINDS_KEY}`, this.onKeybindsChanged, ); + this.listenerAbort?.abort(); + this.listenerAbort = null; this.activeKeys.clear(); this.lastGestureScale = null; this.keybindAndEvent = []; + this.keybinds = {}; } } diff --git a/tests/InputHandler.test.ts b/tests/InputHandler.test.ts index 5d7c86aee3..22eb647456 100644 --- a/tests/InputHandler.test.ts +++ b/tests/InputHandler.test.ts @@ -1,4 +1,5 @@ import { + AlternateViewEvent, AutoUpgradeEvent, ConfirmGhostStructureEvent, ContextMenuEvent, @@ -1294,3 +1295,92 @@ describe("InputHandler right-click cancels unit selection (#4692)", () => { ).toBe(true); }); }); + +describe("InputHandler teardown (OPE-411)", () => { + const makeHandler = (canvas: HTMLElement, eventBus: EventBus) => + new InputHandler( + { + inSpawnPhase: () => false, + myPlayer: () => ({ isAlive: () => true }), + } as unknown as GameView, + { + attackRatio: 20, + ghostStructure: null, + rocketDirectionUp: true, + upgradeMultiplier: 1, + }, + canvas, + eventBus, + ); + + let inputHandler: InputHandler; + let eventBus: EventBus; + let canvas: HTMLCanvasElement; + + beforeEach(() => { + new UserSettings().removeCached(KEYBINDS_KEY, false); + canvas = document.createElement("canvas"); + canvas.width = 800; + canvas.height = 600; + eventBus = new EventBus(); + inputHandler = makeHandler(canvas, eventBus); + inputHandler.initialize(); + }); + + afterEach(() => inputHandler.destroy()); + + it("emits AlternateViewEvent on Space while alive", () => { + const emit = vi.spyOn(eventBus, "emit"); + window.dispatchEvent(new KeyboardEvent("keydown", { code: "Space" })); + expect( + emit.mock.calls.some( + (c: unknown[]) => c[0] instanceof AlternateViewEvent, + ), + ).toBe(true); + }); + + it("emits nothing on a window keydown after destroy()", () => { + inputHandler.destroy(); + const emit = vi.spyOn(eventBus, "emit"); + window.dispatchEvent(new KeyboardEvent("keydown", { code: "Space" })); + window.dispatchEvent(new KeyboardEvent("keyup", { code: "Space" })); + expect(emit).not.toHaveBeenCalled(); + }); + + it("emits nothing on a canvas event after destroy()", () => { + inputHandler.destroy(); + const emit = vi.spyOn(eventBus, "emit"); + canvas.dispatchEvent( + new MouseEvent("contextmenu", { clientX: 100, clientY: 100 }), + ); + expect(emit).not.toHaveBeenCalled(); + }); + + it("clears keybinds and the keybind dispatch table on destroy()", () => { + inputHandler.destroy(); + expect(inputHandler["keybinds"]).toEqual({}); + expect(inputHandler["keybindAndEvent"]).toEqual([]); + }); + + it("destroying one handler leaves a later handler working", () => { + const secondBus = new EventBus(); + const secondCanvas = document.createElement("canvas"); + const second = makeHandler(secondCanvas, secondBus); + second.initialize(); + + inputHandler.destroy(); + + const deadEmit = vi.spyOn(eventBus, "emit"); + const liveEmit = vi.spyOn(secondBus, "emit"); + window.dispatchEvent(new KeyboardEvent("keydown", { code: "Space" })); + + expect(deadEmit).not.toHaveBeenCalled(); + expect( + liveEmit.mock.calls.some( + (c: unknown[]) => c[0] instanceof AlternateViewEvent, + ), + ).toBe(true); + + second.destroy(); + }); +}); From 35f005a25bf03e5920ea06b99ce0c6e3eaba91b4 Mon Sep 17 00:00:00 2001 From: Josh Harris Date: Sat, 12 Sep 2026 11:22:39 +0100 Subject: [PATCH 2/6] fix(client): destroy the InputHandler when the game stops (OPE-411) Nothing ever called InputHandler.destroy(): ClientGameRunner constructs one per game and only ever calls initialize(). Main.ts stops the current game and joins a new one in place ("joining lobby, stopping existing game"), so without this every in-page game transition left the previous handler's window listeners live, which is what OPE-411 is about. Call this.input.destroy() in ClientGameRunner.stop(), alongside the other always-run idempotent disposals, before disposeRenderer() removes the input overlay. stop() can be re-entered (the worker-error path stops the game and the player can still leave afterwards), so destroy() must be idempotent; it is, and there are now tests for both. Game end does not stop the runner -- only leaving, joining another game, a worker error or page teardown do -- so a player who wins keeps working input while spectating. Co-Authored-By: Claude Fable 5.1 --- src/client/ClientGameRunner.ts | 5 ++++ tests/InputHandler.test.ts | 9 +++++++ tests/client/ClientGameRunnerActions.test.ts | 24 +++++++++++++++++-- tests/client/ClientGameRunnerMessages.test.ts | 2 +- 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/client/ClientGameRunner.ts b/src/client/ClientGameRunner.ts index 2ec410a6f6..24f04eb6a7 100644 --- a/src/client/ClientGameRunner.ts +++ b/src/client/ClientGameRunner.ts @@ -1121,6 +1121,11 @@ export class ClientGameRunner { public stop() { this.soundManager.dispose(); this.graphicsListenerAbort?.abort(); + // Detach the input handler's window/canvas listeners. Nothing else ever + // did, so a handler from a finished game kept translating keys into events + // on a dead bus, and joining another game without a page reload stacked a + // second live handler on top. Idempotent, like the disposals around it. + this.input.destroy(); this.disposeRenderer?.(); if (!this.isActive) return; diff --git a/tests/InputHandler.test.ts b/tests/InputHandler.test.ts index 22eb647456..50c1629904 100644 --- a/tests/InputHandler.test.ts +++ b/tests/InputHandler.test.ts @@ -1362,6 +1362,15 @@ describe("InputHandler teardown (OPE-411)", () => { expect(inputHandler["keybindAndEvent"]).toEqual([]); }); + it("is safe to destroy twice", () => { + inputHandler.destroy(); + expect(() => inputHandler.destroy()).not.toThrow(); + + const emit = vi.spyOn(eventBus, "emit"); + window.dispatchEvent(new KeyboardEvent("keydown", { code: "Space" })); + expect(emit).not.toHaveBeenCalled(); + }); + it("destroying one handler leaves a later handler working", () => { const secondBus = new EventBus(); const secondCanvas = document.createElement("canvas"); diff --git a/tests/client/ClientGameRunnerActions.test.ts b/tests/client/ClientGameRunnerActions.test.ts index 4b8f52aa82..257a61555c 100644 --- a/tests/client/ClientGameRunnerActions.test.ts +++ b/tests/client/ClientGameRunnerActions.test.ts @@ -108,6 +108,7 @@ function makeRunner(overrides: { playerByClientID: vi.fn(overrides.playerByClientID ?? (() => myPlayer)), euclideanDistSquared: () => overrides.boatDistSquared ?? 0, }; + const input = { initialize: vi.fn(), destroy: vi.fn() }; const runner = new ClientGameRunner( { gameID: "game1234" } as LobbyConfig, "c0000001", @@ -120,7 +121,7 @@ function makeRunner(overrides: { screenToWorldCoordinates: vi.fn(() => ({ x: 1, y: 2 })), }, } as never, - { initialize: vi.fn() } as never, + input as never, { updateCallback: vi.fn(), rejoinGame: vi.fn(), @@ -133,7 +134,7 @@ function makeRunner(overrides: { { goToPlayer: () => false } as never, ); runner.start(); - return { runner, eventBus, gameView, myPlayer }; + return { runner, eventBus, gameView, myPlayer, input }; } const flushPromises = () => new Promise((r) => setTimeout(r, 0)); @@ -234,3 +235,22 @@ describe("auto boat", () => { expect(boats).toHaveLength(0); }); }); + +describe("stop() (OPE-411)", () => { + it("destroys the input handler so its listeners stop firing", () => { + const { runner, input } = makeRunner({}); + + runner.stop(); + + expect(input.destroy).toHaveBeenCalledTimes(1); + }); + + it("tolerates a second stop()", () => { + const { runner, input } = makeRunner({}); + + runner.stop(); + runner.stop(); + + expect(input.destroy).toHaveBeenCalledTimes(2); + }); +}); diff --git a/tests/client/ClientGameRunnerMessages.test.ts b/tests/client/ClientGameRunnerMessages.test.ts index 77549dcf26..6522b1103a 100644 --- a/tests/client/ClientGameRunnerMessages.test.ts +++ b/tests/client/ClientGameRunnerMessages.test.ts @@ -146,7 +146,7 @@ function makeStartedRunner(withStartInfo: boolean) { "c0000001", eventBus, renderer as never, - { initialize: vi.fn() } as never, + { initialize: vi.fn(), destroy: vi.fn() } as never, transport as never, worker as never, gameView as never, From d36adbe7389684ee37116f9b041113ced0fc578b Mon Sep 17 00:00:00 2001 From: Josh Harris Date: Sat, 12 Sep 2026 11:35:57 +0100 Subject: [PATCH 3/6] test(client): make the InputHandler teardown tests actually detect a regression (OPE-411) Cold review findings. The window-side probes used Space, whose emit is gated on this.keybinds.toggleView, which destroy() also clears -- so they passed even with the abort signal dropped from the keydown registration. They now probe Escape, whose CloseViewEvent is emitted unconditionally. Verified by mutation: removing { signal } from the keydown registration fails three tests, and removing the re-initialize guard fails one. Also: assert keybinds is non-empty before destroy() so that test cannot pass trivially; destroy the second handler in a finally so a failed expectation cannot leak a live window listener into later tests; add fake-timer tests for the pan/zoom interval; and guard initializePointerAndKeyboardEvents() against a second call so a re-initialize cannot orphan the first listener set or interval. Production never calls initialize() twice on one handler, and never after destroy() -- each "start" message builds a fresh runner and a fresh handler, and reconnects go through transport.rejoinGame() -- so the guard is belt-and-braces, not a bug fix. The doc comment said "window/document/canvas"; there are no document listeners in this file. Co-Authored-By: Claude Fable 5.1 --- src/client/InputHandler.ts | 11 ++++- tests/InputHandler.test.ts | 82 +++++++++++++++++++++++++++++++++----- 2 files changed, 81 insertions(+), 12 deletions(-) diff --git a/src/client/InputHandler.ts b/src/client/InputHandler.ts index 78f500a642..7b9a988c76 100644 --- a/src/client/InputHandler.ts +++ b/src/client/InputHandler.ts @@ -252,7 +252,7 @@ export class InputHandler { private readonly LONG_PRESS_MS = 800; private moveInterval: NodeJS.Timeout | null = null; - /** Aborts every window/document/canvas listener added in + /** Aborts every window/canvas listener added in * `initializePointerAndKeyboardEvents()`. */ private listenerAbort: AbortController | null = null; private activeKeys = new Set(); @@ -477,6 +477,15 @@ export class InputHandler { } private initializePointerAndKeyboardEvents() { + // A second initialize() would otherwise orphan the first listener set and + // interval: nothing else holds the old controller, so they could never be + // removed. Production only initializes once, but this keeps that from + // being load-bearing. + this.listenerAbort?.abort(); + if (this.moveInterval !== null) { + clearInterval(this.moveInterval); + this.moveInterval = null; + } this.listenerAbort = new AbortController(); const { signal } = this.listenerAbort; this.canvas.addEventListener("pointerdown", (e) => this.onPointerDown(e), { diff --git a/tests/InputHandler.test.ts b/tests/InputHandler.test.ts index 50c1629904..109634041b 100644 --- a/tests/InputHandler.test.ts +++ b/tests/InputHandler.test.ts @@ -1,6 +1,7 @@ import { AlternateViewEvent, AutoUpgradeEvent, + CloseViewEvent, ConfirmGhostStructureEvent, ContextMenuEvent, InputHandler, @@ -1339,9 +1340,22 @@ describe("InputHandler teardown (OPE-411)", () => { ).toBe(true); }); + it("emits CloseViewEvent on Escape while alive", () => { + const emit = vi.spyOn(eventBus, "emit"); + window.dispatchEvent(new KeyboardEvent("keydown", { code: "Escape" })); + expect( + emit.mock.calls.some((c: unknown[]) => c[0] instanceof CloseViewEvent), + ).toBe(true); + }); + it("emits nothing on a window keydown after destroy()", () => { inputHandler.destroy(); const emit = vi.spyOn(eventBus, "emit"); + // Escape is the load-bearing probe: its CloseViewEvent is emitted + // unconditionally, so it still fires if the keydown listener survives + // destroy(). Space goes through this.keybinds, which destroy() also + // clears, so a Space-only probe would pass even with the abort reverted. + window.dispatchEvent(new KeyboardEvent("keydown", { code: "Escape" })); window.dispatchEvent(new KeyboardEvent("keydown", { code: "Space" })); window.dispatchEvent(new KeyboardEvent("keyup", { code: "Space" })); expect(emit).not.toHaveBeenCalled(); @@ -1357,6 +1371,9 @@ describe("InputHandler teardown (OPE-411)", () => { }); it("clears keybinds and the keybind dispatch table on destroy()", () => { + expect(Object.keys(inputHandler["keybinds"]).length).toBeGreaterThan(0); + expect(inputHandler["keybindAndEvent"].length).toBeGreaterThan(0); + inputHandler.destroy(); expect(inputHandler["keybinds"]).toEqual({}); expect(inputHandler["keybindAndEvent"]).toEqual([]); @@ -1377,19 +1394,62 @@ describe("InputHandler teardown (OPE-411)", () => { const second = makeHandler(secondCanvas, secondBus); second.initialize(); - inputHandler.destroy(); + try { + inputHandler.destroy(); - const deadEmit = vi.spyOn(eventBus, "emit"); - const liveEmit = vi.spyOn(secondBus, "emit"); - window.dispatchEvent(new KeyboardEvent("keydown", { code: "Space" })); + const deadEmit = vi.spyOn(eventBus, "emit"); + const liveEmit = vi.spyOn(secondBus, "emit"); + window.dispatchEvent(new KeyboardEvent("keydown", { code: "Escape" })); + + expect(deadEmit).not.toHaveBeenCalled(); + expect( + liveEmit.mock.calls.some( + (c: unknown[]) => c[0] instanceof CloseViewEvent, + ), + ).toBe(true); + } finally { + // Must run even if an expectation throws, or a live window listener + // leaks into every later test in this file. + second.destroy(); + } + }); - expect(deadEmit).not.toHaveBeenCalled(); - expect( - liveEmit.mock.calls.some( - (c: unknown[]) => c[0] instanceof AlternateViewEvent, - ), - ).toBe(true); + it("clears the pan/zoom interval on destroy()", () => { + vi.useFakeTimers(); + const handler = makeHandler( + document.createElement("canvas"), + new EventBus(), + ); + try { + handler.initialize(); + expect(vi.getTimerCount()).toBe(1); + + handler.destroy(); + expect(vi.getTimerCount()).toBe(0); + } finally { + handler.destroy(); + vi.useRealTimers(); + } + }); - second.destroy(); + it("a second initialize() does not orphan the first listeners or interval", () => { + vi.useFakeTimers(); + const bus = new EventBus(); + const handler = makeHandler(document.createElement("canvas"), bus); + try { + handler.initialize(); + handler.initialize(); + expect(vi.getTimerCount()).toBe(1); + + handler.destroy(); + expect(vi.getTimerCount()).toBe(0); + + const emit = vi.spyOn(bus, "emit"); + window.dispatchEvent(new KeyboardEvent("keydown", { code: "Escape" })); + expect(emit).not.toHaveBeenCalled(); + } finally { + handler.destroy(); + vi.useRealTimers(); + } }); }); From dd208c6f963cb095264e944e88609c7375c6f28d Mon Sep 17 00:00:00 2001 From: Josh Harris Date: Sat, 12 Sep 2026 11:52:09 +0100 Subject: [PATCH 4/6] fix(client): cancel the pending long-press timer on teardown (OPE-411) CodeRabbit review. A touch pointerdown arms an 800ms long-press timer that the AbortController does not cover, so stopping a game within that window (back button, joining another lobby, a worker error) let the timer fire afterwards: emitting TouchLongPressStartEvent on the dead bus and setting the cursor on a canvas disposeRenderer() had already removed. Same defect class as the listeners this ticket is about. Clear it and reset longPressActive/suppressNextTap in destroy() and in the re-initialize guard, alongside the existing moveInterval cleanup. Both paths are covered by tests; removing the destroy() clear fails the new test. Co-Authored-By: Claude Fable 5.1 --- src/client/InputHandler.ts | 16 ++++++++++ tests/InputHandler.test.ts | 61 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/src/client/InputHandler.ts b/src/client/InputHandler.ts index 7b9a988c76..3562b4ce0b 100644 --- a/src/client/InputHandler.ts +++ b/src/client/InputHandler.ts @@ -486,6 +486,12 @@ export class InputHandler { clearInterval(this.moveInterval); this.moveInterval = null; } + if (this.longPressTimer !== null) { + clearTimeout(this.longPressTimer); + this.longPressTimer = null; + } + this.longPressActive = false; + this.suppressNextTap = false; this.listenerAbort = new AbortController(); const { signal } = this.listenerAbort; this.canvas.addEventListener("pointerdown", (e) => this.onPointerDown(e), { @@ -1274,6 +1280,16 @@ export class InputHandler { ); this.listenerAbort?.abort(); this.listenerAbort = null; + // A touch pointerdown arms an 800ms long-press timer. Aborting the + // listeners does not cancel it, so without this it can still fire after + // teardown: emitting TouchLongPressStartEvent on a dead bus and setting + // the cursor on a canvas the renderer has already removed. + if (this.longPressTimer !== null) { + clearTimeout(this.longPressTimer); + this.longPressTimer = null; + } + this.longPressActive = false; + this.suppressNextTap = false; this.activeKeys.clear(); this.lastGestureScale = null; this.keybindAndEvent = []; diff --git a/tests/InputHandler.test.ts b/tests/InputHandler.test.ts index 109634041b..85f4c91227 100644 --- a/tests/InputHandler.test.ts +++ b/tests/InputHandler.test.ts @@ -5,6 +5,7 @@ import { ConfirmGhostStructureEvent, ContextMenuEvent, InputHandler, + TouchLongPressStartEvent, UnitSelectionEvent, WarshipSelectionBoxCancelEvent, WarshipSelectionBoxCompleteEvent, @@ -1414,6 +1415,66 @@ describe("InputHandler teardown (OPE-411)", () => { } }); + it("cancels a pending long-press timer on destroy()", () => { + vi.useFakeTimers(); + const bus = new EventBus(); + const handler = makeHandler(document.createElement("canvas"), bus); + try { + handler.initialize(); + handler["onPointerDown"]( + new PointerEvent("pointerdown", { + button: 0, + clientX: 10, + clientY: 20, + pointerId: 1, + pointerType: "touch", + }), + ); + expect(handler["longPressTimer"]).not.toBeNull(); + + handler.destroy(); + const emit = vi.spyOn(bus, "emit"); + vi.advanceTimersByTime(2000); + + expect(emit).not.toHaveBeenCalled(); + expect(handler["longPressActive"]).toBe(false); + } finally { + handler.destroy(); + vi.useRealTimers(); + } + }); + + it("cancels a pending long-press timer on re-initialize", () => { + vi.useFakeTimers(); + const bus = new EventBus(); + const handler = makeHandler(document.createElement("canvas"), bus); + try { + handler.initialize(); + handler["onPointerDown"]( + new PointerEvent("pointerdown", { + button: 0, + clientX: 10, + clientY: 20, + pointerId: 1, + pointerType: "touch", + }), + ); + + handler.initialize(); + const emit = vi.spyOn(bus, "emit"); + vi.advanceTimersByTime(2000); + + expect( + emit.mock.calls.some( + (c: unknown[]) => c[0] instanceof TouchLongPressStartEvent, + ), + ).toBe(false); + } finally { + handler.destroy(); + vi.useRealTimers(); + } + }); + it("clears the pan/zoom interval on destroy()", () => { vi.useFakeTimers(); const handler = makeHandler( From 7bf069f2f314888f92ffd45c2766fa92cadb9fb3 Mon Sep 17 00:00:00 2001 From: Josh Harris Date: Sat, 12 Sep 2026 13:31:04 +0100 Subject: [PATCH 5/6] fix(client): release the InputHandler's EventBus subscription on destroy (OPE-411) Stand-in review. The EventBus is created once per page (Main.ts:247) and handed to every joinLobby(), so "the bus dies with the game" was wrong: the UnitSelectionEvent subscription that initialize() added kept a finished game's InputHandler -- and the GameView, uiState and canvas overlay it closes over -- reachable for the rest of the session, once per in-game transition, with its closure running against the next game's events. Hold the callback in a field (onUnitSelection), off() it in destroy(), and off()-before-on() in initialize() so a second call cannot double the subscription. Corrected the two comments that claimed the bus dies with the game, here and in ClientGameRunner.stop(). Tests: destroy() then emitting UnitSelectionEvent leaves the cursor and unitSelectionActive untouched (both probes discriminating), and the second-initialize test now asserts the callback fires exactly once. Mutation-checked: dropping the destroy() off() fails two tests, dropping the off()-before-on() fails one. Also renamed the ClientGameRunner test to "calls input.destroy()", which is what it actually asserts. Co-Authored-By: Claude Fable 5.1 --- src/client/ClientGameRunner.ts | 10 ++-- src/client/InputHandler.ts | 54 ++++++++++++-------- tests/InputHandler.test.ts | 36 +++++++++++++ tests/client/ClientGameRunnerActions.test.ts | 2 +- 4 files changed, 75 insertions(+), 27 deletions(-) diff --git a/src/client/ClientGameRunner.ts b/src/client/ClientGameRunner.ts index 24f04eb6a7..9642d9fecd 100644 --- a/src/client/ClientGameRunner.ts +++ b/src/client/ClientGameRunner.ts @@ -1121,10 +1121,12 @@ export class ClientGameRunner { public stop() { this.soundManager.dispose(); this.graphicsListenerAbort?.abort(); - // Detach the input handler's window/canvas listeners. Nothing else ever - // did, so a handler from a finished game kept translating keys into events - // on a dead bus, and joining another game without a page reload stacked a - // second live handler on top. Idempotent, like the disposals around it. + // Detach the input handler's window/canvas listeners and its EventBus + // subscription. Nothing else ever did, and the bus is created once per + // page, so a handler from a finished game kept translating keys into + // events that the next game receives, and joining another game without a + // page reload stacked a second live handler on top. Idempotent, like the + // disposals around it. this.input.destroy(); this.disposeRenderer?.(); if (!this.isActive) return; diff --git a/src/client/InputHandler.ts b/src/client/InputHandler.ts index 3562b4ce0b..49911803d3 100644 --- a/src/client/InputHandler.ts +++ b/src/client/InputHandler.ts @@ -283,30 +283,38 @@ export class InputHandler { this.onKeybindsChanged, ); - // Listen for warship selection to change cursor - this.eventBus.on(UnitSelectionEvent, (e) => { - this.unitSelectionActive = - e.isSelected && (e.unit !== null || (e.units ?? []).length > 0); - if (e.isSelected && (e.units ?? []).length > 0) { - // Multi-selection active - this.multiSelectionActive = true; - this.canvas.style.cursor = "crosshair"; - } else if (e.isSelected) { - // Single warship selected — cursor crosshair, but not multi - this.multiSelectionActive = false; - this.canvas.style.cursor = "crosshair"; - } else { - // Deselected - this.multiSelectionActive = false; - if (!this.selectionBoxActive) { - this.canvas.style.cursor = ""; - } - } - }); + // Listen for warship selection to change cursor. Held in a field so + // destroy() can release it: the EventBus is created once per page in + // Main.ts and handed to every joinLobby(), so a subscription left behind + // keeps this handler -- and the GameView, uiState and overlay it closes + // over -- alive for the rest of the session, and runs against the next + // game's events. off() first so a second initialize() cannot double it. + this.eventBus.off(UnitSelectionEvent, this.onUnitSelection); + this.eventBus.on(UnitSelectionEvent, this.onUnitSelection); this.initializePointerAndKeyboardEvents(); } + private onUnitSelection = (e: UnitSelectionEvent) => { + this.unitSelectionActive = + e.isSelected && (e.unit !== null || (e.units ?? []).length > 0); + if (e.isSelected && (e.units ?? []).length > 0) { + // Multi-selection active + this.multiSelectionActive = true; + this.canvas.style.cursor = "crosshair"; + } else if (e.isSelected) { + // Single warship selected — cursor crosshair, but not multi + this.multiSelectionActive = false; + this.canvas.style.cursor = "crosshair"; + } else { + // Deselected + this.multiSelectionActive = false; + if (!this.selectionBoxActive) { + this.canvas.style.cursor = ""; + } + } + }; + private onKeybindsChanged = () => { this.buildKeybindTable(); }; @@ -1280,10 +1288,12 @@ export class InputHandler { ); this.listenerAbort?.abort(); this.listenerAbort = null; + this.eventBus.off(UnitSelectionEvent, this.onUnitSelection); // A touch pointerdown arms an 800ms long-press timer. Aborting the // listeners does not cancel it, so without this it can still fire after - // teardown: emitting TouchLongPressStartEvent on a dead bus and setting - // the cursor on a canvas the renderer has already removed. + // teardown: emitting TouchLongPressStartEvent on the page-global bus, + // into the next game, and setting the cursor on a canvas the renderer + // has already removed. if (this.longPressTimer !== null) { clearTimeout(this.longPressTimer); this.longPressTimer = null; diff --git a/tests/InputHandler.test.ts b/tests/InputHandler.test.ts index 85f4c91227..0d60668cb9 100644 --- a/tests/InputHandler.test.ts +++ b/tests/InputHandler.test.ts @@ -1475,6 +1475,27 @@ describe("InputHandler teardown (OPE-411)", () => { } }); + it("releases its EventBus subscription on destroy()", () => { + const unit = { id: () => 1 } as unknown as UnitView; + + // Control: while alive the subscription drives the cursor. + eventBus.emit(new UnitSelectionEvent(unit, true)); + expect(canvas.style.cursor).toBe("crosshair"); + canvas.style.cursor = ""; + + inputHandler.destroy(); + + // The EventBus is page-global, so a subscription left behind would keep + // this handler alive and run it against the next game's events. Both + // probes are discriminating: a live subscription would set the crosshair + // on the first, and clear unitSelectionActive on the second. + eventBus.emit(new UnitSelectionEvent(unit, true)); + expect(canvas.style.cursor).toBe(""); + + eventBus.emit(new UnitSelectionEvent(null, false)); + expect(inputHandler["unitSelectionActive"]).toBe(true); + }); + it("clears the pan/zoom interval on destroy()", () => { vi.useFakeTimers(); const handler = makeHandler( @@ -1497,14 +1518,29 @@ describe("InputHandler teardown (OPE-411)", () => { vi.useFakeTimers(); const bus = new EventBus(); const handler = makeHandler(document.createElement("canvas"), bus); + // The bus captures this field's value at initialize() time, so swapping + // it first lets us count how many times the subscription is registered. + const onUnitSelection = vi.fn(); + handler["onUnitSelection"] = onUnitSelection; try { handler.initialize(); handler.initialize(); expect(vi.getTimerCount()).toBe(1); + bus.emit( + new UnitSelectionEvent({ id: () => 1 } as unknown as UnitView, true), + ); + expect(onUnitSelection).toHaveBeenCalledTimes(1); + handler.destroy(); expect(vi.getTimerCount()).toBe(0); + onUnitSelection.mockClear(); + bus.emit( + new UnitSelectionEvent({ id: () => 1 } as unknown as UnitView, true), + ); + expect(onUnitSelection).not.toHaveBeenCalled(); + const emit = vi.spyOn(bus, "emit"); window.dispatchEvent(new KeyboardEvent("keydown", { code: "Escape" })); expect(emit).not.toHaveBeenCalled(); diff --git a/tests/client/ClientGameRunnerActions.test.ts b/tests/client/ClientGameRunnerActions.test.ts index 257a61555c..2251884487 100644 --- a/tests/client/ClientGameRunnerActions.test.ts +++ b/tests/client/ClientGameRunnerActions.test.ts @@ -237,7 +237,7 @@ describe("auto boat", () => { }); describe("stop() (OPE-411)", () => { - it("destroys the input handler so its listeners stop firing", () => { + it("calls input.destroy()", () => { const { runner, input } = makeRunner({}); runner.stop(); From 6e546de170ab839cc9dbf9b8570a2bf0bb630950 Mon Sep 17 00:00:00 2001 From: Josh Harris Date: Sun, 13 Sep 2026 21:49:23 +0100 Subject: [PATCH 6/6] fix(client): drop in-flight pointer state on teardown and re-initialize (OPE-411) Claude review. destroy() and the re-initialize guard reset the long-press flags but not pointerDown, selectionBoxActive or multiSelectionActive. Because initialize() clears the pointers map unconditionally, a re-initialize while a pointer was physically down left pointerDown true with an empty map, so the next ordinary pointermove was treated as a drag from a stale origin. Extract the reset the blur handler already performed into a private resetPointerState() and call it from all three places. Blur's observable behaviour is unchanged: it captures whether a selection was active before the call and re-emits WarshipSelectionBoxCancelEvent afterwards, so the same events fire in the same order and the cursor reset still runs last. resetPointerState() itself emits nothing, which is what the other two callers need -- destroy() must not push events onto the page-global bus. Tests cover the drag-from-stale-origin case, the state after destroy(), and all three halves of the blur contract. Mutation-checked: dropping `pointerDown = false` fails two tests. Co-Authored-By: Claude Fable 5.1 --- src/client/InputHandler.ts | 63 +++++++++++---------- tests/InputHandler.test.ts | 112 +++++++++++++++++++++++++++++++------ 2 files changed, 127 insertions(+), 48 deletions(-) diff --git a/src/client/InputHandler.ts b/src/client/InputHandler.ts index 49911803d3..a37b7d345a 100644 --- a/src/client/InputHandler.ts +++ b/src/client/InputHandler.ts @@ -319,6 +319,28 @@ export class InputHandler { this.buildKeybindTable(); }; + /** + * Drops every piece of in-flight pointer/drag/long-press state. Shared by + * the blur handler, the re-initialize guard and destroy(): each has to leave + * the handler with nothing latched, or a pointer that was physically down + * stays recorded as down while `pointers` is empty, and the next ordinary + * move is treated as a drag from a stale origin. Deliberately emits + * nothing -- blur re-emits the events it owes around this call. + */ + private resetPointerState() { + this.pointerDown = false; + this.pointers.clear(); + this.lastGestureScale = null; + if (this.longPressTimer !== null) { + clearTimeout(this.longPressTimer); + this.longPressTimer = null; + } + this.longPressActive = false; + this.suppressNextTap = false; + this.selectionBoxActive = false; + this.multiSelectionActive = false; + } + /** Re-read the player's keybinds and rebuild the key dispatch table. */ private buildKeybindTable() { this.keybinds = this.userSettings.keybinds(Platform.isMac); @@ -494,12 +516,7 @@ export class InputHandler { clearInterval(this.moveInterval); this.moveInterval = null; } - if (this.longPressTimer !== null) { - clearTimeout(this.longPressTimer); - this.longPressTimer = null; - } - this.longPressActive = false; - this.suppressNextTap = false; + this.resetPointerState(); this.listenerAbort = new AbortController(); const { signal } = this.listenerAbort; this.canvas.addEventListener("pointerdown", (e) => this.onPointerDown(e), { @@ -572,18 +589,10 @@ export class InputHandler { this.alternateView = false; this.eventBus.emit(new AlternateViewEvent(false)); } - this.pointerDown = false; - this.pointers.clear(); - this.lastGestureScale = null; - if (this.longPressTimer !== null) { - clearTimeout(this.longPressTimer); - this.longPressTimer = null; - } - this.longPressActive = false; - this.suppressNextTap = false; - if (this.selectionBoxActive || this.multiSelectionActive) { - this.selectionBoxActive = false; - this.multiSelectionActive = false; + const hadSelection = + this.selectionBoxActive || this.multiSelectionActive; + this.resetPointerState(); + if (hadSelection) { this.eventBus.emit(new WarshipSelectionBoxCancelEvent()); } this.canvas.style.cursor = ""; @@ -1289,19 +1298,13 @@ export class InputHandler { this.listenerAbort?.abort(); this.listenerAbort = null; this.eventBus.off(UnitSelectionEvent, this.onUnitSelection); - // A touch pointerdown arms an 800ms long-press timer. Aborting the - // listeners does not cancel it, so without this it can still fire after - // teardown: emitting TouchLongPressStartEvent on the page-global bus, - // into the next game, and setting the cursor on a canvas the renderer - // has already removed. - if (this.longPressTimer !== null) { - clearTimeout(this.longPressTimer); - this.longPressTimer = null; - } - this.longPressActive = false; - this.suppressNextTap = false; + // Includes the 800ms long-press timer a touch pointerdown arms: aborting + // the listeners does not cancel it, so without this it can still fire + // after teardown, emitting TouchLongPressStartEvent on the page-global + // bus, into the next game, and setting the cursor on a canvas the + // renderer has already removed. + this.resetPointerState(); this.activeKeys.clear(); - this.lastGestureScale = null; this.keybindAndEvent = []; this.keybinds = {}; } diff --git a/tests/InputHandler.test.ts b/tests/InputHandler.test.ts index 0d60668cb9..64f55eb91d 100644 --- a/tests/InputHandler.test.ts +++ b/tests/InputHandler.test.ts @@ -4,7 +4,9 @@ import { CloseViewEvent, ConfirmGhostStructureEvent, ContextMenuEvent, + DragEvent, InputHandler, + MouseOverEvent, TouchLongPressStartEvent, UnitSelectionEvent, WarshipSelectionBoxCancelEvent, @@ -1421,15 +1423,7 @@ describe("InputHandler teardown (OPE-411)", () => { const handler = makeHandler(document.createElement("canvas"), bus); try { handler.initialize(); - handler["onPointerDown"]( - new PointerEvent("pointerdown", { - button: 0, - clientX: 10, - clientY: 20, - pointerId: 1, - pointerType: "touch", - }), - ); + touchOrMouseDown(handler, "touch"); expect(handler["longPressTimer"]).not.toBeNull(); handler.destroy(); @@ -1450,15 +1444,7 @@ describe("InputHandler teardown (OPE-411)", () => { const handler = makeHandler(document.createElement("canvas"), bus); try { handler.initialize(); - handler["onPointerDown"]( - new PointerEvent("pointerdown", { - button: 0, - clientX: 10, - clientY: 20, - pointerId: 1, - pointerType: "touch", - }), - ); + touchOrMouseDown(handler, "touch"); handler.initialize(); const emit = vi.spyOn(bus, "emit"); @@ -1496,6 +1482,96 @@ describe("InputHandler teardown (OPE-411)", () => { expect(inputHandler["unitSelectionActive"]).toBe(true); }); + const touchOrMouseDown = (handler: InputHandler, pointerType: string) => + handler["onPointerDown"]( + new PointerEvent("pointerdown", { + button: 0, + clientX: 100, + clientY: 100, + pointerId: 1, + pointerType, + }), + ); + + const movePointer = (handler: InputHandler) => + handler["onPointerMove"]( + new PointerEvent("pointermove", { + button: 0, + clientX: 400, + clientY: 400, + pointerId: 1, + pointerType: "mouse", + }), + ); + + it("drops in-flight pointer state on re-initialize", () => { + // pointers.clear() runs unconditionally on initialize, so leaving + // pointerDown latched would make the next ordinary move a drag from a + // stale origin. + touchOrMouseDown(inputHandler, "mouse"); + expect(inputHandler["pointerDown"]).toBe(true); + + inputHandler.initialize(); + + const emit = vi.spyOn(eventBus, "emit"); + movePointer(inputHandler); + + expect( + emit.mock.calls.some((c: unknown[]) => c[0] instanceof DragEvent), + ).toBe(false); + expect( + emit.mock.calls.some((c: unknown[]) => c[0] instanceof MouseOverEvent), + ).toBe(true); + }); + + // resetPointerState() is shared with the blur handler; blur owes a cancel + // event that the other two callers must not emit, so lock both halves. + it("window blur still cancels an active selection box", () => { + inputHandler["selectionBoxActive"] = true; + const emit = vi.spyOn(eventBus, "emit"); + + window.dispatchEvent(new Event("blur")); + + expect( + emit.mock.calls.some( + (c: unknown[]) => c[0] instanceof WarshipSelectionBoxCancelEvent, + ), + ).toBe(true); + expect(inputHandler["selectionBoxActive"]).toBe(false); + expect(inputHandler["pointerDown"]).toBe(false); + }); + + it("window blur emits no cancel when nothing was selected", () => { + const emit = vi.spyOn(eventBus, "emit"); + + window.dispatchEvent(new Event("blur")); + + expect( + emit.mock.calls.some( + (c: unknown[]) => c[0] instanceof WarshipSelectionBoxCancelEvent, + ), + ).toBe(false); + }); + + it("destroy() emits nothing even with a selection box active", () => { + inputHandler["selectionBoxActive"] = true; + const emit = vi.spyOn(eventBus, "emit"); + + inputHandler.destroy(); + + expect(emit).not.toHaveBeenCalled(); + }); + + it("drops in-flight pointer state on destroy()", () => { + touchOrMouseDown(inputHandler, "mouse"); + inputHandler.destroy(); + + expect(inputHandler["pointerDown"]).toBe(false); + expect(inputHandler["pointers"].size).toBe(0); + expect(inputHandler["selectionBoxActive"]).toBe(false); + expect(inputHandler["multiSelectionActive"]).toBe(false); + }); + it("clears the pan/zoom interval on destroy()", () => { vi.useFakeTimers(); const handler = makeHandler(