-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Click and hold nukes #5374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Click and hold nukes #5374
Changes from all commits
9a45e09
507bfec
fcbb807
23680c4
49a93f3
7f85047
ceccca4
dbf155e
639dc98
7428351
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -251,6 +251,17 @@ | |
| private suppressNextTap: boolean = false; | ||
| private readonly LONG_PRESS_MS = 800; | ||
|
|
||
| // Wait in MS before assuming mouse stationary. | ||
| public readonly HOLD_POINTER_WAIT_MS = 100; | ||
| private isClickHoldPastGrace = false; | ||
| private clickHoldGrace: ReturnType<typeof setTimeout> | null = null; | ||
| // Wait in MS before starting repeat | ||
| public readonly HOLD_SECOND_ACTION_DELAY_MS = 500; | ||
| private clickHoldEnsureIntent: ReturnType<typeof setTimeout> | null = null; | ||
| // Repeated trigger behavior | ||
| public readonly HOLD_REPEATED_ACTION_TRIGGER_RATE = 90; // hold-to-deploy firerate (multiplier affects this) | ||
| private clickHoldRepeat: ReturnType<typeof setInterval> | null = null; | ||
|
|
||
| private moveInterval: NodeJS.Timeout | null = null; | ||
| private activeKeys = new Set<string>(); | ||
| private keybinds: Record<string, string> = {}; | ||
|
|
@@ -786,7 +797,10 @@ | |
| this.lastPointerDownY = event.clientY; | ||
|
|
||
| this.eventBus.emit(new MouseDownEvent(event.clientX, event.clientY)); | ||
|
|
||
| // clickHold only for real mouse | ||
| if (event.pointerType === "mouse") { | ||
| this.clickHold(); | ||
| } | ||
| // Start long-press timer for touch devices | ||
| if (event.pointerType === "touch") { | ||
| this.longPressActive = false; | ||
|
|
@@ -832,6 +846,7 @@ | |
| } | ||
| this.pointerDown = false; | ||
| this.pointers.clear(); | ||
| this.clickHoldCleanup(); | ||
|
|
||
| // Clean up long-press state | ||
| if (this.longPressTimer !== null) { | ||
|
|
@@ -989,16 +1004,20 @@ | |
| if (this.pointers.size === 1) { | ||
| const deltaX = event.clientX - this.lastPointerX; | ||
| const deltaY = event.clientY - this.lastPointerY; | ||
| const moveDist = | ||
| Math.abs(event.clientX - this.lastPointerDownX) + | ||
| Math.abs(event.clientY - this.lastPointerDownY); | ||
|
|
||
| // Cancel long-press if finger moved significantly before timer fires | ||
| if (this.longPressTimer !== null) { | ||
| const moveDist = | ||
| Math.abs(event.clientX - this.lastPointerDownX) + | ||
| Math.abs(event.clientY - this.lastPointerDownY); | ||
| if (moveDist >= this.DRAG_THRESHOLD_PX) { | ||
| if (moveDist >= this.DRAG_THRESHOLD_PX) { | ||
| // Cancel long-press if finger moved significantly before timer fires | ||
| if (this.longPressTimer !== null) { | ||
| clearTimeout(this.longPressTimer); | ||
| this.longPressTimer = null; | ||
| } | ||
| // Cancel clickHold if dragged quickly | ||
| if (!this.isClickHoldPastGrace) { | ||
| this.clickHoldCleanup(); | ||
| } | ||
| } | ||
|
|
||
| // If shift is held OR touch long-press is active OR selection box already | ||
|
|
@@ -1223,6 +1242,61 @@ | |
| return false; | ||
| } | ||
|
|
||
| private clickHold() { | ||
| // for redefining valid ghosts | ||
| const isValidTarget = () => { | ||
| switch (this.uiState.ghostStructure) { | ||
| case UnitType.AtomBomb: | ||
| case UnitType.HydrogenBomb: | ||
| // MIRV seemed excessive to click hold. | ||
| return true; | ||
| default: | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| // Saves performance via guard clause and prevents some potential bugs | ||
| if (!isValidTarget()) { | ||
| return; | ||
| } | ||
|
|
||
| const repeatBehavior = () => { | ||
| isValidTarget() | ||
| ? this.eventBus.emit(new ConfirmGhostStructureEvent()) | ||
| : this.clickHoldCleanup(); | ||
| }; | ||
|
|
||
| // first: ensure grace period for click+drag has passed | ||
| this.clickHoldGrace = setTimeout(() => { | ||
| this.isClickHoldPastGrace = true; | ||
| // second: launch first event, and wait before repeating | ||
| repeatBehavior(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Suppress the release confirmation after a hold confirmation. After 🤖 Prompt for AI Agents |
||
| // finally, we are past initial hold delay | ||
| // and have launched the first event. | ||
| this.clickHoldEnsureIntent = setTimeout(() => { | ||
| // if mouse still held down, begin repeated events | ||
| // HOWEVER: we do not need to delay the first repeat behavior | ||
| repeatBehavior(); | ||
| this.clickHoldRepeat = setInterval(() => { | ||
| repeatBehavior(); | ||
| }, this.HOLD_REPEATED_ACTION_TRIGGER_RATE); | ||
| }, this.HOLD_SECOND_ACTION_DELAY_MS); | ||
| }, this.HOLD_POINTER_WAIT_MS); | ||
| } | ||
|
|
||
| private clickHoldCleanup() { | ||
| this.isClickHoldPastGrace = false; | ||
| if (this.clickHoldGrace !== null) { | ||
| clearTimeout(this.clickHoldGrace); | ||
| } | ||
| if (this.clickHoldEnsureIntent !== null) { | ||
| clearTimeout(this.clickHoldEnsureIntent); | ||
| } | ||
| if (this.clickHoldRepeat !== null) { | ||
| clearInterval(this.clickHoldRepeat); | ||
| } | ||
| } | ||
|
|
||
| destroy() { | ||
| if (this.moveInterval !== null) { | ||
| clearInterval(this.moveInterval); | ||
|
|
@@ -1231,6 +1305,7 @@ | |
| `${USER_SETTINGS_CHANGED_EVENT}:${KEYBINDS_KEY}`, | ||
| this.onKeybindsChanged, | ||
| ); | ||
| this.clickHoldCleanup(); | ||
| this.activeKeys.clear(); | ||
| this.lastGestureScale = null; | ||
| this.keybindAndEvent = []; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -992,6 +992,251 @@ | |
| }); | ||
| }); | ||
|
|
||
| describe("Click and hold when ghost is bomb", () => { | ||
| let inputHandler: InputHandler; | ||
| let mockGameView: GameView; | ||
| let eventBus: EventBus; | ||
| let mockCanvas: HTMLCanvasElement; | ||
| let uiState: UIState; | ||
|
|
||
| beforeEach(() => { | ||
| mockGameView = { | ||
| inSpawnPhase: () => false, | ||
| myPlayer: () => ({ isAlive: () => true }), | ||
| } as GameView; | ||
| mockCanvas = document.createElement("canvas"); | ||
| mockCanvas.width = 800; | ||
| mockCanvas.height = 600; | ||
|
|
||
| eventBus = new EventBus(); | ||
| uiState = { | ||
| attackRatio: 20, | ||
| ghostStructure: UnitType.AtomBomb, | ||
| rocketDirectionUp: true, | ||
| upgradeMultiplier: 1, | ||
| } as UIState; | ||
| inputHandler = new InputHandler( | ||
| mockGameView, | ||
| uiState, | ||
| mockCanvas, | ||
| eventBus, | ||
| ); | ||
| inputHandler.initialize(); | ||
|
Comment on lines
+1003
to
+1024
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift Use the required full-game test setup. This suite manually creates a mock Replace this setup with the required game instance and assert the bomb-launch behavior through the simulation. As per coding guidelines, tests “use a 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| }); | ||
|
|
||
| afterEach(() => { | ||
| inputHandler.destroy(); | ||
| }); | ||
|
|
||
| test("does not prevent single-click behavior within grace period", () => { | ||
| vi.useFakeTimers(); | ||
| const mockEmit = vi.spyOn(eventBus, "emit"); | ||
|
|
||
| const downEvent = new PointerEvent("pointerdown", { | ||
| button: 0, | ||
| clientX: 100, | ||
| clientY: 100, | ||
| pointerId: 1, | ||
| }); | ||
| const upEvent = new PointerEvent("pointerup", { | ||
| button: 0, | ||
| clientX: 100, | ||
| clientY: 100, | ||
| pointerId: 1, | ||
| }); | ||
| inputHandler["onPointerDown"](downEvent); | ||
|
|
||
| vi.advanceTimersByTime(inputHandler.HOLD_POINTER_WAIT_MS - 1); | ||
| inputHandler["onPointerUp"](upEvent); | ||
|
|
||
| const emittedTypes = mockEmit.mock.calls.map( | ||
| (call) => call[0].constructor.name, | ||
| ); | ||
| expect(emittedTypes).toContain("MouseUpEvent"); | ||
| vi.useRealTimers(); | ||
| }); | ||
|
|
||
| test("triggers events on expected timeline when fully stationary", () => { | ||
| vi.useFakeTimers(); | ||
| const mockEmit = vi.spyOn(eventBus, "emit"); | ||
| let el = 0; // expected launches | ||
| let multi = 15; | ||
|
|
||
| const downEvent = new PointerEvent("pointerdown", { | ||
| button: 0, | ||
| clientX: 100, | ||
| clientY: 100, | ||
| pointerId: 1, | ||
| }); | ||
|
|
||
| inputHandler["onPointerDown"](downEvent); | ||
|
|
||
| vi.advanceTimersByTime(inputHandler.HOLD_POINTER_WAIT_MS - 1); | ||
| expect( | ||
| mockEmit.mock.calls.filter( | ||
| ([event]) => event instanceof ConfirmGhostStructureEvent, | ||
| ), | ||
| ).toHaveLength(el); | ||
|
|
||
| vi.advanceTimersByTime(2); | ||
| el++; | ||
| expect( | ||
| mockEmit.mock.calls.filter( | ||
| ([event]) => event instanceof ConfirmGhostStructureEvent, | ||
| ), | ||
| ).toHaveLength(el); | ||
|
|
||
| vi.advanceTimersByTime(inputHandler.HOLD_SECOND_ACTION_DELAY_MS); | ||
| el++; | ||
| expect( | ||
| mockEmit.mock.calls.filter( | ||
| ([event]) => event instanceof ConfirmGhostStructureEvent, | ||
| ), | ||
| ).toHaveLength(el); | ||
|
|
||
| vi.advanceTimersByTime(inputHandler.HOLD_REPEATED_ACTION_TRIGGER_RATE); | ||
| el++; | ||
|
|
||
| expect( | ||
| mockEmit.mock.calls.filter( | ||
| ([event]) => event instanceof ConfirmGhostStructureEvent, | ||
| ), | ||
| ).toHaveLength(el); | ||
|
|
||
| vi.advanceTimersByTime( | ||
| inputHandler.HOLD_REPEATED_ACTION_TRIGGER_RATE * multi, | ||
| ); | ||
| el = el + multi; | ||
| expect( | ||
| mockEmit.mock.calls.filter( | ||
| ([event]) => event instanceof ConfirmGhostStructureEvent, | ||
| ), | ||
| ).toHaveLength(el); | ||
|
|
||
| const emittedTypes = mockEmit.mock.calls.map( | ||
| (call) => call[0].constructor.name, | ||
| ); | ||
| expect(emittedTypes).toContain("MouseDownEvent"); | ||
| expect(emittedTypes).toContain("ConfirmGhostStructureEvent"); | ||
|
|
||
| vi.useRealTimers(); | ||
| }); | ||
|
|
||
| test("triggers event on expected timeline when drag started after grace period", () => { | ||
| vi.useFakeTimers(); | ||
| const mockEmit = vi.spyOn(eventBus, "emit"); | ||
| let el = 0; // expected launches | ||
| let multi = 15; | ||
|
|
||
| const downEvent = new PointerEvent("pointerdown", { | ||
| button: 0, | ||
| clientX: 100, | ||
| clientY: 100, | ||
| pointerId: 1, | ||
| }); | ||
|
|
||
| const moveEvent = new PointerEvent("pointermove", { | ||
| button: 0, | ||
| clientX: 130, // 30px move > DRAG_THRESHOLD_PX (10) | ||
| clientY: 100, | ||
| pointerId: 1, | ||
| }); | ||
|
|
||
| inputHandler["onPointerDown"](downEvent); | ||
|
|
||
| vi.advanceTimersByTime(inputHandler.HOLD_POINTER_WAIT_MS - 2); | ||
| // right before grace period | ||
| expect( | ||
| mockEmit.mock.calls.filter( | ||
| ([event]) => event instanceof ConfirmGhostStructureEvent, | ||
| ), | ||
| ).toHaveLength(el); | ||
|
|
||
| vi.advanceTimersByTime(2); | ||
| // right after grace period, move the mouse | ||
| inputHandler["onPointerMove"](moveEvent); | ||
| el++; | ||
| expect( | ||
| mockEmit.mock.calls.filter( | ||
| ([event]) => event instanceof ConfirmGhostStructureEvent, | ||
| ), | ||
| ).toHaveLength(el); | ||
|
|
||
| vi.advanceTimersByTime(inputHandler.HOLD_SECOND_ACTION_DELAY_MS); | ||
| el++; | ||
| expect( | ||
| mockEmit.mock.calls.filter( | ||
| ([event]) => event instanceof ConfirmGhostStructureEvent, | ||
| ), | ||
| ).toHaveLength(el); | ||
|
|
||
| vi.advanceTimersByTime(inputHandler.HOLD_REPEATED_ACTION_TRIGGER_RATE); | ||
| el++; | ||
|
|
||
| expect( | ||
| mockEmit.mock.calls.filter( | ||
| ([event]) => event instanceof ConfirmGhostStructureEvent, | ||
| ), | ||
| ).toHaveLength(el); | ||
|
|
||
| vi.advanceTimersByTime( | ||
| inputHandler.HOLD_REPEATED_ACTION_TRIGGER_RATE * multi, | ||
| ); | ||
| el = el + multi; | ||
| expect( | ||
| mockEmit.mock.calls.filter( | ||
| ([event]) => event instanceof ConfirmGhostStructureEvent, | ||
| ), | ||
| ).toHaveLength(el); | ||
|
|
||
| const emittedTypes = mockEmit.mock.calls.map( | ||
| (call) => call[0].constructor.name, | ||
| ); | ||
| expect(emittedTypes).toContain("MouseDownEvent"); | ||
| expect(emittedTypes).toContain("ConfirmGhostStructureEvent"); | ||
|
|
||
| vi.useRealTimers(); | ||
| }); | ||
|
|
||
| test("clickHold does nothing when pointer moved before the grace period completes", () => { | ||
| vi.useFakeTimers(); | ||
| const mockEmit = vi.spyOn(eventBus, "emit"); | ||
|
|
||
| const downEvent = new PointerEvent("pointerdown", { | ||
| button: 0, | ||
| clientX: 100, | ||
| clientY: 100, | ||
| pointerId: 1, | ||
| }); | ||
|
|
||
| const moveEvent = new PointerEvent("pointermove", { | ||
| button: 0, | ||
| clientX: 130, // 30px move > DRAG_THRESHOLD_PX (10) | ||
| clientY: 100, | ||
| pointerId: 1, | ||
| }); | ||
|
|
||
| inputHandler["onPointerDown"](downEvent); | ||
| vi.advanceTimersByTime(1); | ||
| inputHandler["onPointerMove"](moveEvent); | ||
|
|
||
| vi.advanceTimersByTime(inputHandler.HOLD_POINTER_WAIT_MS - 2); | ||
| // still within grace period | ||
| inputHandler["onPointerMove"](moveEvent); | ||
|
|
||
| vi.advanceTimersByTime( | ||
| inputHandler.HOLD_REPEATED_ACTION_TRIGGER_RATE + | ||
| inputHandler.HOLD_REPEATED_ACTION_TRIGGER_RATE, | ||
| ); | ||
|
|
||
| const confirmCalls = mockEmit.mock.calls.filter( | ||
| ([event]) => event instanceof ConfirmGhostStructureEvent, | ||
| ); | ||
| expect(confirmCalls).toHaveLength(0); | ||
| vi.useRealTimers(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("Warship box selection (Shift+drag)", () => { | ||
| let inputHandler: InputHandler; | ||
| let eventBus: EventBus; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the side-effect ternary with
ifandelse.tseslint.configs.recommendedenables@typescript-eslint/no-unused-expressions. The standalone ternary at lines 1264–1266 is therefore reported bylint:eslint, which makes the declaredlintcommand fail.Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 ESLint
[error] 1264-1266: Expected an assignment or function call and instead saw an expression.
(
@typescript-eslint/no-unused-expressions)🪛 GitHub Check: 🔍 Lint
[failure] 1264-1264:
Expected an assignment or function call and instead saw an expression. (
@typescript-eslint/no-unused-expressions)🤖 Prompt for AI Agents