Hotkey manager: drop duplicate commands

We use both SDL_KEYDOWN and SDL_TEXTINPUT events for hotkeys. It's possible
for both events (caused by the same keypress) to trigger the hotkey command
and we don't want that. Hence, let's drop duplicate commands.

Fixes #1736.

(cherry-picked from commit 8667e5bbdd)

changelog.md

@@ -66,6 +66,7 @@
    * Add clear_shroud in [move_unit] to clear shroud as the unit moves
  ### Miscellaneous and bug fixes
    * Fixed minimap buttons not doing anything (bug #2681)
+   * Fixed some hotkeys triggering multiple commands on GNU/Linux (bug #1736)
    * Fixed events with an id but no name being rejected
    * Fixed assertion when using [inspect]
    * Fixed inability to deselect modifications in single-player

players_changelog.md

@@ -11,6 +11,7 @@ changelog: https://github.com/wesnoth/wesnoth/blob/master/changelog.md
      Galician, Polish, Scottish Gaelic, Spanish, Ukrainian.
  ### Miscellaneous and bug fixes
    * Fixed minimap buttons not doing anything (bug #2681)
+   * Fixed some hotkeys triggering multiple commands on GNU/Linux (bug #1736)
 
 
 ## Version 1.13.12

src/controller_base.cpp

@@ -25,6 +25,7 @@
 #include "preferences/game.hpp"
 #include "scripting/plugins/context.hpp"
 #include "show_dialog.hpp" //gui::in_dialog
+#include "gui/core/event/handler.hpp" // gui2::is_in_dialog
 #include "soundsource.hpp"
 static lg::log_domain log_display("display");
 #define ERR_DP LOG_STREAM(err, log_display)
@@ -142,6 +143,15 @@ void controller_base::handle_event(const SDL_Event& event)
 	}
 }
 
+void controller_base::process(events::pump_info&)
+{
+	if(gui2::is_in_dialog()) {
+		return;
+	}
+
+	hotkey::run_events(get_hotkey_command_executor());
+}
+
 void controller_base::keyup_listener::handle_event(const SDL_Event& event)
 {
 	if(event.type == SDL_KEYUP) {

src/controller_base.hpp

@@ -60,7 +60,7 @@ namespace soundsource
 class manager;
 }
 
-class controller_base : public video2::draw_layering
+class controller_base : public video2::draw_layering, public events::pump_monitor
 {
 public:
 	controller_base(const config& game_config);
@@ -147,9 +147,9 @@ protected:
 	 * Calls various virtual function to allow specialized
 	 * behavior of derived classes.
 	 */
-	void handle_event(const SDL_Event& event);
+	void handle_event(const SDL_Event& event) override;
 
-	void handle_window_event(const SDL_Event& /*event*/)
+	void handle_window_event(const SDL_Event& /*event*/) override
 	{
 		// No action by default
 	}
@@ -160,6 +160,8 @@ protected:
 		// No action by default
 	}
 
+	virtual void process(events::pump_info&) override;
+
 	/** Process keydown (always). Overridden in derived classes */
 	virtual void process_keydown_event(const SDL_Event& /*event*/)
 	{

src/hotkey/command_executor.cpp

@@ -39,6 +39,7 @@
 
 #include <cassert>
 #include <ios>
+#include <set>
 
 static lg::log_domain log_config("config");
 static lg::log_domain log_hotkey("hotkey");
@@ -63,7 +64,7 @@ void make_screenshot(const std::string& name, bool map_screenshot)
 }
 namespace hotkey {
 
-static void event_execute(const SDL_Event& event, command_executor* executor);
+static void event_queue(const SDL_Event& event, command_executor* executor);
 
 bool command_executor::do_execute_command(const hotkey_command&  cmd, int /*index*/, bool press, bool release)
 {
@@ -523,23 +524,23 @@ void command_executor::get_menu_images(display& disp, std::vector<config>& items
 
 void mbutton_event(const SDL_Event& event, command_executor* executor)
 {
-	event_execute(event, executor);
+	event_queue(event, executor);
 }
 
 void jbutton_event(const SDL_Event& event, command_executor* executor)
 {
-	event_execute(event, executor);
+	event_queue(event, executor);
 }
 
 void jhat_event(const SDL_Event& event, command_executor* executor)
 {
-	event_execute(event, executor);
+	event_queue(event, executor);
 }
 
 void key_event(const SDL_Event& event, command_executor* executor)
 {
 	if (!executor) return;
-	event_execute(event,executor);
+	event_queue(event,executor);
 }
 
 void keyup_event(const SDL_Event&, command_executor* executor)
@@ -548,14 +549,20 @@ void keyup_event(const SDL_Event&, command_executor* executor)
 	executor->handle_keyup();
 }
 
-static void event_execute(const SDL_Event& event, command_executor* executor)
+void run_events(command_executor* executor)
+{
+	if(!executor) return;
+	executor->run_queued_commands();
+}
+
+static void event_queue(const SDL_Event& event, command_executor* executor)
 {
 	if (!executor) return;
-	executor->execute_command(event);
+	executor->queue_command(event);
 	executor->set_button_state();
 }
 
-void command_executor::execute_command(const SDL_Event& event, int index)
+void command_executor::queue_command(const SDL_Event& event, int index)
 {
 	LOG_HK << "event 0x" << std::hex << event.type << std::dec << std::endl;
 	if(event.type == SDL_TEXTINPUT) {
@@ -581,16 +588,21 @@ void command_executor::execute_command(const SDL_Event& event, int index)
 		press_event_sent_ = true;
 	}
 
-	if (!can_execute_command(command, index)
-			|| do_execute_command(command, index, press, release)) {
+	command_queue_.emplace_back(command, index, press, release);
+}
+
+void command_executor::execute_command_wrap(const command_executor::queued_command& command)
+{
+	if (!can_execute_command(*command.command, command.index)
+			|| do_execute_command(*command.command, command.index, command.press, command.release)) {
 		return;
 	}
 
-	if (!press) {
+	if (!command.press) {
 		return; // none of the commands here respond to a key release
     }
 
-	switch (command.id) {
+	switch (command.command->id) {
 		case HOTKEY_FULLSCREEN:
 			CVideo::get_singleton().toggle_fullscreen();
 			break;
@@ -628,7 +640,7 @@ void command_executor::execute_command(const SDL_Event& event, int index)
 			}
 			break;
 		default:
-			DBG_G << "command_executor: unknown command number " << command.id << ", ignoring.\n";
+			DBG_G << "command_executor: unknown command number " << command.command->id << ", ignoring.\n";
 			break;
 	}
 }
@@ -695,6 +707,33 @@ void command_executor_default::set_button_state()
 	}
 }
 
+// Removes duplicate commands caused by both SDL_KEYDOWN and SDL_TEXTINPUT triggering hotkeys.
+// See https://github.com/wesnoth/wesnoth/issues/1736
+std::vector<command_executor::queued_command> command_executor::filter_command_queue()
+{
+	std::vector<queued_command> filtered_commands;
+	std::set<const hotkey_command*> seen_commands;
+
+	for(const queued_command& cmd : command_queue_) {
+		if(seen_commands.find(cmd.command) == seen_commands.end()) {
+			seen_commands.insert(cmd.command);
+			filtered_commands.push_back(cmd);
+		}
+	}
+
+	command_queue_.clear();
+
+	return filtered_commands;
+}
+
+void command_executor::run_queued_commands()
+{
+	std::vector<queued_command> commands = filter_command_queue();
+	for(const queued_command& cmd : commands) {
+		execute_command_wrap(cmd);
+	}
+}
+
 void command_executor_default::recalculate_minimap()
 {
 	get_display().recalculate_minimap();

src/hotkey/command_executor.hpp

@@ -134,7 +134,8 @@ public:
 	void execute_action(const std::vector<std::string>& items_arg, int xloc, int yloc, bool context_menu, display& gui);
 
 	virtual bool can_execute_command(const hotkey_command& command, int index=-1) const = 0;
-	void execute_command(const SDL_Event& event, int index = -1);
+	void queue_command(const SDL_Event& event, int index = -1);
+	void run_queued_commands();
 	void execute_quit_command()
 	{
 		const hotkey_command& quit_hotkey = hotkey_command::get_command_by_command(hotkey::HOTKEY_QUIT_GAME);
@@ -150,7 +151,23 @@ protected:
 	virtual bool do_execute_command(const hotkey_command& command, int index=-1, bool press=true, bool release=false);
 
 private:
+	struct queued_command
+	{
+		queued_command(const hotkey_command& command_, int index_, bool press_, bool release_)
+			: command(&command_), index(index_), press(press_), release(release_)
+		{}
+
+		const hotkey_command* command;
+		int index;
+		bool press;
+		bool release;
+	};
+
+	void execute_command_wrap(const queued_command& command);
+	std::vector<queued_command> filter_command_queue();
+
 	bool press_event_sent_ = false;
+	std::vector<queued_command> command_queue_;
 };
 class command_executor_default : public command_executor
 {
@@ -176,5 +193,7 @@ void jhat_event(const SDL_Event& event, command_executor* executor);
 void key_event(const SDL_Event& event, command_executor* executor);
 void keyup_event(const SDL_Event& event, command_executor* executor);
 void mbutton_event(const SDL_Event& event, command_executor* executor);
+// Function to call to process the events.
+void run_events(command_executor* executor);
 
 }