Skip to content

Commit

Permalink
make flg manager not store confg& side_
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gfgtdf committed Jun 29, 2018
1 parent ef4ae1c commit e72a92a
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 42 deletions.
76 changes: 39 additions & 37 deletions src/game_initialization/flg_manager.cpp
Expand Up @@ -33,11 +33,18 @@ namespace ng
flg_manager::flg_manager(const std::vector<const config*>& 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_()
Expand All @@ -47,14 +54,14 @@ flg_manager::flg_manager(const std::vector<const config*>& 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();
Expand All @@ -63,7 +70,7 @@ flg_manager::flg_manager(const std::vector<const config*>& 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();
Expand All @@ -84,9 +91,18 @@ flg_manager::flg_manager(const std::vector<const config*>& 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)
Expand All @@ -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)
Expand Down Expand Up @@ -245,7 +261,7 @@ void flg_manager::resolve_random(randomness::mt_rng& rng, const std::vector<std:
void flg_manager::update_available_factions()
{
const config* custom_faction = nullptr;
const bool show_custom_faction = get_default_faction(side_)["faction"] == "Custom" || !has_no_recruits_ || faction_lock_;
const bool show_custom_faction = original_faction_ == "Custom" || !has_no_recruits_ || faction_lock_;

for(const config* faction : era_factions_) {
if((*faction)["id"] == "Custom" && !show_custom_faction) {
Expand All @@ -258,7 +274,7 @@ void flg_manager::update_available_factions()
}

// Add default faction to the top of the list.
if(get_default_faction(side_)["faction"] == (*faction)["id"]) {
if(original_faction_ == (*faction)["id"]) {
available_factions_.insert(available_factions_.begin(), faction);
} else {
available_factions_.push_back(faction);
Expand Down Expand Up @@ -327,17 +343,8 @@ void flg_manager::update_available_genders()
available_genders_.clear();

if(saved_game_) {
std::string gender;

for(const config& side_unit : side_.child_range("unit")) {
if(current_leader_ == side_unit["type"] && side_unit["canrecruit"].to_bool()) {
gender = side_unit["gender"].str();
break;
}
}

if(!gender.empty()) {
available_genders_.push_back(gender);
if(!savegame_gender_.empty()) {
available_genders_.push_back(savegame_gender_);
}
} else {
if(const unit_type* unit = unit_types.find(current_leader_)) {
Expand Down Expand Up @@ -421,7 +428,7 @@ void flg_manager::update_choosable_genders()

void flg_manager::select_default_faction()
{
const config::attribute_value& default_faction = get_default_faction(side_)["faction"];
const std::string& default_faction = original_faction_;
auto default_faction_it = std::find_if(choosable_factions_.begin(), choosable_factions_.end(),
[&default_faction](const config* faction) {
return (*faction)["id"] == default_faction;
Expand All @@ -439,17 +446,17 @@ int flg_manager::find_suitable_faction() const
std::vector<std::string> 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");
Expand Down Expand Up @@ -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);
}
Expand All @@ -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<std::string> 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")) {
Expand Down
13 changes: 10 additions & 3 deletions src/game_initialization/flg_manager.hpp
Expand Up @@ -104,8 +104,16 @@ class flg_manager

const std::vector<const config*>& 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<std::string> original_recruit_;
const std::string choose_faction_by_leader_;
const bool saved_game_;
const bool has_no_recruits_;

Expand All @@ -129,7 +137,6 @@ class flg_manager
std::string default_leader_gender_;
const config* default_leader_cfg_;

static std::vector<std::string> get_original_recruits(const config& cfg);
static const config& get_default_faction(const config& cfg);
};

Expand Down
11 changes: 9 additions & 2 deletions src/gui/dialogs/multiplayer/mp_join_game.cpp
Expand Up @@ -209,8 +209,12 @@ bool mp_join_game::fetch_game_config()
const std::string color = (*side_choice)["color"].str();

std::vector<const config*> 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();
Expand Down Expand Up @@ -332,8 +336,11 @@ void mp_join_game::show_flg_select(int side_num)
const std::string color = side_choice["color"].str();

std::vector<const config*> 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();
Expand Down

0 comments on commit e72a92a

Please sign in to comment.