Improvements to the config interfaces, mainly iterators

- config::attribute_count() now excludes blank (ie, unset) attributes
  Blank attributes in the map are generally a result of the mutating
  behaviour of operator[], so they're not "logically" existing attributes.
- Non-const versions of attribute iterator and all-children iterator
- Attribute iterators are now bi-directional
- vconfig all-children iterator is now a bi-directional Boost iterator range
- vconfig now has a bidirectional attribute iterator

src/config.cpp

@@ -648,6 +648,13 @@ unsigned config::all_children_count() const
 	return ordered_children.size();
 }
 
+unsigned config::attribute_count() const
+{
+	return std::count_if(values.begin(), values.end(), [](const attribute& v) {
+		return !v.second.blank();
+	});
+}
+
 bool config::has_child(const std::string &key) const
 {
 	check_valid();
@@ -987,8 +994,26 @@ config::const_attr_itors config::attribute_range() const
 {
 	check_valid();
 
-	return const_attr_itors(const_attribute_iterator(values.begin()),
+	const_attr_itors range (const_attribute_iterator(values.begin()),
 	                        const_attribute_iterator(values.end()));
+
+	// Ensure the first element is not blank, as a few places assume this
+	while(range.begin() != range.end() && range.begin()->second.blank()) {
+		range.pop_front();
+	}
+	return range;
+}
+
+config::attr_itors config::attribute_range()
+{
+	check_valid();
+	attr_itors range(attribute_iterator(values.begin()), attribute_iterator(values.end()));
+
+	// Ensure the first element is not blank, as a few places assume this
+	while(range.begin() != range.end() && range.begin()->second.blank()) {
+		range.pop_front();
+	}
+	return range;
 }
 
 namespace {
@@ -1116,17 +1141,49 @@ config::all_children_iterator::reference config::all_children_iterator::operator
 	return any_child(&i_->pos->first, i_->pos->second[i_->index]);
 }
 
-config::all_children_iterator config::ordered_begin() const
+config::const_all_children_iterator::reference config::const_all_children_iterator::operator*() const
+{
+	return any_child(&i_->pos->first, i_->pos->second[i_->index]);
+}
+
+config::const_all_children_iterator config::ordered_begin() const
+{
+	return const_all_children_iterator(ordered_children.cbegin());
+}
+
+config::const_all_children_iterator config::ordered_cbegin() const
+{
+	return const_all_children_iterator(ordered_children.cbegin());
+}
+
+config::const_all_children_iterator config::ordered_end() const
+{
+	return const_all_children_iterator(ordered_children.cend());
+}
+
+config::const_all_children_iterator config::ordered_cend() const
+{
+	return const_all_children_iterator(ordered_children.cend());
+}
+
+config::const_all_children_itors config::all_children_range() const
+{
+	return const_all_children_itors(
+		const_all_children_iterator(ordered_children.cbegin()),
+		const_all_children_iterator(ordered_children.cend()));
+}
+
+config::all_children_iterator config::ordered_begin()
 {
 	return all_children_iterator(ordered_children.begin());
 }
 
-config::all_children_iterator config::ordered_end() const
+config::all_children_iterator config::ordered_end()
 {
 	return all_children_iterator(ordered_children.end());
 }
 
-config::all_children_itors config::all_children_range() const
+config::all_children_itors config::all_children_range()
 {
 	return all_children_itors(
 		all_children_iterator(ordered_children.begin()),
@@ -1535,7 +1592,7 @@ bool operator==(const config& a, const config& b)
 	if (a.values != b.values)
 		return false;
 
-	config::all_children_itors x = a.all_children_range(), y = b.all_children_range();
+	config::const_all_children_itors x = a.all_children_range(), y = b.all_children_range();
 	for (; !x.empty() && !y.empty(); x.pop_front(), y.pop_front()) {
 		if (x.front().key != y.front().key || x.front().cfg != y.front().cfg) {
 			return false;

src/config.hpp

@@ -142,11 +142,13 @@ public:
 		child_iterator &operator--() { --i_; return *this; }
 		child_iterator operator--(int) { return child_iterator(i_--); }
 
-		config &operator*() const { return **i_; }
+		reference operator*() const { return **i_; }
-		config *operator->() const { return &**i_; }
+		pointer operator->() const { return &**i_; }
 
 		bool operator==(const child_iterator &i) const { return i_ == i.i_; }
 		bool operator!=(const child_iterator &i) const { return i_ != i.i_; }
+		bool operator==(const const_child_iterator &i) const { return i == *this; }
+		bool operator!=(const const_child_iterator &i) const { return i == *this; }
 
 		friend bool operator<(const this_type& a, const this_type& b) { return a.i_ < b.i_; }
 		friend bool operator<=(const this_type& a, const this_type& b) { return a.i_ <= b.i_; }
@@ -168,7 +170,7 @@ public:
 
 	struct const_child_iterator
 	{
-		typedef config value_type;
+		typedef const config value_type;
 		typedef std::random_access_iterator_tag iterator_category;
 		typedef int difference_type;
 		typedef const config *pointer;
@@ -183,11 +185,13 @@ public:
 		const_child_iterator &operator--() { --i_; return *this; }
 		const_child_iterator operator--(int) { return const_child_iterator(i_--); }
 
-		const config &operator*() const { return **i_; }
+		reference operator*() const { return **i_; }
-		const config *operator->() const { return &**i_; }
+		pointer operator->() const { return &**i_; }
 
 		bool operator==(const const_child_iterator &i) const { return i_ == i.i_; }
 		bool operator!=(const const_child_iterator &i) const { return i_ != i.i_; }
+		bool operator==(const child_iterator &i) const { return i_ == i.i_; }
+		bool operator!=(const child_iterator &i) const { return i_ != i.i_; }
 
 		friend bool operator<(const this_type& a, const this_type& b) { return a.i_ < b.i_; }
 		friend bool operator<=(const this_type& a, const this_type& b) { return a.i_ <= b.i_; }
@@ -388,39 +392,74 @@ public:
 #endif
 	> attribute_map;
 	typedef attribute_map::value_type attribute;
+	struct const_attribute_iterator;
+
+	struct attribute_iterator
+	{
+		typedef attribute value_type;
+		typedef std::bidirectional_iterator_tag iterator_category;
+		typedef int difference_type;
+		typedef attribute *pointer;
+		typedef attribute &reference;
+		typedef attribute_map::iterator Itor;
+		explicit attribute_iterator(const Itor &i): i_(i) {}
+
+		attribute_iterator &operator++() { ++i_; return *this; }
+		attribute_iterator operator++(int) { return attribute_iterator(i_++); }
+		attribute_iterator &operator--() { --i_; return *this; }
+		attribute_iterator operator--(int) { return attribute_iterator(i_--); }
+
+		reference operator*() const { return *i_; }
+		pointer operator->() const { return &*i_; }
+
+		bool operator==(const attribute_iterator &i) const { return i_ == i.i_; }
+		bool operator!=(const attribute_iterator &i) const { return i_ != i.i_; }
+		bool operator==(const const_attribute_iterator &i) const { return i == *this; }
+		bool operator!=(const const_attribute_iterator &i) const { return i == *this; }
+
+	private:
+		friend struct config::const_attribute_iterator;
+		Itor i_;
+	};
 
 	struct const_attribute_iterator
 	{
-		typedef attribute value_type;
+		typedef const attribute value_type;
-		typedef std::forward_iterator_tag iterator_category;
+		typedef std::bidirectional_iterator_tag iterator_category;
 		typedef int difference_type;
 		typedef const attribute *pointer;
 		typedef const attribute &reference;
 		typedef attribute_map::const_iterator Itor;
 		explicit const_attribute_iterator(const Itor &i): i_(i) {}
+		const_attribute_iterator(attribute_iterator& i): i_(i.i_) {}
 
 		const_attribute_iterator &operator++() { ++i_; return *this; }
 		const_attribute_iterator operator++(int) { return const_attribute_iterator(i_++); }
 
-		const attribute &operator*() const { return *i_; }
+		const_attribute_iterator &operator--() { --i_; return *this; }
-		const attribute *operator->() const { return &*i_; }
+		const_attribute_iterator operator--(int) { return const_attribute_iterator(i_--); }
+
+		reference operator*() const { return *i_; }
+		pointer operator->() const { return &*i_; }
 
 		bool operator==(const const_attribute_iterator &i) const { return i_ == i.i_; }
 		bool operator!=(const const_attribute_iterator &i) const { return i_ != i.i_; }
+		bool operator==(const attribute_iterator &i) const { return i_ == i.i_; }
+		bool operator!=(const attribute_iterator &i) const { return i_ != i.i_; }
 
 	private:
 		Itor i_;
 	};
 
 	typedef boost::iterator_range<const_attribute_iterator> const_attr_itors;
+	typedef boost::iterator_range<attribute_iterator> attr_itors;
 
 	child_itors child_range(const std::string& key);
 	const_child_itors child_range(const std::string& key) const;
 	unsigned child_count(const std::string &key) const;
 	unsigned all_children_count() const;
-	/** Note: this function also counts the 'blank' attributes, so it might return more than one might expect */
+	/** Count the number of non-blank attributes */
-	unsigned attribute_count() const
+	unsigned attribute_count() const;
-	{ return values.size(); }
 
 	/**
 	 * Determine whether a config has a child or not.
@@ -572,6 +611,7 @@ public:
 	}
 
 	const_attr_itors attribute_range() const;
+	attr_itors attribute_range();
 
 	/**
 	 * Returns the first child of tag @a key with a @a name attribute
@@ -623,10 +663,12 @@ public:
 	struct any_child
 	{
 		const child_map::key_type &key;
-		const config &cfg;
+		config &cfg;
-		any_child(const child_map::key_type *k, const config *c): key(*k), cfg(*c) {}
+		any_child(const child_map::key_type *k, config *c): key(*k), cfg(*c) {}
 	};
 
+	struct const_all_children_iterator;
+
 	struct all_children_iterator
 	{
 		struct arrow_helper
@@ -639,9 +681,9 @@ public:
 		typedef any_child value_type;
 		typedef std::random_access_iterator_tag iterator_category;
 		typedef int difference_type;
-		typedef const arrow_helper pointer;
+		typedef arrow_helper pointer;
-		typedef const any_child reference;
+		typedef any_child reference;
-		typedef std::vector<child_pos>::const_iterator Itor;
+		typedef std::vector<child_pos>::iterator Itor;
 		typedef all_children_iterator this_type;
 		explicit all_children_iterator(const Itor &i): i_(i) {}
 
@@ -653,6 +695,61 @@ public:
 		reference operator*() const;
 		pointer operator->() const { return *this; }
 
+		bool operator==(const all_children_iterator &i) const { return i_ == i.i_; }
+		bool operator!=(const all_children_iterator &i) const { return i_ != i.i_; }
+		bool operator==(const const_all_children_iterator &i) const { return i_ == i.i_; }
+		bool operator!=(const const_all_children_iterator &i) const { return i_ != i.i_; }
+
+		friend bool operator<(const this_type& a, const this_type& b) { return a.i_ < b.i_; }
+		friend bool operator<=(const this_type& a, const this_type& b) { return a.i_ <= b.i_; }
+		friend bool operator>=(const this_type& a, const this_type& b) { return a.i_ >= b.i_; }
+		friend bool operator>(const this_type& a, const this_type& b) { return a.i_ > b.i_; }
+
+		this_type& operator+=(difference_type n) { i_ += n; return *this; }
+		this_type& operator-=(difference_type n) { i_ -= n; return *this; }
+
+		reference operator[](difference_type n) const { return any_child(&i_[n].pos->first, i_[n].pos->second[i_->index]); }
+		friend difference_type operator-(const this_type& a, const this_type& b) { return a.i_ - b.i_; }
+		friend this_type operator-(const this_type& a, difference_type n) { return this_type(a.i_ - n); }
+		friend this_type operator+(const this_type& a, difference_type n) { return this_type(a.i_ + n); }
+		friend this_type operator+(difference_type n, const this_type& a) { return this_type(a.i_ + n); }
+
+	private:
+		Itor i_;
+
+		friend class config;
+		friend struct const_all_children_iterator;
+	};
+
+	struct const_all_children_iterator
+	{
+		struct arrow_helper
+		{
+			const any_child data;
+			arrow_helper(const const_all_children_iterator &i): data(*i) {}
+			const any_child *operator->() const { return &data; }
+		};
+
+		typedef const any_child value_type;
+		typedef std::random_access_iterator_tag iterator_category;
+		typedef int difference_type;
+		typedef const arrow_helper pointer;
+		typedef const any_child reference;
+		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(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_++); }
+		this_type &operator--() { --i_; return *this; }
+		this_type operator--(int) { return this_type(i_--); }
+
+		reference operator*() const;
+		pointer operator->() const { return *this; }
+
+		bool operator==(const const_all_children_iterator &i) const { return i_ == i.i_; }
+		bool operator!=(const const_all_children_iterator &i) const { return i_ != i.i_; }
 		bool operator==(const all_children_iterator &i) const { return i_ == i.i_; }
 		bool operator!=(const all_children_iterator &i) const { return i_ != i.i_; }
 
@@ -677,12 +774,18 @@ public:
 	};
 
 	typedef boost::iterator_range<all_children_iterator> all_children_itors;
+	typedef boost::iterator_range<const_all_children_iterator> const_all_children_itors;
 
 	/** In-order iteration over all children. */
-	all_children_itors all_children_range() const;
+	const_all_children_itors all_children_range() const;
+	all_children_itors all_children_range();
 
-	all_children_iterator ordered_begin() const;
+	const_all_children_iterator ordered_cbegin() const;
-	all_children_iterator ordered_end() const;
+	const_all_children_iterator ordered_cend() const;
+	const_all_children_iterator ordered_begin() const;
+	const_all_children_iterator ordered_end() const;
+	all_children_iterator ordered_begin();
+	all_children_iterator ordered_end();
 	all_children_iterator erase(const all_children_iterator& i);
 
 	/**

src/game_initialization/mp_options.cpp

@@ -243,9 +243,9 @@ const config& manager::get_value_cfg_or_empty(const std::string& id) const
 
 config::any_child manager::get_option_parent(const std::string& id) const
 {
-	static const config empty;
+	static config empty;
 	static const std::string empty_key = "";
-	static config::any_child not_found(&empty_key, &empty);
+	static const config::any_child not_found(&empty_key, &empty);
 
 	for (const config::any_child& i : options_info_.all_children_range()) {
 		for (const config::any_child& j : i.cfg.all_children_range()) {

src/gui/core/window_builder.cpp

@@ -154,7 +154,7 @@ register_builder_widget(const std::string& id,
 
 tbuilder_widget_ptr create_builder_widget(const config& cfg)
 {
-	config::all_children_itors children = cfg.all_children_range();
+	config::const_all_children_itors children = cfg.all_children_range();
 	size_t nb_children = std::distance(children.begin(), children.end());
 	VALIDATE(nb_children == 1, "Grid cell does not have exactly 1 child.");
 

src/replay.cpp

@@ -696,7 +696,7 @@ REPLAY_RETURN do_replay_handle(bool one_move)
 		}
 
 
-		const config::all_children_itors ch_itors = cfg->all_children_range();
+		const config::const_all_children_itors ch_itors = cfg->all_children_range();
 		//if there is an empty command tag or a start tag
 		if (ch_itors.empty() || cfg->has_child("start"))
 		{

src/units/animation.cpp

@@ -74,11 +74,11 @@ struct animation_branch
 	}
 
 	config attributes;
-	std::vector<config::all_children_iterator> children;
+	std::vector<config::const_all_children_iterator> children;
 	config merge() const
 	{
 		config result = attributes;
-		for (const config::all_children_iterator &i : children) {
+		for (const config::const_all_children_iterator &i : children) {
 			result.add_child(i->key, i->cfg);
 		}
 		return result;
@@ -89,7 +89,7 @@ typedef std::list<animation_branch> animation_branches;
 
 struct animation_cursor
 {
-	config::all_children_itors itors;
+	config::const_all_children_itors itors;
 	animation_branches branches;
 	animation_cursor *parent;
 	animation_cursor(const config &cfg):
@@ -638,7 +638,7 @@ void unit_animation::add_anims( std::vector<unit_animation> & animations, const
 		anim["apply_to"] = "standing";
 		anim["cycles"] = "true";
 		// add cycles to all frames within a standing animation block
-		for (config::all_children_iterator ci : ab.children)
+		for (config::const_all_children_iterator ci : ab.children)
 		{
 			std::string sub_frame_name = ci->key;
 			size_t pos = sub_frame_name.find("_frame");
@@ -656,7 +656,7 @@ void unit_animation::add_anims( std::vector<unit_animation> & animations, const
 		config anim = ab.merge();
 		anim["apply_to"] = "default";
 		anim["cycles"] = "true";
-		for (config::all_children_iterator ci : ab.children)
+		for (config::const_all_children_iterator ci : ab.children)
 		{
 			std::string sub_frame_name = ci->key;
 			size_t pos = sub_frame_name.find("_frame");

src/variable.cpp

@@ -309,6 +309,25 @@ config::attribute_value vconfig::expand(const std::string &key) const
 	return val;
 }
 
+vconfig::attribute_iterator::reference vconfig::attribute_iterator::operator*() const
+{
+	config::attribute val = *i_;
+	if(resources::gamedata) {
+		val.second.apply_visitor(vconfig_expand_visitor(val.second));
+	}
+	return val;
+}
+
+vconfig::attribute_iterator::pointer vconfig::attribute_iterator::operator->() const
+{
+	config::attribute val = *i_;
+	if(resources::gamedata) {
+		val.second.apply_visitor(vconfig_expand_visitor(val.second));
+	}
+	pointer_proxy p = {val};
+	return p;
+}
+
 vconfig::all_children_iterator::all_children_iterator(const Itor &i) :
 	i_(i), inner_index_(0), cache_()
 {
@@ -351,6 +370,25 @@ vconfig::all_children_iterator vconfig::all_children_iterator::operator++(int)
 	return i;
 }
 
+vconfig::all_children_iterator& vconfig::all_children_iterator::operator--()
+{
+	if(inner_index_ >= 0 && i_->key == "insert_tag") {
+		if(--inner_index_ >= 0) {
+			return *this;
+		}
+		inner_index_ = 0;
+	}
+	--i_;
+	return *this;
+}
+
+vconfig::all_children_iterator vconfig::all_children_iterator::operator--(int)
+{
+	vconfig::all_children_iterator i = *this;
+	this->operator--();
+	return i;
+}
+
 vconfig::all_children_iterator::reference vconfig::all_children_iterator::operator*() const
 {
 	return value_type(get_key(), get_child());

src/variable.hpp

@@ -90,21 +90,56 @@ public:
 	bool has_attribute(const std::string& key) const { return cfg_->has_attribute(key); }
 	bool empty() const { return (null() || cfg_->empty()); }
 
+	struct attribute_iterator
+	{
+		struct pointer_proxy;
+
+		typedef const config::attribute value_type;
+		typedef std::bidirectional_iterator_tag iterator_category;
+		typedef int difference_type;
+		typedef const pointer_proxy pointer;
+		typedef const config::attribute reference;
+		typedef config::const_attribute_iterator Itor;
+		explicit attribute_iterator(const Itor &i): i_(i) {}
+
+		attribute_iterator &operator++() { ++i_; return *this; }
+		attribute_iterator operator++(int) { return attribute_iterator(i_++); }
+
+		attribute_iterator &operator--() { --i_; return *this; }
+		attribute_iterator operator--(int) { return attribute_iterator(i_--); }
+
+		reference operator*() const;
+		pointer operator->() const;
+
+		bool operator==(const attribute_iterator &i) const { return i_ == i.i_; }
+		bool operator!=(const attribute_iterator &i) const { return i_ != i.i_; }
+
+	private:
+		Itor i_;
+	};
+
+	boost::iterator_range<attribute_iterator> attribute_range() {
+		config::const_attr_itors range = cfg_->attribute_range();
+		return boost::make_iterator_range(attribute_iterator(range.begin()), attribute_iterator(range.end()));
+	}
+
 	struct all_children_iterator
 	{
 		struct pointer_proxy;
 
 		typedef std::pair<std::string, vconfig> value_type;
-		typedef std::forward_iterator_tag iterator_category;
+		typedef std::bidirectional_iterator_tag iterator_category;
 		typedef int difference_type;
 		typedef const pointer_proxy pointer;
 		typedef const value_type reference;
-		typedef config::all_children_iterator Itor;
+		typedef config::const_all_children_iterator Itor;
 		explicit all_children_iterator(const Itor &i);
 		all_children_iterator(const Itor &i, const std::shared_ptr<const config> & cache);
 
 		all_children_iterator& operator++();
 		all_children_iterator  operator++(int);
+		all_children_iterator& operator--();
+		all_children_iterator  operator--(int);
 
 		reference operator*() const;
 		pointer operator->() const;
@@ -141,8 +176,8 @@ public:
 	/** In-order iteration over all children. */
 	all_children_iterator ordered_begin() const;
 	all_children_iterator ordered_end() const;
-	std::pair<all_children_iterator,all_children_iterator> all_ordered() const {
+	boost::iterator_range<all_children_iterator> all_ordered() const {
-		return std::make_pair(ordered_begin(), ordered_end());
+		return boost::make_iterator_range(ordered_begin(), ordered_end());
 	}
 
 private:
@@ -157,6 +192,12 @@ private:
 	static const config default_empty_config;
 };
 
+struct vconfig::attribute_iterator::pointer_proxy
+{
+	value_type p;
+	const value_type *operator->() const { return &p; }
+};
+
 struct vconfig::all_children_iterator::pointer_proxy
 {
 	value_type p;