Revert "put [resource] before other content; fixes #3345"

This reverts commit 7067402209.

The commit used the `pos` iterator after invalidating
it (passing an iterator to `config::add_child_at()`
invalidates it), resulting in undefined behavior.
This commit is contained in:
Jyrki Vesterinen 2018-10-17 19:53:09 +03:00
parent 2bf39f7ee8
commit b3e1680f88
4 changed files with 24 additions and 61 deletions

View file

@ -542,42 +542,6 @@ config& config::add_child_at(config_key_type key, const config& val, unsigned in
return *v[index];
}
config& config::add_child_at(config_key_type key, const config &val, const const_all_children_iterator& pos)
{
if(pos == ordered_end()) {
//optimisation
config::add_child(key, val);
}
auto end = ordered_children.end();
assert(pos.i_ <= end);
auto pos_it = ordered_children.begin() + (pos.i_ - ordered_children.begin()); //'const cast'.
auto next = std::find_if(pos_it, end,[&](const child_pos& p){ return p.pos->first == key; });
if(next == end) {
config& res = config::add_child(key, val);
std::rotate( pos_it, ordered_children.end(), ordered_children.end());
return res;
}
auto pl = next->pos;
child_list& l = pl->second;
unsigned int index = next->index;
config& res = **(l.emplace(l.begin() + index, new config(val)));
for(auto ord = next; ord != end; ++ord) {
//this changes next->index and all later refernces to that tag.
if(ord->pos == pl) {
++ord->index;
}
}
//finally insert our new child in ordered_children.
ordered_children.insert(pos.i_, { pl, index });
return res;
}
namespace
{
struct remove_ordered

View file

@ -624,7 +624,7 @@ public:
typedef std::vector<child_pos>::const_iterator Itor;
typedef const_all_children_iterator this_type;
explicit const_all_children_iterator(const Itor &i): i_(i) {}
const_all_children_iterator(const all_children_iterator& i): i_(i.i_) {}
const_all_children_iterator(all_children_iterator& i): i_(i.i_) {}
const_all_children_iterator &operator++() { ++i_; return *this; }
const_all_children_iterator operator++(int) { return const_all_children_iterator(i_++); }
@ -659,8 +659,6 @@ public:
friend class config;
};
config& add_child_at(config_key_type key, const config &val, const const_all_children_iterator& pos);
typedef boost::iterator_range<all_children_iterator> all_children_itors;
typedef boost::iterator_range<const_all_children_iterator> const_all_children_itors;

View file

@ -298,7 +298,7 @@ struct modevents_entry
} // end anon namespace
void saved_game::load_mod(const std::string& type, const std::string& id, config::all_children_iterator pos)
void saved_game::load_mod(const std::string& type, const std::string& id)
{
if(const config& cfg = game_config_manager::get()->game_config().find_child(type, "id", id)) {
// Note the addon_id if this mod is required to play the game in mp.
@ -323,18 +323,18 @@ void saved_game::load_mod(const std::string& type, const std::string& id, config
if(modevent["enable_if"].empty()
|| variable_to_bool(carryover_.child_or_empty("variables"), modevent["enable_if"])
) {
this->starting_point_.add_child_at("event", modevent, pos++);
this->starting_point_.add_child("event", modevent);
}
}
// Copy lua
for(const config& modlua : cfg.child_range("lua")) {
this->starting_point_.add_child_at("lua", modlua, pos++);
this->starting_point_.add_child("lua", modlua);
}
// Copy load_resource
for(const config& load_resource : cfg.child_range("load_resource")) {
this->starting_point_.add_child_at("load_resource", load_resource, pos++);
this->starting_point_.add_child("load_resource", load_resource);
}
} else {
// TODO: A user message instead?
@ -367,24 +367,25 @@ void saved_game::expand_mp_events()
mods.emplace_back("campaign", classification_.campaign);
}
for(modevents_entry& mod : mods) {
load_mod(mod.type, mod.id, this->starting_point_.ordered_end());
}
mods.clear();
while(starting_point_.has_child("load_resource")) {
//todo: this looks like too much exposure of configs internals.
auto pos = std::find_if(starting_point_.ordered_begin(), starting_point_.ordered_end(), [&](const config::any_child& can){ return can.key == "load_resource"; });
assert(pos != starting_point_.ordered_end());
//assert(pos->index == 0);
std::string id = pos->cfg["id"];
starting_point_.remove_child("load_resource", 0);
//pos is still valid because starting_point_.ordered_begin() is a vector iterator.
if(loaded_resources.find(id) == loaded_resources.end()) {
loaded_resources.insert(id);
load_mod("resource", id, pos);
// In the first iteration mod contains no [resource]s in all other iterations, mods contains only [resource]s.
do {
for(modevents_entry& mod : mods) {
load_mod(mod.type, mod.id);
}
}
mods.clear();
for(const config& cfg : starting_point_.child_range("load_resource")) {
if(loaded_resources.find(cfg["id"].str()) == loaded_resources.end()) {
mods.emplace_back("resource", cfg["id"].str());
loaded_resources.insert(cfg["id"].str());
}
}
starting_point_.clear_children("load_resource");
} while(!mods.empty());
this->starting_point_["has_mod_events"] = true;
}
}

View file

@ -72,7 +72,7 @@ public:
/// should be called after expand_scenario() but before expand_carryover()
void expand_mp_events();
/// helper for expand_mp_events();
void load_mod(const std::string& type, const std::string& id, config::all_children_iterator pos);
void load_mod(const std::string& type, const std::string& id);
/// adds values of [option]s into [carryover_sides_start][variables] so that they are applied in the next level.
/// Note that since [variabels] are persistent we only use this once at the beginning
/// of a campaign but calling it multiple times is no harm eigher