Skip to content

bugfix(input): Prevent command execution when attempting to exit command context while moving the camera#1501

Merged
xezon merged 2 commits into
TheSuperHackers:mainfrom
Stubbjax:fix-command-context-exit
Sep 9, 2025
Merged

bugfix(input): Prevent command execution when attempting to exit command context while moving the camera#1501
xezon merged 2 commits into
TheSuperHackers:mainfrom
Stubbjax:fix-command-context-exit

Conversation

@Stubbjax

Copy link
Copy Markdown

This change resolves a bug where right-clicking to cancel a context command such as a super weapon will actually trigger it if the camera is moving with a high enough scroll speed. This occurs because MSG_RAW_MOUSE_RIGHT_BUTTON_UP is not consumed for clicks determined to be invalid due to camera movement, allowing the message to be processed in CommandXlat where the same condition is not present.

An alternative approach would be to add the same camera movement condition to CommandXlat, but it feels counterintuitive to be unable to cancel context commands while the camera is moving with a high enough scroll speed. There are no apparent consequences to removing the condition.

@Stubbjax Stubbjax self-assigned this Aug 27, 2025
@Stubbjax Stubbjax added Bug Something is not working right, typically is user facing Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Input labels Aug 27, 2025
Bool isClick = TheMouse->isClick(&m_deselectFeedbackAnchor, &pixel, m_lastClick, currentTime);

if (isClick &&
cameraPos.length() > TheMouse->m_dragTolerance3D)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Mouse.ini:

  DragTolerance = 25                                ; How many pixels should we allow before it is a drag?
  DragTolerance3D = 25                              ; How many feet in worldspace should we allow before it is a drag?
  DragToleranceMS = 250                             ; if the mouse is held down for this long, we consider it a drag, not a click.

This is the only user of m_dragTolerance3D. So we can remove that field as well?

Perhaps the cameraPos.length() > TheMouse->m_dragTolerance3D was meant to be inside isClick function? It looks to be intentional, so I wonder if there is something to it we are missing.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The field can indeed be removed. I left it for posterity in case it could be used for something else.

It is a reasonable expectation that right-clicking cancels the active context command regardless of camera movement. It otherwise introduces a seemingly arbitrary behavioural inconsistency (especially as it is dependent on the player's camera scroll speed set in the game options). A player might right-click while moving the camera (or immediately after pressing spacebar or a hotkey to zoom to a location), not realise their command is still active, and then accidentally bomb themselves or a useless location.

I think it probably was meant to be inside isClick, but I cannot come up with any valid cases for keeping the condition.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also, judging by the descriptions, it appears to be redundant to DragTolerance.

@xezon xezon changed the title bugfix: Prevent command execution when attempting to exit context while moving the camera bugfix(input): Prevent command execution when attempting to exit context while moving the camera Sep 1, 2025
@xezon

xezon commented Sep 7, 2025

Copy link
Copy Markdown

Can we come up with a more intuitive change description? Even I have trouble understand what this means.

@Stubbjax

Stubbjax commented Sep 8, 2025

Copy link
Copy Markdown
Author

Can we come up with a more intuitive change description? Even I have trouble understand what this means.

Do you mean the description or title? The description is more technical / developer-focused. Perhaps we can come up with one based on the repro steps.

  1. Ensure scroll speed is above ~30% in the game options
  2. Select a ready special power or superweapon ability
  3. While using arrow keys to pan the camera, right-click anywhere on the battlefield
  4. Expect the ability to be cancelled
  5. Observe the ability gets used instead of cancelled

The change essentially ensures that right-clicking consistently exits the active command context - when the user has selected an ability to use and the game is awaiting input for a location / target.

@xezon

xezon commented Sep 8, 2025

Copy link
Copy Markdown

Replacing "context" with "command context" helps readability a bit but otherwise I have no better idea. I expect most readers will not understand this.

"Prevent command execution when attempting to exit command context while moving the camera"

@xezon xezon changed the title bugfix(input): Prevent command execution when attempting to exit context while moving the camera bugfix(input): Prevent command execution when attempting to exit command context while moving the camera Sep 9, 2025
@xezon xezon merged commit 481f473 into TheSuperHackers:main Sep 9, 2025
19 checks passed
@Stubbjax Stubbjax deleted the fix-command-context-exit branch October 21, 2025 11:55
fbraz3 pushed a commit to fbraz3/GeneralsX that referenced this pull request Nov 10, 2025
fbraz3 pushed a commit to fbraz3/GeneralsX that referenced this pull request Feb 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is not working right, typically is user facing Gen Relates to Generals Input Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants