KEYMAPPER: More complete keyboard repeat handling

backends/keymapper/action.cpp

@@ -28,7 +28,8 @@ namespace Common {
 
 Action::Action(const char *i, const String &des) :
 		id(i),
-		description(des) {
+		description(des),
+		_shouldTriggerOnKbdRepeats(false) {
 	assert(i);
 }
 

backends/keymapper/action.h

@@ -49,6 +49,7 @@ struct Action {
 
 private:
 	Array<String> _defaultInputMapping;
+	bool _shouldTriggerOnKbdRepeats;
 
 public:
 	Action(const char *id, const String &description);
@@ -117,11 +118,19 @@ public:
 	/**
 	 * Allows an action bound to a keyboard event to be repeatedly
 	 * triggered by key repeats
+	 *
+	 * Note that key repeat events should probably not be used for anything
+	 * else than text input as they do not trigger when the action is bound
+	 * to something else than a keyboard key. Furthermore, the frequency at
+	 * which they trigger and whether they trigger at all is operating system
+	 * controlled.
 	 */
-	void allowKbdReapets() {
-		event.kbdRepeat = true;
+	void allowKbdRepeats() {
+		_shouldTriggerOnKbdRepeats = true;
 	}
 
+	bool shouldTriggerOnKbdRepeats() const { return _shouldTriggerOnKbdRepeats; }
+
 	/**
 	 * Add a default input mapping for the action
 	 *

backends/keymapper/keymapper.cpp

@@ -171,11 +171,12 @@ List<Event> Keymapper::mapEvent(const Event &ev) {
 	hardcodedEventMapping(ev);
 
 	List<Event> mappedEvents;
-	if (!mapEvent(ev, _enabledKeymapType, mappedEvents)) {
+	bool matchedAction = mapEvent(ev, _enabledKeymapType, mappedEvents);
+	if (!matchedAction) {
 		// If we found actions matching this input in the game / gui keymaps,
 		// no need to look at the global keymaps. An input resulting in actions
 		// from system and game keymaps would lead to unexpected user experience.
-		mapEvent(ev, Keymap::kKeymapTypeGlobal, mappedEvents);
+		matchedAction = mapEvent(ev, Keymap::kKeymapTypeGlobal, mappedEvents);
 	}
 
 	if (ev.type == EVENT_JOYAXIS_MOTION && ev.joystick.axis < ARRAYSIZE(_joystickAxisPreviouslyPressed)) {
@@ -186,14 +187,7 @@ List<Event> Keymapper::mapEvent(const Event &ev) {
 		}
 	}
 
-	// Ignore keyboard repeat events unless the action explicitly allows it.
-	// This is a convenience to prevent actions getting triggered multiple times
-	if (!mappedEvents.empty() && ev.type == EVENT_KEYDOWN && ev.kbdRepeat
-			&& !mappedEvents.front().kbdRepeat) {
-		return List<Event>();
-	}
-
-	if (mappedEvents.empty()) {
+	if (!matchedAction) {
 		// if it didn't get mapped, just pass it through
 		mappedEvents.push_back(ev);
 	}
@@ -295,6 +289,11 @@ Event Keymapper::executeAction(const Action *action, const Event &incomingEvent)
 		return outgoingEvent;
 	}
 
+	if (incomingEvent.type == EVENT_KEYDOWN && incomingEvent.kbdRepeat && !action->shouldTriggerOnKbdRepeats()) {
+		outgoingEvent.type = EVENT_INVALID;
+		return outgoingEvent;
+	}
+
 	EventType convertedType = convertStartToEnd(outgoingEvent.type);
 
 	// hardware keys need to send up instead when they are up
@@ -302,6 +301,10 @@ Event Keymapper::executeAction(const Action *action, const Event &incomingEvent)
 		outgoingEvent.type = convertedType;
 	}
 
+	if (outgoingEvent.type == EVENT_KEYDOWN && incomingEvent.type == EVENT_KEYDOWN) {
+		outgoingEvent.kbdRepeat = incomingEvent.kbdRepeat;
+	}
+
 	if (isMouseEvent(outgoingEvent)) {
 		if (isMouseEvent(incomingEvent)) {
 			outgoingEvent.mouse = incomingEvent.mouse;

engines/ultima/ultima4/meta_engine.cpp

@@ -158,7 +158,7 @@ Common::KeymapArray MetaEngine::initKeymaps() {
 			if (r->_action == KEYBIND_UP || r->_action == KEYBIND_DOWN ||
 					r->_action == KEYBIND_LEFT || r->_action == KEYBIND_RIGHT)
 				// Allow movement actions to be triggered on keyboard repeats
-				act->allowKbdReapets();
+				act->allowKbdRepeats();
 
 			keyMap->addAction(act);
 		}