Opened 9 years ago

Closed 9 years ago

Last modified 10 months ago

#9153 closed patch

GSoC: GUI: Text input + Right Alt (bug #2978736)

Reported by: Pidgeot Owned by: lordhoto
Priority: normal Component: GUI
Keywords: Cc:
Game:

Description

Patch for bug 2978736. For reference, this is from my comment on that bug:

The problem is that in backends/platform/sdl/events.cpp, the key up event
is explicitly swallowed when both Ctrl and Alt is held down - and since
Ctrl+Alt is equivalent to AltGr, the event manager never realizes the
character key has been released. This is, presumably, done because we
didn't expect to ever have sent a down event - Ctrl+Alt is used for scaler
hotkeys, and the event is handled in the backend.

I see two ways of fixing this: either pass on the up key event if we're
not releasing one of the keys bound to a hotkey, or never send key down
events when both Ctrl and Alt is held down.

The problem with never sending the key down event is that at least some
games (such as LSL1SCI) can use a CTRL+ALT+<something> input - I can't
think of any where it's *required* to play the game, but we would have to
either provide a workaround (holding down a different set of modifier keys)
or ignore that part - and it seems a little silly to require a workaround
if it isn't really required.

In my opinion, it would be best to take the second option - look at the
key being released and determine if it's a scaler hotkey.

I noticed that the GP2X backend also provides the handleScalerHotkey
method, but it doesn't appear to get called anywhere - so I haven't
attempted to do anything to that. I've only fixed the SDL backend.

Ticket imported from: #2980597. Ticket imported from: patches/1258.

Attachments (2)

bug_2978736.patch (1.9 KB) - added by Pidgeot 9 years ago.
Patch for bug 2978736
bug_2978736_commonevent.patch (2.1 KB) - added by Pidgeot 9 years ago.
Move modifier checking into isScalerHotkey

Download all attachments as: .zip

Change History (8)

Changed 9 years ago by Pidgeot

Attachment: bug_2978736.patch added

Patch for bug 2978736

comment:1 Changed 9 years ago by Pidgeot

Summary: GUI: Text input + Right Alt (bug #2978736)GSoC: GUI: Text input + Right Alt (bug #2978736)

comment:2 Changed 9 years ago by lordhoto

First of all thanks for your patch. IMHO the right direction was taken.

A small side note though. The added "isScalerHotkey" method has quite unintuitive behavior to me. IMHO it should also check for the modifiers so that it really checks whether the full hotkey is represented by the event, like the name suggests. That would also remove the need for the if which checks that before isScalerHotkey gets called.

Changed 9 years ago by Pidgeot

Move modifier checking into isScalerHotkey

comment:3 Changed 9 years ago by Pidgeot

Yes, that makes sense. I've attached an alternative patch which works on a Common::Event instead. This is less consistent with the implementation of handleScalerHotkey, but as you mentioned in IRC, it might make sense to change that one anyway.

comment:4 Changed 9 years ago by lordhoto

Ok I committed the second patch as is now. Personally I think that suffices as patch for your GSoC application.

comment:5 Changed 9 years ago by lordhoto

Owner: set to lordhoto
Status: newclosed

comment:6 Changed 10 months ago by digitall

Component: GUI
Note: See TracTickets for help on using tickets.