bugfix(mouse): Fix bad drag tolerances with high scroll speed factors

[xezon]

• Fixes Army gets deselected when moving camera with drag right click #2778

This change fixes the bad drag tolerances with high scroll speed factors, which is a retail bug but is more pronounced in this Project because players are encouraged to set way higher scroll factors after #1026 when higher frame rates no longer increase the camera movement.

[greptile-apps]

Greptile Summary

This PR fixes click-vs-drag misclassification that occurs when players use high keyboard scroll speed factors. It introduces getDragToleranceAdjustedForScrollFactor() that scales m_dragTolerance inversely with the current m_keyboardScrollFactor, so a higher scroll speed reduces the pixel tolerance required to register a drag.

• Mouse::getDragToleranceAdjustedForScrollFactor() is added to Mouse.cpp and declared in Mouse.h; Mouse::isClick() is updated to use this adjusted value instead of the raw m_dragTolerance.
• The OptionPreferences::getScrollFactor() path already clamps the scroll factor to a minimum of 0.01f, but the formula inside getDragToleranceAdjustedForScrollFactor() divides by m_keyboardScrollFactor without a zero-guard — a zero value (reachable via direct INI parsing before preferences override) would produce +infinity as the tolerance, silently breaking all drag detection for the session.

Confidence Score: 3/5

The drag tolerance scaling logic is directionally correct and well-scoped, but the new helper function divides by m_keyboardScrollFactor without checking for zero first.

The new getDragToleranceAdjustedForScrollFactor() function computes m_dragTolerance * (default / current) without guarding against m_keyboardScrollFactor == 0. When m_keyboardScrollFactor is zero — reachable if INI parsing sets it before preferences override it — the result is +infinity, which silently makes isClick() return TRUE for all mouse movement, breaking box selection and right-click drag for the rest of the session. The fix otherwise looks correct and contained to a single function.

Core/GameEngine/Source/GameClient/Input/Mouse.cpp — the new getDragToleranceAdjustedForScrollFactor() method needs a zero-guard on m_keyboardScrollFactor.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameClient/Input/Mouse.cpp	Introduces getDragToleranceAdjustedForScrollFactor() to scale drag tolerance inversely with m_keyboardScrollFactor; the division lacks a zero-guard, which can produce +infinity if the scroll factor is ever 0.
Core/GameEngine/Include/GameClient/Mouse.h	Adds the getDragToleranceAdjustedForScrollFactor() public const method declaration; straightforward header addition, no issues.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User as Player
    participant Input as Input Handler
    participant Mouse as Mouse::isClick()
    participant Adj as getDragToleranceAdjustedForScrollFactor()
    participant GD as TheGlobalData

    User->>Input: press + release mouse button
    Input->>Mouse: isClick(anchor, dest, t0, t1)
    Mouse->>Adj: getDragToleranceAdjustedForScrollFactor()
    Adj->>GD: read m_keyboardDefaultScrollFactor
    Adj->>GD: read m_keyboardScrollFactor
    Note over Adj: dragTolerance = m_dragTolerance * (default / current)<br/>⚠ Division by zero if m_keyboardScrollFactor == 0
    Adj-->>Mouse: dragTolerance (Real)
    Mouse->>Mouse: "compare abs(delta.x/y) > dragTolerance"
    Mouse-->>Input: TRUE (click) / FALSE (drag)

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant User as Player
    participant Input as Input Handler
    participant Mouse as Mouse::isClick()
    participant Adj as getDragToleranceAdjustedForScrollFactor()
    participant GD as TheGlobalData

    User->>Input: press + release mouse button
    Input->>Mouse: isClick(anchor, dest, t0, t1)
    Mouse->>Adj: getDragToleranceAdjustedForScrollFactor()
    Adj->>GD: read m_keyboardDefaultScrollFactor
    Adj->>GD: read m_keyboardScrollFactor
    Note over Adj: dragTolerance = m_dragTolerance * (default / current)<br/>⚠ Division by zero if m_keyboardScrollFactor == 0
    Adj-->>Mouse: dragTolerance (Real)
    Mouse->>Mouse: "compare abs(delta.x/y) > dragTolerance"
    Mouse-->>Input: TRUE (click) / FALSE (drag)

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
Core/GameEngine/Source/GameClient/Input/Mouse.cpp:414
**Potential division by zero in scroll factor adjustment**

`m_keyboardScrollFactor` is written directly via `INI::parseReal` without clamping. If a game or mod INI file sets `KeyboardScrollSpeedFactor = 0`, this line produces `+infinity` (IEEE 754), making `dragTolerance` infinite. Every subsequent call to `isClick` would then return `TRUE` for any movement, breaking box selection and right-click drag detection entirely. The `OptionPreferences::getScrollFactor()` path clamps the minimum to `1` (→ `0.01f`), but that overwrite only happens after INI parsing, so a corrupt preference state or a modded INI with zero could still reach here with `m_keyboardScrollFactor == 0.0f`. Adding a guard like `if (TheGlobalData->m_keyboardScrollFactor > 0.0f)` before performing the division would prevent this.

```suggestion
	if (TheGlobalData->m_keyboardScrollFactor > 0.0f)
		return m_dragTolerance * (TheGlobalData->m_keyboardDefaultScrollFactor / TheGlobalData->m_keyboardScrollFactor);
	return m_dragTolerance;
```

_{Reviews (1): Last reviewed commit: "bugfix(mouse): Fix bad drag tolerances w..." | Re-trigger Greptile}

[greptile-apps]

Potential division by zero in scroll factor adjustment

m_keyboardScrollFactor is written directly via INI::parseReal without clamping. If a game or mod INI file sets KeyboardScrollSpeedFactor = 0, this line produces +infinity (IEEE 754), making dragTolerance infinite. Every subsequent call to isClick would then return TRUE for any movement, breaking box selection and right-click drag detection entirely. The OptionPreferences::getScrollFactor() path clamps the minimum to 1 (→ 0.01f), but that overwrite only happens after INI parsing, so a corrupt preference state or a modded INI with zero could still reach here with m_keyboardScrollFactor == 0.0f. Adding a guard like if (TheGlobalData->m_keyboardScrollFactor > 0.0f) before performing the division would prevent this.

Suggested change

	return m_dragTolerance * (TheGlobalData->m_keyboardDefaultScrollFactor / TheGlobalData->m_keyboardScrollFactor);
	if (TheGlobalData->m_keyboardScrollFactor > 0.0f)
	return m_dragTolerance * (TheGlobalData->m_keyboardDefaultScrollFactor / TheGlobalData->m_keyboardScrollFactor);
	return m_dragTolerance;

Prompt To Fix With AI

This is a comment left during a code review.
Path: Core/GameEngine/Source/GameClient/Input/Mouse.cpp
Line: 414

Comment:
**Potential division by zero in scroll factor adjustment**

`m_keyboardScrollFactor` is written directly via `INI::parseReal` without clamping. If a game or mod INI file sets `KeyboardScrollSpeedFactor = 0`, this line produces `+infinity` (IEEE 754), making `dragTolerance` infinite. Every subsequent call to `isClick` would then return `TRUE` for any movement, breaking box selection and right-click drag detection entirely. The `OptionPreferences::getScrollFactor()` path clamps the minimum to `1` (→ `0.01f`), but that overwrite only happens after INI parsing, so a corrupt preference state or a modded INI with zero could still reach here with `m_keyboardScrollFactor == 0.0f`. Adding a guard like `if (TheGlobalData->m_keyboardScrollFactor > 0.0f)` before performing the division would prevent this.

```suggestion
	if (TheGlobalData->m_keyboardScrollFactor > 0.0f)
		return m_dragTolerance * (TheGlobalData->m_keyboardDefaultScrollFactor / TheGlobalData->m_keyboardScrollFactor);
	return m_dragTolerance;
```

How can I resolve this? If you propose a fix, please make it concise.

[xezon]

If it is zero then we cannot scroll and have more problems than that.

[greptile-apps]

Fair point — a scroll factor of zero means camera scrolling is already non-functional, so defending against infinite drag tolerance in that broken state adds noise without real benefit. I'll withdraw the concern.

[Caball009]

#1501 Seems related.

Maybe using DragTolerance3D (or the idea behind it: distance in the actual game world) makes more sense than accounting for scroll factor?

[xezon]

Maybe using DragTolerance3D (or the idea behind it: distance in the actual game world) makes more sense than accounting for scroll factor?

In principle yes, but that would require

DragTolerance3D = 30 ; How many feet in world space should we allow before it is a drag?

to be a reasonable value and the arguments of the isClick function to take in world positions instead of mouse positions.

#1501 Seems related.

It looks related yes.