Improve KeyPress handling (#15923)

* Pass KeyPress by value
* TouchControls: add setting change callback for keybindings

src/client/inputhandler.cpp

@@ -77,7 +77,7 @@ void KeyCache::populate()
 	if (handler) {
 		// First clear all keys, then re-add the ones we listen for
 		handler->dontListenForKeys();
-		for (const KeyPress &k : key) {
+		for (auto k : key) {
 			handler->listenForKey(k);
 		}
 		handler->listenForKey(EscapeKey);
@@ -111,7 +111,7 @@ bool MyEventReceiver::OnEvent(const SEvent &event)
 
 	// This is separate from other keyboard handling so that it also works in menus.
 	if (event.EventType == EET_KEY_INPUT_EVENT) {
-		const KeyPress keyCode(event.KeyInput);
+		KeyPress keyCode(event.KeyInput);
 		if (keyCode == getKeySetting("keymap_fullscreen")) {
 			if (event.KeyInput.PressedDown && !fullscreen_is_down) {
 				IrrlichtDevice *device = RenderingEngine::get_raw_device();
@@ -142,7 +142,7 @@ bool MyEventReceiver::OnEvent(const SEvent &event)
 
 	// Remember whether each key is down or up
 	if (event.EventType == irr::EET_KEY_INPUT_EVENT) {
-		const KeyPress keyCode(event.KeyInput);
+		KeyPress keyCode(event.KeyInput);
 		if (keyCode && keysListenedFor[keyCode]) { // ignore key input that is invalid or irrelevant for the game.
 			if (event.KeyInput.PressedDown) {
 				if (!IsKeyDown(keyCode))

src/client/inputhandler.h

@@ -53,7 +53,7 @@ class KeyList : private std::list<KeyPress>
 	typedef super::iterator iterator;
 	typedef super::const_iterator const_iterator;
 
-	virtual const_iterator find(const KeyPress &key) const
+	virtual const_iterator find(KeyPress key) const
 	{
 		const_iterator f(begin());
 		const_iterator e(end());
@@ -68,7 +68,7 @@ class KeyList : private std::list<KeyPress>
 		return e;
 	}
 
-	virtual iterator find(const KeyPress &key)
+	virtual iterator find(KeyPress key)
 	{
 		iterator f(begin());
 		iterator e(end());
@@ -86,13 +86,13 @@ class KeyList : private std::list<KeyPress>
 public:
 	void clear() { super::clear(); }
 
-	void set(const KeyPress &key)
+	void set(KeyPress key)
 	{
 		if (find(key) == end())
 			push_back(key);
 	}
 
-	void unset(const KeyPress &key)
+	void unset(KeyPress key)
 	{
 		iterator p(find(key));
 
@@ -100,7 +100,7 @@ public:
 			erase(p);
 	}
 
-	void toggle(const KeyPress &key)
+	void toggle(KeyPress key)
 	{
 		iterator p(this->find(key));
 
@@ -112,12 +112,12 @@ public:
 
 	void append(const KeyList &other)
 	{
-		for (const KeyPress &key : other) {
+		for (auto key : other) {
 			set(key);
 		}
 	}
 
-	bool operator[](const KeyPress &key) const { return find(key) != end(); }
+	bool operator[](KeyPress key) const { return find(key) != end(); }
 };
 
 class MyEventReceiver : public IEventReceiver
@@ -126,10 +126,10 @@ public:
 	// This is the one method that we have to implement
 	virtual bool OnEvent(const SEvent &event);
 
-	bool IsKeyDown(const KeyPress &keyCode) const { return keyIsDown[keyCode]; }
+	bool IsKeyDown(KeyPress keyCode) const { return keyIsDown[keyCode]; }
 
 	// Checks whether a key was down and resets the state
-	bool WasKeyDown(const KeyPress &keyCode)
+	bool WasKeyDown(KeyPress keyCode)
 	{
 		bool b = keyWasDown[keyCode];
 		if (b)
@@ -139,13 +139,13 @@ public:
 
 	// Checks whether a key was just pressed. State will be cleared
 	// in the subsequent iteration of Game::processPlayerInteraction
-	bool WasKeyPressed(const KeyPress &keycode) const { return keyWasPressed[keycode]; }
+	bool WasKeyPressed(KeyPress keycode) const { return keyWasPressed[keycode]; }
 
 	// Checks whether a key was just released. State will be cleared
 	// in the subsequent iteration of Game::processPlayerInteraction
-	bool WasKeyReleased(const KeyPress &keycode) const { return keyWasReleased[keycode]; }
+	bool WasKeyReleased(KeyPress keycode) const { return keyWasReleased[keycode]; }
 
-	void listenForKey(const KeyPress &keyCode)
+	void listenForKey(KeyPress keyCode)
 	{
 		keysListenedFor.set(keyCode);
 	}
@@ -247,7 +247,7 @@ public:
 	virtual void clearWasKeyPressed() {}
 	virtual void clearWasKeyReleased() {}
 
-	virtual void listenForKey(const KeyPress &keyCode) {}
+	virtual void listenForKey(KeyPress keyCode) {}
 	virtual void dontListenForKeys() {}
 
 	virtual v2s32 getMousePos() = 0;
@@ -316,7 +316,7 @@ public:
 		m_receiver->clearWasKeyReleased();
 	}
 
-	virtual void listenForKey(const KeyPress &keyCode)
+	virtual void listenForKey(KeyPress keyCode)
 	{
 		m_receiver->listenForKey(keyCode);
 	}

src/client/keycode.cpp

@@ -367,7 +367,7 @@ bool KeyPress::loadFromScancode(const std::string &name)
 }
 
 std::unordered_map<std::string, KeyPress> specialKeyCache;
-const KeyPress &KeyPress::getSpecialKey(const std::string &name)
+KeyPress KeyPress::getSpecialKey(const std::string &name)
 {
 	auto &key = specialKeyCache[name];
 	if (!key)
@@ -382,7 +382,7 @@ const KeyPress &KeyPress::getSpecialKey(const std::string &name)
 // A simple cache for quicker lookup
 static std::unordered_map<std::string, KeyPress> g_key_setting_cache;
 
-const KeyPress &getKeySetting(const std::string &settingname)
+KeyPress getKeySetting(const std::string &settingname)
 {
 	auto n = g_key_setting_cache.find(settingname);
 	if (n != g_key_setting_cache.end())

src/client/keycode.h

@@ -10,7 +10,9 @@
 #include <string>
 #include <variant>
 
-/* A key press, consisting of a scancode or a keycode */
+/* A key press, consisting of a scancode or a keycode.
+ * This fits into 64 bits, so prefer passing this by value.
+*/
 class KeyPress
 {
 public:
@@ -40,10 +42,10 @@ public:
 		return 0;
 	}
 
-	bool operator==(const KeyPress &o) const {
+	bool operator==(KeyPress o) const {
 		return scancode == o.scancode;
 	}
-	bool operator!=(const KeyPress &o) const {
+	bool operator!=(KeyPress o) const {
 		return !(*this == o);
 	}
 
@@ -55,7 +57,7 @@ public:
 			std::get<u32>(scancode) != 0;
 	}
 
-	static const KeyPress &getSpecialKey(const std::string &name);
+	static KeyPress getSpecialKey(const std::string &name);
 
 private:
 	bool loadFromScancode(const std::string &name);
@@ -74,7 +76,7 @@ private:
 #define RMBKey KeyPress::getSpecialKey("KEY_RBUTTON")
 
 // Key configuration getter
-const KeyPress &getKeySetting(const std::string &settingname);
+KeyPress getKeySetting(const std::string &settingname);
 
 // Clear fast lookup cache
 void clearKeyCache();

src/gui/guiKeyChangeMenu.cpp

@@ -205,6 +205,8 @@ void GUIKeyChangeMenu::drawMenu()
 
 bool GUIKeyChangeMenu::acceptInput()
 {
+	clearKeyCache();
+
 	for (key_setting *k : key_settings) {
 		std::string default_key;
 		Settings::getLayer(SL_DEFAULTS)->getNoEx(k->setting_name, default_key);
@@ -231,8 +233,6 @@ bool GUIKeyChangeMenu::acceptInput()
 			g_settings->setBool("autojump", ((gui::IGUICheckBox*)e)->isChecked());
 	}
 
-	clearKeyCache();
-
 	g_gamecallback->signalKeyConfigChange();
 
 	return true;

src/gui/touchcontrols.cpp

@@ -31,7 +31,7 @@
 
 TouchControls *g_touchcontrols;
 
-void TouchControls::emitKeyboardEvent(const KeyPress &key, bool pressed)
+void TouchControls::emitKeyboardEvent(KeyPress key, bool pressed)
 {
 	SEvent e{};
 	e.EventType              = EET_KEY_INPUT_EVENT;
@@ -142,12 +142,8 @@ bool TouchControls::buttonsStep(std::vector<button_info> &buttons, float dtime)
 	return has_pointers;
 }
 
-static const KeyPress &id_to_keypress(touch_gui_button_id id)
+static std::string id_to_setting(touch_gui_button_id id)
 {
-	// ESC isn't part of the keymap.
-	if (id == exit_id)
-		return EscapeKey;
-
 	std::string key = "";
 	switch (id) {
 		case dig_id:
@@ -204,11 +200,22 @@ static const KeyPress &id_to_keypress(touch_gui_button_id id)
 		default:
 			break;
 	}
-	assert(!key.empty());
-	auto &kp = getKeySetting("keymap_" + key);
+	return key.empty() ? key : "keymap_" + key;
+}
+
+static KeyPress id_to_keypress(touch_gui_button_id id)
+{
+	// ESC isn't part of the keymap.
+	if (id == exit_id)
+		return EscapeKey;
+
+	auto setting_name = id_to_setting(id);
+
+	assert(!setting_name.empty());
+	auto kp = getKeySetting(setting_name);
 	if (!kp)
 		warningstream << "TouchControls: Unbound or invalid key for "
-				<< key << ", hiding button." << std::endl;
+				<< setting_name << ", hiding button." << std::endl;
 	return kp;
 }
 
@@ -232,6 +239,11 @@ TouchControls::TouchControls(IrrlichtDevice *device, ISimpleTextureSource *tsrc)
 	readSettings();
 	for (auto name : setting_names)
 		g_settings->registerChangedCallback(name, settingChangedCallback, this);
+
+	// Also update layout when keybindings change (e.g. for convertibles)
+	for (u8 id = 0; id < touch_gui_button_id_END; id++)
+		if (auto name = id_to_setting((touch_gui_button_id)id); !name.empty())
+			g_settings->registerChangedCallback(name, settingChangedCallback, this);
 }
 
 void TouchControls::settingChangedCallback(const std::string &name, void *data)

src/gui/touchcontrols.h

@@ -204,7 +204,7 @@ private:
 	// for its buttons. We only want static image display, not interactivity,
 	// from Irrlicht.
 
-	void emitKeyboardEvent(const KeyPress &keycode, bool pressed);
+	void emitKeyboardEvent(KeyPress keycode, bool pressed);
 
 	void loadButtonTexture(IGUIImage *gui_button, const std::string &path);
 	void buttonEmitAction(button_info &btn, bool action);