Merge pull request #564 from Wedge009/bug_21969_fix

Resolve preferences file polluted with null-command hot-keys (Bug #21969)

changelog

@@ -73,7 +73,9 @@ Version 1.13.2+dev:
    * Fixed oos caused by mp replay turn feature.
    * Fixed oos bugs caused by plattform dependent rounding from double to int in lua.
    * Fixed custom (lua-defined) scenario tags beeing removed from [replay_start]
-   * fixed savefile bloat caused by unit variations (walking corpses)
+   * Fixed savefile bloat caused by unit variations (walking corpses)
+   * Fixed preferences file bloat caused by null-command hot-keys (bug #21969)
+   * Fixed clearing of default hot-keys not working (bugs #21983/#22218/#23981)
 
 Version 1.13.2:
  * Add-ons client:

src/hotkey/command_executor.cpp

@@ -514,7 +514,7 @@ static void event_execute( const SDL_Event& event, command_executor* executor)
 {
 	if (!executor) return;
 	const hotkey_ptr hk = get_hotkey(event);
-	if (!hk->active()) {
+	if (!hk->active() || hk->is_disabled()) {
 		return;
 	}
 

src/hotkey/hotkey_item.cpp

@@ -144,7 +144,7 @@ bool hotkey_base::matches(const SDL_Event &event) const
 	unsigned int mods = sdl_get_mods();
 
 	if (!hotkey::is_scope_active(hotkey::get_hotkey_command(get_command()).scope) ||
-			!active()) {
+			!active() || is_disabled()) {
 		return false;
 	}
 
@@ -158,6 +158,7 @@ bool hotkey_base::matches(const SDL_Event &event) const
 void hotkey_base::save(config& item) const
 {
 	item["command"] = get_command();
+	item["disabled"] = is_disabled();
 
 	item["shift"] = !!(mod_ & KMOD_SHIFT);
 	item["ctrl"] = !!(mod_ & KMOD_CTRL);
@@ -270,6 +271,8 @@ hotkey_ptr load_from_config(const config& cfg)
 	base->set_mods(mods);
 	base->set_command(cfg["command"].str());
 
+	cfg["disabled"].to_bool() ? base->disable() : base->enable();
+
 	return base;
 }
 
@@ -401,8 +404,12 @@ void add_hotkey(const hotkey_ptr item)
 void clear_hotkeys(const std::string& command)
 {
 	BOOST_FOREACH(hotkey::hotkey_ptr item, hotkeys_) {
-		if (item->get_command() == command) {
-			item->clear();
+		if (item->get_command() == command)
+		{
+			if (item->is_default())
+				item->disable();
+			else
+				item->clear();
 		}
 	}
 }
@@ -462,18 +469,19 @@ void save_hotkeys(config& cfg)
 	cfg.clear_children("hotkey");
 
 	BOOST_FOREACH(hotkey_ptr item, hotkeys_) {
-		if (!item->is_default()) {
+		if ((!item->is_default() && item->active()) ||
+			(item->is_default() && item->is_disabled())) {
 			item->save(cfg.add_child("hotkey"));
 		}
 	}
 }
 
 std::string get_names(std::string id)
 {
-
+	// Names are used in places like the hot-key preferences menu
 	std::vector<std::string> names;
 	BOOST_FOREACH(const hotkey::hotkey_ptr item, hotkeys_) {
-		if (item->get_command() == id && (!item->null())) {
+		if (item->get_command() == id && !item->null() && !item->is_disabled()) {
 			names.push_back(item->get_name());
 		}
 	}

src/hotkey/hotkey_item.hpp

@@ -49,7 +49,7 @@ class hotkey_base
 	/**
 	 * Initialises a new empty hotkey that will be disabled
 	 */
-	hotkey_base() : command_("null"), is_default_(true), mod_(0)
+	hotkey_base() : command_("null"), is_default_(true), is_disabled_(false), mod_(0)
 	{}
 
 	void set_command(const std::string& command)
@@ -110,6 +110,19 @@ class hotkey_base
 		is_default_ = false;
 	}
 
+	bool is_disabled() const
+	{
+		return is_disabled_;
+	}
+	void disable()
+	{
+		is_disabled_ = true;
+	}
+	void enable()
+	{
+		is_disabled_ = false;
+	}
+
 	/**
 	 * Unbind this hotkey by linking it to the null-command
 	 */
@@ -203,12 +216,21 @@ class hotkey_base
 	std::string command_;
 
 	/**
-	 *
+	 * is_default_ is true if the hot-key is part of the default hot-key list defined in data/core/hotkeys.cfg.
+	 * is_default_ is false if it is not, in which case it would be defined in the user's preferences file.
 	 */
 	bool is_default_;
+
+	/*
+	 * The original design of using a "null" command to indicate a disabled hot-key is ambiguous with regards
+	 * to when to save a user hot-key to preferences as well as when a default hot-key should be flagged as
+	 * disabled. So introduce a separate disabled flag to resolve the ambiguity.
+	 * Where the flag is true, the hot-key should not be written to preferences unless it is a default hot-key.
+	 */
+	bool is_disabled_;
+
 	/*
 	 * Keyboard modifiers. Treat as opaque, only do comparisons.
-	 *
 	 */
 	unsigned int mod_;
 };