Skip to content
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

RightClickOverlay: check focus before clearing selection_pos #1367

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

ChrisJohnsen
Copy link
Contributor

I found myself sometimes needing to "exit twice" to exit out of a dig designation that didn't have a selection started.

Problem Reproduction

  • optionally, boot DF from scratch
  • monitor .selection_pos with gm-editor
    • gui/gm-editor buildreq.selection_pos, Alt-a to enable live updates
  • if needed, set .selection_pos to (0,0,0) to simulate a fresh DF start
    • the values can be edited in gm-editor, or
      • :lua ~buildreq.selection_pos:assign(xyz2pos(0,0,0))
  • start one of the other (non-building placement) modes covered by RightClickOverlay (e.g. dig designation), but don't start a selection
  • trigger LEAVESCREEN or right-click
    • Expect: selection mode is exited
    • Result: selection mode remains active
      • the gm-editor window will show that .selection_pos was changed to (-30k,-30k,-30k)
  • a second trigger of LEAVESCREEN or right-click
    • the overlay will let the input pass and DF will exit the mode

Alternate Fix

Just swapping the if and elseif mostly works okay and avoids having to do focus matching.

This works because all the components of selection_rect start at -30k in a fresh boot and this overlay makes sure the "start" components are -30k before exiting the modes that change them.

There would still be some edge cases though:

  • cancelling some kinds of selection_rect selections while this overlay is (temporarily) disabled
    • DF leaves the selection_rect "start" parts set when cancelling stockpile, zone, burrow selections
  • new selection_rect uses in DF that this overlay hasn't been updated to also handle

Either could create a "cancel input suppressed" situations for building placement mode.

Check for the relevant focus before clearing .selection_pos. Otherwise,
the overlay will erroneously consume a LEAVESCREEN or right-click input
in the other modes when .selection_pos.x >= 0.

Note: .selection_pos is (0,0,0) after DF starts from scratch.
Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

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

This solution looks fine (with the focus string check). This only runs in response to a manual action, so performance isn't really an issue here.

Could you add a line to the Fixes section in the changelog?

@ChrisJohnsen
Copy link
Contributor Author

Oops! Sorry for leaving out a changelog entry.

@myk002 myk002 merged commit f071f7a into DFHack:master Jan 10, 2025
10 checks passed
@ChrisJohnsen ChrisJohnsen deleted the cj/right-click-overlay-check-focus branch January 10, 2025 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants