Merge pull request #24 from OpenVicProject/cleanup

Various fixes, refactors and general cleanup

12 files changed, 111 insertions, 89 deletions

diff --git a/src/openvic-simulation/dataloader/NodeTools.cpp b/src/openvic-simulation/dataloader/NodeTools.cpp
index 7dd6e16..2367f86 100644
--- a/src/openvic-simulation/dataloader/NodeTools.cpp
+++ b/src/openvic-simulation/dataloader/NodeTools.cpp
@@ -220,6 +220,20 @@ node_callback_t NodeTools::expect_list(node_callback_t callback) {
 	return expect_list_and_length(default_length_callback, callback);
 }
 
+node_callback_t NodeTools::expect_length(callback_t<size_t> callback) {
+	return [callback](ast::NodeCPtr node) -> bool {
+		bool ret = true;
+		ret &= expect_list_and_length(
+			[callback, &ret](size_t size) -> size_t {
+				ret &= callback(size);
+				return 0;
+			},
+			success_callback
+		)(node);
+		return ret;
+	};
+}
+
 node_callback_t NodeTools::expect_dictionary_and_length(length_callback_t length_callback, key_value_callback_t callback) {
 	return expect_list_and_length(length_callback, expect_assign(callback));
 }
diff --git a/src/openvic-simulation/dataloader/NodeTools.hpp b/src/openvic-simulation/dataloader/NodeTools.hpp
index 51bbfa9..5c22e11 100644
--- a/src/openvic-simulation/dataloader/NodeTools.hpp
+++ b/src/openvic-simulation/dataloader/NodeTools.hpp
@@ -41,6 +41,7 @@ namespace OpenVic {
 		node_callback_t expect_list_and_length(length_callback_t length_callback, node_callback_t callback);
 		node_callback_t expect_list_of_length(size_t length, node_callback_t callback);
 		node_callback_t expect_list(node_callback_t callback);
+		node_callback_t expect_length(callback_t<size_t> callback);
 
 		node_callback_t expect_dictionary_and_length(length_callback_t length_callback, key_value_callback_t callback);
 		node_callback_t expect_dictionary(key_value_callback_t callback);
@@ -136,6 +137,28 @@ namespace OpenVic {
 			};
 		}
 
+		template<typename T>
+		requires requires(T& t) {
+			t += T {};
+		}
+		callback_t<T> add_variable_callback(T& var) {
+			return [&var](T val) -> bool {
+				var += val;
+				return true;
+			};
+		}
+
+		template<typename T>
+		requires requires(T& t) {
+			t--;
+		}
+		node_callback_t decrement_callback(T& var, node_callback_t callback) {
+			return [&var, callback](ast::NodeCPtr node) -> bool {
+				var--;
+				return callback(node);
+			};
+		}
+
 		template<typename I, typename T>
 		requires(std::integral<I>, std::integral<T>)
 		callback_t<I> _assign_variable_callback_int(const std::string_view name, T& var) {
diff --git a/src/openvic-simulation/economy/Good.cpp b/src/openvic-simulation/economy/Good.cpp
index 8675369..f1da8bd 100644
--- a/src/openvic-simulation/economy/Good.cpp
+++ b/src/openvic-simulation/economy/Good.cpp
@@ -97,13 +97,7 @@ bool GoodManager::load_goods_file(ast::NodeCPtr root) {
 	bool ret = expect_dictionary_reserve_length(
 		good_categories,
 		[this, &total_expected_goods](std::string_view key, ast::NodeCPtr value) -> bool {
-			bool ret = expect_list_and_length(
-				[&total_expected_goods](size_t size) -> size_t {
-					total_expected_goods += size;
-					return 0;
-				},
-				success_callback
-			)(value);
+			bool ret = expect_length(add_variable_callback(total_expected_goods))(value);
 			ret &= add_good_category(key);
 			return ret;
 		}
diff --git a/src/openvic-simulation/map/Map.cpp b/src/openvic-simulation/map/Map.cpp
index 3b06c66..5082906 100644
--- a/src/openvic-simulation/map/Map.cpp
+++ b/src/openvic-simulation/map/Map.cpp
@@ -316,6 +316,10 @@ bool Map::add_mapmode(const std::string_view identifier, Mapmode::colour_func_t
 	return mapmodes.add_item({ identifier, mapmodes.size(), colour_func });
 }
 
+Mapmode const* Map::get_mapmode_by_index(size_t index) const {
+	return mapmodes.get_item_by_index(index);
+}
+
 bool Map::generate_mapmode_colours(Mapmode::index_t index, uint8_t* target) const {
 	if (target == nullptr) {
 		Logger::error("Mapmode colour target pointer is null!");
@@ -500,34 +504,28 @@ bool Map::load_region_file(ast::NodeCPtr root) {
 	return ret;
 }
 
+/* REQUIREMENTS:
+ * MAP-19, MAP-84
+ */
 bool Map::_generate_province_adjacencies() {
 	bool changed = false;
 
-	auto generate_adjacency = [&] (shape_pixel_t cur_pixel, size_t x, size_t y) -> bool {
-		size_t idx = x + y * width;
-		shape_pixel_t neighbour_pixel = province_shape_image[idx];
-		if (cur_pixel.index != neighbour_pixel.index) {
-			Province* cur = get_province_by_index(cur_pixel.index);
-			Province* neighbour = get_province_by_index(neighbour_pixel.index);
-			if(cur != nullptr && neighbour != nullptr) {
-				cur->add_adjacency(neighbour, 0, 0);
-				neighbour->add_adjacency(cur, 0, 0);
-				return true;
-			} else Logger::error(
-				"Couldn't find province(s): current = ", cur, " (", cur_pixel.index,
-				"), neighbour = ", neighbour, " (", neighbour_pixel.index, ")"
-			);
+	auto generate_adjacency = [&](Province* cur, size_t x, size_t y) -> bool {
+		Province* neighbour = get_province_by_index(province_shape_image[x + y * width].index);
+		if (neighbour != nullptr && cur != neighbour) {
+			return cur->add_adjacency(neighbour, 0, 0) | neighbour->add_adjacency(cur, 0, 0);
 		}
 		return false;
 	};
 	
 	for (size_t y = 0; y < height; ++y) {
 		for (size_t x = 0; x < width; ++x) {
-			shape_pixel_t cur = province_shape_image[x + y * width];
-			if(x + 1 < width)
-				changed |= generate_adjacency(cur, x + 1, y);
-			if(y + 1 < height)
-				changed |= generate_adjacency(cur, x, y + 1);
+			Province* cur = get_province_by_index(province_shape_image[x + y * width].index);
+			if (cur != nullptr) {
+				changed |= generate_adjacency(cur, (x + 1) % width, y);
+				if (y + 1 < height)
+					changed |= generate_adjacency(cur, x, y + 1);
+			}
 		}
 	}
 
diff --git a/src/openvic-simulation/map/Map.hpp b/src/openvic-simulation/map/Map.hpp
index 163a21f..f81b9c1 100644
--- a/src/openvic-simulation/map/Map.hpp
+++ b/src/openvic-simulation/map/Map.hpp
@@ -39,6 +39,7 @@ namespace OpenVic {
 		using terrain_variant_map_t = std::map<colour_t, terrain_t>;
 
 #pragma pack(push, 1)
+		/* Used to represent tightly packed 3-byte integer pixel information. */
 		struct shape_pixel_t {
 			Province::index_t index;
 			terrain_t terrain;
@@ -93,6 +94,7 @@ namespace OpenVic {
 
 		bool add_mapmode(const std::string_view identifier, Mapmode::colour_func_t colour_func);
 		IDENTIFIER_REGISTRY_ACCESSORS(Mapmode, mapmode)
+		Mapmode const* get_mapmode_by_index(size_t index) const;
 		static constexpr size_t MAPMODE_COLOUR_SIZE = 4;
 		bool generate_mapmode_colours(Mapmode::index_t index, uint8_t* target) const;
 
diff --git a/src/openvic-simulation/map/Province.cpp b/src/openvic-simulation/map/Province.cpp
index db7e784..6ec17b1 100644
--- a/src/openvic-simulation/map/Province.cpp
+++ b/src/openvic-simulation/map/Province.cpp
@@ -174,4 +174,4 @@ bool Province::add_adjacency(Province const* province, distance_t distance, flag
 
 std::vector<Province::adjacency_t> const& Province::get_adjacencies() const {
 	return adjacencies;
-}
\ No newline at end of file
+}
diff --git a/src/openvic-simulation/politics/Ideology.cpp b/src/openvic-simulation/politics/Ideology.cpp
index 43adfaa..7c15c3c 100644
--- a/src/openvic-simulation/politics/Ideology.cpp
+++ b/src/openvic-simulation/politics/Ideology.cpp
@@ -5,9 +5,9 @@ using namespace OpenVic::NodeTools;
 
 IdeologyGroup::IdeologyGroup(const std::string_view new_identifier) : HasIdentifier { new_identifier } {}
 
-Ideology::Ideology(const std::string_view new_identifier, colour_t new_colour, IdeologyGroup const& new_group, bool uncivilised, Date spawn_date)
-	: HasIdentifierAndColour { new_identifier, new_colour, true, false }, group { new_group }, uncivilised { uncivilised },
-	spawn_date { spawn_date } {}
+Ideology::Ideology(const std::string_view new_identifier, colour_t new_colour, IdeologyGroup const& new_group, bool new_uncivilised, bool new_can_reduce_militancy, Date new_spawn_date)
+	: HasIdentifierAndColour { new_identifier, new_colour, true, false }, group { new_group }, uncivilised { new_uncivilised },
+	can_reduce_militancy { new_can_reduce_militancy }, spawn_date { new_spawn_date } {}
 
 IdeologyGroup const& Ideology::get_group() const {
 	return group;
@@ -17,6 +17,10 @@ bool Ideology::is_uncivilised() const {
 	return uncivilised;
 }
 
+bool Ideology::get_can_reduce_militancy() const {
+	return can_reduce_militancy;
+}
+
 Date const& Ideology::get_spawn_date() const {
 	return spawn_date;
 }
@@ -32,7 +36,7 @@ bool IdeologyManager::add_ideology_group(const std::string_view identifier) {
 	return ideology_groups.add_item({ identifier });
 }
 
-bool IdeologyManager::add_ideology(const std::string_view identifier, colour_t colour, IdeologyGroup const* group, bool uncivilised, Date spawn_date) {
+bool IdeologyManager::add_ideology(const std::string_view identifier, colour_t colour, IdeologyGroup const* group, bool uncivilised, bool can_reduce_militancy, Date spawn_date) {
 	if (identifier.empty()) {
 		Logger::error("Invalid ideology identifier - empty!");
 		return false;
@@ -48,7 +52,7 @@ bool IdeologyManager::add_ideology(const std::string_view identifier, colour_t c
 		return false;
 	}
 
-	return ideologies.add_item({ identifier, colour, *group, uncivilised, spawn_date });
+	return ideologies.add_item({ identifier, colour, *group, uncivilised, can_reduce_militancy, spawn_date });
 }
 
 /* REQUIREMENTS:
@@ -59,13 +63,7 @@ bool IdeologyManager::load_ideology_file(ast::NodeCPtr root) {
 	bool ret = expect_dictionary_reserve_length(
 		ideology_groups,
 		[this, &expected_ideologies](std::string_view key, ast::NodeCPtr value) -> bool {
-			bool ret = expect_list_and_length(
-				[&expected_ideologies](size_t size) -> size_t {
-					expected_ideologies += size;
-					return 0;
-				},
-				success_callback
-			)(value);
+			bool ret = expect_length(add_variable_callback(expected_ideologies))(value);
 			ret &= add_ideology_group(key);
 			return ret;
 		}
@@ -80,15 +78,22 @@ bool IdeologyManager::load_ideology_file(ast::NodeCPtr root) {
 			return expect_dictionary(
 				[this, ideology_group](std::string_view key, ast::NodeCPtr value) -> bool {
 					colour_t colour = NULL_COLOUR;
-					bool uncivilised = true;
+					bool uncivilised = true, can_reduce_militancy = false;
 					Date spawn_date;
 
 					bool ret = expect_dictionary_keys(
 						"uncivilized", ZERO_OR_ONE, expect_bool(assign_variable_callback(uncivilised)),
 						"color", ONE_EXACTLY, expect_colour(assign_variable_callback(colour)),
-						"date", ZERO_OR_ONE, expect_date(assign_variable_callback(spawn_date))
+						"date", ZERO_OR_ONE, expect_date(assign_variable_callback(spawn_date)),
+						"can_reduce_militancy", ZERO_OR_ONE, expect_bool(assign_variable_callback(can_reduce_militancy)),
+						"add_political_reform", ONE_EXACTLY, success_callback,
+						"remove_political_reform", ONE_EXACTLY, success_callback,
+						"add_social_reform", ONE_EXACTLY, success_callback,
+						"remove_social_reform", ONE_EXACTLY, success_callback,
+						"add_military_reform", ZERO_OR_ONE, success_callback,
+						"add_economic_reform", ZERO_OR_ONE, success_callback
 					)(value);
-					ret &= add_ideology(key, colour, ideology_group, uncivilised, spawn_date);
+					ret &= add_ideology(key, colour, ideology_group, uncivilised, can_reduce_militancy, spawn_date);
 					return ret;
 				}
 			)(ideology_group_value);
diff --git a/src/openvic-simulation/politics/Ideology.hpp b/src/openvic-simulation/politics/Ideology.hpp
index b72c176..f5ed5f1 100644
--- a/src/openvic-simulation/politics/Ideology.hpp
+++ b/src/openvic-simulation/politics/Ideology.hpp
@@ -22,17 +22,19 @@ namespace OpenVic {
 
 	private:
 		IdeologyGroup const& group;
-		const bool uncivilised;
+		const bool uncivilised, can_reduce_militancy;
 		const Date spawn_date;
 
 		//TODO - willingness to repeal/pass reforms (and its modifiers)
 
-		Ideology(const std::string_view new_identifier, colour_t new_colour, IdeologyGroup const& new_group, bool uncivilised, Date spawn_date);
+		Ideology(const std::string_view new_identifier, colour_t new_colour, IdeologyGroup const& new_group, bool new_uncivilised, bool new_can_reduce_militancy, Date new_spawn_date);
 
 	public:
 		Ideology(Ideology&&) = default;
+
 		IdeologyGroup const& get_group() const;
 		bool is_uncivilised() const;
+		bool get_can_reduce_militancy() const;
 		Date const& get_spawn_date() const;
 	};
 
@@ -47,7 +49,7 @@ namespace OpenVic {
 		bool add_ideology_group(const std::string_view identifier);
 		IDENTIFIER_REGISTRY_ACCESSORS(IdeologyGroup, ideology_group)
 
-		bool add_ideology(const std::string_view identifier, colour_t colour, IdeologyGroup const* group, bool uncivilised, Date spawn_date);
+		bool add_ideology(const std::string_view identifier, colour_t colour, IdeologyGroup const* group, bool uncivilised, bool can_reduce_militancy, Date spawn_date);
 		IDENTIFIER_REGISTRY_ACCESSORS_CUSTOM_PLURAL(Ideology, ideology, ideologies)
 
 		bool load_ideology_file(ast::NodeCPtr root);
diff --git a/src/openvic-simulation/politics/Issue.cpp b/src/openvic-simulation/politics/Issue.cpp
index b11b8a0..8ee01ea 100644
--- a/src/openvic-simulation/politics/Issue.cpp
+++ b/src/openvic-simulation/politics/Issue.cpp
@@ -5,8 +5,8 @@ using namespace OpenVic::NodeTools;
 
 IssueType::IssueType(const std::string_view new_identifier) : HasIdentifier { new_identifier } {}
 
-IssueGroup::IssueGroup(const std::string_view new_identifier, IssueType const& new_type, bool ordered) 
-	: HasIdentifier { new_identifier }, type { new_type }, ordered { ordered } {}
+IssueGroup::IssueGroup(const std::string_view new_identifier, IssueType const& new_type, bool new_ordered, bool new_administrative) 
+	: HasIdentifier { new_identifier }, type { new_type }, ordered { new_ordered }, administrative { new_administrative } {}
 
 IssueType const& IssueGroup::get_type() const {
 	return type;
@@ -16,6 +16,10 @@ bool IssueGroup::is_ordered() const {
 	return ordered;
 }
 
+bool IssueGroup::is_administrative() const {
+	return administrative;
+}
+
 Issue::Issue(const std::string_view new_identifier, IssueGroup const& new_group, size_t ordinal)
 	: HasIdentifier { new_identifier }, group { new_group }, ordinal { ordinal } {}
 
@@ -42,7 +46,7 @@ bool IssueManager::add_issue_type(const std::string_view identifier) {
 	return issue_types.add_item({ identifier });
 }
 
-bool IssueManager::add_issue_group(const std::string_view identifier, IssueType const* type, bool ordered) {
+bool IssueManager::add_issue_group(const std::string_view identifier, IssueType const* type, bool ordered, bool administrative) {
 	if (identifier.empty()) {
 		Logger::error("Invalid issue group identifier - empty!");
 		return false;
@@ -53,7 +57,7 @@ bool IssueManager::add_issue_group(const std::string_view identifier, IssueType
 		return false;
 	}
 
-	return issue_groups.add_item({ identifier, *type, ordered });
+	return issue_groups.add_item({ identifier, *type, ordered, administrative });
 }
 
 bool IssueManager::add_issue(const std::string_view identifier, IssueGroup const* group, size_t ordinal) {
@@ -71,17 +75,17 @@ bool IssueManager::add_issue(const std::string_view identifier, IssueGroup const
 }
 
 bool IssueManager::_load_issue_group(size_t& expected_issues, const std::string_view identifier, IssueType const* type, ast::NodeCPtr node) {
-	bool ordered = false;
-	return expect_dictionary_keys_and_length(
+	bool ordered = false, administrative = false;
+	bool ret = expect_dictionary_keys_and_length(
 		[&expected_issues](size_t size) -> size_t {
 			expected_issues += size;
 			return size;
 		}, ALLOW_OTHER_KEYS,
-		"next_step_only", ONE_EXACTLY, [&expected_issues, &ordered](ast::NodeCPtr node) -> bool {
-			expected_issues--;
-			return expect_bool(assign_variable_callback(ordered))(node);
-		}
-	)(node) && add_issue_group(identifier, type, ordered);
+		"next_step_only", ZERO_OR_ONE, decrement_callback(expected_issues, expect_bool(assign_variable_callback(ordered))),
+		"administrative", ZERO_OR_ONE, decrement_callback(expected_issues, expect_bool(assign_variable_callback(administrative)))
+	)(node);
+	ret &= add_issue_group(identifier, type, ordered, administrative);
+	return ret;
 }
 
 bool IssueManager::_load_issue(size_t& ordinal, const std::string_view identifier, IssueGroup const* group, ast::NodeCPtr node) {
@@ -102,12 +106,9 @@ bool IssueManager::load_issues_file(ast::NodeCPtr root) {
 	size_t expected_issue_groups = 0;
 	bool ret = expect_dictionary_reserve_length(issue_types, 
 		[this, &expected_issue_groups](std::string_view key, ast::NodeCPtr value) -> bool {
-			return expect_list_and_length(
-				[&expected_issue_groups](size_t size) -> size_t {
-					expected_issue_groups += size;
-					return 0;
-				}, success_callback
-			)(value) && add_issue_type(key);
+			bool ret = expect_length(add_variable_callback(expected_issue_groups))(value);
+			ret &= add_issue_type(key);
+			return ret;
 		}
 	)(root);
 	lock_issue_types();
@@ -132,7 +133,7 @@ bool IssueManager::load_issues_file(ast::NodeCPtr root) {
 			IssueGroup const* issue_group = get_issue_group_by_identifier(group_key);
 			size_t ordinal = 0;
 			return expect_dictionary([this, issue_group, &ordinal](std::string_view key, ast::NodeCPtr value) -> bool {
-				if (key == "next_step_only") return true;
+				if (key == "next_step_only" || key == "administrative") return true;
 				bool ret = _load_issue(ordinal, key, issue_group, value);
 				ordinal++;
 				return ret;
diff --git a/src/openvic-simulation/politics/Issue.hpp b/src/openvic-simulation/politics/Issue.hpp
index 546b3ea..f002f45 100644
--- a/src/openvic-simulation/politics/Issue.hpp
+++ b/src/openvic-simulation/politics/Issue.hpp
@@ -25,14 +25,15 @@ namespace OpenVic {
 
 	private:
 		IssueType const& type;
-		const bool ordered;
+		const bool ordered, administrative;
 
-		IssueGroup(const std::string_view new_identifier, IssueType const& new_type, bool ordered);
+		IssueGroup(const std::string_view new_identifier, IssueType const& new_type, bool new_ordered, bool new_administrative);
 	
 	public:
 		IssueGroup(IssueGroup&&) = default;
 		IssueType const& get_type() const;
 		bool is_ordered() const;
+		bool is_administrative() const;
 	};
 
 	//Issue type (i.e. yes_slavery)
@@ -70,7 +71,7 @@ namespace OpenVic {
 		bool add_issue_type(const std::string_view identifier);
 		IDENTIFIER_REGISTRY_ACCESSORS(IssueType, issue_type)
 
-		bool add_issue_group(const std::string_view identifier, IssueType const* type, bool ordered);
+		bool add_issue_group(const std::string_view identifier, IssueType const* type, bool ordered, bool administrative);
 		IDENTIFIER_REGISTRY_ACCESSORS(IssueGroup, issue_group)
 
 		bool add_issue(const std::string_view identifier, IssueGroup const* group, size_t ordinal);
diff --git a/src/openvic-simulation/pop/Culture.cpp b/src/openvic-simulation/pop/Culture.cpp
index dadc0a6..69e1543 100644
--- a/src/openvic-simulation/pop/Culture.cpp
+++ b/src/openvic-simulation/pop/Culture.cpp
@@ -124,22 +124,10 @@ bool CultureManager::_load_culture_group(size_t& total_expected_cultures,
 			return size;
 		},
 		ALLOW_OTHER_KEYS,
-		"leader", ONE_EXACTLY, [&total_expected_cultures, &leader](ast::NodeCPtr node) -> bool {
-			total_expected_cultures--;
-			return expect_identifier(assign_variable_callback(leader))(node);
-		},
-		"unit", ZERO_OR_ONE, [this, &total_expected_cultures, &unit_graphical_culture_type](ast::NodeCPtr node) -> bool {
-			total_expected_cultures--;
-			return expect_graphical_culture_type_identifier(assign_variable_callback_pointer(unit_graphical_culture_type))(node);
-		},
-		"union", ZERO_OR_ONE, [&total_expected_cultures](ast::NodeCPtr) -> bool {
-			total_expected_cultures--;
-			return true;
-		},
-		"is_overseas", ZERO_OR_ONE, [&total_expected_cultures, &is_overseas](ast::NodeCPtr node) -> bool {
-			total_expected_cultures--;
-			return expect_bool(assign_variable_callback(is_overseas))(node);
-		}
+		"leader", ONE_EXACTLY, decrement_callback(total_expected_cultures, expect_identifier(assign_variable_callback(leader))),
+		"unit", ZERO_OR_ONE, decrement_callback(total_expected_cultures, expect_graphical_culture_type_identifier(assign_variable_callback_pointer(unit_graphical_culture_type))),
+		"union", ZERO_OR_ONE, decrement_callback(total_expected_cultures, success_callback),
+		"is_overseas", ZERO_OR_ONE, decrement_callback(total_expected_cultures, expect_bool(assign_variable_callback(is_overseas)))
 	)(culture_group_node);
 	ret &= add_culture_group(culture_group_key, leader, unit_graphical_culture_type, is_overseas);
 	return ret;
diff --git a/src/openvic-simulation/pop/Religion.cpp b/src/openvic-simulation/pop/Religion.cpp
index ec919fe..acdcad0 100644
--- a/src/openvic-simulation/pop/Religion.cpp
+++ b/src/openvic-simulation/pop/Religion.cpp
@@ -73,13 +73,7 @@ bool ReligionManager::load_religion_file(ast::NodeCPtr root) {
 	bool ret = expect_dictionary_reserve_length(
 		religion_groups,
 		[this, &total_expected_religions](std::string_view key, ast::NodeCPtr value) -> bool {
-			bool ret = expect_list_and_length(
-				[&total_expected_religions](size_t size) -> size_t {
-					total_expected_religions += size;
-					return 0;
-				},
-				success_callback
-			)(value);
+			bool ret = expect_length(add_variable_callback(total_expected_religions))(value);
 			ret &= add_religion_group(key);
 			return ret;
 		}