1
0
forked from 0ad/0ad

Fix console not preventing hotkeys from firing / Clean up hotkey-input conflict.

- Fix mistake in 4b46c09222 (similar to one in a4852c4c01) that allowed
hotkeys to fire while typing in the console.
- Prevent Alt+key to fire hotkeys in input boxes & console as that is
often text input.
- Split the hotkey triggering logic in two: a preparatory phase & an
actual firing phase.
This allows the GUI code to check which hotkeys are about to fire and
selectively allow them to go through. This makes it easier to change
hardcoded hotkeys, such as the console toggling one.

Differential Revision: https://code.wildfiregames.com/D3786
This was SVN commit r25180.
This commit is contained in:
wraitii 2021-04-02 14:18:20 +00:00
parent 6842eacde9
commit 2d53308e1b
7 changed files with 115 additions and 37 deletions

View File

@ -218,29 +218,35 @@ InReaction CInput::ManuallyHandleKeys(const SDL_Event_* ev)
return IN_HANDLED;
}
case SDL_KEYDOWN:
case SDL_KEYUP:
{
// Since the GUI framework doesn't handle to set settings
// in Unicode (CStrW), we'll simply retrieve the actual
// pointer and edit that.
SDL_Keycode keyCode = ev->ev.key.keysym.sym;
// Regular text input is handled in SDL_TEXTINPUT, however keydown events are still sent (and they come first).
// To make things work correctly, we pass through 'escape' which is a non-character key.
// TODO: there are probably other keys that we could ignore, but recognizing "non-glyph" keys isn't that trivial.
// Further, don't input text if modifiers other than shift are pressed (the user is presumably trying to perform a hotkey).
if (keyCode == SDLK_ESCAPE ||
// We have a probably printable key - we should return HANDLED so it can't trigger hotkeys.
// However, if Ctrl/Meta modifiers are active, just pass it through instead,
// assuming that we are indeed trying to trigger hotkeys (e.g. copy/paste).
// Escape & the "cancel" hotkey are also passed through to allow closing dialogs easily.
// See also similar logic in CConsole.cpp
// NB: this assumes that Ctrl/GUI aren't used in the Manually* functions below,
// as those code paths would obviously never be taken.
if (keyCode == SDLK_ESCAPE || EventWillFireHotkey(ev, "cancel") ||
g_scancodes[SDL_SCANCODE_LCTRL] || g_scancodes[SDL_SCANCODE_RCTRL] ||
g_scancodes[SDL_SCANCODE_LALT] || g_scancodes[SDL_SCANCODE_RALT] ||
g_scancodes[SDL_SCANCODE_LGUI] || g_scancodes[SDL_SCANCODE_RGUI])
return IN_PASS;
if (m_ComposingText)
return IN_HANDLED;
ManuallyImmutableHandleKeyDownEvent(keyCode);
ManuallyMutableHandleKeyDownEvent(keyCode);
if (ev->ev.type == SDL_KEYDOWN)
{
ManuallyImmutableHandleKeyDownEvent(keyCode);
ManuallyMutableHandleKeyDownEvent(keyCode);
UpdateBufferPositionSetting();
UpdateBufferPositionSetting();
}
return IN_HANDLED;
}
default:

View File

@ -1,4 +1,4 @@
/* Copyright (C) 2020 Wildfire Games.
/* Copyright (C) 2021 Wildfire Games.
*
* Permission is hereby granted, free of charge, to any person obtaining
* a copy of this software and associated documentation files (the
@ -33,7 +33,7 @@
#include <stdio.h>
#include <stdlib.h>
const size_t MAX_HANDLERS = 9;
const size_t MAX_HANDLERS = 10;
static InHandler handler_stack[MAX_HANDLERS];
static size_t handler_stack_top = 0;

View File

@ -1,4 +1,4 @@
/* Copyright (C) 2020 Wildfire Games.
/* Copyright (C) 2021 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -622,11 +622,11 @@ static bool isUnprintableChar(SDL_Keysym key)
case SDLK_LEFT: case SDLK_RIGHT:
case SDLK_UP: case SDLK_DOWN:
case SDLK_PAGEUP: case SDLK_PAGEDOWN:
return false;
return true;
// Ignore the others
default:
return true;
return false;
}
}
@ -671,7 +671,7 @@ InReaction conInputHandler(const SDL_Event_* ev)
// In SDL2, we no longer get Unicode wchars via SDL_Keysym
// we use text input events instead and they provide UTF-8 chars
if (ev->ev.type == SDL_TEXTINPUT && !HotkeyIsPressed("console.toggle"))
if (ev->ev.type == SDL_TEXTINPUT)
{
// TODO: this could be more efficient with an interface to insert UTF-8 strings directly
std::wstring wstr = wstring_from_utf8(ev->ev.text.text);
@ -681,19 +681,28 @@ InReaction conInputHandler(const SDL_Event_* ev)
}
// TODO: text editing events for IME support
if (ev->ev.type != SDL_KEYDOWN)
if (ev->ev.type != SDL_KEYDOWN && ev->ev.type != SDL_KEYUP)
return IN_PASS;
int sym = ev->ev.key.keysym.sym;
// Stop unprintable characters (ctrl+, alt+ and escape),
// also prevent ` and/or ~ appearing in console every time it's toggled.
if (!isUnprintableChar(ev->ev.key.keysym) &&
// Stop unprintable characters (ctrl+, alt+ and escape).
if (ev->ev.type == SDL_KEYDOWN && isUnprintableChar(ev->ev.key.keysym) &&
!HotkeyIsPressed("console.toggle"))
{
g_Console->InsertChar(sym, 0);
return IN_HANDLED;
}
return IN_PASS;
// We have a probably printable key - we should return HANDLED so it can't trigger hotkeys.
// However, if Ctrl/Meta modifiers are active (or it's escape), just pass it through instead,
// assuming that we are indeed trying to trigger hotkeys (e.g. copy/paste).
// Also ignore the key if we are trying to toggle the console off.
// See also similar logic in CInput.cpp
if (EventWillFireHotkey(ev, "console.toggle") ||
g_scancodes[SDL_SCANCODE_LCTRL] || g_scancodes[SDL_SCANCODE_RCTRL] ||
g_scancodes[SDL_SCANCODE_LGUI] || g_scancodes[SDL_SCANCODE_RGUI])
return IN_PASS;
return IN_HANDLED;
}

View File

@ -539,7 +539,7 @@ void InitInput()
in_add_handler(CProfileViewer::InputThunk);
in_add_handler(HotkeyInputHandler);
in_add_handler(HotkeyInputActualHandler);
// gui_handler needs to be registered after (i.e. called before!) the
// hotkey handler so that input boxes can be typed in without
@ -550,11 +550,17 @@ void InitInput()
in_add_handler(touch_input_handler);
// must be registered after (called before) the GUI which relies on these globals
in_add_handler(GlobalsInputHandler);
// Should be called after scancode map update (i.e. after the global input, but before UI).
// This never blocks the event, but it does some processing necessary for hotkeys,
// which are triggered later down the input chain.
// (by calling this before the UI, we can use 'EventWouldTriggerHotkey' in the UI).
in_add_handler(HotkeyInputPrepHandler);
// Should be called first, this updates our hotkey press state
// so that js calls to HotkeyIsPressed are synched with events.
// These two must be called first (i.e. pushed last)
// GlobalsInputHandler deals with some important global state,
// such as which scancodes are being pressed, mouse buttons pressed, etc.
// while HotkeyStateChange updates the map of active hotkeys.
in_add_handler(GlobalsInputHandler);
in_add_handler(HotkeyStateChange);
}

View File

@ -51,6 +51,15 @@ namespace {
bool wasRetriggered;
};
// 'In-flight' state used because the hotkey triggering process is split in two phase.
// These hotkeys may still be stopped if the event responsible for triggering them is handled
// before it can be used to generate the hotkeys.
std::vector<PressedHotkey> newPressedHotkeys;
// Stores the 'specificity' of the newly pressed hotkeys.
size_t closestMapMatch = 0;
// This is merely used to ensure consistency in EventWillFireHotkey.
const SDL_Event_* currentEvent;
// List of currently pressed hotkeys. This is used to quickly reset hotkeys.
// This is an unsorted vector because there will generally be very few elements,
// so it's presumably faster than std::set.
@ -161,10 +170,14 @@ InReaction HotkeyStateChange(const SDL_Event_* ev)
return IN_PASS;
}
InReaction HotkeyInputHandler(const SDL_Event_* ev)
InReaction HotkeyInputPrepHandler(const SDL_Event_* ev)
{
int scancode = SDL_SCANCODE_UNKNOWN;
// Restore default state.
newPressedHotkeys.clear();
currentEvent = nullptr;
switch(ev->ev.type)
{
case SDL_KEYDOWN:
@ -223,30 +236,32 @@ InReaction HotkeyInputHandler(const SDL_Event_* ev)
{
phantom.ev.key.keysym.scancode = static_cast<SDL_Scancode>(UNIFIED_SHIFT);
unified[0] = (phantom.ev.type == SDL_KEYDOWN);
HotkeyInputHandler(&phantom);
return HotkeyInputPrepHandler(&phantom);
}
else if (scancode == SDL_SCANCODE_LCTRL || scancode == SDL_SCANCODE_RCTRL)
{
phantom.ev.key.keysym.scancode = static_cast<SDL_Scancode>(UNIFIED_CTRL);
unified[1] = (phantom.ev.type == SDL_KEYDOWN);
HotkeyInputHandler(&phantom);
return HotkeyInputPrepHandler(&phantom);
}
else if (scancode == SDL_SCANCODE_LALT || scancode == SDL_SCANCODE_RALT)
{
phantom.ev.key.keysym.scancode = static_cast<SDL_Scancode>(UNIFIED_ALT);
unified[2] = (phantom.ev.type == SDL_KEYDOWN);
HotkeyInputHandler(&phantom);
return HotkeyInputPrepHandler(&phantom);
}
else if (scancode == SDL_SCANCODE_LGUI || scancode == SDL_SCANCODE_RGUI)
{
phantom.ev.key.keysym.scancode = static_cast<SDL_Scancode>(UNIFIED_SUPER);
unified[3] = (phantom.ev.type == SDL_KEYDOWN);
HotkeyInputHandler(&phantom);
return HotkeyInputPrepHandler(&phantom);
}
// Check whether we have any hotkeys registered that include this scancode.
if (g_HotkeyMap.find(scancode) == g_HotkeyMap.end())
return (IN_PASS);
return IN_PASS;
currentEvent = ev;
/**
* Hotkey behaviour spec (see also tests):
@ -285,9 +300,6 @@ InReaction HotkeyInputHandler(const SDL_Event_* ev)
activeScancodes.emplace_back(scancode);
}
std::vector<ReleasedHotkey> releasedHotkeys;
std::vector<PressedHotkey> newPressedHotkeys;
std::set<SDL_Scancode_> triggers;
if (!isReleasedKey || isInstantaneous)
triggers.insert(scancode);
@ -298,7 +310,7 @@ InReaction HotkeyInputHandler(const SDL_Event_* ev)
// Now check if we need to trigger new hotkeys / retrigger hotkeys.
// We'll need the match-level and the keys in play to release currently pressed hotkeys.
size_t closestMapMatch = 0;
closestMapMatch = 0;
for (SDL_Scancode_ code : triggers)
for (const SHotkeyMapping& hotkey : g_HotkeyMap[code])
{
@ -331,6 +343,20 @@ InReaction HotkeyInputHandler(const SDL_Event_* ev)
}
}
return IN_PASS;
}
InReaction HotkeyInputActualHandler(const SDL_Event_* ev)
{
if (!currentEvent)
return IN_PASS;
bool isInstantaneous = ev->ev.type == SDL_MOUSEWHEEL;
// TODO: it's probably possible to break hotkeys somewhat if the "Up" event that would release a hotkey is handled
// by a priori handler - it might be safer to do that in the 'Prep' phase.
std::vector<ReleasedHotkey> releasedHotkeys;
// For instantaneous events, we don't update the pressedHotkeys (i.e. currently active hotkeys),
// we just fire/release the triggered hotkeys transiently.
// Therefore, skip the whole 'check pressedHotkeys & swap with newPressedHotkeys' logic.
@ -424,6 +450,16 @@ InReaction HotkeyInputHandler(const SDL_Event_* ev)
return IN_PASS;
}
bool EventWillFireHotkey(const SDL_Event_* ev, const CStr& keyname)
{
// Sanity check of sort. This parameter mostly exists because it looks right from the caller's perspective.
if (ev != currentEvent || !currentEvent)
return false;
return std::find_if(newPressedHotkeys.begin(), newPressedHotkeys.end(),
[&keyname](const PressedHotkey& v){ return v.mapping->name == keyname; }) != newPressedHotkeys.end();
}
bool HotkeyIsPressed(const CStr& keyname)
{
return g_HotkeyStatus[keyname];

View File

@ -77,9 +77,29 @@ class CConfigDB;
extern void LoadHotkeys(CConfigDB& configDB);
extern void UnloadHotkeys();
/**
* Updates g_HotkeyMap.
*/
extern InReaction HotkeyStateChange(const SDL_Event_* ev);
extern InReaction HotkeyInputHandler(const SDL_Event_* ev);
/**
* Detects hotkeys that should fire. This allows using EventWillFireHotkey,
* (and then possibly preventing those hotkeys from firing by handling the event).
*/
extern InReaction HotkeyInputPrepHandler(const SDL_Event_* ev);
/**
* Actually fires hotkeys.
*/
extern InReaction HotkeyInputActualHandler(const SDL_Event_* ev);
/**
* @return whether the event @param ev will fire the hotkey @param keyname.
*/
extern bool EventWillFireHotkey(const SDL_Event_* ev, const CStr& keyname);
/**
* @return whether the hotkey is currently pressed (i.e. active).
*/
extern bool HotkeyIsPressed(const CStr& keyname);
#endif // INCLUDED_HOTKEY

View File

@ -45,7 +45,8 @@ private:
ev.ev.key.repeat = 0;
ev.ev.key.keysym.scancode = SDL_GetScancodeFromName(key);
GlobalsInputHandler(&ev);
HotkeyInputHandler(&ev);
HotkeyInputPrepHandler(&ev);
HotkeyInputActualHandler(&ev);
hotkeyPress = false;
hotkeyUp = false;
while(in_poll_priority_event(&ev))