Some refactoring of preferences-related hotkey info storage (fixes #2953)

This commit does a few things:

First, it moves the hotkey category names out of the preferences dialog and into
the more appropriate hotkey file. Each name is now properly mapped to its corresponding
category enum, so we don't have to worry about that anymore.

Second, it adds a new mapping of hotkey categories to commands so one can easily fetch
a list of hotkeys in a given category.

Third, it excludes categories with no hotkeys from the filter dropdown in the Prefs
dialog (this is the part that actually fixes the bug above).

This also includes a slight behavior change to hotkey type filtering. Previously, if a
hotkey's category didn't match any of the ones listed in the dropdown, that hotkey's
row visibility would be set to the toggle state of the first row in the filter dropdown.
Now it gets set to false.

src/gui/dialogs/preferences_dialog.cpp

@@ -22,7 +22,6 @@
 #include "formatter.hpp"
 #include "formula/string_utils.hpp"
 #include "preferences/game.hpp"
-#include "hotkey/hotkey_command.hpp"
 #include "hotkey/hotkey_item.hpp"
 #include "preferences/credentials.hpp"
 #include "preferences/lobby.hpp"
@@ -133,24 +132,12 @@ preferences_dialog::preferences_dialog(const config& game_cfg, const PREFERENCE_
 			return lhs["name"].t_str().str() < rhs["name"].t_str().str();
 		});
 
-	cat_names_ = {
+	for(const auto& name : hotkey::get_category_names()) {
-		// TODO: This list needs to be synchronized with the hotkey::HOTKEY_CATEGORY enum
+		// Don't include categories with no hotkeys
-		// Find some way to do that automatically
+		if(!hotkey::get_hotkeys_by_category(name.first).empty()) {
-		_("General"),
+			cat_names_[name.first] = t_string(name.second, "wesnoth-lib");
-		_("Saved Games"),
+		}
-		_("Map Commands"),
+	}
-		_("Unit Commands"),
-		_("Player Chat"),
-		_("Replay Control"),
-		_("Planning Mode"),
-		_("Scenario Editor"),
-		_("Editor Palettes"),
-		_("Editor Tools"),
-		_("Editor Clipboard"),
-		_("Debug Commands"),
-		_("Custom WML Commands"),
-		// HKCAT_PLACEHOLDER intentionally excluded (it shouldn't have any anyway)
-	};
 }
 
 // Helper function to refresh resolution list
@@ -745,7 +732,7 @@ void preferences_dialog::post_build(window& window)
 
 	std::vector<config> hotkey_category_entries;
 	for(const auto& name : cat_names_) {
-		hotkey_category_entries.emplace_back("label", name, "checkbox", false);
+		hotkey_category_entries.emplace_back("label", name.second, "checkbox", false);
 	}
 
 	multimenu_button& hotkey_menu = find_widget<multimenu_button>(&window, "hotkey_category_menu", false);
@@ -925,16 +912,19 @@ void preferences_dialog::hotkey_type_filter_callback(window& window) const
 		for(std::size_t h = 0; h < visible_hotkeys_.size(); ++h) {
 			int index = 0;
 
-			for(std::size_t i = 0; i < cat_names_.size(); ++i) {
+			for(const auto& name : cat_names_) {
-				hotkey::HOTKEY_CATEGORY cat = hotkey::HOTKEY_CATEGORY(i);
+				if(visible_hotkeys_[h]->category == name.first) {
-
-				if(visible_hotkeys_[h]->category == cat) {
-					index = i;
 					break;
+				} else {
+					++index;
 				}
 			}
 
-			res[h] = toggle_states[index];
+			if(index < toggle_states.size()) {
+				res[h] = toggle_states[index];
+			} else {
+				res[h] = false;
+			}
 		}
 	} else {
 		// Nothing selected. It means that *all* categories are shown.

src/gui/dialogs/preferences_dialog.hpp

@@ -16,10 +16,11 @@
 #pragma once
 
 #include "config.hpp"
-#include "preferences/game.hpp"
-#include "utils/make_enum.hpp"
 #include "gui/dialogs/modal_dialog.hpp"
 #include "gui/widgets/group.hpp"
+#include "hotkey/hotkey_command.hpp"
+#include "preferences/game.hpp"
+#include "utils/make_enum.hpp"
 
 #include <boost/dynamic_bitset.hpp>
 
@@ -79,7 +80,7 @@ public:
 		preferences_dialog(game_cfg, initial_view).show();
 	}
 
-	typedef std::vector<const hotkey::hotkey_command*> t_visible_hotkeys;
+	typedef std::vector<const hotkey::hotkey_command*> visible_hotkeys_t;
 
 private:
 	/** Inherited from modal_dialog, implemented by REGISTER_DIALOG. */
@@ -136,9 +137,9 @@ private:
 	int last_selected_item_;
 
 	std::vector<double> accl_speeds_;
-	t_visible_hotkeys visible_hotkeys_;
+	visible_hotkeys_t visible_hotkeys_;
 
-	std::vector<t_string> cat_names_;
+	std::map<hotkey::HOTKEY_CATEGORY, t_string> cat_names_;
 
 	// The page/tab index pairs for setting visible pages
 	const std::pair<int, int>& initial_index_;

src/hotkey/hotkey_command.cpp

@@ -30,9 +30,27 @@ static lg::log_domain log_config("config");
 #define DBG_G LOG_STREAM(debug, lg::general())
 #define ERR_CF LOG_STREAM(err, log_config)
 
+namespace hotkey
+{
 namespace
 {
-using namespace hotkey;
+const category_name_map_t category_names {
+	{ HKCAT_GENERAL,    N_("General") },
+	{ HKCAT_SAVING,     N_("Saved Games") },
+	{ HKCAT_MAP,        N_("Map Commands") },
+	{ HKCAT_UNITS,      N_("Unit Commands") },
+	{ HKCAT_CHAT,       N_("Player Chat") },
+	{ HKCAT_REPLAY,     N_("Replay Control") },
+	{ HKCAT_WHITEBOARD, N_("Planning Mode") },
+	{ HKCAT_SCENARIO,   N_("Scenario Editor") },
+	{ HKCAT_PALETTE,    N_("Editor Palettes") },
+	{ HKCAT_TOOLS,      N_("Editor Tools") },
+	{ HKCAT_CLIPBOARD,  N_("Editor Clipboard") },
+	{ HKCAT_DEBUG,      N_("Debug Commands") },
+	{ HKCAT_CUSTOM,     N_("Custom WML Commands") },
+};
+
+std::map<HOTKEY_CATEGORY, std::list<HOTKEY_COMMAND>> hotkeys_by_category;
 
 // Make them global ?
 hk_scopes scope_game(1 << SCOPE_GAME);
@@ -46,7 +64,7 @@ hk_scopes scope_main(1 << SCOPE_MAIN_MENU);
 // Since HOTKEY_NULL is the last entry in said enum, we can use its index to dynamically
 // size the array.
 //
-std::array<hotkey_command_temp, HOTKEY_NULL - 1> hotkey_list_ {{
+std::array<hotkey_command_temp, HOTKEY_NULL - 1> master_hotkey_list {{
 	{ HOTKEY_SCROLL_UP, "scroll-up", N_("Scroll Up"), false, scope_game | scope_editor, HKCAT_GENERAL, "" },
 	{ HOTKEY_SCROLL_DOWN, "scroll-down", N_("Scroll Down"), false, scope_game | scope_editor, HKCAT_GENERAL, "" },
 	{ HOTKEY_SCROLL_LEFT, "scroll-left", N_("Scroll Left"), false, scope_game | scope_editor, HKCAT_GENERAL, "" },
@@ -290,18 +308,16 @@ std::set<HOTKEY_COMMAND> toggle_commands {
 	HOTKEY_SCROLL_RIGHT
 };
 
-// Contains copies of hotkey_list_ and all current active wml menu hotkeys
+// Contains copies of master_hotkey_list and all current active wml menu hotkeys
 // TODO: Maybe known_hotkeys is not a fitting name anymore.
 std::vector<hotkey::hotkey_command> known_hotkeys;
 
-// Index map for known_hotkeys. Since known_hotkeys begins with hotkey_list_, they are also indexes for hotkey_list_.
+// Index map for known_hotkeys. Since known_hotkeys begins with master_hotkey_list, they are also indexes for master_hotkey_list.
 std::map<std::string, std::size_t> command_map_;
 
 hk_scopes scope_active_(0);
 } // end anon namespace
 
-namespace hotkey
-{
 scope_changer::scope_changer()
 	: prev_scope_active_(scope_active_)
 {
@@ -496,8 +512,8 @@ const hotkey_command& hotkey_command::get_command_by_command(hotkey::HOTKEY_COMM
 
 const hotkey_command& get_hotkey_null()
 {
-	// It is the last entry in that array, and the indexes in hotkey_list_ and known_hotkeys are the same.
+	// It is the last entry in that array, and the indexes in master_hotkey_list and known_hotkeys are the same.
-	return known_hotkeys[hotkey_list_.size() - 1];
+	return known_hotkeys[master_hotkey_list.size() - 1];
 }
 
 void delete_all_wml_hotkeys()
@@ -507,6 +523,8 @@ void delete_all_wml_hotkeys()
 
 		known_hotkeys.pop_back();
 	}
+
+	hotkeys_by_category[HKCAT_CUSTOM].clear();
 }
 
 const std::string& get_description(const std::string& command)
@@ -523,19 +541,24 @@ const std::string& get_tooltip(const std::string& command)
 void init_hotkey_commands()
 {
 	known_hotkeys.clear();
+	hotkeys_by_category.clear();
 
 	// Reserve enough space for the built-in hotkeys and 20 extra spaces for WML hotkeys. This is
 	// to avoid reallocation of this huge vector when any of the latter are added. 20 is honestly
 	// overkill, since there's really no reason to ever have near that many hotkey-enabled WML menu
 	// items, but it doesn't cost us anything to have extra.
-	known_hotkeys.reserve(hotkey_list_.size() + 20);
+	known_hotkeys.reserve(master_hotkey_list.size() + 20);
 
 	std::size_t i = 0;
-	for(hotkey_command_temp& cmd : hotkey_list_) {
+	for(const hotkey_command_temp& cmd : master_hotkey_list) {
+		// Initialize the full hotkey from the temp data.
 		known_hotkeys.emplace_back(cmd);
 
-		command_map_[cmd.command] = i;
+		// Note the known_hotkeys index associated with this command.
-		++i;
+		command_map_[cmd.command] = i++;
+
+		// Record this hotkey's id in the appropriate category list.
+		hotkeys_by_category[cmd.category].push_back(cmd.id);
 	}
 }
 
@@ -548,4 +571,15 @@ HOTKEY_COMMAND get_id(const std::string& command)
 {
 	return get_hotkey_command(command).id;
 }
+
+const category_name_map_t& get_category_names()
+{
+	return category_names;
 }
+
+std::list<HOTKEY_COMMAND> get_hotkeys_by_category(HOTKEY_CATEGORY category)
+{
+	return hotkeys_by_category[category];
+}
+
+} // namespace hotkey

src/hotkey/hotkey_command.hpp

@@ -18,6 +18,8 @@
 #include "tstring.hpp"
 
 #include <bitset>
+#include <list>
+#include <map>
 #include <vector>
 
 class config;
@@ -206,10 +208,24 @@ enum HOTKEY_CATEGORY {
 	HKCAT_PLACEHOLDER // Keep this one last
 };
 
+using category_name_map_t = std::map<HOTKEY_CATEGORY, std::string>;
+
+/**
+ * Returns the map of hotkey categories and their display names.
+ *
+ * These aren't translated and need be converted to a t_string before
+ * being displayed to the player.
+ */
+const category_name_map_t& get_category_names();
+
+/** Returns a list of all the hotkeys belonging to the given category. */
+std::list<HOTKEY_COMMAND> get_hotkeys_by_category(HOTKEY_CATEGORY category);
+
 typedef std::bitset<SCOPE_COUNT> hk_scopes;
 
 /// Do not use this outside hotkeys.cpp.
-/// hotkey_command uses t_string which might cause bugs when used at program startup, so use this for the hotkey_list_ (and only there).
+/// hotkey_command uses t_string which might cause bugs when used at program startup,
+/// so use this for the master hotkey list (and only there).
 struct hotkey_command_temp
 {
 	HOTKEY_COMMAND id;