bugfix(mouse): Fix bad drag tolerances with high scroll speed factors#2823
bugfix(mouse): Fix bad drag tolerances with high scroll speed factors#2823xezon wants to merge 2 commits into
Conversation
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Source/GameClient/Input/Mouse.cpp | Core fix: isClick now calls getDragToleranceAdjustedForScrollFactor() and compares circular distance; the adjusted tolerance is correctly used in the condition. A two-line comment at the if-guard has inverted wording that contradicts the actual greater-than checks. |
| Core/GameEngine/Include/GameClient/Mouse.h | isClick signature updated from raw pointers to const references; getDragToleranceAdjustedForScrollFactor() declared as a new const getter. Both changes are clean. |
| Core/Libraries/Include/Lib/BaseType.h | Adds arithmetic operators (+, -, +=, -=), set/add/sub helpers, and lengthSqr() to ICoord2D, Coord2D, Coord3D, and ICoord3D. All implementations look correct for typical screen-coordinate values. |
| Core/GameEngine/Source/GameClient/MessageStream/SelectionXlat.cpp | Removes the 3D camera-position check (m_deselectDownCameraPosition) that was computed but never actually passed to isClick, making it dead code; now calls isClick with references instead of pointers. |
| Core/GameEngine/Include/GameClient/SelectionXlat.h | Removes the m_deselectDownCameraPosition field that is no longer needed after the dead camera-position check was deleted. |
| Core/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp | Two call-sites updated to pass ICoord2D by const reference instead of pointer; no logic change. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Caller as CommandXlat / SelectionXlat
participant Mouse
participant GlobalData as TheGlobalData
Caller->>Mouse: isClick(anchor0, anchor1, t0, t1)
Mouse->>Mouse: "delta = anchor1 - anchor0"
Mouse->>Mouse: "timeDelta = t1 - t0"
Mouse->>Mouse: getDragToleranceAdjustedForScrollFactor()
Mouse->>GlobalData: m_keyboardDefaultScrollFactor
Mouse->>GlobalData: m_keyboardScrollFactor
GlobalData-->>Mouse: "ratio = default / current"
Mouse->>Mouse: "dragTolerance = m_dragTolerance * ratio"
Mouse->>Mouse: "reject if timeDelta > m_dragToleranceMS"
Mouse->>Mouse: "reject if delta.lengthSqr() > sqr(dragTolerance)"
Mouse-->>Caller: TRUE (click) / FALSE (drag)
%%{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 Caller as CommandXlat / SelectionXlat
participant Mouse
participant GlobalData as TheGlobalData
Caller->>Mouse: isClick(anchor0, anchor1, t0, t1)
Mouse->>Mouse: "delta = anchor1 - anchor0"
Mouse->>Mouse: "timeDelta = t1 - t0"
Mouse->>Mouse: getDragToleranceAdjustedForScrollFactor()
Mouse->>GlobalData: m_keyboardDefaultScrollFactor
Mouse->>GlobalData: m_keyboardScrollFactor
GlobalData-->>Mouse: "ratio = default / current"
Mouse->>Mouse: "dragTolerance = m_dragTolerance * ratio"
Mouse->>Mouse: "reject if timeDelta > m_dragToleranceMS"
Mouse->>Mouse: "reject if delta.lengthSqr() > sqr(dragTolerance)"
Mouse-->>Caller: TRUE (click) / FALSE (drag)
Reviews (5): Last reviewed commit: "bugfix(mouse): Fix bad drag tolerances w..." | Re-trigger Greptile
|
#1501 Seems related. Maybe using |
In principle yes, but that would require to be a reasonable value and the arguments of the isClick function to take in world positions instead of mouse positions.
It looks related yes. |
Skyaero42
left a comment
There was a problem hiding this comment.
I think for now this is a sufficient fix.
…oord3D, ICoord2D, ICoord3D (#2823)
2cecc31 to
a2f38ca
Compare
a2f38ca to
8ca8f81
Compare
8ca8f81 to
b7b643d
Compare
|
|
||
| Real toAngle() const; ///< turn 2D vector into angle (where angle 0 is down the +x axis) | ||
|
|
||
| void add( const Coord2D *a ) |
There was a problem hiding this comment.
Why do these function takes the argument by pointer?
There was a problem hiding this comment.
Because that is the current standard in these classes here.
There was a problem hiding this comment.
I'm a bit skeptical about leaning into a worse option for consistency. Seems to me like it'd just create more work for the eventual refactor.
There was a problem hiding this comment.
Yes is not ideal. Do you want the refactor to happen before adding new functions?
There was a problem hiding this comment.
Whatever is most convenient to you. I wouldn't want it to delay this PR.
Merge with Rebase
This change fixes the bad drag tolerances with high scroll speed factors, which was introduced by #1501 and is especially 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.