From 5320c7120d3914f05fda793c7c1f8d67b5f6e5d9 Mon Sep 17 00:00:00 2001 From: Charles Dang Date: Thu, 4 Jul 2019 15:45:36 +1100 Subject: [PATCH] Refactor team_data usage and fix issue with Game Stats's Scroll To functionality This removes unnecessary struct members that can be accessed directly from the team class and fixes an issue where hidden teams could cause the Game Stats dialog to break when scrolling to leader (fixes #4029) --- src/display_context.cpp | 16 ++++++---------- src/display_context.hpp | 27 ++++++++------------------- src/gui/dialogs/game_stats.cpp | 27 ++++++++++++++------------- src/gui/dialogs/game_stats.hpp | 8 ++++---- src/menu_events.cpp | 6 +++--- src/reports.cpp | 8 ++++---- src/scripting/lua_team.cpp | 4 ++-- 7 files changed, 41 insertions(+), 55 deletions(-) diff --git a/src/display_context.cpp b/src/display_context.cpp index 21178477f808..99482828373f 100644 --- a/src/display_context.cpp +++ b/src/display_context.cpp @@ -159,15 +159,11 @@ int display_context::side_upkeep(int side) const return res; } -team_data display_context::calculate_team_data(const team& tm) const +team_data::team_data(const display_context& dc, const team& tm) + : side(tm.side()) + , units(dc.side_units(side)) + , upkeep(dc.side_upkeep(side)) + , expenses(std::max(0, upkeep - tm.support())) + , net_income(tm.total_income() - expenses) { - team_data res; - res.units = side_units(tm.side()); - res.upkeep = side_upkeep(tm.side()); - res.villages = tm.villages().size(); - res.expenses = std::max(0,res.upkeep - tm.support()); - res.net_income = tm.total_income() - res.expenses; - res.gold = tm.gold(); - res.teamname = tm.user_team_name(); - return res; } diff --git a/src/display_context.hpp b/src/display_context.hpp index ac84224ee9d2..d56ee8f73848 100644 --- a/src/display_context.hpp +++ b/src/display_context.hpp @@ -31,24 +31,8 @@ class unit_map; class unit; struct map_location; -struct team_data +class display_context { - team_data() : - units(0), - upkeep(0), - villages(0), - expenses(0), - net_income(0), - gold(0), - teamname() - { - } - - int units, upkeep, villages, expenses, net_income, gold; - std::string teamname; -}; - -class display_context { public: virtual const std::vector & teams() const = 0; virtual const gamemap & map() const = 0; @@ -99,8 +83,6 @@ class display_context { int side_upkeep(int side_num) const ; - team_data calculate_team_data(const class team& tm) const; - // Accessor from team.cpp /// Check if we are an observer in this game @@ -110,3 +92,10 @@ class display_context { virtual ~display_context() {} }; + +struct team_data +{ + team_data(const display_context& dc, const team& tm); + + int side = 0, units = 0, upkeep = 0, expenses = 0, net_income = 0; +}; diff --git a/src/gui/dialogs/game_stats.cpp b/src/gui/dialogs/game_stats.cpp index 6f1de5d74f26..3b6c0aac4fd6 100644 --- a/src/gui/dialogs/game_stats.cpp +++ b/src/gui/dialogs/game_stats.cpp @@ -48,14 +48,11 @@ namespace dialogs REGISTER_DIALOG(game_stats) -game_stats::game_stats(const display_context& board, const int viewing_team, int& selected_index) +game_stats::game_stats(const display_context& board, const int viewing_team, int& selected_side_number) : board_(board) , viewing_team_(board_.teams()[viewing_team]) - , selected_index_(selected_index) + , selected_side_number_(selected_side_number) { - for(const auto& team : board_.teams()) { - team_data_.push_back(board_.calculate_team_data(team)); - } } unit_const_ptr game_stats::get_leader(const int side) @@ -85,13 +82,15 @@ void game_stats::pre_show(window& window) continue; } + team_data_.emplace_back(board_, team); + std::map row_data_stats; string_map column_stats; const bool known = viewing_team_.knows_about_team(team.side() - 1); const bool enemy = viewing_team_.is_enemy(team.side()); - const team_data& data = team_data_[team.side() - 1]; + const team_data& data = team_data_.back(); unit_const_ptr leader = get_leader(team.side()); @@ -130,20 +129,20 @@ void game_stats::pre_show(window& window) column_stats["label"] = leader_name + "\n" + controller_name(team); row_data_stats.emplace("team_leader_name", column_stats); - column_stats["label"] = data.teamname.empty() ? team.team_name() : data.teamname; + column_stats["label"] = team.user_team_name().empty() ? team.team_name() : team.user_team_name(); row_data_stats.emplace("team_name", column_stats); // Only fill in the rest of the info if the side is known... if(known || game_config::debug) { std::string gold_str; if(game_config::debug || !enemy || !viewing_team_.uses_fog()) { - gold_str = utils::half_signed_value(data.gold); + gold_str = utils::half_signed_value(team.gold()); } - column_stats["label"] = data.gold < 0 ? "" + gold_str + "" : gold_str; + column_stats["label"] = team.gold() < 0 ? "" + gold_str + "" : gold_str; row_data_stats.emplace("team_gold", column_stats); - std::string village_count = std::to_string(data.villages); + std::string village_count = std::to_string(team.villages().size()); if(!viewing_team_.uses_fog() && !viewing_team_.uses_shroud()) { village_count += "/" + std::to_string(board_.map().villages().size()); } @@ -205,7 +204,8 @@ void game_stats::pre_show(window& window) // Sorting options for the status list stats_list.register_translatable_sorting_option(0, [this](const int i) { unit_const_ptr leader = get_leader(i + 1); - return leader ? leader->name().str() : ""; }); + return leader ? leader->name().str() : ""; + }); stats_list.register_translatable_sorting_option(1, [this](const int i) { return board_.teams()[i].user_team_name().str(); }); @@ -218,7 +218,8 @@ void game_stats::pre_show(window& window) // Sorting options for the settings list settings_list.register_translatable_sorting_option(0, [this](const int i) { unit_const_ptr leader = get_leader(i + 1); - return leader ? leader->name().str() : ""; }); + return leader ? leader->name().str() : ""; + }); settings_list.register_sorting_option(1, [this](const int i) { return board_.teams()[i].side(); }); settings_list.register_sorting_option(2, [this](const int i) { return board_.teams()[i].start_gold(); }); @@ -258,7 +259,7 @@ void game_stats::post_show(window& window) const int selected_tab = find_widget(&window, "tab_bar", false).get_selected_row(); const std::string list_id = selected_tab == 0 ? "game_stats_list" : "scenario_settings_list"; - selected_index_ = find_widget(&window, list_id, false).get_selected_row(); + selected_side_number_ = team_data_[find_widget(&window, list_id, false).get_selected_row()].side; } } diff --git a/src/gui/dialogs/game_stats.hpp b/src/gui/dialogs/game_stats.hpp index acd2b19439ee..47baad79f3a3 100644 --- a/src/gui/dialogs/game_stats.hpp +++ b/src/gui/dialogs/game_stats.hpp @@ -35,16 +35,16 @@ namespace dialogs class game_stats : public modal_dialog { public: - game_stats(const display_context& board, const int viewing_team, int& selected_index); + game_stats(const display_context& board, const int viewing_team, int& selected_side_number); - static bool execute(game_board& board, const int viewing_team, int& selected_index) + static bool execute(game_board& board, const int viewing_team, int& selected_side_number) { if(std::all_of(board.teams().begin(), board.teams().end(), [](team& team) { return team.hidden(); })) { show_transient_message("", _("No visible sides found.")); return false; } - return game_stats(board, viewing_team, selected_index).show(); + return game_stats(board, viewing_team, selected_side_number).show(); } private: @@ -55,7 +55,7 @@ class game_stats : public modal_dialog std::vector team_data_; - int& selected_index_; + int& selected_side_number_; unit_const_ptr get_leader(const int side); diff --git a/src/menu_events.cpp b/src/menu_events.cpp index 7bc47bc436e9..83294100c99c 100644 --- a/src/menu_events.cpp +++ b/src/menu_events.cpp @@ -160,10 +160,10 @@ void menu_handler::unit_list() void menu_handler::status_table() { - int selected_index; + int selected_side; - if(gui2::dialogs::game_stats::execute(board(), gui_->viewing_team(), selected_index)) { - gui_->scroll_to_leader(teams()[selected_index].side()); + if(gui2::dialogs::game_stats::execute(board(), gui_->viewing_team(), selected_side)) { + gui_->scroll_to_leader(selected_side); } } diff --git a/src/reports.cpp b/src/reports.cpp index deafbb160156..b604418c39a9 100644 --- a/src/reports.cpp +++ b/src/reports.cpp @@ -677,7 +677,7 @@ static config unit_moves(reports::context & rc, const unit* u, bool is_visible_u for (const terrain_movement& tm : terrain_moves) { tooltip << tm.name << ": "; - + std::string color; //movement - range: 1 .. 5, movetype::UNREACHABLE=impassable const bool cannot_move = tm.moves > u->total_movement(); @@ -1440,7 +1440,7 @@ REPORT_GENERATOR(upkeep, rc) std::ostringstream str; int viewing_side = rc.screen().viewing_side(); const team &viewing_team = rc.dc().get_team(viewing_side); - team_data td = rc.dc().calculate_team_data(viewing_team); + team_data td(rc.dc(), viewing_team); str << td.expenses << " (" << td.upkeep << ")"; return gray_inactive(rc,str.str(), _("Upkeep") + "\n\n" + _("The expenses incurred at the end of every turn to maintain your army. The first number is the amount of gold that will be deducted. The second is the total cost of upkeep, including that covered by villages — in other words, the amount of gold that would be deducted if you lost all villages.")); } @@ -1449,7 +1449,7 @@ REPORT_GENERATOR(expenses, rc) { int viewing_side = rc.screen().viewing_side(); const team &viewing_team = rc.dc().get_team(viewing_side); - team_data td = rc.dc().calculate_team_data(viewing_team); + team_data td(rc.dc(), viewing_team); return gray_inactive(rc,std::to_string(td.expenses)); } @@ -1458,7 +1458,7 @@ REPORT_GENERATOR(income, rc) std::ostringstream str; int viewing_side = rc.screen().viewing_side(); const team &viewing_team = rc.dc().get_team(viewing_side); - team_data td = rc.dc().calculate_team_data(viewing_team); + team_data td(rc.dc(), viewing_team); char const *end = naps; if (viewing_side != rc.screen().playing_side()) { if (td.net_income < 0) { diff --git a/src/scripting/lua_team.cpp b/src/scripting/lua_team.cpp index 010a301a6b16..4abde41799ad 100644 --- a/src/scripting/lua_team.cpp +++ b/src/scripting/lua_team.cpp @@ -60,6 +60,7 @@ static int impl_side_get(lua_State *L) return_tstring_attrib("objectives", t.objectives()); return_int_attrib("village_gold", t.village_gold()); return_int_attrib("village_support", t.village_support()); + return_int_attrib("num_villages", t.villages().size()); return_int_attrib("recall_cost", t.recall_cost()); return_int_attrib("base_income", t.base_income()); return_int_attrib("total_income", t.total_income()); @@ -104,10 +105,9 @@ static int impl_side_get(lua_State *L) // These are blocked together because they are all part of the team_data struct. // Some of these values involve iterating over the units map to calculate them. - auto d = [&](){ return resources::gameboard->calculate_team_data(t); }; + auto d = [&](){ return team_data(*resources::gameboard, t); }; return_int_attrib("num_units", d().units); return_int_attrib("total_upkeep", d().upkeep); - return_int_attrib("num_villages", d().villages); return_int_attrib("expenses", d().expenses); return_int_attrib("net_income", d().net_income);