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.

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

Jump to bottom

Menu closes immediately on opening with "ESC" #756

Closed
frankmilliron opened this issue Apr 19, 2023 · 7 comments
Closed

Menu closes immediately on opening with "ESC" #756

frankmilliron opened this issue Apr 19, 2023 · 7 comments
Labels
bug external Problem exists elsewhere (emulator, tooling) ToolKits MouseGraphics, Icon, Button, Line Edit, List Box, etc.
Milestone
1.3

Comments

@frankmilliron
Copy link
Contributor

To replicate:
place arrow pointer in farthest upper-left corner, hit "ESC" to open menus.
Menu closes immediately upon opening.
@frankmilliron frankmilliron added bug Selector The optional Selector app labels Apr 19, 2023
@inexorabletash inexorabletash added ToolKits MouseGraphics, Icon, Button, Line Edit, List Box, etc. and removed Selector The optional Selector app labels Apr 20, 2023
@inexorabletash
Copy link
Collaborator

Proximal cause is that in MenuSelectImpl when lda #find_mode_by_coord / jsr find_menu runs, the set_pos_params are not the correct mouse coords, so menu is dismissed.

@inexorabletash
Copy link
Collaborator

This is weird - once mouse mode is entered, the mouse should be moved to the keyboard mouse position (aligned with the relevant menu item) via PositionKbdMouse, but if the mouse is at x=0 the wrong coordinates are returned via the GetAndReturnEvent call, for some iteration of the event loop inside MenuSelectImpl.

@inexorabletash
Copy link
Collaborator

I still haven't spotted it, but there's some subtlety where mouse_state (where we think the mouse is) and set_pos_params (where we try and SETMOUSE) get out of sync, so set_pos_params gets overwritten with real (?) values instead of keyboard values during the GetEvent call.

@inexorabletash
Copy link
Collaborator

Hey @frankmilliron - was this in Virtual ][ ? I can repro in Virtual ][ but not in MAME (via Ample) so I'm wondering if this is actually an emulation issue.

@frankmilliron
Copy link
Contributor Author

Indeed this is an emulation issue with Virtual ][. MAME correctly moves the mouse pointer to the active menu window as well, which V2 does not. I need to set up my //c so I can finally test this on real hardware.

@inexorabletash inexorabletash added the external Problem exists elsewhere (emulator, tooling) label Apr 20, 2023
@inexorabletash
Copy link
Collaborator

I'll report to Gerard. Thanks for confirming!

inexorabletash added a commit that referenced this issue Apr 30, 2023
@inexorabletash
MGTK: Rework keyboard menu logic. Saves 151 bytes.
2ab7202 
Keyboard menu support (rejiggered only slightly in 5582bf0) was
basically a hack on top of mouse menu control. During event checking,
if keyboard control was enabled, then keys would effectively be
translated into mouse deltas, which would in turn drive the menu
selection. This caused the pointer to move during keyboard menu
operations, and to be nice necessitated position save/restore logic.

This is rewritten to make keyboard control inherent in the menu event
loop. The current highlighted item is now decoupled from the mouse
position, so keyboard control of the menu doesn't move the mouse at
all.

One feature or bug - I've heard it both ways - that goes away is that
the previously keyboard-selected menu item is not the first one shown
when Esc is pressed to enter the menu selection mode. Now, selection
always starts on the first menu. This may be less convenient for power
users, but is more predictable. Similarly, the unused and undocumented
SetMenuSelection MGTK call is removed, since it controlled this state.

As a side effect, this resolves #756, since menu behavior is not
dependent on having POSMOUSE work correctly.

And as usual in this code-base, any change which makes things work
more sensibly tends to save bytes.
@inexorabletash inexorabletash added this to the 1.3 milestone Apr 30, 2023
@inexorabletash
Copy link
Collaborator

New keyboard menu logic works around this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug external Problem exists elsewhere (emulator, tooling) ToolKits MouseGraphics, Icon, Button, Line Edit, List Box, etc.
Projects
None yet
Milestone
1.3
Development

No branches or pull requests
2 participants
@inexorabletash @frankmilliron