Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc cleanups #6570

Merged
merged 5 commits into from
Mar 22, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
101 changes: 50 additions & 51 deletions src/ai/actions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1000,14 +1000,57 @@ static void execute_or_check(action_result& action, bool execute)
}
}

const std::map<int, std::string> actions::error_names_ {
{action_result::AI_ACTION_SUCCESS, "action_result::AI_ACTION_SUCCESS"},
{action_result::AI_ACTION_STARTED, "action_result::AI_ACTION_STARTED"},
{action_result::AI_ACTION_FAILURE, "action_result::AI_ACTION_FAILURE"},

{attack_result::E_EMPTY_ATTACKER, "attack_result::E_EMPTY_ATTACKER"},
{attack_result::E_EMPTY_DEFENDER, "attack_result::E_EMPTY_DEFENDER"},
{attack_result::E_INCAPACITATED_ATTACKER, "attack_result::E_INCAPACITATED_ATTACKER"},
{attack_result::E_INCAPACITATED_DEFENDER, "attack_result::E_INCAPACITATED_DEFENDER"},
{attack_result::E_NOT_OWN_ATTACKER, "attack_result::E_NOT_OWN_ATTACKER"},
{attack_result::E_NOT_ENEMY_DEFENDER, "attack_result::E_NOT_ENEMY_DEFENDER"},
{attack_result::E_NO_ATTACKS_LEFT, "attack_result::E_NO_ATTACKS_LEFT"},
{attack_result::E_WRONG_ATTACKER_WEAPON, "attack_result::E_WRONG_ATTACKER_WEAPON"},
{attack_result::E_UNABLE_TO_CHOOSE_ATTACKER_WEAPON, "attack_result::E_UNABLE_TO_CHOOSE_ATTACKER_WEAPON"},
{attack_result::E_ATTACKER_AND_DEFENDER_NOT_ADJACENT," attack_result::E_ATTACKER_AND_DEFENDER_NOT_ADJACENT"},

{move_result::E_EMPTY_MOVE, "move_result::E_EMPTY_MOVE"},
{move_result::E_NO_UNIT, "move_result::E_NO_UNIT"},
{move_result::E_NOT_OWN_UNIT, "move_result::E_NOT_OWN_UNIT"},
{move_result::E_INCAPACITATED_UNIT, "move_result::E_INCAPACITATED_UNIT"},
{move_result::E_AMBUSHED, "move_result::E_AMBUSHED"},
{move_result::E_FAILED_TELEPORT, "move_result::E_FAILED_TELEPORT"},
{move_result::E_NO_ROUTE, "move_result::E_NO_ROUTE"},
{move_result::E_OFF_MAP, "move_result::E_OFF_MAP"},

{recall_result::E_NOT_AVAILABLE_FOR_RECALLING, "recall_result::E_NOT_AVAILABLE_FOR_RECALLING"},
{recall_result::E_NO_GOLD, "recall_result::E_NO_GOLD"},
{recall_result::E_NO_LEADER," recall_result::E_NO_LEADER"},
{recall_result::E_LEADER_NOT_ON_KEEP, "recall_result::E_LEADER_NOT_ON_KEEP"},
{recall_result::E_BAD_RECALL_LOCATION, "recall_result::E_BAD_RECALL_LOCATION"},

{recruit_result::E_NOT_AVAILABLE_FOR_RECRUITING, "recruit_result::E_NOT_AVAILABLE_FOR_RECRUITING"},
{recruit_result::E_UNKNOWN_OR_DUMMY_UNIT_TYPE, "recruit_result::E_UNKNOWN_OR_DUMMY_UNIT_TYPE"},
{recruit_result::E_NO_GOLD, "recruit_result::E_NO_GOLD"},
{recruit_result::E_NO_LEADER, "recruit_result::E_NO_LEADER"},
{recruit_result::E_LEADER_NOT_ON_KEEP, "recruit_result::E_LEADER_NOT_ON_KEEP"},
{recruit_result::E_BAD_RECRUIT_LOCATION, "recruit_result::E_BAD_RECRUIT_LOCATION"},

{stopunit_result::E_NO_UNIT, "stopunit_result::E_NO_UNIT"},
{stopunit_result::E_NOT_OWN_UNIT, "stopunit_result::E_NOT_OWN_UNIT"},
{stopunit_result::E_INCAPACITATED_UNIT, "stopunit_result::E_INCAPACITATED_UNIT"},
};

attack_result_ptr actions::execute_attack_action(side_number side,
bool execute,
const map_location& attacker_loc,
const map_location& defender_loc,
int attacker_weapon,
double aggression)
{
attack_result_ptr action(new attack_result(side, attacker_loc, defender_loc, attacker_weapon, aggression));
auto action = std::make_shared<attack_result>(side, attacker_loc, defender_loc, attacker_weapon, aggression);
execute_or_check(*action, execute);
return action;
}
Expand All @@ -1019,7 +1062,7 @@ move_result_ptr actions::execute_move_action( side_number side,
bool remove_movement,
bool unreach_is_ok)
{
move_result_ptr action(new move_result(side, from, to, remove_movement, unreach_is_ok));
auto action = std::make_shared<move_result>(side, from, to, remove_movement, unreach_is_ok);
execute_or_check(*action, execute);
return action;
}
Expand All @@ -1030,7 +1073,7 @@ recall_result_ptr actions::execute_recall_action( side_number side,
const map_location& where,
const map_location& from)
{
recall_result_ptr action(new recall_result(side, unit_id, where, from));
auto action = std::make_shared<recall_result>(side, unit_id, where, from);
execute_or_check(*action, execute);
return action;
}
Expand All @@ -1041,7 +1084,7 @@ recruit_result_ptr actions::execute_recruit_action( side_number side,
const map_location& where,
const map_location& from)
{
recruit_result_ptr action(new recruit_result(side, unit_name, where, from));
auto action = std::make_shared<recruit_result>(side, unit_name, where, from);
execute_or_check(*action, execute);
return action;
}
Expand All @@ -1052,7 +1095,7 @@ stopunit_result_ptr actions::execute_stopunit_action( side_number side,
bool remove_movement,
bool remove_attacks)
{
stopunit_result_ptr action(new stopunit_result(side, unit_location, remove_movement, remove_attacks));
auto action = std::make_shared<stopunit_result>(side, unit_location, remove_movement, remove_attacks);
execute_or_check(*action, execute);
return action;
}
Expand All @@ -1062,56 +1105,14 @@ synced_command_result_ptr actions::execute_synced_command_action( side_number si
const std::string& lua_code,
const map_location& location)
{
synced_command_result_ptr action(new synced_command_result(side, lua_code, location));
auto action = std::make_shared<synced_command_result>(side, lua_code, location);
execute_or_check(*action, execute);
return action;
}

const std::string& actions::get_error_name(int error_code)
{
if (error_names_.empty()){
error_names_.emplace(action_result::AI_ACTION_SUCCESS, "action_result::AI_ACTION_SUCCESS");
error_names_.emplace(action_result::AI_ACTION_STARTED, "action_result::AI_ACTION_STARTED");
error_names_.emplace(action_result::AI_ACTION_FAILURE, "action_result::AI_ACTION_FAILURE");

error_names_.emplace(attack_result::E_EMPTY_ATTACKER, "attack_result::E_EMPTY_ATTACKER");
error_names_.emplace(attack_result::E_EMPTY_DEFENDER, "attack_result::E_EMPTY_DEFENDER");
error_names_.emplace(attack_result::E_INCAPACITATED_ATTACKER, "attack_result::E_INCAPACITATED_ATTACKER");
error_names_.emplace(attack_result::E_INCAPACITATED_DEFENDER, "attack_result::E_INCAPACITATED_DEFENDER");
error_names_.emplace(attack_result::E_NOT_OWN_ATTACKER, "attack_result::E_NOT_OWN_ATTACKER");
error_names_.emplace(attack_result::E_NOT_ENEMY_DEFENDER, "attack_result::E_NOT_ENEMY_DEFENDER");
error_names_.emplace(attack_result::E_NO_ATTACKS_LEFT, "attack_result::E_NO_ATTACKS_LEFT");
error_names_.emplace(attack_result::E_WRONG_ATTACKER_WEAPON, "attack_result::E_WRONG_ATTACKER_WEAPON");
error_names_.emplace(attack_result::E_UNABLE_TO_CHOOSE_ATTACKER_WEAPON, "attack_result::E_UNABLE_TO_CHOOSE_ATTACKER_WEAPON");
error_names_.emplace(attack_result::E_ATTACKER_AND_DEFENDER_NOT_ADJACENT," attack_result::E_ATTACKER_AND_DEFENDER_NOT_ADJACENT");

error_names_.emplace(move_result::E_EMPTY_MOVE, "move_result::E_EMPTY_MOVE");
error_names_.emplace(move_result::E_NO_UNIT, "move_result::E_NO_UNIT");
error_names_.emplace(move_result::E_NOT_OWN_UNIT, "move_result::E_NOT_OWN_UNIT");
error_names_.emplace(move_result::E_INCAPACITATED_UNIT, "move_result::E_INCAPACITATED_UNIT");
error_names_.emplace(move_result::E_AMBUSHED, "move_result::E_AMBUSHED");
error_names_.emplace(move_result::E_FAILED_TELEPORT, "move_result::E_FAILED_TELEPORT");
error_names_.emplace(move_result::E_NO_ROUTE, "move_result::E_NO_ROUTE");
error_names_.emplace(move_result::E_OFF_MAP, "move_result::E_OFF_MAP");

error_names_.emplace(recall_result::E_NOT_AVAILABLE_FOR_RECALLING, "recall_result::E_NOT_AVAILABLE_FOR_RECALLING");
error_names_.emplace(recall_result::E_NO_GOLD, "recall_result::E_NO_GOLD");
error_names_.emplace(recall_result::E_NO_LEADER," recall_result::E_NO_LEADER");
error_names_.emplace(recall_result::E_LEADER_NOT_ON_KEEP, "recall_result::E_LEADER_NOT_ON_KEEP");
error_names_.emplace(recall_result::E_BAD_RECALL_LOCATION, "recall_result::E_BAD_RECALL_LOCATION");

error_names_.emplace(recruit_result::E_NOT_AVAILABLE_FOR_RECRUITING, "recruit_result::E_NOT_AVAILABLE_FOR_RECRUITING");
error_names_.emplace(recruit_result::E_UNKNOWN_OR_DUMMY_UNIT_TYPE, "recruit_result::E_UNKNOWN_OR_DUMMY_UNIT_TYPE");
error_names_.emplace(recruit_result::E_NO_GOLD, "recruit_result::E_NO_GOLD");
error_names_.emplace(recruit_result::E_NO_LEADER, "recruit_result::E_NO_LEADER");
error_names_.emplace(recruit_result::E_LEADER_NOT_ON_KEEP, "recruit_result::E_LEADER_NOT_ON_KEEP");
error_names_.emplace(recruit_result::E_BAD_RECRUIT_LOCATION, "recruit_result::E_BAD_RECRUIT_LOCATION");

error_names_.emplace(stopunit_result::E_NO_UNIT, "stopunit_result::E_NO_UNIT");
error_names_.emplace(stopunit_result::E_NOT_OWN_UNIT, "stopunit_result::E_NOT_OWN_UNIT");
error_names_.emplace(stopunit_result::E_INCAPACITATED_UNIT, "stopunit_result::E_INCAPACITATED_UNIT");
}
std::map<int,std::string>::iterator i = error_names_.find(error_code);
auto i = error_names_.find(error_code);
if (i==error_names_.end()){
ERR_AI_ACTIONS << "error name not available for error #"<<error_code << std::endl;
i = error_names_.find(-1);
Expand All @@ -1120,8 +1121,6 @@ const std::string& actions::get_error_name(int error_code)
return i->second;
}

std::map<int,std::string> actions::error_names_;

void sim_gamestate_changed(action_result *result, bool gamestate_changed){
if(gamestate_changed){
result->set_gamestate_changed();
Expand Down
2 changes: 1 addition & 1 deletion src/ai/actions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ const static std::string& get_error_name(int error_code);

private:

static std::map<int,std::string> error_names_;
static const std::map<int, std::string> error_names_;

};

Expand Down
14 changes: 1 addition & 13 deletions src/display.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ gui::button::TYPE display::string_to_button_type(const std::string& type)

static const std::string& get_direction(std::size_t n)
{
static const std::array<std::string, 6> dirs {{ "-n", "-ne", "-se", "-s", "-sw", "-nw" }};
static const std::array<std::string, 6> dirs { "-n", "-ne", "-se", "-s", "-sw", "-nw" };
return dirs[n >= dirs.size() ? 0 : n];
}

Expand Down Expand Up @@ -1222,18 +1222,6 @@ void display::drawing_buffer_add(const drawing_layer layer,
drawing_buffer_.emplace_back(layer, loc, x, y, surf, clip);
}

// FIXME: temporary method. Group splitting should be made
// public into the definition of drawing_layer
//
// The drawing is done per layer_group, the range per group is [low, high).
const std::array<display::drawing_layer, 4> display::drawing_buffer_key::layer_groups {{
LAYER_TERRAIN_BG,
LAYER_UNIT_FIRST,
LAYER_UNIT_MOVE_DEFAULT,
// Make sure the movement doesn't show above fog and reachmap.
LAYER_REACHMAP
}};

enum {
// you may adjust the following when needed:

Expand Down
12 changes: 11 additions & 1 deletion src/display.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,17 @@ class display : public video2::draw_layering
private:
unsigned int key_;

static const std::array<drawing_layer, 4> layer_groups;
// FIXME: temporary method. Group splitting should be made
// public into the definition of drawing_layer
//
// The drawing is done per layer_group, the range per group is [low, high).
static inline const std::array layer_groups {
LAYER_TERRAIN_BG,
LAYER_UNIT_FIRST,
LAYER_UNIT_MOVE_DEFAULT,
// Make sure the movement doesn't show above fog and reachmap.
LAYER_REACHMAP
};

public:
drawing_buffer_key(const map_location &loc, drawing_layer layer);
Expand Down
12 changes: 8 additions & 4 deletions src/enum_base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,13 @@ struct enum_base : public T
}
};

#define ENUM_AND_ARRAY(...) \
static constexpr std::size_t count = std::tuple_size<decltype(std::make_tuple(__VA_ARGS__))>::value; \
enum class type { __VA_ARGS__ }; \
static constexpr std::array<const char*, count> values{ __VA_ARGS__ };
#define ENUM_AND_ARRAY(...) \
enum class type { __VA_ARGS__ }; \
\
/** Provide a variable template for an array of matching size. */ \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a variable template. A variable template is like template<typename T> bool whatever = something<T>::value;. Please fix the comment.

Also, why remove count? You could leave the count declaration and just use it here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right, I meant alias template. I removed count since I made values use the alias so I figured might as well subsume it.

template<typename T> \
using sized_array = std::array<T, std::tuple_size<decltype(std::make_tuple(__VA_ARGS__))>::value>; \
\
static constexpr sized_array<const char*> values{__VA_ARGS__};

} // namespace string_enums
2 changes: 1 addition & 1 deletion src/font/font_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ namespace
#ifdef CAIRO_HAS_WIN32_FONT
bool is_valid_font_file(const std::string& file)
{
static const std::array<std::string, 3> font_exts { ".ttf", ".ttc", ".otf" };
static const std::array font_exts { ".ttf", ".ttc", ".otf" };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea if it's a problem. but this change means three std::string values are constructed every time this function is called.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using auto two lines down should help, depending on the API of filesystem::ends_with. Or append s to each of the constants.


for(const std::string& ext : font_exts) {
if(filesystem::ends_with(file, ext)) {
Expand Down
10 changes: 5 additions & 5 deletions src/game_initialization/connect_engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ static lg::log_domain log_network("network");

namespace
{
const std::array<std::string, 5> controller_names {{
const std::array controller_names {
side_controller::human,
side_controller::human,
side_controller::ai,
side_controller::none,
side_controller::reserved
}};
};

const std::set<std::string> children_to_swap {
"village",
Expand Down Expand Up @@ -317,7 +317,7 @@ void connect_engine::update_level()
}
}

void connect_engine::update_and_send_diff(bool /*update_time_of_day*/)
void connect_engine::update_and_send_diff()
{
config old_level = level_;
update_level();
Expand Down Expand Up @@ -457,7 +457,7 @@ void connect_engine::start_game()
config lock("stop_updates");
mp::send_to_server(lock);

update_and_send_diff(true);
update_and_send_diff();

save_reserved_sides_information();

Expand Down Expand Up @@ -523,7 +523,7 @@ void connect_engine::start_game_commandline(const commandline_options& cmdline_o
side->resolve_random(rng);
} // end top-level loop

update_and_send_diff(true);
update_and_send_diff();

// Update sides with commandline parameters.
if(cmdline_opts.multiplayer_turns) {
Expand Down
2 changes: 1 addition & 1 deletion src/game_initialization/connect_engine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class connect_engine
// Import all sides into the level.
void update_level();
// Updates the level and sends a diff to the clients.
void update_and_send_diff(bool update_time_of_day = false);
void update_and_send_diff();

bool can_start_game() const;
void start_game();
Expand Down
2 changes: 1 addition & 1 deletion src/generators/lua_map_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ lua_map_generator::lua_map_generator(const config & cfg, const config* vars)
, generator_data_(cfg)
{
lk_.load_core();
const std::array<std::string, 3> required {{"id", "config_name", "create_map"}};
const std::array required {"id", "config_name", "create_map"};
for(const std::string& req : required) {
if (!cfg.has_attribute(req)) {
if(req == "create_map" && cfg.has_attribute("create_scenario")) {
Expand Down
2 changes: 1 addition & 1 deletion src/gui/dialogs/game_stats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ unit_const_ptr game_stats::get_leader(const int side)

static std::string controller_name(const team& t)
{
static const std::array<t_string, side_controller::size()> names {{_("controller^Idle"), _("controller^Human"), _("controller^AI"), _("controller^Reserved")}};
static const side_controller::sized_array<t_string> names {_("controller^Idle"), _("controller^Human"), _("controller^AI"), _("controller^Reserved")};
return "<span color='#808080'><small>" + names[static_cast<int>(t.controller())] + "</small></span>";
}

Expand Down
6 changes: 3 additions & 3 deletions src/gui/dialogs/multiplayer/mp_create_game.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,12 +253,12 @@ void mp_create_game::pre_show(window& win)
//
// Set up random faction mode menu_button
//
static const std::array<t_string, random_faction_mode::size()> names {{_("Independent"), _("No Mirror"), _("No Ally Mirror")}};
static const std::array<t_string, random_faction_mode::size()> tooltips {{
static const random_faction_mode::sized_array<t_string> names {_("Independent"), _("No Mirror"), _("No Ally Mirror")};
static const random_faction_mode::sized_array<t_string> tooltips {
_("Independent: Random factions assigned independently"),
_("No Mirror: No two players will get the same faction"),
_("No Ally Mirror: No two allied players will get the same faction")
}};
};
std::vector<config> rfm_options;
for(std::size_t i = 0; i < random_faction_mode::size(); i++) {
rfm_options.emplace_back("label", names[i]);
Expand Down
2 changes: 1 addition & 1 deletion src/replay.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ static void verify(const unit_map& units, const config& cfg) {
u->write(u_cfg);

bool is_ok = true;
static const std::array<std::string, 4> fields {{"type","hitpoints","experience","side"}};
static const std::array fields {"type","hitpoints","experience","side"};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again you've changed the type from std::string to const char*. Same as previous comment.

for(const std::string& field : fields) {
if (u_cfg[field] != un[field]) {
errbuf << "ERROR IN FIELD '" << field << "' for unit at "
Expand Down
4 changes: 2 additions & 2 deletions src/scripting/game_lua_kernel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4726,7 +4726,7 @@ void game_lua_kernel::set_game_display(game_display * gd) {
* elsewhere (in the C++ code).
* Any child tags not in this list will be passed to Lua's on_load event.
*/
static const std::array<std::string, 24> handled_file_tags {{
static const std::array handled_file_tags {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing again.

(Aside: It seems to me that the function using this could just be an std::find call.)

"color_palette",
"color_range",
"display",
Expand All @@ -4751,7 +4751,7 @@ static const std::array<std::string, 24> handled_file_tags {{
"tunnel",
"undo_stack",
"variables"
}};
};

static bool is_handled_file_tag(const std::string& s)
{
Expand Down
9 changes: 3 additions & 6 deletions src/terrain/builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,6 @@ static map_location legacy_difference(const map_location& me, const map_location
*
*/

terrain_builder::building_ruleset terrain_builder::building_rules_;
const game_config_view* terrain_builder::rules_cfg_ = nullptr;

terrain_builder::rule_image::rule_image(int layer, int x, int y, bool global_image, int cx, int cy, bool is_water)
: layer(layer)
, basex(x)
Expand Down Expand Up @@ -730,7 +727,7 @@ void terrain_builder::add_images_from_config(rule_imagelist& images, const confi
// If an integer is given then assign that, but if a bool is given, then assign -1 if true and 0 if false
int random_start = variant["random_start"].to_bool(true) ? variant["random_start"].to_int(-1) : 0;

images.back().variants.push_back(rule_image_variant(name, variations, tod, has_flag, random_start));
images.back().variants.emplace_back(name, variations, tod, has_flag, random_start);
}

// Adds the main (default) variant of the image at the end,
Expand All @@ -740,7 +737,7 @@ void terrain_builder::add_images_from_config(rule_imagelist& images, const confi

int random_start = img["random_start"].to_bool(true) ? img["random_start"].to_int(-1) : 0;

images.back().variants.push_back(rule_image_variant(name, variations, random_start));
images.back().variants.emplace_back(name, variations, random_start);
}
}

Expand Down Expand Up @@ -1093,7 +1090,7 @@ void terrain_builder::apply_rule(const terrain_builder::building_rule& rule, con

if(!constraint.no_draw) {
for(const rule_image& img : constraint.images) {
btile.images.push_back(tile::rule_image_rand(&img, rand_seed));
btile.images.emplace_back(&img, rand_seed);
}
}

Expand Down