Don't consider input events as keypresses without release in between

When the player holds a key down, the OS generates multiple key press and
text input events. We can't assume that every such event is a real key
press; instead, we need to track whether there has been a key up event
since we last sent a key press event.

Fixes #1711.

I also moved hotkey::execute_command() into the hotkey::command_executor
class, and renamed the existing member function of same name to
do_execute_command(). I did that because the command_executor class was
the best place to store the "press event already sent" flag.

RELEASE_NOTES

@@ -26,6 +26,7 @@ CHANGES
 
 [rawarn="Deprecations and breaking changes"]
 [list]
+[*][set_menu_item] no longer fires repeatedly if the player holds the hotkey (bug #1711). If you were relying on repeated firing, add repeat_on_hold=yes to [default_hotkey].
 [/list]
 [/rawarn]
 

changelog

@@ -10,6 +10,9 @@ Version 1.13.11+dev:
    * Fixed units shown with [move_units_fake] disappearing between steps
      (bug #1516).
    * [modify_side] now supports side_name
+   * [set_menu_item] no longer fires repeatedly if the player holds the
+     hotkey (bug #1711). If you were relying on repeated firing, add
+     repeat_on_hold=yes to [default_hotkey].
  * Miscellaneous and bug fixes:
    * Fixed standing animation toggle not taking immediate effect (bug
      #1653).

src/controller_base.cpp

@@ -51,9 +51,6 @@ void controller_base::handle_event(const SDL_Event& event)
 		return;
 	}
 
-	static const hotkey::hotkey_command& quit_hotkey
-			= hotkey::hotkey_command::get_command_by_command(hotkey::HOTKEY_QUIT_GAME);
-
 	events::mouse_handler_base& mh_base = get_mouse_handler_base();
 
 	switch(event.type) {
@@ -78,7 +75,7 @@ void controller_base::handle_event(const SDL_Event& event)
 		// in which case the key press events should go only to it.
 		if(have_keyboard_focus()) {
 			if(event.key.keysym.sym == SDLK_ESCAPE) {
-				hotkey::execute_command(quit_hotkey, get_hotkey_command_executor());
+				get_hotkey_command_executor()->execute_quit_command();
 				break;
 			}
 

src/editor/controller/editor_controller.cpp

@@ -583,7 +583,7 @@ hotkey::ACTION_STATE editor_controller::get_action_state(hotkey::HOTKEY_COMMAND
 	}
 }
 
-bool editor_controller::execute_command(const hotkey::hotkey_command& cmd, int index, bool press)
+bool editor_controller::do_execute_command(const hotkey::hotkey_command& cmd, int index, bool press)
 {
 	hotkey::HOTKEY_COMMAND command = cmd.id;
 	SCOPE_ED;
@@ -591,7 +591,7 @@ bool editor_controller::execute_command(const hotkey::hotkey_command& cmd, int i
 
 	// nothing here handles release; fall through to base implementation
 	if (!press) {
-		return hotkey::command_executor::execute_command(cmd, index, press);
+		return hotkey::command_executor::do_execute_command(cmd, index, press);
 	}
 
 	switch (command) {
@@ -987,7 +987,7 @@ bool editor_controller::execute_command(const hotkey::hotkey_command& cmd, int i
 			return true;
 		}
 		default:
-			return hotkey::command_executor::execute_command(cmd, index, press);
+			return hotkey::command_executor::do_execute_command(cmd, index, press);
 	}
 }
 

src/editor/controller/editor_controller.hpp

@@ -108,7 +108,7 @@ class editor_controller : public controller_base,
 		hotkey::ACTION_STATE get_action_state(hotkey::HOTKEY_COMMAND command, int index) const override;
 
 		/** command_executor override */
-		bool execute_command(const hotkey::hotkey_command& command, int index = -1, bool press=true) override;
+		bool do_execute_command(const hotkey::hotkey_command& command, int index = -1, bool press=true) override;
 
 		/** controller_base override */
 		void show_menu(const std::vector<config>& items_arg, int xloc, int yloc, bool context_menu, display& disp) override;

src/game_events/menu_item.hpp

@@ -75,6 +75,12 @@ public:
 		return use_wml_menu_;
 	}
 
+	/** If true, holding the hotkey will trigger this item repeatedly. */
+	bool hotkey_repeat() const
+	{
+		return default_hotkey_["repeat_on_hold"].to_bool(false);
+	}
+
 	/**
 	 * Returns whether or not *this is applicable given the context.
 	 * Assumes game variables x1, y1, and unit have been set.

src/game_events/wmi_manager.cpp

@@ -76,12 +76,14 @@ bool wmi_manager::erase(const std::string& id)
  * play_controller::execute_command() needs something different.
  */
 bool wmi_manager::fire_item(
-		const std::string& id, const map_location& hex, game_data& gamedata, filter_context& fc, unit_map& units) const
+		const std::string& id, const map_location& hex, game_data& gamedata, filter_context& fc, unit_map& units, bool is_key_hold_repeat) const
 {
 	// Does this item exist?
 	item_ptr wmi = get_item(id);
 	if(!wmi) {
 		return false;
+	} else if(is_key_hold_repeat && !wmi->hotkey_repeat()) {
+		return false;
 	}
 
 	// Prepare for can show().

src/game_events/wmi_manager.hpp

@@ -58,7 +58,8 @@ public:
 			const map_location& hex,
 			game_data& gamedata,
 			filter_context& fc,
-			unit_map& units) const;
+			unit_map& units,
+			bool is_key_hold_repeat = false) const;
 
 	/**
 	 * Gets the menu item with the specified ID.

src/hotkey/command_executor.cpp

@@ -77,7 +77,7 @@ namespace hotkey {
 
 static void event_execute(const SDL_Event& event, command_executor* executor);
 
-bool command_executor::execute_command(const hotkey_command&  cmd, int /*index*/, bool press)
+bool command_executor::do_execute_command(const hotkey_command&  cmd, int /*index*/, bool press)
 {
 	// hotkey release handling
 	if (!press) {
@@ -399,7 +399,7 @@ void command_executor::show_menu(const std::vector<config>& items_arg, int xloc,
 		this->show_menu(submenu->items(), x, y, submenu->is_context(), gui);
 	} else {
 		const hotkey::hotkey_command& cmd = hotkey::get_hotkey_command(items[res]["id"]);
-		hotkey::execute_command(cmd,this,res);
+		do_execute_command(cmd, res);
 		set_button_state();
 	}
 }
@@ -415,7 +415,7 @@ void command_executor::execute_action(const std::vector<std::string>& items_arg,
 	while(i != items.end()) {
 		const hotkey_command &command = hotkey::get_hotkey_command(*i);
 		if (can_execute_command(command)) {
-			hotkey::execute_command(command, this);
+			do_execute_command(command);
 			set_button_state();
 		}
 		++i;
@@ -524,35 +524,43 @@ void key_event(const SDL_Event& event, command_executor* executor)
 	event_execute(event,executor);
 }
 
-static void event_execute( const SDL_Event& event, command_executor* executor)
+static void event_execute(const SDL_Event& event, command_executor* executor)
 {
 	if (!executor) return;
+	executor->execute_command(event);
+	executor->set_button_state();
+}
 
+void command_executor::execute_command(const SDL_Event& event, int index)
+{
 	LOG_HK << "event 0x" << std::hex << event.type << std::dec << std::endl;
 	if(event.type == SDL_TEXTINPUT) {
 		LOG_HK << "SDL_TEXTINPUT \"" << event.text.text << "\"\n";
 	}
 
+	if(event.type == SDL_KEYUP) {
+		LOG_HK << "key-up received, next key-down or text input will be a press event\n";
+		press_event_sent_ = false;
+	}
+
 	const hotkey_ptr hk = get_hotkey(event);
-	if (!hk->active() || hk->is_disabled()) {
+	if(!hk->active() || hk->is_disabled()) {
 		return;
 	}
 
-	bool press = event.type == SDL_KEYDOWN ||
-			event.type == SDL_JOYBUTTONDOWN ||
-			event.type == SDL_MOUSEBUTTONDOWN ||
-			event.type == SDL_TEXTINPUT;
+	const hotkey_command& command = hotkey::get_hotkey_command(hk->get_command());
+	bool press = (event.type == SDL_KEYDOWN ||
+		event.type == SDL_JOYBUTTONDOWN ||
+		event.type == SDL_MOUSEBUTTONDOWN ||
+		event.type == SDL_TEXTINPUT) &&
+		!press_event_sent_;
+	if(press) {
+		LOG_HK << "sending press event\n";
+		press_event_sent_ = true;
+	}
 
-	execute_command(hotkey::get_hotkey_command(hk->get_command()), executor, -1, press);
-	executor->set_button_state();
-}
-
-void execute_command(const hotkey_command& command, command_executor* executor, int index, bool press)
-{
-	assert(executor != nullptr);
-
-	if (!executor->can_execute_command(command, index)
-			|| executor->execute_command(command, index, press)) {
+	if (!can_execute_command(command, index)
+			|| do_execute_command(command, index, press)) {
 		return;
 	}
 
@@ -564,23 +572,23 @@ void execute_command(const hotkey_command& command, command_executor* executor,
 
 		case HOTKEY_MINIMAP_DRAW_TERRAIN:
 			preferences::toggle_minimap_draw_terrain();
-			executor->recalculate_minimap();
+			recalculate_minimap();
 			break;
 		case HOTKEY_MINIMAP_CODING_TERRAIN:
 			preferences::toggle_minimap_terrain_coding();
-			executor->recalculate_minimap();
+			recalculate_minimap();
 			break;
 		case HOTKEY_MINIMAP_CODING_UNIT:
 			preferences::toggle_minimap_movement_coding();
-			executor->recalculate_minimap();
+			recalculate_minimap();
 			break;
 		case HOTKEY_MINIMAP_DRAW_UNITS:
 			preferences::toggle_minimap_draw_units();
-			executor->recalculate_minimap();
+			recalculate_minimap();
 			break;
 		case HOTKEY_MINIMAP_DRAW_VILLAGES:
 			preferences::toggle_minimap_draw_villages();
-			executor->recalculate_minimap();
+			recalculate_minimap();
 			break;
 		case HOTKEY_FULLSCREEN:
 			CVideo::get_singleton().toggle_fullscreen();

src/hotkey/command_executor.hpp

@@ -135,7 +135,18 @@ 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;
-	virtual bool execute_command(const hotkey_command& command, int index=-1, bool press=true);
+	void execute_command(const SDL_Event& event, int index = -1);
+	void execute_quit_command()
+	{
+		const hotkey_command& quit_hotkey = hotkey_command::get_command_by_command(hotkey::HOTKEY_QUIT_GAME);
+		do_execute_command(quit_hotkey);
+	}
+
+protected:
+	virtual bool do_execute_command(const hotkey_command& command, int index=-1, bool press=true);
+
+private:
+	bool press_event_sent_ = false;
 };
 class command_executor_default : public command_executor
 {
@@ -161,8 +172,4 @@ void jhat_event(const SDL_Event& event, command_executor* executor);
 void key_event(const SDL_Event& event, command_executor* executor);
 void mbutton_event(const SDL_Event& event, command_executor* executor);
 
-
-//TODO
-void execute_command(const hotkey_command& command, command_executor* executor, int index=-1, bool press=true);
-
 }

src/hotkey/hotkey_handler.cpp

@@ -238,7 +238,7 @@ void play_controller::hotkey_handler::scroll_right(bool on)
 	play_controller_.set_scroll_right(on);
 }
 
-bool play_controller::hotkey_handler::execute_command(const hotkey::hotkey_command& cmd, int index, bool press)
+bool play_controller::hotkey_handler::do_execute_command(const hotkey::hotkey_command& cmd, int index, bool press)
 {
 	hotkey::HOTKEY_COMMAND command = cmd.id;
 	if(index >= 0) {
@@ -253,14 +253,14 @@ bool play_controller::hotkey_handler::execute_command(const hotkey::hotkey_comma
 		}
 	}
 	int prefixlen = wml_menu_hotkey_prefix.length();
-	if(press && command == hotkey::HOTKEY_WML && cmd.command.compare(0, prefixlen, wml_menu_hotkey_prefix) == 0)
+	if(command == hotkey::HOTKEY_WML && cmd.command.compare(0, prefixlen, wml_menu_hotkey_prefix) == 0)
 	{
 		std::string name = cmd.command.substr(prefixlen);
 		const map_location& hex = mouse_handler_.get_last_hex();
 
-		return gamestate().get_wml_menu_items().fire_item(name, hex, gamestate().gamedata_, gamestate(), gamestate().board_.units_);
+		return gamestate().get_wml_menu_items().fire_item(name, hex, gamestate().gamedata_, gamestate(), gamestate().board_.units_, !press);
 	}
-	return command_executor::execute_command(cmd, index, press);
+	return command_executor::do_execute_command(cmd, index, press);
 }
 
 bool play_controller::hotkey_handler::can_execute_command(const hotkey::hotkey_command& cmd, int index) const

src/hotkey/hotkey_handler.hpp

@@ -123,7 +123,7 @@ public:
 	virtual hotkey::ACTION_STATE get_action_state(hotkey::HOTKEY_COMMAND command, int index) const override;
 	/** Check if a command can be executed. */
 	virtual bool can_execute_command(const hotkey::hotkey_command& command, int index=-1) const override;
-	virtual bool execute_command(const hotkey::hotkey_command& command, int index=-1, bool press=true) override;
+	virtual bool do_execute_command(const hotkey::hotkey_command& command, int index=-1, bool press=true) override;
 	void show_menu(const std::vector<config>& items_arg, int xloc, int yloc, bool context_menu, display& disp) override;
 
 	/**