Skip to content

feat: hold middle-click to continuously upgrade buildings #3821

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.

Sign up for GitHub

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

Open
MoritzCMD wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: hold middle-click to continuously upgrade buildings #3821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions resources/lang/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,10 @@
"lobby_id_visibility_label": "Hidden Lobby IDs",
"lobby_id_visibility_desc": "Hide Lobby ID in private lobby creation",
"toggle_visibility": "Toggle Visibility",
"hold_middle_click_label": "Hold Middle-Click to Upgrade",
"hold_middle_click_desc": "Hold the middle mouse button to continuously upgrade a building.",
"hold_middle_click_speed_label": "Upgrade Hold Speed",
"hold_middle_click_speed_desc": "How fast buildings upgrade while holding middle-click. Lower = faster. (50–500ms)",
"left_click_label": "Left Click to Open Menu",
"left_click_desc": "When ON, left-click opens menu and sword button attacks. When OFF, left-click attacks directly.",
"left_click_menu": "Left Click Menu",
Expand Down
19 changes: 19 additions & 0 deletions src/client/InputHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,11 @@ export class InputHandler {

private readonly userSettings: UserSettings = new UserSettings();

private holdUpgradeInterval: ReturnType<typeof setInterval> | undefined =
undefined;
private holdUpgradeTimeout: ReturnType<typeof setTimeout> | undefined =
undefined;

constructor(
private gameView: GameView,
public uiState: UIState,
Expand Down Expand Up @@ -581,6 +586,16 @@ export class InputHandler {
if (event.button === 1) {
event.preventDefault();
this.eventBus.emit(new AutoUpgradeEvent(event.clientX, event.clientY));

if (this.userSettings.holdMiddleClickUpgrade()) {
this.holdUpgradeTimeout = setTimeout(() => {
this.holdUpgradeInterval = setInterval(() => {
this.eventBus.emit(
new AutoUpgradeEvent(event.clientX, event.clientY),
);
}, this.userSettings.holdMiddleClickUpgradeSpeed());
}, 400);
}
Comment on lines +590 to +598
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clear the middle-click timers outside pointerup too.

If focus changes or the handler is torn down while the button is still held, these timer handles can survive and keep emitting AutoUpgradeEvents in the background. Please route the cleanup through one helper and call it from blur/teardown as well.

🔧 Suggested cleanup helper 
+  private clearHoldUpgradeTimers() {
+    clearTimeout(this.holdUpgradeTimeout);
+    clearInterval(this.holdUpgradeInterval);
+    this.holdUpgradeTimeout = undefined;
+    this.holdUpgradeInterval = undefined;
+  }

Call it from the middle-button release path, the blur handler, and destroy().

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/client/InputHandler.ts` around lines 590 - 598, Create a single helper
(e.g. clearHoldMiddleClickTimers) that clears and nulls this.holdUpgradeTimeout
and this.holdUpgradeInterval (use clearTimeout/clearInterval) to stop
AutoUpgradeEvent emission; replace the existing inline clear logic in the
middle-button release path with a call to this helper and also call it from the
blur handler and destroy() so timers cannot survive focus loss or teardown.
Ensure references to this.userSettings.holdMiddleClickUpgradeSpeed(),
this.eventBus.emit, and AutoUpgradeEvent remain unchanged.

Copy link
Copy Markdown
Collaborator

@evanpelle evanpelle May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@evanpelle, here's the fix — a single clearHoldUpgradeTimers() helper called from all three places:

// Add this private helper method to InputHandler class
private clearHoldUpgradeTimers(): void {
  clearTimeout(this.holdUpgradeTimeout);
  clearInterval(this.holdUpgradeInterval);
  this.holdUpgradeTimeout = undefined;
  this.holdUpgradeInterval = undefined;
}

1. Replace the inline cleanup in onPointerUp:

   onPointerUp(event: PointerEvent) {
     if (event.button === 1) {
       event.preventDefault();
-      clearTimeout(this.holdUpgradeTimeout);
-      clearInterval(this.holdUpgradeInterval);
-      this.holdUpgradeTimeout = undefined;
-      this.holdUpgradeInterval = undefined;
+      this.clearHoldUpgradeTimers();
       return;
     }

2. Add cleanup in the blur handler (inside initialize()):

     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.clearHoldUpgradeTimers();
       if (this.longPressTimer !== null) {

3. Add cleanup in destroy():

   destroy() {
     if (this.moveInterval !== null) {
       clearInterval(this.moveInterval);
     }
+    this.clearHoldUpgradeTimers();
     this.activeKeys.clear();
   }

This keeps the logic in one place and makes sure the upgrade loop stops on focus loss or teardown — not just on mouse-up.

🧠 Learnings used 
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2874
File: src/server/MapLandTiles.ts:7-11
Timestamp: 2026-01-12T21:37:01.156Z
Learning: In this repository's OpenFrontIO deployment, inter-service HTTP calls to the master should target http://localhost:3000 (master at port 3000) as the canonical address. Apply this as the standard for all server-side TypeScript code that communicates with the master. Avoid hardcoding non-master URLs; centralize the master address (e.g., via config or env) when possible, and ensure internal service communication uses localhost:3000 in this architecture.

Learnt from: ryanbarlow97
Repo: openfrontio/OpenFrontIO PR: 2946
File: src/client/InputHandler.ts:21-21
Timestamp: 2026-01-18T15:42:50.689Z
Learning: In the OpenFrontIO repository, ensure that InputHandler.ts uses a MOUSE_CODES set that excludes MouseRight. Only MouseLeft and MouseMiddle should be supported for keybind actions; MouseRight should not be included in keybind mappings. Verify by inspecting the MOUSE_CODES definition and any keybind bindings to confirm MouseRight is not present, and add a test or assertion if possible to prevent regression.

Learnt from: evanpelle
Repo: openfrontio/OpenFrontIO PR: 3114
File: src/client/graphics/layers/AttacksDisplay.ts:64-77
Timestamp: 2026-02-10T05:02:28.715Z
Learning: In the OpenFrontIO codebase, GameUpdateType maps always have an array value for each key (never undefined). When iterating, you can safely use: for (const event of updates[GameUpdateType.X]) without optional chaining or guards. Treat updates as a Record<GameUpdateType, any[]> and rely on its guaranteed array values. If you modify the type, ensure the guarantee remains (e.g., initialize missing keys to []) to preserve safe iteration.

Learnt from: wozniakpl
Repo: openfrontio/OpenFrontIO PR: 3317
File: src/client/InputHandler.ts:344-350
Timestamp: 2026-03-07T11:35:24.071Z
Learning: Ensure ghost preservation logic for special units (AtomBomb and HydrogenBomb) is implemented via shouldPreserveGhostAfterBuild(unitType) and that ghost removal is conditional on !shouldPreserveGhostAfterBuild(unitType) within createStructure. Confirm that both mouse (MouseUpEvent) and keyboard (ConfirmGhostStructureEvent) confirmation paths go through requestConfirmStructure → createStructure, and that Enter/NumpadEnter confirmations do not break the locked-nuke multi-placement since the ghost should be preserved by design. Add tests to cover shouldPreserveGhostAfterBuild returning true for AtomBomb/HydrogenBomb and verify that ghost removal only occurs for other units. Applies to files in the repository OpenFrontIO (src/client/InputHandler.ts and src/client/graphics/layers/StructureIconsLayer.ts).

Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 3549
File: tests/pathfinding/playground/server.ts:246-250
Timestamp: 2026-03-31T17:02:41.798Z
Learning: When migrating/implementing Express 5+ code, use the Express 5 `app.listen(PORT, (error) => { ... })` callback signature: Express 5 passes listen errors (e.g., `EADDRINUSE`) as the first argument to the callback. Handle the error inside that callback instead of relying on Node.js `http.Server`’s `'error'` event semantics.

Learnt from: ryanbarlow97
Repo: openfrontio/OpenFrontIO PR: 3625
File: src/client/ClanApi.ts:164-180
Timestamp: 2026-04-10T00:17:12.810Z
Learning: When reviewing OpenFrontIO TypeScript client code (e.g., src/client/*.ts), do not require runtime Zod validation for response bodies in cases where the client calls an internal endpoint with a guaranteed, fixed response shape. For `joinClan()` in src/client/ClanApi.ts, allow returning the success body using a TypeScript `as { status: "joined" | "requested" }` cast without additional runtime Zod validation, since `/clans/:tag/join` is internal and always returns exactly `{ status: "joined" | "requested" }`.

Learnt from: baculinivan-web
Repo: openfrontio/OpenFrontIO PR: 3688
File: src/client/graphics/layers/GameRightSidebar.ts:235-248
Timestamp: 2026-04-22T08:54:50.345Z
Learning: In the OpenFrontIO/OpenFrontIO repository, do not raise review findings for standard web accessibility (a11y) keyboard/semantic issues that would normally require button semantics (e.g., using clickable `<div>` elements instead of `<button>`, or similar keyboard accessibility/ARIA patterns). This repository intentionally does not enforce those semantic/keyboard-accessibility conventions because gameplay requires touch or mouse input and is not operable via keyboard alone.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

return;
}

Expand Down Expand Up @@ -637,6 +652,10 @@ export class InputHandler {
onPointerUp(event: PointerEvent) {
if (event.button === 1) {
event.preventDefault();
clearTimeout(this.holdUpgradeTimeout);
clearInterval(this.holdUpgradeInterval);
this.holdUpgradeTimeout = undefined;
this.holdUpgradeInterval = undefined;
return;
}

Expand Down
28 changes: 28 additions & 0 deletions src/client/UserSettingModal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,34 @@ export class UserSettingModal extends BaseModal {
@change=${this.toggleCursorCostLabel}
></setting-toggle>

<!-- 🖱️ Hold Middle-Click Upgrade -->
<setting-toggle
label="${translateText("user_setting.hold_middle_click_label")}"
description="${translateText("user_setting.hold_middle_click_desc")}"
id="hold-middle-click-upgrade-toggle"
.checked=${this.userSettings.holdMiddleClickUpgrade()}
@change=${() => {
this.userSettings.toggleHoldMiddleClickUpgrade();
this.requestUpdate();
}}
></setting-toggle>

<!-- ⏱️ Upgrade Hold Speed -->
<setting-slider
label="${translateText("user_setting.hold_middle_click_speed_label")}"
description="${translateText(
"user_setting.hold_middle_click_speed_desc",
)}"
min="50"
max="500"
step="25"
unit="ms"
.value=${this.userSettings.holdMiddleClickUpgradeSpeed()}
@change=${(e: CustomEvent<{ value: number }>) => {
this.userSettings.setHoldMiddleClickUpgradeSpeed(e.detail.value);
}}
></setting-slider>

<!-- 🖱️ Left Click Menu -->
<setting-toggle
label="${translateText("user_setting.left_click_label")}"
Expand Down
3 changes: 2 additions & 1 deletion src/client/components/baseComponents/setting/SettingSlider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { customElement, property } from "lit/decorators.js";
export class SettingSlider extends LitElement {
@property() label = "Setting";
@property() description = "";
@property() unit = "%";
@property({ type: Number }) value = 0;
@property({ type: Number }) min = 0;
@property({ type: Number }) max = 100;
Expand Down Expand Up @@ -65,7 +66,7 @@ export class SettingSlider extends LitElement {
<div class="flex items-center gap-2 w-full">
<span
class="text-white font-bold text-sm shrink-0 text-right min-w-[3ch]"
>${this.value}%</span
>${this.value}${this.unit}</span
>
<input
type="range"
Expand Down
19 changes: 19 additions & 0 deletions src/core/game/UserSettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,25 @@ export class UserSettings {
return this.getBool("settings.leftClickOpensMenu", false);
}

holdMiddleClickUpgrade() {
return this.getBool("settings.holdMiddleClickUpgrade", true);
}

holdMiddleClickUpgradeSpeed(): number {
return this.getFloat("settings.holdMiddleClickUpgradeSpeed", 150);
}

toggleHoldMiddleClickUpgrade() {
this.setBool(
"settings.holdMiddleClickUpgrade",
!this.holdMiddleClickUpgrade(),
);
}

setHoldMiddleClickUpgradeSpeed(value: number): void {
this.setFloat("settings.holdMiddleClickUpgradeSpeed", value);
Comment on lines +166 to +178
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clamp the hold speed before persisting it.

A negative or NaN delay from storage/manual edits can turn the upgrade loop into a tight spin. Keep this within the documented 50-500ms window on both read and write.

🛠️ Suggested fix 
   holdMiddleClickUpgradeSpeed(): number {
-    return this.getFloat("settings.holdMiddleClickUpgradeSpeed", 150);
+    const raw = this.getFloat("settings.holdMiddleClickUpgradeSpeed", 150);
+    if (!Number.isFinite(raw)) return 150;
+    return Math.min(500, Math.max(50, Math.round(raw)));
   }
@@
   setHoldMiddleClickUpgradeSpeed(value: number): void {
-    this.setFloat("settings.holdMiddleClickUpgradeSpeed", value);
+    const normalized = Number.isFinite(value) ? Math.round(value) : 150;
+    this.setFloat(
+      "settings.holdMiddleClickUpgradeSpeed",
+      Math.min(500, Math.max(50, normalized)),
+    );
   }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/core/game/UserSettings.ts` around lines 166 - 178, Clamp the
hold-middle-click upgrade speed to the documented 50–500ms range on both read
and write to prevent negative or NaN values causing a tight loop: in get method
holdMiddleClickUpgradeSpeed() apply a clamp and NaN fallback after calling
getFloat("settings.holdMiddleClickUpgradeSpeed", 150) so it always returns a
number between 50 and 500, and in setter setHoldMiddleClickUpgradeSpeed(value:
number) sanitize the incoming value (handle NaN) and clamp it to 50–500 before
calling setFloat("settings.holdMiddleClickUpgradeSpeed", value); keep the
boolean toggle method holdMiddleClickUpgrade() unchanged.

Copy link
Copy Markdown
Collaborator

@evanpelle evanpelle May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Clamp holdMiddleClickUpgradeSpeed() getter to [50, 500] with a NaN / non-finite fallback to 150.
  • Clamp setHoldMiddleClickUpgradeSpeed() setter the same way before persisting.
🧠 Learnings used 
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2874
File: src/server/MapLandTiles.ts:7-11
Timestamp: 2026-01-12T21:37:01.156Z
Learning: In this repository's OpenFrontIO deployment, inter-service HTTP calls to the master should target http://localhost:3000 (master at port 3000) as the canonical address. Apply this as the standard for all server-side TypeScript code that communicates with the master. Avoid hardcoding non-master URLs; centralize the master address (e.g., via config or env) when possible, and ensure internal service communication uses localhost:3000 in this architecture.

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 3292
File: src/core/execution/TrainExecution.ts:70-84
Timestamp: 2026-02-25T02:38:42.961Z
Learning: Preserve the intentional duplication of boundary tiles when constructing train motion plan paths. Specifically, do not deduplicate tiles at station boundaries in train motion plan logic. This affects both server-side tracking (TrainExecution.ts saveTraversedTiles) and client-side processing (GameView.ts train motion plan processing). Reviewers should ensure changes do not remove or consolidate boundary tiles, as duplicates are expected and documented in MotionPlans.ts, and are necessary for correct car spacing and positioning, especially when speed > 1.

Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 3549
File: tests/pathfinding/playground/server.ts:246-250
Timestamp: 2026-03-31T17:02:41.798Z
Learning: When migrating/implementing Express 5+ code, use the Express 5 `app.listen(PORT, (error) => { ... })` callback signature: Express 5 passes listen errors (e.g., `EADDRINUSE`) as the first argument to the callback. Handle the error inside that callback instead of relying on Node.js `http.Server`’s `'error'` event semantics.

Learnt from: baculinivan-web
Repo: openfrontio/OpenFrontIO PR: 3688
File: src/client/graphics/layers/GameRightSidebar.ts:235-248
Timestamp: 2026-04-22T08:54:50.345Z
Learning: In the OpenFrontIO/OpenFrontIO repository, do not raise review findings for standard web accessibility (a11y) keyboard/semantic issues that would normally require button semantics (e.g., using clickable `<div>` elements instead of `<button>`, or similar keyboard accessibility/ARIA patterns). This repository intentionally does not enforce those semantic/keyboard-accessibility conventions because gameplay requires touch or mouse input and is not operable via keyboard alone.

Failed to handle agent chat message. Please try again.

}

territoryPatterns() {
return this.getBool("settings.territoryPatterns", true);
}
Expand Down
Loading