Skip to content

Commit

Permalink
Hotkeys: cleanup and simplification of hotkey_command list
Browse files Browse the repository at this point in the history 
* Used an std::array instead of C array for hotkey_list_.
* Used an std::vector instead of boost::ptr_array for known_hotkeys.
* Made hotkey_command non-copyable and movable in order to avoid hundreds of copies.
* Added a hotkey_command ctor that takes a hotkey_command_temp reference.
* Hopefully prevent some reallocations by reserving an appropriate amount of memory for known_hotkeys
  before filling it up.
* Search from the end of know_hotkeys when calling remove_wml_hotkey(). This avoids traversing the almost
  200 vector members that likely will not match.
* Deployed auto/range-for.
* For some reason I needed to add a <cassert> include to gui2/widgets/grid.hpp.
  • Loading branch information
@Vultraz
Vultraz committed Nov 22, 2017
1 parent 1a98327 commit 2cc3f99
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 58 deletions.
2 changes: 2 additions & 0 deletions src/gui/widgets/grid.hpp
View file
Expand Up @@ -16,6 +16,8 @@

#include "gui/widgets/widget.hpp"

#include <cassert>

namespace gui2
{

Expand Down
74 changes: 46 additions & 28 deletions src/hotkey/hotkey_command.cpp
View file
Expand Up @@ -22,6 +22,8 @@
#include "log.hpp"
#include "preferences/general.hpp"

#include <array>

static lg::log_domain log_config("config");
#define ERR_G LOG_STREAM(err, lg::general())
#define LOG_G LOG_STREAM(info, lg::general())
Expand All @@ -37,8 +39,14 @@ hk_scopes scope_game(1 << SCOPE_GAME);
hk_scopes scope_editor(1 << SCOPE_EDITOR);
hk_scopes scope_main(1 << SCOPE_MAIN_MENU);

// This contains all static hotkeys
hotkey_command_temp hotkey_list_[] {
//
// All static hotkeys.
//
// This array should always have the same number of entries as the HOTKEY_COMMAND enum.
// 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_ {{
{ 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, "" },
Expand Down Expand Up @@ -272,9 +280,9 @@ hotkey_command_temp hotkey_list_[] {

//This list item must stay at the end since it is used as terminator for iterating.
{ HOTKEY_NULL, "null", N_("Unrecognized Command"), true, SCOPE_COUNT, HKCAT_PLACEHOLDER, "" }
};
}};

std::set<HOTKEY_COMMAND> toggle_commands = {
std::set<HOTKEY_COMMAND> toggle_commands {
HOTKEY_SCROLL_UP,
HOTKEY_SCROLL_DOWN,
HOTKEY_SCROLL_LEFT,
Expand All @@ -283,9 +291,9 @@ std::set<HOTKEY_COMMAND> toggle_commands = {

// Contains copies of hotkey_list_ and all current active wml menu hotkeys
// TODO: Maybe known_hotkeys is not a fitting name anymore.
boost::ptr_vector<hotkey::hotkey_command> known_hotkeys;
std::vector<hotkey::hotkey_command> known_hotkeys;

// Index map for known_hotkeys. Since known_hotkeys begins with input_list_, they are also indexes for input_list_.
// Index map for known_hotkeys. Since known_hotkeys begins with hotkey_list_, they are also indexes for hotkey_list_.
std::map<std::string, size_t> command_map_;

hk_scopes scope_active_(0);
Expand Down Expand Up @@ -342,7 +350,7 @@ const hotkey_command& get_hotkey_command(const std::string& command)
return known_hotkeys[command_map_[command]];
}

const boost::ptr_vector<hotkey_command>& get_hotkey_commands()
const std::vector<hotkey_command>& get_hotkey_commands()
{
return known_hotkeys;
}
Expand All @@ -361,18 +369,20 @@ bool remove_wml_hotkey(const std::string& id)
} else {
LOG_G << "removing wml hotkey with id=" + id + "\n";

for(boost::ptr_vector<hotkey_command>::iterator itor = known_hotkeys.begin(); itor != known_hotkeys.end();
++itor) {
// Iterate in reverse since it's almost certain the appropriate hotkey will be near
// the end, since this function is used to removed WML hotkeys, which are added after
// the built-ins.
for(auto itor = known_hotkeys.rbegin(); itor != known_hotkeys.rend(); ++itor) {
if(itor->command == id) {
known_hotkeys.erase(itor);
known_hotkeys.erase(std::next(itor).base());
break;
}
}

// command_map_ might be all wrong now, so we need to rebuild.
command_map_.clear();

for(size_t index = 0; index < known_hotkeys.size(); index++) {
for(size_t index = 0; index < known_hotkeys.size(); ++index) {
command_map_[known_hotkeys[index].command] = index;
}

Expand All @@ -399,8 +409,7 @@ void add_wml_hotkey(const std::string& id, const t_string& description, const co

DBG_G << "Added wml hotkey with id = '" << id << "' and description = '" << description << "'.\n";

known_hotkeys.push_back(new hotkey_command(
hotkey::HOTKEY_WML, id, description, false, false, scope_game, HKCAT_CUSTOM, t_string("")));
known_hotkeys.emplace_back(hotkey::HOTKEY_WML, id, description, false, false, scope_game, HKCAT_CUSTOM, t_string(""));

command_map_[id] = known_hotkeys.size() - 1;

Expand All @@ -417,6 +426,18 @@ void add_wml_hotkey(const std::string& id, const t_string& description, const co
}
}

hotkey_command::hotkey_command(const hotkey_command_temp& temp_command)
: id(temp_command.id)
, command(temp_command.command)
, description(temp_command.description, "wesnoth-lib")
, hidden(temp_command.hidden)
, toggle(toggle_commands.count(temp_command.id) > 0)
, scope(temp_command.scope)
, category(temp_command.category)
, tooltip(temp_command.tooltip, "wesnoth-lib")
{
}

hotkey_command::hotkey_command(hotkey::HOTKEY_COMMAND cmd,
const std::string& id_,
const t_string& desc,
Expand Down Expand Up @@ -474,21 +495,16 @@ 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.
return known_hotkeys[sizeof(hotkey_list_) / sizeof(hotkey_list_[0]) - 1];
// It is the last entry in that array, and the indexes in hotkey_list_ and known_hotkeys are the same.
return known_hotkeys[hotkey_list_.size() - 1];
}

void delete_all_wml_hotkeys()
{
while(known_hotkeys.back().id == hotkey::HOTKEY_WML) {
command_map_.erase(known_hotkeys.back().command);

// according to some page in the Internet .back() returns a reference not an iterator, so i use this.
boost::ptr_vector<hotkey_command>::iterator last_element = known_hotkeys.end();
--last_element;

// boost::ptr_vector<hotkey_command> will manage the deleting of the object for me.
known_hotkeys.erase(last_element);
known_hotkeys.pop_back();
}
}

Expand All @@ -505,18 +521,20 @@ const std::string& get_tooltip(const std::string& command)

void init_hotkey_commands()
{
// the size value is just random set.
boost::ptr_vector<hotkey_command> known_hotkeys_temp(200);
known_hotkeys = known_hotkeys_temp;
known_hotkeys.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);

size_t i = 0;
for(hotkey_command_temp& cmd : hotkey_list_) {
known_hotkeys.push_back(new hotkey_command(cmd.id, cmd.command, t_string(cmd.description, "wesnoth-lib"),
cmd.hidden, toggle_commands.count(cmd.id) > 0, cmd.scope, cmd.category,
t_string(cmd.tooltip, "wesnoth-lib")));
known_hotkeys.emplace_back(cmd);

command_map_[cmd.command] = i;
i++;
++i;
}
}

Expand Down
84 changes: 54 additions & 30 deletions src/hotkey/hotkey_command.hpp
View file
Expand Up @@ -16,9 +16,10 @@

#include "tooltips.hpp"
#include "tstring.hpp"
#include <boost/ptr_container/ptr_vector.hpp>

#include <bitset>
#include <vector>

class config;

namespace hotkey {
Expand Down Expand Up @@ -207,69 +208,92 @@ enum HOTKEY_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).
struct hotkey_command_temp
{
HOTKEY_COMMAND id;

std::string command;

/// description, tooltip are untranslated
std::string description;

bool hidden;

hk_scopes scope;
HOTKEY_CATEGORY category;

std::string tooltip;
};

/// Stores all information related to functions that can be bound to hotkeys.
/// this is currently a semi struct: it haves a constructor, but only const-public members.
struct hotkey_command {
public:
struct hotkey_command
{
hotkey_command() = delete;

/** Constuct a new command from a temporary static hotkey object. */
hotkey_command(const hotkey_command_temp& temp_command);

hotkey_command(HOTKEY_COMMAND cmd, const std::string& id, const t_string& desc, bool hidden, bool toggle, hk_scopes scope, HOTKEY_CATEGORY category, const t_string& tooltip);

/** Needed for vector::erase. */
hotkey_command(hotkey_command&&) = default;
hotkey_command& operator=(hotkey_command&&) = default;

/** This shouldn't be copyable. */
hotkey_command(const hotkey_command&) = delete;
hotkey_command& operator=(const hotkey_command&) = default;

/// the names are strange: the "hotkey::HOTKEY_COMMAND" is named id, and the string to identify the object is called "command"
/// there is some inconstancy with that names in this file.
/// This binds the command to a function. Does not need to be unique.
const HOTKEY_COMMAND id;
HOTKEY_COMMAND id;

/// The command is unique.
const std::string command;
std::string command;

// since the wml_menu hotkey_command s can have different textdomains we need t_string now.
const t_string description;
t_string description;

/// If hidden then don't show the command in the hotkey preferences.
const bool hidden;
bool hidden;

/// Toggle hotkeys have some restrictions on what can be bound to them.
/// They require a binding that has two states, "pressed" and "released".
const bool toggle;
bool toggle;

/// The visibility scope of the command.
const hk_scopes scope;
hk_scopes scope;

/// The category of the command.
const HOTKEY_CATEGORY category;
HOTKEY_CATEGORY category;

const t_string tooltip;
t_string tooltip;

/// checks weather this is the null hotkey_command
bool null() const;

/// returns the command that is treated as null
static const hotkey_command& null_command();

/// the execute_command argument was changed from HOTKEY_COMMAND to hotkey_command,
/// to be able to call it with HOTKEY_COMMAND, this function was created
static const hotkey_command& get_command_by_command(HOTKEY_COMMAND command);
};

/// 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).
struct hotkey_command_temp {
HOTKEY_COMMAND id;

const char* command;
/// description, tooltip are untranslated
const char* description;

bool hidden;

hk_scopes scope;
HOTKEY_CATEGORY category;

const char* tooltip;
};

class scope_changer {
public:
scope_changer();
~scope_changer();
private:
hk_scopes prev_scope_active_;
};
typedef boost::ptr_vector<hotkey_command> hotkey_command_list;

/// returns a container that contains all currently active hotkey_commands.
/// everything that wants a hotkey, must be in this container.
const boost::ptr_vector<hotkey_command>& get_hotkey_commands();
const std::vector<hotkey_command>& get_hotkey_commands();

/// returns the hotkey_command with the given name
const hotkey_command& get_hotkey_command(const std::string& command);
Expand Down

0 comments on commit 2cc3f99

Please sign in to comment.