Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#669 closed defect (fixed)

Minimap dragging reads mouse position after left button is released

Reported by: historic_bruno Owned by:
Priority: Nice to Have Milestone: Alpha 3
Component: Core engine Keywords: mouse, minimap, SDL
Cc: Patch:

Description

If you use the minimap to change your view in the game, there's a short time after releasing the left mouse button where the camera is still moving. Ideally the instant you release the button, the view should stop changing because some people have mouse cursors that move fast and even 1/4 sec is enough time to move very far from where you intended.

Change History (5)

comment:1 by historic_bruno, 13 years ago

Keywords: SDL added

I logged the GUI messages that the minimap receives during this process. Notice at the end there is a huge jump in the mouse coordinates, that's because during that time the mouse moved that much (also outside the minimap object, triggering a GUIM_MOUSE_LEAVE message but a GUIM_MOUSE_MOTION message first). The message handler expects a GUIM_MOUSE_RELEASE_LEFT message when the user is done dragging on the minimap but it's missing.

Possible solutions:

  1. Minimap could delay changing camera position until it receives the next message (basically to make sure the next message isn't GUIM_MOUSE_LEAVE, in which case it would ignore the mouse motion). This feels too much like a hack.
  2. Sort out why the GUIM_MOUSE_RELEASE_LEFT is never received - is the corresponding SDL event somehow dropped?
***SGUIMessage {type: GUIM_MOUSE_ENTER, position: 125.000000, 583.000000}***
SGUIMessage {type: GUIM_MOUSE_OVER, position: 125.000000, 583.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 108.000000, 582.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 108.000000, 582.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 107.000000, 582.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 107.000000, 582.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 106.000000, 582.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 106.000000, 582.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 106.000000, 582.000000}
***SGUIMessage {type: GUIM_MOUSE_PRESS_LEFT, position: 106.000000, 582.000000}***
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 104.000000, 585.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 104.000000, 585.000000}
SGUIMessage {type: GUIM_SETTINGS_UPDATED, position: 104.000000, 585.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 101.000000, 594.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 101.000000, 594.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 99.000000, 597.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 99.000000, 597.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 96.000000, 606.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 96.000000, 606.000000}
SGUIMessage {type: GUIM_SETTINGS_UPDATED, position: 96.000000, 606.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 94.000000, 608.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 94.000000, 608.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 94.000000, 612.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 94.000000, 612.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 92.000000, 617.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 92.000000, 617.000000}
SGUIMessage {type: GUIM_SETTINGS_UPDATED, position: 92.000000, 617.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 92.000000, 619.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 92.000000, 619.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 91.000000, 620.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 91.000000, 620.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 91.000000, 622.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 91.000000, 622.000000}
SGUIMessage {type: GUIM_SETTINGS_UPDATED, position: 91.000000, 622.000000}

****************** Missing GUIM_MOUSE_RELEASE_LEFT message here ******************

***SGUIMessage {type: GUIM_MOUSE_MOTION, position: 475.000000, 502.000000}***
***SGUIMessage {type: GUIM_MOUSE_LEAVE, position: 475.000000, 502.000000}***

comment:2 by historic_bruno, 13 years ago

After a suggestion from Philip, I added some more logging, and found that in fact, the SDL_BUTTON_LEFT event comes after the mouse has already left the minimap object. That means the nearest GUI object to the cursor (at the time) is no longer the minimap, but NULL so it's not handled.

...
SGUIMessage {type: GUIM_MOUSE_OVER, position: 96.000000, 633.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 96.000000, 634.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 96.000000, 634.000000}
SGUIMessage {type: GUIM_SETTINGS_UPDATED, position: 96.000000, 634.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 652.000000, 308.000000}
SGUIMessage {type: GUIM_MOUSE_LEAVE, position: 652.000000, 308.000000}
Nearest object @ SDL_BUTTON_LEFT up: NULL
...

Also this bug doesn't consistently reproduce, most of the time it does on my system, but sometimes it works as expected. So perhaps it's a timing issue when polling SDL events. (If it just happens to be time for a mouse button state update while the mouse is over the minimap it works?) Here's a log when it works:

...
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 96.000000, 618.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 96.000000, 618.000000}
SGUIMessage {type: GUIM_MOUSE_OVER, position: 96.000000, 618.000000}
Nearest object @ SDL_BUTTON_LEFT up: minimap
SGUIMessage {type: GUIM_MOUSE_RELEASE_LEFT, position: 96.000000, 618.000000}
SGUIMessage {type: GUIM_SETTINGS_UPDATED, position: 96.000000, 618.000000}
SGUIMessage {type: GUIM_MOUSE_MOTION, position: 384.000000, 486.000000}
SGUIMessage {type: GUIM_MOUSE_LEAVE, position: 384.000000, 486.000000}
...

Note that GUIM_MOUSE_RELEASE_LEFT occurs just before the large change in mouse position.

comment:3 by historic_bruno, 13 years ago

According to the SDL Documentation, SDL_MouseButtonEvent stores the mouse position from when the button state changed. So we should be using (ev->ev.button.x, ev->ev.button.y) and not relying on where the last mouse motion ended, since there may be delays in receiving button state events - as in the current bug.

Probably this only affects the minimap right now, but any GUI object where continuous mouse input is important would experience similar bugs (I'm guessing).

comment:4 by ben, 13 years ago

Resolution: fixed
Status: newclosed

(In [8610]) Changed GUI mouse event handling so that button events use the mouse position at the time of the button state change. Fixed minimap message handling to respect out-of-order events. Fixes #669

comment:5 by Kieran P, 13 years ago

Milestone: BacklogAlpha 3
Note: See TracTickets for help on using tickets.