Expect modifier value + keys function

9 files changed, 131 insertions, 120 deletions

diff --git a/src/headless/main.cpp b/src/headless/main.cpp
index 04728b6..3935c2d 100644
--- a/src/headless/main.cpp
+++ b/src/headless/main.cpp
@@ -52,7 +52,7 @@ static bool headless_load(Dataloader::path_vector_t const& roots) {
 		ret = false;
 	}
 
-	
+
 	Testing testing = Testing(&game_manager);
 	std::cout << std::endl << "Testing Loaded" << std::endl << std::endl;
 	testing.execute_all_scripts();
diff --git a/src/openvic-simulation/Modifier.cpp b/src/openvic-simulation/Modifier.cpp
index 9fa0691..3ec5474 100644
--- a/src/openvic-simulation/Modifier.cpp
+++ b/src/openvic-simulation/Modifier.cpp
@@ -131,8 +131,8 @@ bool ModifierManager::setup_modifier_effects() {
 	return ret;
 }
 
-node_callback_t ModifierManager::expect_modifier_value(callback_t<ModifierValue&&> callback, key_value_callback_t default_callback) const {
-	return [this, callback, default_callback](ast::NodeCPtr root) -> bool {
+node_callback_t ModifierManager::expect_modifier_value(callback_t<ModifierValue&&> modifier_callback, key_value_callback_t default_callback) const {
+	return [this, modifier_callback, default_callback](ast::NodeCPtr root) -> bool {
 		ModifierValue modifier;
 		bool ret = expect_dictionary(
 			[this, &modifier, default_callback](std::string_view key, ast::NodeCPtr value) -> bool {
@@ -149,7 +149,17 @@ node_callback_t ModifierManager::expect_modifier_value(callback_t<ModifierValue&
 				return default_callback(key, value);
 			}
 		)(root);
-		ret &= callback(std::move(modifier));
+		ret &= modifier_callback(std::move(modifier));
+		return ret;
+	};
+}
+
+node_callback_t ModifierManager::_expect_modifier_value_and_keys(callback_t<ModifierValue&&> modifier_callback, key_map_t&& key_map) const {
+	return [this, modifier_callback, key_map = std::move(key_map)](ast::NodeCPtr node) mutable -> bool {
+		bool ret = expect_modifier_value(
+			modifier_callback, dictionary_keys_callback(key_map, false)
+		)(node);
+		ret &= check_key_map_counts(key_map);
 		return ret;
 	};
 }
diff --git a/src/openvic-simulation/Modifier.hpp b/src/openvic-simulation/Modifier.hpp
index ea302b2..bfd9ca7 100644
--- a/src/openvic-simulation/Modifier.hpp
+++ b/src/openvic-simulation/Modifier.hpp
@@ -92,6 +92,17 @@ namespace OpenVic {
 		IdentifierRegistry<ModifierEffect> modifier_effects;
 		IdentifierRegistry<Modifier> modifiers;
 
+		NodeTools::node_callback_t _expect_modifier_value_and_keys(NodeTools::callback_t<ModifierValue&&> modifier_callback, NodeTools::key_map_t&& key_map) const;
+
+		template<typename... Args>
+		NodeTools::node_callback_t _expect_modifier_value_and_keys(
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, NodeTools::key_map_t&& key_map,
+			const std::string_view key, NodeTools::dictionary_entry_t::expected_count_t expected_count, NodeTools::node_callback_t callback,
+			Args... args) const {
+			NodeTools::add_key_map_entry(key_map, key, expected_count, callback);
+			return _expect_modifier_value_and_keys(modifier_callback, std::move(key_map), args...);
+		}
+
 	public:
 		ModifierManager();
 
@@ -103,6 +114,11 @@ namespace OpenVic {
 
 		bool setup_modifier_effects();
 
-		NodeTools::node_callback_t expect_modifier_value(NodeTools::callback_t<ModifierValue&&> callback, NodeTools::key_value_callback_t default_callback) const;
+		NodeTools::node_callback_t expect_modifier_value(NodeTools::callback_t<ModifierValue&&> modifier_callback, NodeTools::key_value_callback_t default_callback) const;
+
+		template<typename... Args>
+		NodeTools::node_callback_t expect_modifier_value_and_keys(NodeTools::callback_t<ModifierValue&&> modifier_callback, Args... args) const {
+			return _expect_modifier_value_and_keys(modifier_callback, {}, args...);
+		}
 	};
 }
diff --git a/src/openvic-simulation/dataloader/NodeTools.cpp b/src/openvic-simulation/dataloader/NodeTools.cpp
index 2367f86..1c7ddf0 100644
--- a/src/openvic-simulation/dataloader/NodeTools.cpp
+++ b/src/openvic-simulation/dataloader/NodeTools.cpp
@@ -242,32 +242,49 @@ node_callback_t NodeTools::expect_dictionary(key_value_callback_t callback) {
 	return expect_dictionary_and_length(default_length_callback, callback);
 }
 
+void NodeTools::add_key_map_entry(key_map_t& key_map, const std::string_view key, dictionary_entry_t::expected_count_t expected_count, node_callback_t callback) {
+	if (key_map.find(key) == key_map.end()) {
+		key_map.emplace(key, dictionary_entry_t { expected_count, callback });
+	} else {
+		Logger::error("Duplicate expected dictionary key: ", key);
+	}
+}
+
+key_value_callback_t NodeTools::dictionary_keys_callback(key_map_t& key_map, bool allow_other_keys) {
+	return [&key_map, allow_other_keys](std::string_view key, ast::NodeCPtr value) -> bool {
+		const key_map_t::iterator it = key_map.find(key);
+		if (it == key_map.end()) {
+			if (allow_other_keys) return true;
+			Logger::error("Invalid dictionary key: ", key);
+			return false;
+		}
+		dictionary_entry_t& entry = it->second;
+		if (++entry.count > 1 && !entry.can_repeat()) {
+			Logger::error("Invalid repeat of dictionary key: ", key);
+			return false;
+		}
+		return entry.callback(value);
+	};
+}
+
+bool NodeTools::check_key_map_counts(key_map_t const& key_map) {
+	bool ret = true;
+	for (key_map_t::value_type const& key_entry : key_map) {
+		dictionary_entry_t const& entry = key_entry.second;
+		if (entry.must_appear() && entry.count < 1) {
+			Logger::error("Mandatory dictionary key not present: ", key_entry.first);
+			ret = false;
+		}
+	}
+	return ret;
+}
+
 node_callback_t NodeTools::_expect_dictionary_keys_and_length(length_callback_t length_callback, bool allow_other_keys, key_map_t&& key_map) {
 	return [length_callback, allow_other_keys, key_map = std::move(key_map)](ast::NodeCPtr node) mutable -> bool {
 		bool ret = expect_dictionary_and_length(
-			length_callback,
-			[&key_map, allow_other_keys](std::string_view key, ast::NodeCPtr value) -> bool {
-				const key_map_t::iterator it = key_map.find(key);
-				if (it == key_map.end()) {
-					if (allow_other_keys) return true;
-					Logger::error("Invalid dictionary key: ", key);
-					return false;
-				}
-				dictionary_entry_t& entry = it->second;
-				if (++entry.count > 1 && !entry.can_repeat()) {
-					Logger::error("Invalid repeat of dictionary key: ", key);
-					return false;
-				}
-				return entry.callback(value);
-			}
+			length_callback, dictionary_keys_callback(key_map, allow_other_keys)
 		)(node);
-		for (key_map_t::value_type const& key_entry : key_map) {
-			dictionary_entry_t const& entry = key_entry.second;
-			if (entry.must_appear() && entry.count < 1) {
-				Logger::error("Mandatory dictionary key not present: ", key_entry.first);
-				ret = false;
-			}
-		}
+		ret &= check_key_map_counts(key_map);
 		return ret;
 	};
 }
diff --git a/src/openvic-simulation/dataloader/NodeTools.hpp b/src/openvic-simulation/dataloader/NodeTools.hpp
index e49cab6..2dae05c 100644
--- a/src/openvic-simulation/dataloader/NodeTools.hpp
+++ b/src/openvic-simulation/dataloader/NodeTools.hpp
@@ -72,6 +72,10 @@ namespace OpenVic {
 		using enum dictionary_entry_t::expected_count_t;
 		using key_map_t = std::map<std::string, dictionary_entry_t, std::less<void>>;
 
+		void add_key_map_entry(key_map_t& key_map, const std::string_view key, dictionary_entry_t::expected_count_t expected_count, node_callback_t callback);
+		key_value_callback_t dictionary_keys_callback(key_map_t& key_map, bool allow_other_keys);
+		bool check_key_map_counts(key_map_t const& key_map);
+
 		constexpr struct allow_other_keys_t {} ALLOW_OTHER_KEYS;
 
 		node_callback_t _expect_dictionary_keys_and_length(length_callback_t length_callback, bool allow_other_keys, key_map_t&& key_map);
@@ -81,11 +85,7 @@ namespace OpenVic {
 			bool allow_other_keys, key_map_t&& key_map,
 			const std::string_view key, dictionary_entry_t::expected_count_t expected_count, node_callback_t callback,
 			Args... args) {
-			if (key_map.find(key) == key_map.end()) {
-				key_map.emplace(key, dictionary_entry_t { expected_count, callback });
-			} else {
-				Logger::error("Duplicate expected dictionary key: ", key);
-			}
+			add_key_map_entry(key_map, key, expected_count, callback);
 			return _expect_dictionary_keys_and_length(length_callback, allow_other_keys, std::move(key_map), args...);
 		}
 
diff --git a/src/openvic-simulation/economy/ProductionType.cpp b/src/openvic-simulation/economy/ProductionType.cpp
index 40ac2a3..b6f3ffa 100644
--- a/src/openvic-simulation/economy/ProductionType.cpp
+++ b/src/openvic-simulation/economy/ProductionType.cpp
@@ -1,9 +1,6 @@
 #include "ProductionType.hpp"
-#include <string_view>
-#include "dataloader/NodeTools.hpp"
-#include "openvic-dataloader/v2script/AbstractSyntaxTree.hpp"
-#include "pop/Pop.hpp"
-#include "utility/Logger.hpp"
+
+#include <set>
 
 using namespace OpenVic;
 using namespace OpenVic::NodeTools;
@@ -31,7 +28,7 @@ fixed_point_t EmployedPop::get_amount() {
 	return amount;
 }
 
-ProductionType::ProductionType(ARGS(type_t, const Good*)) : HasIdentifier { identifier }, owner { owner },
+ProductionType::ProductionType(PRODUCTION_TYPE_ARGS(type_t, Good const*)) : HasIdentifier { identifier }, owner { owner },
 	employees { employees }, type { type }, workforce { workforce }, input_goods { input_goods }, output_goods { output_goods },
 	value { value }, bonuses { bonuses }, efficiency { efficiency }, coastal { coastal }, farm { farm }, mine { mine } {}
 
@@ -47,15 +44,15 @@ ProductionType::type_t ProductionType::get_type() const {
 	return type;
 }
 
-size_t ProductionType::get_workforce() const {
+Pop::pop_size_t ProductionType::get_workforce() const {
 	return workforce;
 }
 
-std::map<const Good*, fixed_point_t> const& ProductionType::get_input_goods() {
+std::map<Good const*, fixed_point_t> const& ProductionType::get_input_goods() {
 	return input_goods;
 }
 
-const Good* ProductionType::get_output_goods() const {
+Good const* ProductionType::get_output_goods() const {
 	return output_goods;
 }
 
@@ -67,7 +64,7 @@ std::vector<Bonus> const& ProductionType::get_bonuses() {
 	return bonuses;
 }
 
-std::map<const Good*, fixed_point_t> const& ProductionType::get_efficiency() {
+std::map<Good const*, fixed_point_t> const& ProductionType::get_efficiency() {
 	return efficiency;
 }
 
@@ -87,7 +84,7 @@ ProductionTypeManager::ProductionTypeManager() : production_types { "production
 
 node_callback_t ProductionTypeManager::_expect_employed_pop(GoodManager& good_manager, PopManager& pop_manager,
 	callback_t<EmployedPop> cb) {
-	
+
 	return [this, &good_manager, &pop_manager, cb](ast::NodeCPtr node) -> bool {
 		std::string_view pop_type, effect;
 		fixed_point_t effect_multiplier = 1, amount = 1;
@@ -125,7 +122,7 @@ node_callback_t ProductionTypeManager::_expect_employed_pop(GoodManager& good_ma
 
 node_callback_t ProductionTypeManager::_expect_employed_pop_list(GoodManager& good_manager, PopManager& pop_manager,
 	callback_t<std::vector<EmployedPop>> cb) {
-	
+
 	return [this, &good_manager, &pop_manager, cb](ast::NodeCPtr node) -> bool {
 		std::vector<EmployedPop> employed_pops;
 		bool res = expect_list([this, &good_manager, &pop_manager, &employed_pops](ast::NodeCPtr node) -> bool {
@@ -139,12 +136,12 @@ node_callback_t ProductionTypeManager::_expect_employed_pop_list(GoodManager& go
 }
 
 #define POPTYPE_CHECK(employed_pop) \
-	if ((employed_pop.pop_type == nullptr && !employed_pop.artisan) || (employed_pop.pop_type != nullptr && employed_pop.artisan)) {\
+	if ((employed_pop.pop_type == nullptr && !employed_pop.artisan) || (employed_pop.pop_type != nullptr && employed_pop.artisan)) { \
 		Logger::error("Invalid pop type parsed for owner of production type ", identifier, "!"); \
 		return false; \
 	}
 
-bool ProductionTypeManager::add_production_type(ARGS(std::string_view, std::string_view), GoodManager& good_manager) {
+bool ProductionTypeManager::add_production_type(PRODUCTION_TYPE_ARGS(std::string_view, std::string_view), GoodManager& good_manager) {
 	if (identifier.empty()) {
 		Logger::error("Invalid production type identifier - empty!");
 		return false;
@@ -175,15 +172,15 @@ bool ProductionTypeManager::add_production_type(ARGS(std::string_view, std::stri
 		POPTYPE_CHECK(employees[i])
 	}
 
-	const Good* output = good_manager.get_good_by_identifier(output_goods);
+	Good const* output = good_manager.get_good_by_identifier(output_goods);
 	if (output == nullptr) {
 		Logger::error("Invalid output ", output_goods, " for production type ", identifier, "!");
 		return false;
 	}
 
-	return production_types.add_item({ 
+	return production_types.add_item({
 		identifier, owner, employees, type_enum, workforce, input_goods,
-		output, value, bonuses, efficiency, coastal, farm, mine 
+		output, value, bonuses, efficiency, coastal, farm, mine
 	});
 }
 
@@ -191,7 +188,7 @@ bool ProductionTypeManager::add_production_type(ARGS(std::string_view, std::stri
 			"owner", ZERO_OR_ONE, _expect_employed_pop(good_manager, pop_manager, move_variable_callback(owner)), \
 			"employees", ZERO_OR_ONE, _expect_employed_pop_list(good_manager, pop_manager, move_variable_callback(employees)), \
 			"type", ZERO_OR_ONE, expect_identifier(assign_variable_callback(type)), \
-			"workforce", ZERO_OR_ONE, expect_uint(assign_variable_callback(workforce)), \
+			"workforce", ZERO_OR_ONE, expect_uint(assign_variable_callback_uint("workforce", workforce)), \
 			"input_goods", ZERO_OR_ONE, good_manager.expect_good_decimal_map(move_variable_callback(input_goods)), \
 			"output_goods", ZERO_OR_ONE, expect_identifier(assign_variable_callback(output_goods)), \
 			"value", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(value)), \
@@ -204,7 +201,7 @@ bool ProductionTypeManager::add_production_type(ARGS(std::string_view, std::stri
 bool ProductionTypeManager::load_production_types_file(GoodManager& good_manager, PopManager& pop_manager, ast::NodeCPtr root) {
 	size_t expected_types = 0;
 
-	//pass 1: find and store template identifiers
+	// pass 1: find and store template identifiers
 	std::set<std::string_view> templates;
 	std::map<std::string_view, std::string_view> template_target_map;
 	bool ret = expect_dictionary(
@@ -225,8 +222,8 @@ bool ProductionTypeManager::load_production_types_file(GoodManager& good_manager
 		}
 	)(root);
 
-	//pass 2: create and populate the template map
-	std::map<std::string_view, ast::NodeCPtr> template_node_map; 
+	// pass 2: create and populate the template map
+	std::map<std::string_view, ast::NodeCPtr> template_node_map;
 	expect_dictionary(
 		[this, &expected_types, &templates, &template_node_map](std::string_view key, ast::NodeCPtr value) -> bool {
 			if (templates.contains(key)) {
@@ -237,7 +234,7 @@ bool ProductionTypeManager::load_production_types_file(GoodManager& good_manager
 		}
 	)(root);
 
-	//pass 3: actually load production types
+	// pass 3: actually load production types
 	production_types.reserve(production_types.size() + expected_types);
 	ret &= expect_dictionary(
 		[this, &good_manager, &pop_manager, &template_target_map, &template_node_map](std::string_view key, ast::NodeCPtr node) -> bool {
@@ -247,15 +244,15 @@ bool ProductionTypeManager::load_production_types_file(GoodManager& good_manager
 			EmployedPop owner;
 			std::vector<EmployedPop> employees;
 			std::string_view type, output_goods;
-			size_t workforce = 0; //0 is a meaningless value -> unset
-			std::map<const Good*, fixed_point_t> input_goods, efficiency;
-			fixed_point_t value = 0; //0 is a meaningless value -> unset
+			Pop::pop_size_t workforce = 0; // 0 is a meaningless value -> unset
+			std::map<Good const*, fixed_point_t> input_goods, efficiency;
+			fixed_point_t value = 0; // 0 is a meaningless value -> unset
 			std::vector<Bonus> bonuses;
 			bool coastal = false, farm = false, mine = false;
 
 			bool ret = true;
 
-			//apply template first
+			// apply template first
 			if (template_target_map.contains(key)) {
 				std::string_view template_id = template_target_map[key];
 				if (template_node_map.contains(template_id)) {
@@ -266,7 +263,7 @@ bool ProductionTypeManager::load_production_types_file(GoodManager& good_manager
 
 			ret &= PARSE_NODE(node);
 			ret &= add_production_type(
-				key, owner, employees, type, workforce, input_goods, output_goods, value, 
+				key, owner, employees, type, workforce, input_goods, output_goods, value,
 				bonuses, efficiency, coastal, farm, mine, good_manager
 			);
 			return ret;
diff --git a/src/openvic-simulation/economy/ProductionType.hpp b/src/openvic-simulation/economy/ProductionType.hpp
index 1e715d8..50af41e 100644
--- a/src/openvic-simulation/economy/ProductionType.hpp
+++ b/src/openvic-simulation/economy/ProductionType.hpp
@@ -1,19 +1,15 @@
 #pragma once
 
-#include <string_view>
-#include <vector>
-#include <set>
 #include "openvic-simulation/economy/Good.hpp"
 #include "openvic-simulation/pop/Pop.hpp"
 #include "openvic-simulation/types/IdentifierRegistry.hpp"
 #include "openvic-simulation/types/fixed_point/FixedPoint.hpp"
-#include "openvic-simulation/dataloader/NodeTools.hpp"
-#include "openvic-dataloader/v2script/AbstractSyntaxTree.hpp"
 
-#define ARGS(enum_type, output) std::string_view identifier, EmployedPop owner, std::vector<EmployedPop> employees, enum_type type, \
-								size_t workforce, std::map<const Good*, fixed_point_t> input_goods, output output_goods, \
-								fixed_point_t value, std::vector<Bonus> bonuses, std::map<const Good*, fixed_point_t> efficiency, \
-								bool coastal, bool farm, bool mine
+#define PRODUCTION_TYPE_ARGS(enum_type, output) \
+	std::string_view identifier, EmployedPop owner, std::vector<EmployedPop> employees, enum_type type, \
+	Pop::pop_size_t workforce, std::map<Good const*, fixed_point_t> input_goods, output output_goods, \
+	fixed_point_t value, std::vector<Bonus> bonuses, std::map<Good const*, fixed_point_t> efficiency, \
+	bool coastal, bool farm, bool mine
 
 namespace OpenVic {
 	struct ProductionTypeManager;
@@ -22,8 +18,8 @@ namespace OpenVic {
 		friend struct ProductionTypeManager;
 
 	private:
-		PopType const* pop_type; //poptype
-		bool artisan; //set by the parser if the magic "artisan" poptype is passed
+		PopType const* pop_type; // poptype
+		bool artisan; // set by the parser if the magic "artisan" poptype is passed
 		enum struct effect_t {
 			INPUT,
 			OUTPUT,
@@ -33,10 +29,10 @@ namespace OpenVic {
 		fixed_point_t amount;
 
 		EmployedPop(PopType const* pop_type, bool artisan, effect_t effect, fixed_point_t effect_multiplier, fixed_point_t amount);
-	
+
 	public:
-		EmployedPop() = default; 
-	
+		EmployedPop() = default;
+
 		PopType const* get_pop_type();
 		bool is_artisan();
 		effect_t get_effect();
@@ -45,7 +41,7 @@ namespace OpenVic {
 	};
 
 	struct Bonus {
-		//TODO: trigger condition(s)
+		// TODO: trigger condition(s)
 		const fixed_point_t value;
 	};
 
@@ -60,20 +56,20 @@ namespace OpenVic {
 			RGO,
 			ARTISAN
 		} type;
-		const size_t workforce;
+		const Pop::pop_size_t workforce;
 
-		const std::map<const Good*, fixed_point_t> input_goods; //farms generally lack this
-		const Good* output_goods;
+		const std::map<Good const*, fixed_point_t> input_goods; // farms generally lack this
+		Good const* output_goods;
 		const fixed_point_t value;
-		const std::vector<Bonus> bonuses; //some
+		const std::vector<Bonus> bonuses; // some
 
-		const std::map<const Good*, fixed_point_t> efficiency; //some
-		const bool coastal; //is_coastal some(false)
-		
-		const bool farm; //some (false)
-		const bool mine; //some (false)
+		const std::map<Good const*, fixed_point_t> efficiency; // some
+		const bool coastal; // is_coastal some(false)
 
-		ProductionType(ARGS(type_t, const Good*));
+		const bool farm; // some (false)
+		const bool mine; // some (false)
+
+		ProductionType(PRODUCTION_TYPE_ARGS(type_t, Good const*));
 
 	public:
 		ProductionType(ProductionType&&) = default;
@@ -81,14 +77,14 @@ namespace OpenVic {
 		EmployedPop const& get_owner() const;
 		std::vector<EmployedPop> const& get_employees() const;
 		type_t get_type() const;
-		size_t get_workforce() const;
+		Pop::pop_size_t get_workforce() const;
 
-		std::map<const Good*, fixed_point_t> const& get_input_goods();
+		std::map<Good const*, fixed_point_t> const& get_input_goods();
 		const Good* get_output_goods() const;
 		fixed_point_t get_value() const;
 		std::vector<Bonus> const& get_bonuses();
 
-		std::map<const Good*, fixed_point_t> const& get_efficiency();
+		std::map<Good const*, fixed_point_t> const& get_efficiency();
 		bool is_coastal() const;
 
 		bool is_farm() const;
@@ -99,15 +95,15 @@ namespace OpenVic {
 	private:
 		IdentifierRegistry<ProductionType> production_types;
 
-		NodeTools::node_callback_t _expect_employed_pop(GoodManager& good_manager, PopManager& pop_manager, 
+		NodeTools::node_callback_t _expect_employed_pop(GoodManager& good_manager, PopManager& pop_manager,
 			NodeTools::callback_t<EmployedPop> cb);
-		NodeTools::node_callback_t _expect_employed_pop_list(GoodManager& good_manager, PopManager& pop_manager, 
+		NodeTools::node_callback_t _expect_employed_pop_list(GoodManager& good_manager, PopManager& pop_manager,
 			NodeTools::callback_t<std::vector<EmployedPop>> cb);
-	
+
 	public:
 		ProductionTypeManager();
 
-		bool add_production_type(ARGS(std::string_view, std::string_view), GoodManager& good_manager);
+		bool add_production_type(PRODUCTION_TYPE_ARGS(std::string_view, std::string_view), GoodManager& good_manager);
 		IDENTIFIER_REGISTRY_ACCESSORS(ProductionType, production_type)
 
 		bool load_production_types_file(GoodManager& good_manager, PopManager& pop_manager, ast::NodeCPtr root);
diff --git a/src/openvic-simulation/map/Map.cpp b/src/openvic-simulation/map/Map.cpp
index e29f104..7ecff39 100644
--- a/src/openvic-simulation/map/Map.cpp
+++ b/src/openvic-simulation/map/Map.cpp
@@ -406,7 +406,7 @@ bool Map::load_region_file(ast::NodeCPtr root) {
 static constexpr colour_t colour_at(uint8_t const* colour_data, int32_t idx) {
 	/* colour_data is filled with BGR byte triplets - to get pixel idx as a
 	 * single RGB value, multiply idx by 3 to get the index of the corresponding
-	 * triplet, then combine the bytes in reverse order. 
+	 * triplet, then combine the bytes in reverse order.
 	 */
 	idx *= 3;
 	return (colour_data[idx + 2] << 16) | (colour_data[idx + 1] << 8) | colour_data[idx];
diff --git a/src/openvic-simulation/map/TerrainType.cpp b/src/openvic-simulation/map/TerrainType.cpp
index 3134332..e688625 100644
--- a/src/openvic-simulation/map/TerrainType.cpp
+++ b/src/openvic-simulation/map/TerrainType.cpp
@@ -82,35 +82,10 @@ bool TerrainTypeManager::_load_terrain_type_categories(ModifierManager const& mo
 			ModifierValue values;
 			colour_t colour = NULL_COLOUR;
 			bool is_water = false;
-			bool has_colour = false, has_is_water = false;
-			bool ret = modifier_manager.expect_modifier_value(move_variable_callback(values),
-				[&colour, &has_colour, &is_water, &has_is_water](std::string_view key, ast::NodeCPtr value) -> bool {
-					if (key == "color") {
-						if (!has_colour) {
-							has_colour = true;
-							return expect_colour(assign_variable_callback(colour))(value);
-						} else {
-							Logger::error("Duplicate terrain type colour key!");
-							return false;
-						}
-					} else if (key == "is_water") {
-						if (!has_is_water) {
-							has_is_water = true;
-							return expect_bool(assign_variable_callback(is_water))(value);
-						} else {
-							Logger::error("Duplicate terrain type is_water key!");
-							return false;
-						}
-					} else {
-						Logger::error("Invalid terrain type entry key: ", key);
-						return false;
-					}
-				} 
+			bool ret = modifier_manager.expect_modifier_value_and_keys(move_variable_callback(values),
+				"color", ONE_EXACTLY, expect_colour(assign_variable_callback(colour)),
+				"is_water", ZERO_OR_ONE, expect_bool(assign_variable_callback(is_water))
 			)(type_node);
-			if (!has_colour) {
-				Logger::error("Terrain type missing color key: ", type_key);
-				ret = false;
-			}
 			ret &= add_terrain_type(type_key, colour, std::move(values), is_water);
 			return ret;
 		}