From 203698df5dfc94c393951561eee2777593957c0b Mon Sep 17 00:00:00 2001 From: Charles Dang Date: Tue, 31 Oct 2017 12:29:29 +1100 Subject: [PATCH] Lobby Info: cleanup and optimization * Deployed auto for iterators. * Reserve container space when appropriate to reduce unnecessary copying. This is especially useful since the game_info class has a rather lot of stuff in it. * Don't recreate sorted users vector every time said vector is sorted. The only time the actual contents would change are when the source (users) change, so just populate it at that point. * Remove the games_filtered vector. This was essentially just used to get the number of visible games with a filter active, but boost::dynamic_bitset (games_visibility) already provides appropriate functionality, rendering it useless. * Removed an unnecessary inline overload. Default arguments suffice instead. --- src/game_initialization/lobby_info.cpp | 46 ++++++++++++++------------ src/game_initialization/lobby_info.hpp | 12 +------ src/gui/dialogs/multiplayer/lobby.cpp | 18 ++-------- 3 files changed, 27 insertions(+), 49 deletions(-) diff --git a/src/game_initialization/lobby_info.cpp b/src/game_initialization/lobby_info.cpp index 5760aeee6eef..8e6aac1f2f30 100644 --- a/src/game_initialization/lobby_info.cpp +++ b/src/game_initialization/lobby_info.cpp @@ -45,7 +45,6 @@ lobby_info::lobby_info(const config& game_config, const std::vector , rooms_() , games_by_id_() , games_() - , games_filtered_() , users_() , users_sorted_() , whispers_() @@ -158,7 +157,7 @@ bool lobby_info::process_gamelist_diff(const config& data) return false; } - game_info_map::iterator current_i = games_by_id_.find(game_id); + auto current_i = games_by_id_.find(game_id); const std::string& diff_result = c[config::diff_track_attribute]; @@ -211,6 +210,13 @@ void lobby_info::process_userlist() users_.emplace_back(c); } + users_sorted_.reserve(users_.size()); + users_sorted_.clear(); + + for(auto& u : users_) { + users_sorted_.push_back(&u); + } + for(auto& ui : users_) { if(ui.game_id == 0) { continue; @@ -240,7 +246,7 @@ void lobby_info::sync_games_display_status() DBG_LB << "lobby_info::sync_games_display_status"; DBG_LB << "games_by_id_ size: " << games_by_id_.size(); - game_info_map::iterator i = games_by_id_.begin(); + auto i = games_by_id_.begin(); while(i != games_by_id_.end()) { if(i->second.display_status == game_info::DELETED) { @@ -258,13 +264,13 @@ void lobby_info::sync_games_display_status() game_info* lobby_info::get_game_by_id(int id) { - game_info_map::iterator i = games_by_id_.find(id); + auto i = games_by_id_.find(id); return i == games_by_id_.end() ? nullptr : &i->second; } const game_info* lobby_info::get_game_by_id(int id) const { - game_info_map::const_iterator i = games_by_id_.find(id); + auto i = games_by_id_.find(id); return i == games_by_id_.end() ? nullptr : &i->second; } @@ -324,26 +330,30 @@ void lobby_info::close_room(const std::string& name) void lobby_info::make_games_vector() { - games_filtered_.clear(); - games_visibility_.clear(); + games_.reserve(games_by_id_.size()); games_.clear(); for(auto& v : games_by_id_) { games_.push_back(&v.second); } + + // Reset the visibility mask. Its size should then match games_'s and all its bits be true. + games_visibility_.resize(games_.size()); + games_visibility_.reset(); + games_visibility_.flip(); } void lobby_info::apply_game_filter() { - games_filtered_.clear(); - games_visibility_.clear(); - - for(auto g : games_) { - game_info& gi = *g; + // Since games_visibility_ is a visibility mask over games_, + // they need to be the same size or we'll end up with issues. + assert(games_visibility_.size() == games_.size()); + for(unsigned i = 0; i < games_.size(); ++i) { bool show = true; + for(const auto& filter_func : game_filters_) { - show = filter_func(gi); + show = filter_func(*games_[i]); if(!show) { break; @@ -354,10 +364,7 @@ void lobby_info::apply_game_filter() show = !show; } - games_visibility_.push_back(show); - if(show) { - games_filtered_.push_back(&gi); - } + games_visibility_[i] = show; } } @@ -370,11 +377,6 @@ void lobby_info::update_user_statuses(int game_id, const room_info* room) void lobby_info::sort_users(bool by_name, bool by_relation) { - users_sorted_.clear(); - for(auto& u : users_) { - users_sorted_.push_back(&u); - } - std::sort(users_sorted_.begin(), users_sorted_.end(), [&](const user_info* u1, const user_info* u2) { if(by_name && by_relation) { return u1->relation < u2->relation || diff --git a/src/game_initialization/lobby_info.hpp b/src/game_initialization/lobby_info.hpp index 1d576e617673..1d124c842d0d 100644 --- a/src/game_initialization/lobby_info.hpp +++ b/src/game_initialization/lobby_info.hpp @@ -139,11 +139,6 @@ class lobby_info return games_visibility_; } - const std::vector& games_filtered() const - { - return games_filtered_; - } - const std::vector& users() const { return users_; @@ -170,7 +165,6 @@ class lobby_info game_info_map games_by_id_; std::vector games_; - std::vector games_filtered_; std::vector users_; std::vector users_sorted_; @@ -198,9 +192,5 @@ enum notify_mode { NOTIFY_COUNT }; -void do_notify(notify_mode mode, const std::string& sender, const std::string& message); -inline void do_notify(notify_mode mode) -{ - do_notify(mode, "", ""); -} +void do_notify(notify_mode mode, const std::string& sender = "", const std::string& message = ""); } diff --git a/src/gui/dialogs/multiplayer/lobby.cpp b/src/gui/dialogs/multiplayer/lobby.cpp index 8afadddbf7f3..433537f7829b 100644 --- a/src/gui/dialogs/multiplayer/lobby.cpp +++ b/src/gui/dialogs/multiplayer/lobby.cpp @@ -417,7 +417,7 @@ void mp_lobby::update_gamelist_header() { #ifndef GUI2_EXPERIMENTAL_LISTBOX const std::string games_string = vgettext("Games: showing $num_shown out of $num_total", { - {"num_shown", std::to_string(lobby_info_.games_filtered().size())}, + {"num_shown", std::to_string(lobby_info_.games_visibility().count())}, {"num_total", std::to_string(lobby_info_.games().size())} }); @@ -1106,21 +1106,7 @@ void mp_lobby::user_dialog_callback(mp::user_info* info) } selected_game_id_ = info->game_id; - // the commented out code below should be enough, but that'd delete the - // playerlist and the widget calling this callback, so instead the game - // will be selected on the next gamelist update. - /* - if (info->game_id != 0) { - for (unsigned i = 0; i < lobby_info_.games_filtered().size(); ++i) { - game_info& g = *lobby_info_.games_filtered()[i]; - if (info->game_id == g.id) { - gamelistbox_->select_row(i); - update_selected_game(); - break; - } - } - } - */ + // do not update here as it can cause issues with removing the widget // from within it's event handler. Should get updated as soon as possible // update_gamelist();