Opened 14 years ago

Closed 13 years ago

Last modified 5 years ago

#673 closed enhancement (fixed)

[PATCH] Mouse wheel in minimap

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

Description (last modified by elexis)

Mousewheel behaviors that work in the game view should work in the minimap as well:

  1. Mousewheel up/down should change zoom
  2. Shift+mousewheel up/down should rotate the view (like Q + E keys)

Attachments (1)

gui-handlemessage.patch (29.2 KB ) - added by historic_bruno 13 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 by historic_bruno, 14 years ago

Summary: Mouse wheel in iminmapMouse wheel in minimap

comment:2 by historic_bruno, 13 years ago

Looks like perhaps this can be fixed by implementing ManuallyHandleEvent() for CMinimap, and then handling a SDL_HOTKEYDOWN event much the way CGameView::HandleEvent() does it.

comment:3 by historic_bruno, 13 years ago

Keywords: simple added

comment:4 by historic_bruno, 13 years ago

Of course it's not that simple. The GUI input handler runs before the hotkey handler, and currently it always returns IN_HANDLED on mouse events (even if the GUI object doesn't really handle the event), so there is no chance to handle these hotkeys. I'm considering changing the GUI input handler to always pass on mousewheel events or maybe changing HandleMessage so it returns a bool value (true if the event was handled or false).

comment:5 by historic_bruno, 13 years ago

Per discussion with Philip a few days ago, we decided that adding a Skip() method to SGUIMessage would be a good idea. GUI objects can then decide in certain situations to "skip" an event so it's not handled by the GUI, and thus gets passed further up the handler stack. Since HandleMessage() currently expects const references, a bit of refactoring was required, and I cleaned up a few other things while I was there.

by historic_bruno, 13 years ago

Attachment: gui-handlemessage.patch added

comment:6 by historic_bruno, 13 years ago

Keywords: simple removed
Summary: Mouse wheel in minimap[PATCH] Mouse wheel in minimap

Just going to upload this is a patch for now. Due to the refactoring, there may be some unintended consequences, although it seems to work fine for me. Basically any GUI object that wants to not handle an event should call Skip() on the SGUIMessage they receive. Also because messages are sent from several locations, the result will always need to be checked to know how to respond (continue processing the event or pass).

comment:7 by Kieran P, 13 years ago

Milestone: BacklogAlpha 5

comment:8 by Kieran P, 13 years ago

Keywords: review added

comment:9 by Philip Taylor, 13 years ago

// TODO: Why mouserightdoubleclick script event not sent?
// TODO: Why mouserightrelease script event not sent?

I don't think there's any reason, they were probably just forgotten (in r3381).

Why have the no-script-event version of SendEvent (used in CInput::HandleMessage and CList::HandleMessage and CText::HandleMessage), rather than calling HandleMessage(msg) directly (like everywhere else)? It seems potentially simpler if SendEvent always does HandleMessage+ScriptEvent instead of sometimes only doing one of them.

The SGUIMessage constructors should use : ..., skipped(false) {} instead of : ... { skipped = false; } for consistency.

The Skip method comment could probably be clearer about the consequences of calling it and what "skipped" means (e.g. something like wxEvent's description: "This method can be used inside an event handler to control whether further event handlers bound to this event will be called after the current one returns. Without Skip() (or equivalently if Skip(false) is used), the event will not be processed any more. If Skip(true) is called, the event processing system continues searching for a further handler function for this event, even though it has been processed already in the current handler.")

comment:10 by Kieran P, 13 years ago

Cc: historic_bruno added
Owner: set to historic_bruno

comment:11 by ben, 13 years ago

Resolution: fixed
Status: newclosed

(In [9340]) Adds Skip method to SGUIMessage which works more or less like wxEvent.Skip: GUI objects can allow other event handlers to process an input event after they finish. Allows hotkey handling of mousewheel events after minimap. Fixes #673. Adds SendEvent to GUI objects, used in place of separate calls to HandleMessage and ScriptEvent.

comment:12 by sanderd17, 8 years ago

Keywords: review removed

in reply to:  5 comment:13 by elexis, 5 years ago

Description: modified (diff)

Replying to historic_bruno:

Per discussion with Philip a few days ago

From 2011-04-04 #0ad-dev:

--- Day changed Tue Apr 04 2011
23:26 < historic_bruno> Philip`: What do you think about changing HandleMessage on GUI objects (it handles input), so that it returns true if the event was handled, or false otherwise. Currently we assume mouse input is always handled by the GUI object, ignoring mouse hotkeys.
23:36 <@Philip`> historic_bruno: What problem is this for?
23:36  * Philip` is not here, by the way, but should be in ~15 minutes
23:36 < historic_bruno> Philip`: ok lol, it's for handling mousewheel zoom or map rotation while the cursor is over the GUI (especially the minimap)
23:37 < historic_bruno> not a big issue but annoys me slightly to have to move the mouse out of the minimap in order to zoom :)
23:52 <@Philip`> Hmm
23:53 <@Philip`> We'd only want to do that for mouse wheeling, not for clicking, so you can't click on units that are behind the GUI
23:58 <@Philip`> If there's only a few special cases that shouldn't intercept the event, maybe it'd be easier to add a method like SGUIMessage::Skip() (to borrow wxWidgets' terminology) which can be called in the few places that don't want to stop the processing of the event
23:59 <@Philip`> e.g. in CMinimap when receiving wheel events
23:59 < historic_bruno> it gets passed as a const though :/
23:59 < historic_bruno> (the SGUIMessage I mean)
--- Day changed Tue Apr 05 2011
00:00 <@Philip`> Could search-and-replace to make it non-const, or give it a field like "mutable bool m_Skipped;" (though that's probably a disgusting hack)
00:00 < historic_bruno> I agree though it's a pretty specific situation... and its conceivable the in-game GUI will have controls with dropdowns or lists, and then we'll want mousewheel to be trapped there
00:02 <@Philip`> Adding a return value sounds like it'd be more of a pain, particularly since some HandleMessages delegate part of the processing to other HandleMessages so their return values would have to be combined somehow
00:02 <@Philip`> (e.g. CText::HandleMessage)
00:03 < historic_bruno> I'm not sure why SGUIMessage was passed as a const, just because it has never needed to be modified?
00:05 <@Philip`> Yeah, and you don't really want message handlers changing its type or data since that'd confuse anybody else who receives the message later
00:05 <@Philip`> so const is a bit safer
00:06 <@Philip`> but it doesn't seem that important, and unconsting it sounds to me like a simpler and more flexible way to let handlers affect the subsequent processing of the message
00:08 <@Philip`> (More flexible because it would allow e.g. events that propagate to parent GUI objects unless StopPropagation() is called, independently of whether Skip() was called to prevent messages continuing on to the hotkey system, etc)
00:09 <@Philip`> (Not that we'll necessarily ever want that, but it's what http://docs.wxwidgets.org/trunk/classwx_event.html does)
00:10 < historic_bruno> sounds like the best solution then :)
00:10 <@Philip`> (and a single bool return value would prevent anything like that)
00:10 < historic_bruno> I was even thinking of using the message's data field somehow until I saw it was const, but a skip() method is more logical
00:11 < historic_bruno> it makes it pretty clear what's going on
Last edited 5 years ago by elexis (previous) (diff)
Note: See TracTickets for help on using tickets.