Require modifier type when parsing ModifierValues

17 files changed, 222 insertions, 68 deletions

diff --git a/src/openvic-simulation/economy/BuildingType.cpp b/src/openvic-simulation/economy/BuildingType.cpp
index 278cde4..a21361f 100644
--- a/src/openvic-simulation/economy/BuildingType.cpp
+++ b/src/openvic-simulation/economy/BuildingType.cpp
@@ -61,9 +61,13 @@ bool BuildingTypeManager::load_buildings_file(
 		building_types, [this, &good_definition_manager, &production_type_manager, &modifier_manager](
 			std::string_view key, ast::NodeCPtr value
 		) -> bool {
+			using enum Modifier::modifier_type_t;
+
 			BuildingType::building_type_args_t building_type_args {};
 
-			bool ret = modifier_manager.expect_modifier_value_and_keys(move_variable_callback(building_type_args.modifier),
+			bool ret = modifier_manager.expect_modifier_value_and_keys(
+				move_variable_callback(building_type_args.modifier),
+				BUILDING,
 				"type", ONE_EXACTLY, expect_identifier(assign_variable_callback(building_type_args.type)),
 				"on_completion", ZERO_OR_ONE, expect_identifier(assign_variable_callback(building_type_args.on_completion)),
 				"completion_size", ZERO_OR_ONE,
diff --git a/src/openvic-simulation/map/Crime.cpp b/src/openvic-simulation/map/Crime.cpp
index 532be9d..176f71b 100644
--- a/src/openvic-simulation/map/Crime.cpp
+++ b/src/openvic-simulation/map/Crime.cpp
@@ -29,17 +29,23 @@ bool CrimeManager::load_crime_modifiers(ModifierManager const& modifier_manager,
 	const bool ret = expect_dictionary_reserve_length(
 		crime_modifiers,
 		[this, &modifier_manager](std::string_view key, ast::NodeCPtr value) -> bool {
+			using enum Modifier::modifier_type_t;
+
 			ModifierValue modifier_value;
 			IconModifier::icon_t icon = 0;
 			ConditionScript trigger { scope_t::PROVINCE, scope_t::NO_SCOPE, scope_t::NO_SCOPE };
 			bool default_active = false;
+
 			bool ret = modifier_manager.expect_modifier_value_and_keys(
 				move_variable_callback(modifier_value),
+				CRIME,
 				"icon", ZERO_OR_ONE, expect_uint(assign_variable_callback(icon)),
 				"trigger", ONE_EXACTLY, trigger.expect_script(),
 				"active", ZERO_OR_ONE, expect_bool(assign_variable_callback(default_active))
 			)(value);
+
 			ret &= add_crime_modifier(key, std::move(modifier_value), icon, std::move(trigger), default_active);
+
 			return ret;
 		}
 	)(root);
diff --git a/src/openvic-simulation/map/MapDefinition.cpp b/src/openvic-simulation/map/MapDefinition.cpp
index 50fba7d..5fe7189 100644
--- a/src/openvic-simulation/map/MapDefinition.cpp
+++ b/src/openvic-simulation/map/MapDefinition.cpp
@@ -955,8 +955,12 @@ bool MapDefinition::load_climate_file(ModifierManager const& modifier_manager, a
 			bool ret = true;
 			Climate* cur_climate = climates.get_item_by_identifier(identifier);
 			if (cur_climate == nullptr) {
+				using enum Modifier::modifier_type_t;
+
 				ModifierValue values;
-				ret &= modifier_manager.expect_modifier_value(move_variable_callback(values))(node);
+
+				ret &= modifier_manager.expect_modifier_value(move_variable_callback(values), CLIMATE)(node);
+
 				ret &= climates.add_item({ identifier, std::move(values), Modifier::modifier_type_t::CLIMATE });
 			} else {
 				ret &= expect_list_reserve_length(*cur_climate, expect_province_definition_identifier(
@@ -1000,6 +1004,8 @@ bool MapDefinition::load_continent_file(ModifierManager const& modifier_manager,
 	bool ret = expect_dictionary_reserve_length(
 		continents,
 		[this, &modifier_manager](std::string_view identifier, ast::NodeCPtr node) -> bool {
+			using enum Modifier::modifier_type_t;
+
 			if (identifier.empty()) {
 				Logger::error("Invalid continent identifier - empty!");
 				return false;
@@ -1007,7 +1013,9 @@ bool MapDefinition::load_continent_file(ModifierManager const& modifier_manager,
 
 			ModifierValue values;
 			std::vector<ProvinceDefinition const*> prov_list;
-			bool ret = modifier_manager.expect_modifier_value_and_keys(move_variable_callback(values),
+			bool ret = modifier_manager.expect_modifier_value_and_keys(
+				move_variable_callback(values),
+				CONTINENT,
 				"provinces", ONE_EXACTLY, expect_list_reserve_length(prov_list, expect_province_definition_identifier(
 					[&prov_list](ProvinceDefinition const& province) -> bool {
 						if (province.continent == nullptr) {
diff --git a/src/openvic-simulation/map/ProvinceDefinition.cpp b/src/openvic-simulation/map/ProvinceDefinition.cpp
index 14828e8..60c8d6c 100644
--- a/src/openvic-simulation/map/ProvinceDefinition.cpp
+++ b/src/openvic-simulation/map/ProvinceDefinition.cpp
@@ -89,7 +89,7 @@ bool ProvinceDefinition::load_positions(
 					port = true;
 					port_adjacent_province = province;
 				} else {
-					/*  Expected provinces with invalid ports: 39, 296, 1047, 1406, 2044 */
+					/* Expected provinces with invalid ports: 39, 296, 1047, 1406, 2044 */
 					Logger::warning(
 						"Invalid port for province ", get_identifier(), ": facing province ", province,
 						" which has: water = ", province->is_water(), ", adjacent = ", is_adjacent_to(province)
diff --git a/src/openvic-simulation/map/TerrainType.cpp b/src/openvic-simulation/map/TerrainType.cpp
index 2f8423d..1531507 100644
--- a/src/openvic-simulation/map/TerrainType.cpp
+++ b/src/openvic-simulation/map/TerrainType.cpp
@@ -72,14 +72,21 @@ node_callback_t TerrainTypeManager::_load_terrain_type_categories(ModifierManage
 	return [this, &modifier_manager](ast::NodeCPtr root) -> bool {
 		const bool ret = expect_dictionary_reserve_length(terrain_types,
 			[this, &modifier_manager](std::string_view type_key, ast::NodeCPtr type_node) -> bool {
+				using enum Modifier::modifier_type_t;
+
 				ModifierValue values;
 				colour_t colour = colour_t::null();
 				bool is_water = false;
-				bool ret = modifier_manager.expect_modifier_value_and_keys(move_variable_callback(values),
+
+				bool ret = modifier_manager.expect_modifier_value_and_keys(
+					move_variable_callback(values),
+					TERRAIN,
 					"color", ONE_EXACTLY, expect_colour(assign_variable_callback(colour)),
 					"is_water", ZERO_OR_ONE, expect_bool(assign_variable_callback(is_water))
 				)(type_node);
+
 				ret &= add_terrain_type(type_key, colour, std::move(values), is_water);
+
 				return ret;
 			}
 		)(root);
diff --git a/src/openvic-simulation/military/LeaderTrait.cpp b/src/openvic-simulation/military/LeaderTrait.cpp
index 4378a67..85d0890 100644
--- a/src/openvic-simulation/military/LeaderTrait.cpp
+++ b/src/openvic-simulation/military/LeaderTrait.cpp
@@ -32,17 +32,21 @@ bool LeaderTraitManager::load_leader_traits_file(ModifierManager const& modifier
 		return expect_dictionary_reserve_length(
 			leader_traits,
 			[this, &modifier_manager, type](std::string_view trait_identifier, ast::NodeCPtr value) -> bool {
+				using enum Modifier::modifier_type_t;
+
 				static const string_set_t allowed_modifiers = {
 					"attack", "defence", "morale", "organisation", "reconnaissance",
 					"speed", "attrition", "experience", "reliability"
 				};
 
 				ModifierValue modifiers;
+
 				bool ret = modifier_manager.expect_whitelisted_modifier_value(
-					move_variable_callback(modifiers), allowed_modifiers
+					move_variable_callback(modifiers), LEADER, allowed_modifiers
 				)(value);
 
 				ret &= add_leader_trait(trait_identifier, type, std::move(modifiers));
+
 				return ret;
 			}
 		);
diff --git a/src/openvic-simulation/military/UnitType.cpp b/src/openvic-simulation/military/UnitType.cpp
index 815025a..22e108d 100644
--- a/src/openvic-simulation/military/UnitType.cpp
+++ b/src/openvic-simulation/military/UnitType.cpp
@@ -29,8 +29,17 @@ UnitType::UnitType(
 	build_time { unit_args.build_time },
 	build_cost { std::move(unit_args.build_cost) },
 	supply_consumption { unit_args.supply_consumption },
-	supply_cost { std::move(unit_args.supply_cost) },
-	terrain_modifiers { std::move(unit_args.terrain_modifiers) } {}
+	supply_cost { std::move(unit_args.supply_cost) } {
+
+	using enum Modifier::modifier_type_t;
+
+	for (auto [terrain, modifier_value] : mutable_iterator(unit_args.terrain_modifier_values)) {
+		terrain_modifiers.emplace(terrain, Modifier {
+			StringUtils::append_string_views(new_identifier, " ", terrain->get_identifier()), std::move(modifier_value),
+			UNIT_TERRAIN
+		});
+	}
+}
 
 bool UnitTypeBranched<LAND>::allowed_cultures_check_culture_in_country(
 	allowed_cultures_t allowed_cultures, Culture const& culture, CountryInstance const& country
@@ -232,16 +241,20 @@ bool UnitTypeManager::load_unit_type_file(
 				good_definition_manager.expect_good_definition_decimal_map(move_variable_callback(unit_args.supply_cost))
 		);
 
-		auto add_terrain_modifier = [&unit_args, &terrain_type_manager, &modifier_manager](
+		auto add_terrain_modifier_value = [&unit_args, &terrain_type_manager, &modifier_manager](
 			std::string_view default_key, ast::NodeCPtr default_value
 		) -> bool {
 			TerrainType const* terrain_type = terrain_type_manager.get_terrain_type_by_identifier(default_key);
+
 			if (terrain_type != nullptr) {
+				using enum Modifier::modifier_type_t;
+
 				// TODO - restrict what modifier effects can be used here
 				return modifier_manager.expect_modifier_value(
-					map_callback(unit_args.terrain_modifiers, terrain_type)
+					map_callback(unit_args.terrain_modifier_values, terrain_type), UNIT_TERRAIN
 				)(default_value);
 			}
+
 			return key_value_invalid_callback(default_key, default_value);
 		};
 
@@ -275,7 +288,7 @@ bool UnitTypeManager::load_unit_type_file(
 				regiment_type_args.allowed_cultures = RegimentType::allowed_cultures_t::ALL_CULTURES;
 			}
 
-			ret &= expect_dictionary_key_map_and_default(key_map, add_terrain_modifier)(value);
+			ret &= expect_dictionary_key_map_and_default(key_map, add_terrain_modifier_value)(value);
 
 			ret &= add_regiment_type(key, unit_args, regiment_type_args);
 
@@ -302,7 +315,7 @@ bool UnitTypeManager::load_unit_type_file(
 				"torpedo_attack", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(ship_type_args.torpedo_attack))
 			);
 
-			ret &= expect_dictionary_key_map_and_default(key_map, add_terrain_modifier)(value);
+			ret &= expect_dictionary_key_map_and_default(key_map, add_terrain_modifier_value)(value);
 
 			ret &= add_ship_type(key, unit_args, ship_type_args);
 
diff --git a/src/openvic-simulation/military/UnitType.hpp b/src/openvic-simulation/military/UnitType.hpp
index 980a119..9527041 100644
--- a/src/openvic-simulation/military/UnitType.hpp
+++ b/src/openvic-simulation/military/UnitType.hpp
@@ -7,7 +7,7 @@
 
 #include "openvic-simulation/dataloader/NodeTools.hpp"
 #include "openvic-simulation/economy/GoodDefinition.hpp"
-#include "openvic-simulation/modifier/ModifierValue.hpp"
+#include "openvic-simulation/modifier/Modifier.hpp"
 #include "openvic-simulation/types/Date.hpp"
 #include "openvic-simulation/types/IdentifierRegistry.hpp"
 #include "openvic-simulation/types/fixed_point/FixedPoint.hpp"
@@ -20,7 +20,7 @@ namespace OpenVic {
 
 	struct UnitType : HasIdentifier {
 		using icon_t = uint32_t;
-		using terrain_modifiers_t = ordered_map<TerrainType const*, ModifierValue>;
+		using terrain_modifiers_t = ordered_map<TerrainType const*, Modifier>;
 
 		enum struct branch_t : uint8_t { INVALID_BRANCH, LAND, NAVAL };
 		enum struct unit_category_t : uint8_t {
@@ -28,6 +28,8 @@ namespace OpenVic {
 		};
 
 		struct unit_type_args_t {
+			using terrain_modifier_values_t = ordered_map<TerrainType const*, ModifierValue>;
+
 			icon_t icon = 0;
 			unit_category_t unit_category = unit_category_t::INVALID_UNIT_CATEGORY;
 			// TODO defaults for move_sound and select_sound
@@ -38,7 +40,7 @@ namespace OpenVic {
 				supply_consumption = 0;
 			Timespan build_time;
 			GoodDefinition::good_definition_map_t build_cost, supply_cost;
-			terrain_modifiers_t terrain_modifiers;
+			terrain_modifier_values_t terrain_modifier_values;
 
 			unit_type_args_t() = default;
 			unit_type_args_t(unit_type_args_t&&) = default;
diff --git a/src/openvic-simulation/modifier/Modifier.cpp b/src/openvic-simulation/modifier/Modifier.cpp
index 962d2d4..3cec7df 100644
--- a/src/openvic-simulation/modifier/Modifier.cpp
+++ b/src/openvic-simulation/modifier/Modifier.cpp
@@ -2,6 +2,33 @@
 
 using namespace OpenVic;
 
+std::string_view Modifier::modifier_type_to_string(modifier_type_t type) {
+	using enum Modifier::modifier_type_t;
+
+	switch (type) {
+#define _CASE(X) case X: return #X;
+	_CASE(EVENT)
+	_CASE(STATIC)
+	_CASE(TRIGGERED)
+	_CASE(CRIME)
+	_CASE(TERRAIN)
+	_CASE(CLIMATE)
+	_CASE(CONTINENT)
+	_CASE(BUILDING)
+	_CASE(LEADER)
+	_CASE(UNIT_TERRAIN)
+	_CASE(NATIONAL_VALUE)
+	_CASE(NATIONAL_FOCUS)
+	_CASE(ISSUE)
+	_CASE(REFORM)
+	_CASE(TECHNOLOGY)
+	_CASE(INVENTION)
+	_CASE(TECH_SCHOOL)
+#undef _CASE
+	default: return "INVALID MODIFIER TYPE";
+	}
+}
+
 Modifier::Modifier(std::string_view new_identifier, ModifierValue&& new_values, modifier_type_t new_type)
 	: HasIdentifier { new_identifier }, ModifierValue { std::move(new_values) }, type { new_type } {}
 
diff --git a/src/openvic-simulation/modifier/Modifier.hpp b/src/openvic-simulation/modifier/Modifier.hpp
index 16ccd21..1277c76 100644
--- a/src/openvic-simulation/modifier/Modifier.hpp
+++ b/src/openvic-simulation/modifier/Modifier.hpp
@@ -8,14 +8,19 @@
 #include "openvic-simulation/utility/Getters.hpp"
 
 namespace OpenVic {
+	struct UnitType;
+
 	struct Modifier : HasIdentifier, ModifierValue {
 		friend struct ModifierManager;
+		friend struct UnitType;
 
 		enum struct modifier_type_t : uint8_t {
-			EVENT, STATIC, TRIGGERED, CRIME, TERRAIN, CLIMATE, CONTINENT, BUILDING, LEADER, NATIONAL_VALUE, NATIONAL_FOCUS,
-			ISSUE, REFORM, TECHNOLOGY, INVENTION, TECH_SCHOOL
+			EVENT, STATIC, TRIGGERED, CRIME, TERRAIN, CLIMATE, CONTINENT, BUILDING, LEADER, UNIT_TERRAIN,
+			NATIONAL_VALUE, NATIONAL_FOCUS, ISSUE, REFORM, TECHNOLOGY, INVENTION, TECH_SCHOOL
 		};
 
+		static std::string_view modifier_type_to_string(modifier_type_t type);
+
 	private:
 		const modifier_type_t PROPERTY(type);
 
diff --git a/src/openvic-simulation/modifier/ModifierManager.cpp b/src/openvic-simulation/modifier/ModifierManager.cpp
index f21712d..5aacbe6 100644
--- a/src/openvic-simulation/modifier/ModifierManager.cpp
+++ b/src/openvic-simulation/modifier/ModifierManager.cpp
@@ -460,13 +460,15 @@ std::string ModifierManager::get_flat_identifier(
 }
 
 bool ModifierManager::add_event_modifier(std::string_view identifier, ModifierValue&& values, IconModifier::icon_t icon) {
+	using enum Modifier::modifier_type_t;
+
 	if (identifier.empty()) {
 		Logger::error("Invalid event modifier effect identifier - empty!");
 		return false;
 	}
 
 	return event_modifiers.add_item(
-		{ identifier, std::move(values), Modifier::modifier_type_t::EVENT, icon }, duplicate_warning_callback
+		{ identifier, std::move(values), EVENT, icon }, duplicate_warning_callback
 	);
 }
 
@@ -474,13 +476,19 @@ bool ModifierManager::load_event_modifiers(ast::NodeCPtr root) {
 	const bool ret = expect_dictionary_reserve_length(
 		event_modifiers,
 		[this](std::string_view key, ast::NodeCPtr value) -> bool {
+			using enum Modifier::modifier_type_t;
+
 			ModifierValue modifier_value;
 			IconModifier::icon_t icon = 0;
+
 			bool ret = expect_modifier_value_and_keys(
 				move_variable_callback(modifier_value),
+				EVENT,
 				"icon", ZERO_OR_ONE, expect_uint(assign_variable_callback(icon))
 			)(value);
+
 			ret &= add_event_modifier(key, std::move(modifier_value), icon);
+
 			return ret;
 		}
 	)(root);
@@ -491,13 +499,15 @@ bool ModifierManager::load_event_modifiers(ast::NodeCPtr root) {
 }
 
 bool ModifierManager::add_static_modifier(std::string_view identifier, ModifierValue&& values) {
+	using enum Modifier::modifier_type_t;
+
 	if (identifier.empty()) {
 		Logger::error("Invalid static modifier effect identifier - empty!");
 		return false;
 	}
 
 	return static_modifiers.add_item(
-		{ identifier, std::move(values), Modifier::modifier_type_t::STATIC }, duplicate_warning_callback
+		{ identifier, std::move(values), STATIC }, duplicate_warning_callback
 	);
 }
 
@@ -505,9 +515,14 @@ bool ModifierManager::load_static_modifiers(ast::NodeCPtr root) {
 	bool ret = expect_dictionary_reserve_length(
 		static_modifiers,
 		[this](std::string_view key, ast::NodeCPtr value) -> bool {
+			using enum Modifier::modifier_type_t;
+
 			ModifierValue modifier_value;
-			bool ret = expect_modifier_value(move_variable_callback(modifier_value))(value);
+
+			bool ret = expect_modifier_value(move_variable_callback(modifier_value), STATIC)(value);
+
 			ret &= add_static_modifier(key, std::move(modifier_value));
+
 			return ret;
 		}
 	)(root);
@@ -522,13 +537,15 @@ bool ModifierManager::load_static_modifiers(ast::NodeCPtr root) {
 bool ModifierManager::add_triggered_modifier(
 	std::string_view identifier, ModifierValue&& values, IconModifier::icon_t icon, ConditionScript&& trigger
 ) {
+	using enum Modifier::modifier_type_t;
+
 	if (identifier.empty()) {
 		Logger::error("Invalid triggered modifier effect identifier - empty!");
 		return false;
 	}
 
 	return triggered_modifiers.add_item(
-		{ identifier, std::move(values), Modifier::modifier_type_t::TRIGGERED, icon, std::move(trigger) },
+		{ identifier, std::move(values), TRIGGERED, icon, std::move(trigger) },
 		duplicate_warning_callback
 	);
 }
@@ -537,16 +554,21 @@ bool ModifierManager::load_triggered_modifiers(ast::NodeCPtr root) {
 	const bool ret = expect_dictionary_reserve_length(
 		triggered_modifiers,
 		[this](std::string_view key, ast::NodeCPtr value) -> bool {
+			using enum Modifier::modifier_type_t;
+
 			ModifierValue modifier_value;
 			IconModifier::icon_t icon = 0;
 			ConditionScript trigger { scope_t::COUNTRY, scope_t::COUNTRY, scope_t::NO_SCOPE };
 
 			bool ret = expect_modifier_value_and_keys(
 				move_variable_callback(modifier_value),
+				TRIGGERED,
 				"icon", ZERO_OR_ONE, expect_uint(assign_variable_callback(icon)),
 				"trigger", ONE_EXACTLY, trigger.expect_script()
 			)(value);
+
 			ret &= add_triggered_modifier(key, std::move(modifier_value), icon, std::move(trigger));
+
 			return ret;
 		}
 	)(root);
@@ -567,7 +589,8 @@ bool ModifierManager::parse_scripts(DefinitionManager const& definition_manager)
 }
 
 key_value_callback_t ModifierManager::_modifier_effect_callback(
-	ModifierValue& modifier, key_value_callback_t default_callback, ModifierEffectValidator auto effect_validator
+	ModifierValue& modifier, Modifier::modifier_type_t type, key_value_callback_t default_callback,
+	ModifierEffectValidator auto effect_validator
 ) const {
 	const auto add_modifier_cb = [this, &modifier, effect_validator](
 		ModifierEffect const* effect, ast::NodeCPtr value
@@ -578,9 +601,11 @@ key_value_callback_t ModifierManager::_modifier_effect_callback(
 				"local_artisan_output",  "artisan_input",           "artisan_throughput",      "artisan_output",
 				"import_cost",           "unciv_economic_modifier", "unciv_military_modifier"
 			};
+
 			if (no_effect_modifiers.contains(effect->get_identifier())) {
 				Logger::warning("This modifier does nothing: ", effect->get_identifier());
 			}
+
 			return expect_fixed_point(map_callback(modifier.values, effect))(value);
 		} else {
 			Logger::error("Failed to validate modifier effect: ", effect->get_identifier());
@@ -630,43 +655,52 @@ key_value_callback_t ModifierManager::_modifier_effect_callback(
 }
 
 node_callback_t ModifierManager::expect_validated_modifier_value_and_default(
-	callback_t<ModifierValue&&> modifier_callback, key_value_callback_t default_callback,
+	callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type, key_value_callback_t default_callback,
 	ModifierEffectValidator auto effect_validator
 ) const {
-	return [this, modifier_callback, default_callback, effect_validator](ast::NodeCPtr root) -> bool {
+	return [this, modifier_callback, type, default_callback, effect_validator](ast::NodeCPtr root) -> bool {
 		ModifierValue modifier;
+
 		bool ret = expect_dictionary_reserve_length(
 			modifier.values,
-			_modifier_effect_callback(modifier, default_callback, effect_validator)
+			_modifier_effect_callback(modifier, type, default_callback, effect_validator)
 		)(root);
+
 		ret &= modifier_callback(std::move(modifier));
+
 		return ret;
 	};
 }
 
 node_callback_t ModifierManager::expect_validated_modifier_value(
-	callback_t<ModifierValue&&> modifier_callback, ModifierEffectValidator auto effect_validator
+	callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type,
+	ModifierEffectValidator auto effect_validator
 ) const {
-	return expect_validated_modifier_value_and_default(modifier_callback, key_value_invalid_callback, effect_validator);
+	return expect_validated_modifier_value_and_default(modifier_callback, type, key_value_invalid_callback, effect_validator);
 }
 
 node_callback_t ModifierManager::expect_modifier_value_and_default(
-	callback_t<ModifierValue&&> modifier_callback, key_value_callback_t default_callback
+	callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type, key_value_callback_t default_callback
 ) const {
-	return expect_validated_modifier_value_and_default(modifier_callback, default_callback, [](ModifierEffect const&) -> bool {
-		return true;
-	});
+	return expect_validated_modifier_value_and_default(
+		modifier_callback, type, default_callback, [](ModifierEffect const&) -> bool {
+			return true;
+		}
+	);
 }
 
-node_callback_t ModifierManager::expect_modifier_value(callback_t<ModifierValue&&> modifier_callback) const {
-	return expect_modifier_value_and_default(modifier_callback, key_value_invalid_callback);
+node_callback_t ModifierManager::expect_modifier_value(
+	callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type
+) const {
+	return expect_modifier_value_and_default(modifier_callback, type, key_value_invalid_callback);
 }
 
 node_callback_t ModifierManager::expect_whitelisted_modifier_value_and_default(
-	callback_t<ModifierValue&&> modifier_callback, string_set_t const& whitelist, key_value_callback_t default_callback
+	callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type, string_set_t const& whitelist,
+	key_value_callback_t default_callback
 ) const {
 	return expect_validated_modifier_value_and_default(
-		modifier_callback, default_callback,
+		modifier_callback, type, default_callback,
 		[&whitelist](ModifierEffect const& effect) -> bool {
 			return whitelist.contains(effect.get_identifier());
 		}
@@ -674,26 +708,32 @@ node_callback_t ModifierManager::expect_whitelisted_modifier_value_and_default(
 }
 
 node_callback_t ModifierManager::expect_whitelisted_modifier_value(
-	callback_t<ModifierValue&&> modifier_callback, string_set_t const& whitelist
+	callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type, string_set_t const& whitelist
 ) const {
-	return expect_whitelisted_modifier_value_and_default(modifier_callback, whitelist, key_value_invalid_callback);
+	return expect_whitelisted_modifier_value_and_default(modifier_callback, type, whitelist, key_value_invalid_callback);
 }
 
 node_callback_t ModifierManager::expect_modifier_value_and_key_map_and_default(
-	callback_t<ModifierValue&&> modifier_callback, key_value_callback_t default_callback, key_map_t&& key_map
+	callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type, key_value_callback_t default_callback,
+	key_map_t&& key_map
 ) const {
-	return [this, modifier_callback, default_callback, key_map = std::move(key_map)](ast::NodeCPtr node) mutable -> bool {
+	return [this, modifier_callback, type, default_callback, key_map = std::move(key_map)](
+		ast::NodeCPtr node
+	) mutable -> bool {
 		bool ret = expect_modifier_value_and_default(
-			modifier_callback,
-			dictionary_keys_callback(key_map, default_callback)
+			modifier_callback, type, dictionary_keys_callback(key_map, default_callback)
 		)(node);
+
 		ret &= check_key_map_counts(key_map);
+
 		return ret;
 	};
 }
 
 node_callback_t ModifierManager::expect_modifier_value_and_key_map(
-	callback_t<ModifierValue&&> modifier_callback, key_map_t&& key_map
+	callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type, key_map_t&& key_map
 ) const {
-	return expect_modifier_value_and_key_map_and_default(modifier_callback, key_value_invalid_callback, std::move(key_map));
+	return expect_modifier_value_and_key_map_and_default(
+		modifier_callback, type, key_value_invalid_callback, std::move(key_map)
+	);
 }
diff --git a/src/openvic-simulation/modifier/ModifierManager.hpp b/src/openvic-simulation/modifier/ModifierManager.hpp
index 73d974d..f3673b0 100644
--- a/src/openvic-simulation/modifier/ModifierManager.hpp
+++ b/src/openvic-simulation/modifier/ModifierManager.hpp
@@ -36,7 +36,7 @@ namespace OpenVic {
 
 		/* effect_validator takes in ModifierEffect const& */
 		NodeTools::key_value_callback_t _modifier_effect_callback(
-			ModifierValue& modifier, NodeTools::key_value_callback_t default_callback,
+			ModifierValue& modifier, Modifier::modifier_type_t type, NodeTools::key_value_callback_t default_callback,
 			ModifierEffectValidator auto effect_validator
 		) const;
 
@@ -71,56 +71,66 @@ namespace OpenVic {
 		bool parse_scripts(DefinitionManager const& definition_manager);
 
 		NodeTools::node_callback_t expect_validated_modifier_value_and_default(
-			NodeTools::callback_t<ModifierValue&&> modifier_callback, NodeTools::key_value_callback_t default_callback,
-			ModifierEffectValidator auto effect_validator
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type,
+			NodeTools::key_value_callback_t default_callback, ModifierEffectValidator auto effect_validator
 		) const;
 		NodeTools::node_callback_t expect_validated_modifier_value(
-			NodeTools::callback_t<ModifierValue&&> modifier_callback, ModifierEffectValidator auto effect_validator
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type,
+			ModifierEffectValidator auto effect_validator
 		) const;
 
 		NodeTools::node_callback_t expect_modifier_value_and_default(
-			NodeTools::callback_t<ModifierValue&&> modifier_callback, NodeTools::key_value_callback_t default_callback
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type,
+			NodeTools::key_value_callback_t default_callback
+		) const;
+		NodeTools::node_callback_t expect_modifier_value(
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type
 		) const;
-		NodeTools::node_callback_t expect_modifier_value(NodeTools::callback_t<ModifierValue&&> modifier_callback) const;
 
 		NodeTools::node_callback_t expect_whitelisted_modifier_value_and_default(
-			NodeTools::callback_t<ModifierValue&&> modifier_callback, string_set_t const& whitelist,
-			NodeTools::key_value_callback_t default_callback
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type,
+			string_set_t const& whitelist, NodeTools::key_value_callback_t default_callback
 		) const;
 		NodeTools::node_callback_t expect_whitelisted_modifier_value(
-			NodeTools::callback_t<ModifierValue&&> modifier_callback, string_set_t const& whitelist
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type,
+			string_set_t const& whitelist
 		) const;
 
+		// In the functions below, key_map refers to a map from identifier strings to NodeTools::dictionary_entry_t,
+		// allowing any non-modifier effect keys found to be parsed in a custom way, similar to expect_dictionary_keys.
 		NodeTools::node_callback_t expect_modifier_value_and_key_map_and_default(
-			NodeTools::callback_t<ModifierValue&&> modifier_callback, NodeTools::key_value_callback_t default_callback,
-			NodeTools::key_map_t&& key_map
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type,
+			NodeTools::key_value_callback_t default_callback, NodeTools::key_map_t&& key_map
 		) const;
 		NodeTools::node_callback_t expect_modifier_value_and_key_map(
-			NodeTools::callback_t<ModifierValue&&> modifier_callback, NodeTools::key_map_t&& key_map
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type,
+			NodeTools::key_map_t&& key_map
 		) const;
 
 		template<typename... Args>
 		NodeTools::node_callback_t expect_modifier_value_and_key_map_and_default(
-			NodeTools::callback_t<ModifierValue&&> modifier_callback, NodeTools::key_value_callback_t default_callback,
-			NodeTools::key_map_t&& key_map, Args... args
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type,
+			NodeTools::key_value_callback_t default_callback, NodeTools::key_map_t&& key_map, Args... args
 		) const {
 			NodeTools::add_key_map_entries(key_map, args...);
-			return expect_modifier_value_and_key_map_and_default(modifier_callback, default_callback, std::move(key_map));
+			return expect_modifier_value_and_key_map_and_default(
+				modifier_callback, type, default_callback, std::move(key_map)
+			);
 		}
 
 		template<typename... Args>
 		NodeTools::node_callback_t expect_modifier_value_and_keys_and_default(
-			NodeTools::callback_t<ModifierValue&&> modifier_callback, NodeTools::key_value_callback_t default_callback,
-			Args... args
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type,
+			NodeTools::key_value_callback_t default_callback, Args... args
 		) const {
-			return expect_modifier_value_and_key_map_and_default(modifier_callback, default_callback, {}, args...);
+			return expect_modifier_value_and_key_map_and_default(modifier_callback, type, default_callback, {}, args...);
 		}
 		template<typename... Args>
 		NodeTools::node_callback_t expect_modifier_value_and_keys(
-			NodeTools::callback_t<ModifierValue&&> modifier_callback, Args... args
+			NodeTools::callback_t<ModifierValue&&> modifier_callback, Modifier::modifier_type_t type, Args... args
 		) const {
 			return expect_modifier_value_and_key_map_and_default(
-				modifier_callback, NodeTools::key_value_invalid_callback, {}, args...
+				modifier_callback, type, NodeTools::key_value_invalid_callback, {}, args...
 			);
 		}
 	};
diff --git a/src/openvic-simulation/politics/Issue.cpp b/src/openvic-simulation/politics/Issue.cpp
index aa7fd60..1faa345 100644
--- a/src/openvic-simulation/politics/Issue.cpp
+++ b/src/openvic-simulation/politics/Issue.cpp
@@ -178,11 +178,15 @@ bool IssueManager::_load_issue(
 	ModifierManager const& modifier_manager, RuleManager const& rule_manager, std::string_view identifier,
 	IssueGroup const* group, ast::NodeCPtr node
 ) {
+	using enum Modifier::modifier_type_t;
+
 	ModifierValue values;
 	RuleSet rules;
 	bool jingoism = false;
 
-	bool ret = modifier_manager.expect_modifier_value_and_keys(move_variable_callback(values),
+	bool ret = modifier_manager.expect_modifier_value_and_keys(
+		move_variable_callback(values),
+		ISSUE,
 		"is_jingoism", ZERO_OR_ONE, expect_bool(assign_variable_callback(jingoism)),
 		"rules", ZERO_OR_ONE, rule_manager.expect_rule_set(move_variable_callback(rules))
 	)(node);
@@ -215,6 +219,8 @@ bool IssueManager::_load_reform(
 	ModifierManager const& modifier_manager, RuleManager const& rule_manager, size_t ordinal, std::string_view identifier,
 	ReformGroup const* group, ast::NodeCPtr node
 ) {
+	using enum Modifier::modifier_type_t;
+
 	ModifierValue values;
 	RuleSet rules;
 	fixed_point_t administrative_multiplier = 0;
@@ -225,6 +231,7 @@ bool IssueManager::_load_reform(
 
 	bool ret = modifier_manager.expect_modifier_value_and_keys(
 		move_variable_callback(values),
+		REFORM,
 		"administrative_multiplier", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(administrative_multiplier)),
 		"technology_cost", ZERO_OR_ONE, expect_uint(assign_variable_callback(technology_cost)),
 		"allow", ZERO_OR_ONE, allow.expect_script(),
diff --git a/src/openvic-simulation/politics/NationalFocus.cpp b/src/openvic-simulation/politics/NationalFocus.cpp
index 81e6336..4d1546c 100644
--- a/src/openvic-simulation/politics/NationalFocus.cpp
+++ b/src/openvic-simulation/politics/NationalFocus.cpp
@@ -117,6 +117,8 @@ bool NationalFocusManager::load_national_foci_file(
 				[this, &group, &pop_manager, &ideology_manager, &good_definition_manager, &modifier_manager](
 					std::string_view identifier, ast::NodeCPtr node
 				) -> bool {
+					using enum Modifier::modifier_type_t;
+
 					uint8_t icon = 0;
 					bool has_flashpoint = false, own_provinces = true, outliner_show_as_percent = false;
 					fixed_point_t flashpoint_tension = 0;
@@ -132,6 +134,7 @@ bool NationalFocusManager::load_national_foci_file(
 
 					bool ret = modifier_manager.expect_modifier_value_and_keys_and_default(
 						move_variable_callback(modifiers),
+						NATIONAL_FOCUS,
 						[&good_definition_manager, &encourage_goods, &pop_manager, &encourage_pop_types](
 							std::string_view key, ast::NodeCPtr value
 						) -> bool {
diff --git a/src/openvic-simulation/politics/NationalValue.cpp b/src/openvic-simulation/politics/NationalValue.cpp
index 6dd4678..16f723b 100644
--- a/src/openvic-simulation/politics/NationalValue.cpp
+++ b/src/openvic-simulation/politics/NationalValue.cpp
@@ -21,11 +21,14 @@ bool NationalValueManager::load_national_values_file(ModifierManager const& modi
 	bool ret = expect_dictionary_reserve_length(
 		national_values,
 		[this, &modifier_manager](std::string_view national_value_identifier, ast::NodeCPtr value) -> bool {
+			using enum Modifier::modifier_type_t;
+
 			ModifierValue modifiers;
 
-			bool ret = modifier_manager.expect_modifier_value(move_variable_callback(modifiers))(value);
+			bool ret = modifier_manager.expect_modifier_value(move_variable_callback(modifiers), NATIONAL_VALUE)(value);
 
 			ret &= add_national_value(national_value_identifier, std::move(modifiers));
+
 			return ret;
 		}
 	)(root);
diff --git a/src/openvic-simulation/research/Invention.cpp b/src/openvic-simulation/research/Invention.cpp
index d8696a8..808dad4 100644
--- a/src/openvic-simulation/research/Invention.cpp
+++ b/src/openvic-simulation/research/Invention.cpp
@@ -60,7 +60,11 @@ bool InventionManager::load_inventions_file(
 ) {
 	return expect_dictionary_reserve_length(
 		inventions, [this, &modifier_manager, &unit_type_manager, &building_type_manager, &crime_manager](
-			std::string_view identifier, ast::NodeCPtr value) -> bool {
+			std::string_view identifier, ast::NodeCPtr value
+		) -> bool {
+			using enum Modifier::modifier_type_t;
+
+			// TODO - use the same variable for all modifiers rather than combining them at the end?
 			ModifierValue loose_modifiers;
 			ModifierValue modifiers;
 
@@ -75,12 +79,15 @@ bool InventionManager::load_inventions_file(
 			ConditionScript limit { scope_t::COUNTRY, scope_t::COUNTRY, scope_t::NO_SCOPE };
 			ConditionalWeight chance { scope_t::COUNTRY, scope_t::COUNTRY, scope_t::NO_SCOPE };
 
-			bool ret = modifier_manager.expect_modifier_value_and_keys(move_variable_callback(loose_modifiers),
+			bool ret = modifier_manager.expect_modifier_value_and_keys(
+				move_variable_callback(loose_modifiers),
+				INVENTION,
 				"news", ZERO_OR_ONE, expect_bool(assign_variable_callback(news)),
 				"limit", ONE_EXACTLY, limit.expect_script(),
 				"chance", ONE_EXACTLY, chance.expect_conditional_weight(ConditionalWeight::BASE),
 				"effect", ZERO_OR_ONE, modifier_manager.expect_modifier_value_and_keys(
 					move_variable_callback(modifiers),
+					INVENTION,
 					"gas_attack", ZERO_OR_ONE, expect_bool(assign_variable_callback(unlock_gas_attack)),
 					"gas_defence", ZERO_OR_ONE, expect_bool(assign_variable_callback(unlock_gas_defence)),
 					"activate_unit", ZERO_OR_MORE,
diff --git a/src/openvic-simulation/research/Technology.cpp b/src/openvic-simulation/research/Technology.cpp
index 773b3c6..a770a40 100644
--- a/src/openvic-simulation/research/Technology.cpp
+++ b/src/openvic-simulation/research/Technology.cpp
@@ -142,8 +142,13 @@ bool TechnologyManager::load_technology_file_schools(
 			const bool ret = expect_dictionary_reserve_length(
 				technology_schools,
 				[this, &modifier_manager](std::string_view school_key, ast::NodeCPtr school_value) -> bool {
+					using enum Modifier::modifier_type_t;
+
 					ModifierValue modifiers;
-					bool ret = modifier_manager.expect_modifier_value(move_variable_callback(modifiers))(school_value);
+
+					bool ret = modifier_manager.expect_modifier_value(
+						move_variable_callback(modifiers), TECH_SCHOOL
+					)(school_value);
 
 					ret &= add_technology_school(school_key, std::move(modifiers));
 
@@ -165,6 +170,8 @@ bool TechnologyManager::load_technologies_file(
 	return expect_dictionary_reserve_length(technologies, [this, &modifier_manager, &unit_type_manager, &building_type_manager](
 		std::string_view tech_key, ast::NodeCPtr tech_value
 	) -> bool {
+		using enum Modifier::modifier_type_t;
+
 		ModifierValue modifiers;
 		TechnologyArea const* area = nullptr;
 		Date::year_t year = 0;
@@ -177,6 +184,7 @@ bool TechnologyManager::load_technologies_file(
 
 		bool ret = modifier_manager.expect_modifier_value_and_keys(
 			move_variable_callback(modifiers),
+			TECHNOLOGY,
 			"area", ONE_EXACTLY, expect_technology_area_identifier(assign_variable_callback_pointer(area)),
 			"year", ONE_EXACTLY, expect_uint(assign_variable_callback(year)),
 			"cost", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(cost)),