cleanup cfg_ member of unit class.

the cfg_ field of the unit class now doesn’t store the advancements and
the abilities anymore. Instead they are stored in separate fields.

I used boost::ptr_vector<config> because that’s similar to how config
objects store their child values internally. Unfortunately
boost::ptr_vector defines ptr_vector::value_type in a bad way so that it
cannot be used for BOOST_FOREACH loops value types, so i had to
change lua's push_check file to use T::reference to be compatible
with ptr_vector

This commit removes all child tags of the unit cfg_ because the only
tags that are now left in cfg_ are [variation] and [advancefrom] from
[unit_type], which werent used in [unit] and only caused the savefiles
to bloat.

src/config.cpp

@@ -528,6 +528,14 @@ void config::append_children(const config &cfg)
 	}
 }
 
+void config::append_attributes(const config &cfg)
+{
+	check_valid(cfg);
+	BOOST_FOREACH(const attribute &v, cfg.values) {
+		values[v.first] = v.second;
+	}
+}
+
 void config::append_children(const config &cfg, const std::string& key)
 {
 	check_valid(cfg);

src/config.hpp

@@ -674,6 +674,11 @@ class config
 	 */
 	void append_children(const config &cfg, const std::string& key);
 
+	/**
+	 * Adds attributes from @a cfg.
+	 */
+	void append_attributes(const config &cfg);
+
 	/**
 	 * All children with the given key will be merged
 	 * into the first element with that key.

src/scripting/game_lua_kernel.cpp

@@ -301,7 +301,7 @@ static int impl_unit_get(lua_State *L)
 		return 1;
 	}
 	if (strcmp(m, "advancements") == 0) {
-		lua_push(L, boost::iterator_range<config::const_child_iterator>(u.modification_advancements()));
+		lua_push(L, u.modification_advancements());
 		return 1;
 	}
 	if (strcmp(m, "overlays") == 0) {

src/scripting/push_check.hpp

@@ -184,7 +184,7 @@ namespace lua_check_impl
 			for (int i = 1, i_end = lua_rawlen(L, n); i <= i_end; ++i)
 			{
 				lua_rawgeti(L, n, i);
-				res.push_back(lua_check_impl::lua_check<typename remove_constref<typename T::value_type>::type>(L, -1));
+				res.push_back(lua_check_impl::lua_check<typename remove_constref<typename T::reference>::type>(L, -1));
 			}
 			return res;
 		}
@@ -218,7 +218,7 @@ namespace lua_check_impl
 		assert(list.size() >= 0);
 		lua_createtable(L, list.size(), 0);
 		for(size_t i = 0, size = static_cast<size_t>(list.size()); i < size; ++i) {
-			lua_check_impl::lua_push<typename remove_constref<typename T::value_type>::type>(L, list[i]);
+			lua_check_impl::lua_push<typename remove_constref<typename T::reference>::type>(L, list[i]);
 			lua_rawseti(L, -2, i + 1);
 		}
 	}

src/unit.cpp

@@ -245,10 +245,23 @@ unit::unit(const unit& o)
 	, hp_bar_scaling_(o.hp_bar_scaling_)
 	, xp_bar_scaling_(o.xp_bar_scaling_)
 	, modifications_(o.modifications_)
+	, abilities_(o.abilities_)
+	, advancements_(o.advancements_)
 	, invisibility_cache_()
 {
 }
 
+struct ptr_vector_pushback
+{
+	ptr_vector_pushback(boost::ptr_vector<config>&  vec) : vec_(&vec) {}
+	void operator()(const config& cfg)
+	{
+		vec_->push_back(new config(cfg));
+	}
+	//Dont use reference to be copyable.
+	boost::ptr_vector<config>* vec_;
+};
+
 unit::unit(const config &cfg, bool use_traits, const vconfig* vcfg, n_unit::id_manager* id_manager)
 	: ref_count_(0)
 	, cfg_()
@@ -312,6 +325,8 @@ unit::unit(const config &cfg, bool use_traits, const vconfig* vcfg, n_unit::id_m
 	, hp_bar_scaling_(cfg["hp_bar_scaling"].blank() ? type_->hp_bar_scaling() : cfg["hp_bar_scaling"])
 	, xp_bar_scaling_(cfg["xp_bar_scaling"].blank() ? type_->xp_bar_scaling() : cfg["xp_bar_scaling"])
 	, modifications_()
+	, abilities_()
+	, advancements_()
 	, invisibility_cache_()
 {
 	side_ = cfg["side"];
@@ -435,25 +450,19 @@ unit::unit(const config &cfg, bool use_traits, const vconfig* vcfg, n_unit::id_m
 	//If cfg specifies [advancement]s, replace this [advancement]s with them.
 	if(cfg.has_child("advancement"))
 	{
-		cfg_.clear_children("advancement");
-		boost::copy( cfg.child_range("advancement")
-			, boost::make_function_output_iterator(
-				boost::bind(
-					  /* The static_cast is required to select the proper overload in C++11 mode. */
-					  static_cast<config&(config::*)(const std::string &, const config&)>(&config::add_child)
-					, boost::ref(cfg_) /*thisptr*/
-					, "advancement"
-					, _1 )) );
+		this->advancements_.clear();
+		boost::copy( cfg.child_range("advancement"), boost::make_function_output_iterator(ptr_vector_pushback(advancements_)));
 	}
 
 	//don't use the unit_type's abilities if this config has its own defined
+	//Why do we allow multiple [abilities] tags?
 	cfg_range = cfg.child_range("abilities");
 	if(cfg_range.first != cfg_range.second) {
-		cfg_.clear_children("abilities");
-		config &target = cfg_.add_child("abilities");
-		do {
-			target.append(*cfg_range.first);
-		} while(++cfg_range.first != cfg_range.second);
+		this->abilities_.clear();
+		BOOST_FOREACH(const config& abilities, cfg_range)
+		{
+			this->abilities_.append(abilities);
+		}
 	}
 
 	// Adjust the unit's defense, movement, vision, jamming, resistances, and
@@ -629,6 +638,8 @@ unit::unit(const unit_type &u_type, int side, bool real_unit,
 	, getsHit_(0)
 	, hidden_(false)
 	, modifications_()
+	, abilities_()
+	, advancements_()
 	, invisibility_cache_()
 {
 	cfg_["upkeep"]="full";
@@ -872,9 +883,14 @@ void unit::advance_to(const config &old_cfg, const unit_type &u_type,
 	}
 
 	// Inherit from the new unit type.
-	new_cfg.merge_with(new_type.get_cfg_for_units());
-
-	// If unit has specific profile, remember it and keep it after advancing
+	new_cfg.merge_attributes(new_type.get_cfg_for_units());
+
+	abilities_ = new_type.abilities_cfg();
+	advancements_.clear();
+	BOOST_FOREACH(const config& advancement, new_type.advancements()) {
+		advancements_.push_back(new config(advancement));
+	}
+ 	// If unit has specific profile, remember it and keep it after advancing
 	std::string profile = old_cfg["profile"].str();
 	if ( !profile.empty()  &&  profile != old_type.big_profile() ) {
 		new_cfg["profile"] = profile;
@@ -1310,27 +1326,22 @@ void unit::set_state(const std::string &state, bool value)
 
 bool unit::has_ability_by_id(const std::string& ability) const
 {
-	if (const config &abil = cfg_.child("abilities"))
-	{
-		BOOST_FOREACH(const config::any_child &ab, abil.all_children_range()) {
-			if (ab.cfg["id"] == ability)
-				return true;
+	BOOST_FOREACH(const config::any_child &ab, this->abilities_.all_children_range()) {
+		if (ab.cfg["id"] == ability) {
+			return true;
 		}
 	}
 	return false;
 }
 
 void unit::remove_ability_by_id(const std::string &ability)
 {
-	if (config &abil = cfg_.child("abilities"))
-	{
-		config::all_children_iterator i = abil.ordered_begin();
-		while (i != abil.ordered_end()) {
-			if (i->cfg["id"] == ability) {
-				i = abil.erase(i);
-			} else {
-				++i;
-			}
+	config::all_children_iterator i = this->abilities_.ordered_begin();
+	while (i != this->abilities_.ordered_end()) {
+		if (i->cfg["id"] == ability) {
+			i = this->abilities_.erase(i);
+		} else {
+			++i;
 		}
 	}
 }
@@ -1423,7 +1434,14 @@ void unit::write(config& cfg) const
 	}
 	cfg["cost"] = unit_value_;
 	cfg.clear_children("modifications");
-	cfg.add_child("modifications",modifications_);
+	cfg.add_child("modifications", modifications_);
+	cfg.clear_children("abilities");
+	cfg.add_child("abilities", abilities_);
+	cfg.clear_children("advancement");
+	BOOST_FOREACH(const config& advancement, this->advancements_)
+	{
+		cfg.add_child("advancement", advancement);
+	}
 
 }
 
@@ -1624,10 +1642,11 @@ std::vector<config> unit::get_modification_advances() const
 
 void unit::set_advancements(std::vector<config> advancements)
 {
-	cfg_.clear_children("advancement");
+	this->advancements_.clear();
 	BOOST_FOREACH(config& advancement, advancements)
 	{
-		cfg_.add_child("advancement").swap(advancement);
+		this->advancements_.push_back(new config());
+		this->advancements_.back().swap(advancement);
 	}
 }
 
@@ -1911,15 +1930,14 @@ void unit::add_modification(const std::string& mod_type, const config& mod, bool
 						emit_zoc_ = v->to_bool();
 					}
 				} else if (apply_to == "new_ability") {
-					config &ab = cfg_.child_or_add("abilities");
 					if (const config &ab_effect = effect.child("abilities")) {
 						config to_append;
 						BOOST_FOREACH(const config::any_child &ab, ab_effect.all_children_range()) {
 							if(!has_ability_by_id(ab.cfg["id"])) {
 								to_append.add_child(ab.key, ab.cfg);
 							}
 						}
-						ab.append(to_append);
+						this->abilities_.append(to_append);
 					}
 				} else if (apply_to == "remove_ability") {
 					if (const config &ab_effect = effect.child("abilities")) {

src/unit.hpp

@@ -19,6 +19,7 @@
 
 #include <boost/tuple/tuple.hpp>
 #include <boost/scoped_ptr.hpp>
+#include <boost/ptr_container/ptr_vector.hpp>
 
 #include "unit_types.hpp"
 #include "unit_ptr.hpp"
@@ -310,9 +311,11 @@ class unit
 	std::vector<std::pair<std::string,std::string> > amla_icons() const;
 
 	std::vector<config> get_modification_advances() const;
-	config::const_child_itors modification_advancements() const
-	{ return cfg_.child_range("advancement"); }
+
+	typedef boost::ptr_vector<config> t_advancements;
 	void set_advancements(std::vector<config> advancements);
+	const t_advancements& modification_advancements() const
+	{ return advancements_; }
 
 	size_t modification_count(const std::string& type, const std::string& id) const;
 
@@ -496,7 +499,8 @@ class unit
 	double hp_bar_scaling_, xp_bar_scaling_;
 
 	config modifications_;
-
+	config abilities_;
+	t_advancements advancements_;
 	/**
 	 * Hold the visibility status cache for a unit, when not uncovered.
 	 * This is mutable since it is a cache.

src/unit_abilities.cpp

@@ -132,12 +132,11 @@ bool unit::get_ability_bool(const std::string& tag_name, const map_location& loc
 {
 	assert(resources::teams);
 
-	if (const config &abilities = cfg_.child("abilities"))
-	{
-		BOOST_FOREACH(const config &i, abilities.child_range(tag_name)) {
-			if (ability_active(tag_name, i, loc) &&
-			    ability_affects_self(tag_name, i, loc))
-				return true;
+	BOOST_FOREACH(const config &i, this->abilities_.child_range(tag_name)) {
+		if (ability_active(tag_name, i, loc) &&
+			ability_affects_self(tag_name, i, loc))
+		{
+			return true;
 		}
 	}
 
@@ -155,14 +154,13 @@ bool unit::get_ability_bool(const std::string& tag_name, const map_location& loc
 		// ourself.
 		if ( &*it == this )
 			continue;
-		const config &adj_abilities = it->cfg_.child("abilities");
-		if (!adj_abilities)
-			continue;
-		BOOST_FOREACH(const config &j, adj_abilities.child_range(tag_name)) {
+		BOOST_FOREACH(const config &j, it->abilities_.child_range(tag_name)) {
 			if (affects_side(j, *resources::teams, side(), it->side()) &&
 			    it->ability_active(tag_name, j, adjacent[i]) &&
 			    ability_affects_adjacent(tag_name,  j, i, loc, *it))
+			{
 				return true;
+			}
 		}
 	}
 
@@ -175,12 +173,11 @@ unit_ability_list unit::get_abilities(const std::string& tag_name, const map_loc
 
 	unit_ability_list res;
 
-	if (const config &abilities = cfg_.child("abilities"))
-	{
-		BOOST_FOREACH(const config &i, abilities.child_range(tag_name)) {
-			if (ability_active(tag_name, i, loc) &&
-			    ability_affects_self(tag_name, i, loc))
-				res.push_back(unit_ability(&i, loc));
+	BOOST_FOREACH(const config &i, this->abilities_.child_range(tag_name)) {
+		if (ability_active(tag_name, i, loc) &&
+			ability_affects_self(tag_name, i, loc))
+		{
+			res.push_back(unit_ability(&i, loc));
 		}
 	}
 
@@ -198,14 +195,13 @@ unit_ability_list unit::get_abilities(const std::string& tag_name, const map_loc
 		// ourself.
 		if ( &*it == this )
 			continue;
-		const config &adj_abilities = it->cfg_.child("abilities");
-		if (!adj_abilities)
-			continue;
-		BOOST_FOREACH(const config &j, adj_abilities.child_range(tag_name)) {
+		BOOST_FOREACH(const config &j, it->abilities_.child_range(tag_name)) {
 			if (affects_side(j, *resources::teams, side(), it->side()) &&
 			    it->ability_active(tag_name, j, adjacent[i]) &&
 			    ability_affects_adjacent(tag_name, j, i, loc, *it))
+			{
 				res.push_back(unit_ability(&j, adjacent[i]));
+			}
 		}
 	}
 
@@ -217,9 +213,7 @@ std::vector<std::string> unit::get_ability_list() const
 {
 	std::vector<std::string> res;
 
-	const config &abilities = cfg_.child("abilities");
-	if (!abilities) return res;
-	BOOST_FOREACH(const config::any_child &ab, abilities.all_children_range()) {
+	BOOST_FOREACH(const config::any_child &ab, this->abilities_.all_children_range()) {
 		std::string const &id = ab.cfg["id"];
 		if (!id.empty())
 			res.push_back(id);
@@ -272,10 +266,7 @@ std::vector<boost::tuple<t_string,t_string,t_string> > unit::ability_tooltips(st
 	if ( active_list )
 		active_list->clear();
 
-	const config &abilities = cfg_.child("abilities");
-	if (!abilities) return res;
-
-	BOOST_FOREACH(const config::any_child &ab, abilities.all_children_range())
+	BOOST_FOREACH(const config::any_child &ab, this->abilities_.all_children_range())
 	{
 		if ( !active_list || ability_active(ab.key, ab.cfg, loc_) )
 		{
@@ -429,11 +420,8 @@ bool unit::ability_affects_self(const std::string& ability,const config& cfg,con
 
 bool unit::has_ability_type(const std::string& ability) const
 {
-	if (const config &list = cfg_.child("abilities")) {
-		config::const_child_itors itors = list.child_range(ability);
-		return itors.first != itors.second;
-	}
-	return false;
+	config::const_child_itors itors = this->abilities_.child_range(ability);
+	return itors.first != itors.second;
 }