From e72a92ac961a6b5e0b0f3c27c62cdcff4d3e4f4d Mon Sep 17 00:00:00 2001 From: gfgtdf Date: Tue, 26 Jun 2018 22:17:17 +0200 Subject: [PATCH] make flg manager not store confg& side_ in the non-host case the clients might get updates that change the [side] config while the flg dialog is open which might result in invalid pointers, note that flg_manager::default_leader_cfg_ is still there but this is no problm because that is never dereferenced it's only compared to other pointers. we also make a copy of all [multiplayer_side] on the non hosts side for the same reason. --- src/game_initialization/flg_manager.cpp | 76 ++++++++++---------- src/game_initialization/flg_manager.hpp | 13 +++- src/gui/dialogs/multiplayer/mp_join_game.cpp | 11 ++- 3 files changed, 58 insertions(+), 42 deletions(-) diff --git a/src/game_initialization/flg_manager.cpp b/src/game_initialization/flg_manager.cpp index 57eda15f81ed..29d42a39a70d 100644 --- a/src/game_initialization/flg_manager.cpp +++ b/src/game_initialization/flg_manager.cpp @@ -33,11 +33,18 @@ namespace ng flg_manager::flg_manager(const std::vector& era_factions, const config& side, const bool lock_settings, const bool use_map_settings, const bool saved_game) : era_factions_(era_factions) - , side_(side) + , side_num_(side["side"].to_int()) + , faction_from_recruit_(side["faction_from_recruit"].to_bool()) + , original_type_(get_default_faction(side)["type"].str()) + , original_gender_(get_default_faction(side)["gender"].str()) + , savegame_gender_() + , original_faction_(get_default_faction(side)["faction"].str()) + , original_recruit_(utils::split(get_default_faction(side)["recruit"].str())) + , choose_faction_by_leader_(side["leader"].str()) , saved_game_(saved_game) - , has_no_recruits_(get_original_recruits(side_).empty() && side_["previous_recruits"].empty()) - , faction_lock_(side_["faction_lock"].to_bool(lock_settings)) - , leader_lock_(side_["leader_lock"].to_bool(lock_settings)) + , has_no_recruits_(original_recruit_.empty() && side["previous_recruits"].empty()) + , faction_lock_(side["faction_lock"].to_bool(lock_settings)) + , leader_lock_(side["leader_lock"].to_bool(lock_settings)) , available_factions_() , available_leaders_() , available_genders_() @@ -47,14 +54,14 @@ flg_manager::flg_manager(const std::vector& era_factions, , current_faction_(nullptr) , current_leader_("null") , current_gender_("null") - , default_leader_type_(side_["type"]) - , default_leader_gender_(side_["gender"]) + , default_leader_type_(side["type"]) + , default_leader_gender_(side["gender"]) , default_leader_cfg_(nullptr) { - const std::string& leader_id = side_["id"]; + const std::string& leader_id = side["id"]; if(!leader_id.empty()) { // Check if leader was carried over and now is in [unit] tag. - default_leader_cfg_ = &side_.find_child("unit", "id", leader_id); + default_leader_cfg_ = &side.find_child("unit", "id", leader_id); if(*default_leader_cfg_) { default_leader_type_ = (*default_leader_cfg_)["type"].str(); default_leader_gender_ = (*default_leader_cfg_)["gender"].str(); @@ -63,7 +70,7 @@ flg_manager::flg_manager(const std::vector& era_factions, } } else if(default_leader_type_.empty()) { // Find a unit which can recruit. - for(const config& side_unit : side_.child_range("unit")) { + for(const config& side_unit : side.child_range("unit")) { if(side_unit["canrecruit"].to_bool()) { default_leader_type_ = side_unit["type"].str(); default_leader_gender_ = side_unit["gender"].str(); @@ -84,9 +91,18 @@ flg_manager::flg_manager(const std::vector& era_factions, leader_lock_ = leader_lock_ && (use_map_settings || lock_settings || default_leader_type_.empty()); faction_lock_ = faction_lock_ && (use_map_settings || lock_settings); + //TODO: this code looks wrong, we should probably just use default_leader_gender_ from above. + for(const config& side_unit : side.child_range("unit")) { + if(current_leader_ == side_unit["type"] && side_unit["canrecruit"].to_bool()) { + savegame_gender_ = side_unit["gender"].str(); + break; + } + } + update_available_factions(); select_default_faction(); + } void flg_manager::set_current_faction(const unsigned index) @@ -109,7 +125,7 @@ void flg_manager::set_current_faction(const std::string& id) index++; } - ERR_MP << "Faction '" << id << "' is not available for side " << side_["side"] << " Ignoring"; + ERR_MP << "Faction '" << id << "' is not available for side " << side_num_ << " Ignoring"; } void flg_manager::set_current_leader(const unsigned index) @@ -245,7 +261,7 @@ void flg_manager::resolve_random(randomness::mt_rng& rng, const std::vector find; std::string search_field; - if(const config::attribute_value* f = get_default_faction(side_).get("faction")) { + if(!original_faction_.empty()) { // Choose based on faction. - find.push_back(f->str()); + find.push_back(original_faction_); search_field = "id"; - } else if(side_["faction_from_recruit"].to_bool()) { + } else if(faction_from_recruit_) { // Choose based on recruit. - find = get_original_recruits(side_); + find = original_recruit_; search_field = "recruit"; - } else if(const config::attribute_value *l = side_.get("leader")) { + } else if(!choose_faction_by_leader_.empty()) { // Choose based on leader. - find.push_back(*l); + find.push_back(choose_faction_by_leader_); search_field = "leader"; } else { find.push_back("Custom"); @@ -520,7 +527,7 @@ void flg_manager::set_current_leader(const std::string& leader) { int index = leader_index(leader); if(index < 0) { - ERR_MP << "Leader '" << leader << "' is not available for side " << side_["side"] << " Ignoring"; + ERR_MP << "Leader '" << leader << "' is not available for side " << side_num_ << " Ignoring"; } else { set_current_leader(index); } @@ -530,17 +537,12 @@ void flg_manager::set_current_gender(const std::string& gender) { int index = gender_index(gender); if(index < 0) { - ERR_MP << "Gender '" << gender << "' is not available for side " << side_["side"] << " Ignoring"; + ERR_MP << "Gender '" << gender << "' is not available for side " << side_num_ << " Ignoring"; } else { set_current_gender(index); } } -std::vector flg_manager::get_original_recruits(const config& cfg) -{ - return utils::split(get_default_faction(cfg)["recruit"].str()); -} - const config& flg_manager::get_default_faction(const config& cfg) { if(const config& df = cfg.child("default_faction")) { diff --git a/src/game_initialization/flg_manager.hpp b/src/game_initialization/flg_manager.hpp index e5c4a9a9d954..9490ea851c84 100644 --- a/src/game_initialization/flg_manager.hpp +++ b/src/game_initialization/flg_manager.hpp @@ -104,8 +104,16 @@ class flg_manager const std::vector& era_factions_; - const config& side_; - + // not sure how reilable the content of this field is, it's currently only used for debugging info. + const int side_num_; + const bool faction_from_recruit_; + + const std::string original_type_; + const std::string original_gender_; + std::string savegame_gender_; + const std::string original_faction_; + const std::vector original_recruit_; + const std::string choose_faction_by_leader_; const bool saved_game_; const bool has_no_recruits_; @@ -129,7 +137,6 @@ class flg_manager std::string default_leader_gender_; const config* default_leader_cfg_; - static std::vector get_original_recruits(const config& cfg); static const config& get_default_faction(const config& cfg); }; diff --git a/src/gui/dialogs/multiplayer/mp_join_game.cpp b/src/gui/dialogs/multiplayer/mp_join_game.cpp index 20d7e90ce09d..d653f0c416e0 100644 --- a/src/gui/dialogs/multiplayer/mp_join_game.cpp +++ b/src/gui/dialogs/multiplayer/mp_join_game.cpp @@ -209,8 +209,12 @@ bool mp_join_game::fetch_game_config() const std::string color = (*side_choice)["color"].str(); std::vector era_factions; + + //make this safe against changes to level_ that might make possible_sides invalid pointers. + config era_copy; for(const config& side : possible_sides) { - era_factions.push_back(&side); + config& side_new = era_copy.add_child("multiplayer_side", side); + era_factions.push_back(&side_new); } const bool is_mp = state_.classification().is_normal_mp_game(); @@ -332,8 +336,11 @@ void mp_join_game::show_flg_select(int side_num) const std::string color = side_choice["color"].str(); std::vector era_factions; + //make this safe against changes to level_ that might make possible_sides invalid pointers. + config era_copy; for(const config& side : possible_sides) { - era_factions.push_back(&side); + config& side_new = era_copy.add_child("multiplayer_side", side); + era_factions.push_back(&side_new); } const bool is_mp = state_.classification().is_normal_mp_game();