4 files changed, 337 insertions, 193 deletions

diff --git a/src/openvic-simulation/military/Deployment.cpp b/src/openvic-simulation/military/Deployment.cpp
index c06ed95..14640ec 100644
--- a/src/openvic-simulation/military/Deployment.cpp
+++ b/src/openvic-simulation/military/Deployment.cpp
@@ -6,15 +6,15 @@ using namespace OpenVic;
 using namespace OpenVic::NodeTools;
 
 Leader::Leader(
-	std::string_view new_name, Unit::type_t new_type, Date new_date, LeaderTrait const* new_personality,
+	std::string_view new_name, Unit::branch_t new_branch, Date new_date, LeaderTrait const* new_personality,
 	LeaderTrait const* new_background, fixed_point_t new_prestige, std::string_view new_picture
-) : name { new_name }, type { new_type }, date { new_date }, personality { new_personality }, background { new_background },
+) : name { new_name }, branch { new_branch }, date { new_date }, personality { new_personality }, background { new_background },
 	prestige { new_prestige }, picture { new_picture } {}
 
-Regiment::Regiment(std::string_view new_name, Unit const* new_type, Province const* new_home)
+Regiment::Regiment(std::string_view new_name, LandUnit const* new_type, Province const* new_home)
 	: name { new_name }, type { new_type }, home { new_home } {}
 
-Ship::Ship(std::string_view new_name, Unit const* new_type) : name { new_name }, type { new_type } {}
+Ship::Ship(std::string_view new_name, NavalUnit const* new_type) : name { new_name }, type { new_type } {}
 
 Army::Army(std::string_view new_name, Province const* new_location, std::vector<Regiment>&& new_regiments)
 	: name { new_name }, location { new_location }, regiments { std::move(new_regiments) } {}
@@ -71,7 +71,7 @@ bool DeploymentManager::load_oob_file(
 		key_value_success_callback, // TODO: load SOI information
 		"leader", ZERO_OR_MORE, [&leaders, &game_manager](ast::NodeCPtr node) -> bool {
 			std::string_view leader_name {};
-			Unit::type_t leader_type = Unit::type_t::LAND;
+			Unit::branch_t leader_branch = Unit::branch_t::INVALID_BRANCH;
 			Date leader_date {};
 			LeaderTrait const* leader_personality = nullptr;
 			LeaderTrait const* leader_background = nullptr;
@@ -81,7 +81,7 @@ bool DeploymentManager::load_oob_file(
 			bool ret = expect_dictionary_keys(
 				"name", ONE_EXACTLY, expect_identifier_or_string(assign_variable_callback(leader_name)),
 				"date", ONE_EXACTLY, expect_identifier_or_string(expect_date_str(assign_variable_callback(leader_date))),
-				"type", ONE_EXACTLY, expect_identifier(UnitManager::expect_type_str(assign_variable_callback(leader_type))),
+				"type", ONE_EXACTLY, UnitManager::expect_branch_identifier(assign_variable_callback(leader_branch)),
 				"personality", ONE_EXACTLY,
 					game_manager.get_military_manager().get_leader_trait_manager().expect_leader_trait_identifier_or_string(
 						assign_variable_callback_pointer(leader_personality)
@@ -109,7 +109,7 @@ bool DeploymentManager::load_oob_file(
 				ret = false;
 			}
 			leaders.emplace_back(
-				leader_name, leader_type, leader_date, leader_personality, leader_background, leader_prestige, picture
+				leader_name, leader_branch, leader_date, leader_personality, leader_background, leader_prestige, picture
 			);
 			return ret;
 		},
@@ -124,12 +124,12 @@ bool DeploymentManager::load_oob_file(
 					game_manager.get_map().expect_province_identifier(assign_variable_callback_pointer(army_location)),
 				"regiment", ONE_OR_MORE, [&game_manager, &army_regiments](ast::NodeCPtr node) -> bool {
 					std::string_view regiment_name {};
-					Unit const* regiment_type = nullptr;
+					LandUnit const* regiment_type = nullptr;
 					Province const* regiment_home = nullptr;
 					const bool ret = expect_dictionary_keys(
 						"name", ONE_EXACTLY, expect_string(assign_variable_callback(regiment_name)),
 						"type", ONE_EXACTLY, game_manager.get_military_manager().get_unit_manager()
-							.expect_unit_identifier(assign_variable_callback_pointer(regiment_type)),
+							.expect_land_unit_identifier(assign_variable_callback_pointer(regiment_type)),
 						"home", ZERO_OR_ONE, game_manager.get_map()
 							.expect_province_identifier(assign_variable_callback_pointer(regiment_home))
 					)(node);
@@ -156,11 +156,11 @@ bool DeploymentManager::load_oob_file(
 					game_manager.get_map().expect_province_identifier(assign_variable_callback_pointer(navy_location)),
 				"ship", ONE_OR_MORE, [&game_manager, &navy_ships](ast::NodeCPtr node) -> bool {
 					std::string_view ship_name {};
-					Unit const* ship_type = nullptr;
+					NavalUnit const* ship_type = nullptr;
 					const bool ret = expect_dictionary_keys(
 						"name", ONE_EXACTLY, expect_string(assign_variable_callback(ship_name)),
 						"type", ONE_EXACTLY, game_manager.get_military_manager().get_unit_manager()
-							.expect_unit_identifier(assign_variable_callback_pointer(ship_type))
+							.expect_naval_unit_identifier(assign_variable_callback_pointer(ship_type))
 					)(node);
 					navy_ships.emplace_back(ship_name, ship_type);
 					return ret;
diff --git a/src/openvic-simulation/military/Deployment.hpp b/src/openvic-simulation/military/Deployment.hpp
index a952823..e774108 100644
--- a/src/openvic-simulation/military/Deployment.hpp
+++ b/src/openvic-simulation/military/Deployment.hpp
@@ -16,7 +16,7 @@ namespace OpenVic {
 	struct Leader {
 	private:
 		std::string        PROPERTY(name);
-		Unit::type_t       PROPERTY(type);
+		Unit::branch_t     PROPERTY(branch); /* type in defines */
 		Date               PROPERTY(date);
 		LeaderTrait const* PROPERTY(personality);
 		LeaderTrait const* PROPERTY(background);
@@ -25,7 +25,7 @@ namespace OpenVic {
 
 	public:
 		Leader(
-			std::string_view new_name, Unit::type_t new_type, Date new_date, LeaderTrait const* new_personality,
+			std::string_view new_name, Unit::branch_t new_branch, Date new_date, LeaderTrait const* new_personality,
 			LeaderTrait const* new_background, fixed_point_t new_prestige, std::string_view new_picture
 		);
 	};
@@ -33,20 +33,20 @@ namespace OpenVic {
 	struct Regiment {
 	private:
 		std::string     PROPERTY(name);
-		Unit const*     PROPERTY(type);
+		LandUnit const* PROPERTY(type);
 		Province const* PROPERTY(home);
 
 	public:
-		Regiment(std::string_view new_name, Unit const* new_type, Province const* new_home);
+		Regiment(std::string_view new_name, LandUnit const* new_type, Province const* new_home);
 	};
 
 	struct Ship {
 	private:
-		std::string PROPERTY(name);
-		Unit const* PROPERTY(type);
+		std::string      PROPERTY(name);
+		NavalUnit const* PROPERTY(type);
 
 	public:
-		Ship(std::string_view new_name, Unit const* new_type);
+		Ship(std::string_view new_name, NavalUnit const* new_type);
 	};
 
 	struct Army {
diff --git a/src/openvic-simulation/military/Unit.cpp b/src/openvic-simulation/military/Unit.cpp
index d0f6435..54fc763 100644
--- a/src/openvic-simulation/military/Unit.cpp
+++ b/src/openvic-simulation/military/Unit.cpp
@@ -1,200 +1,284 @@
 #include "Unit.hpp"
 
-#define UNIT_ARGS \
-	icon, sprite, active, unit_type, floating_flag, priority, max_strength, default_organisation, maximum_speed, \
-	weighted_value, move_sound, select_sound, build_time, std::move(build_cost), supply_consumption, \
-	std::move(supply_cost)
-
-#define LAND_ARGS \
-	allowed_cultures, sprite_override, sprite_mount, sprite_mount_attach_node, reconnaissance, attack, defence, discipline, \
-	support, maneuver, siege
-
-#define NAVY_ARGS \
-	naval_icon, sail, transport, capital, colonial_points, build_overseas, min_port_level, limit_per_port, \
-	supply_consumption_score, hull, gun_power, fire_range, evasion, torpedo_attack
+#include "openvic-simulation/map/TerrainType.hpp"
 
 using namespace OpenVic;
 using namespace OpenVic::NodeTools;
 
+using enum Unit::branch_t;
+using enum Unit::unit_type_t;
+
 Unit::Unit(
-	std::string_view identifier, type_t type, UNIT_PARAMS
-) : HasIdentifier { identifier }, icon { icon }, type { type }, sprite { sprite }, active { active }, unit_type { unit_type },
-	floating_flag { floating_flag }, priority { priority }, max_strength { max_strength },
-	default_organisation { default_organisation }, maximum_speed { maximum_speed }, weighted_value { weighted_value },
-	move_sound { move_sound }, select_sound { select_sound }, build_time { build_time }, build_cost { std::move(build_cost) },
-	supply_consumption { supply_consumption }, supply_cost { std::move(supply_cost) } {}
+	std::string_view new_identifier, branch_t new_branch, unit_args_t& unit_args
+) : HasIdentifier { new_identifier },
+	branch { new_branch },
+	icon { unit_args.icon },
+	sprite { unit_args.sprite },
+	active { unit_args.active },
+	unit_type { unit_args.unit_type },
+	floating_flag { unit_args.floating_flag },
+	priority { unit_args.priority },
+	max_strength { unit_args.max_strength },
+	default_organisation { unit_args.default_organisation },
+	maximum_speed { unit_args.maximum_speed },
+	weighted_value { unit_args.weighted_value },
+	move_sound { unit_args.move_sound },
+	select_sound { unit_args.select_sound },
+	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) } {}
 
 LandUnit::LandUnit(
-	std::string_view identifier, UNIT_PARAMS, LAND_PARAMS
-) : Unit { identifier, type_t::LAND, UNIT_ARGS }, allowed_cultures { allowed_cultures }, sprite_override { sprite_override },
-	sprite_mount { sprite_mount }, sprite_mount_attach_node { sprite_mount_attach_node }, reconnaissance { reconnaissance },
-	attack { attack }, defence { defence }, discipline { discipline }, support { support }, maneuver { maneuver },
-	siege { siege } {}
+	std::string_view new_identifier, unit_args_t& unit_args, land_unit_args_t const& land_unit_args
+) : Unit { new_identifier, LAND, unit_args },
+	allowed_cultures { land_unit_args.allowed_cultures },
+	sprite_override { land_unit_args.sprite_override },
+	sprite_mount { land_unit_args.sprite_mount },
+	sprite_mount_attach_node { land_unit_args.sprite_mount_attach_node },
+	reconnaissance { land_unit_args.reconnaissance },
+	attack { land_unit_args.attack },
+	defence { land_unit_args.defence },
+	discipline { land_unit_args.discipline },
+	support { land_unit_args.support },
+	maneuver { land_unit_args.maneuver },
+	siege { land_unit_args.siege } {}
 
 NavalUnit::NavalUnit(
-	std::string_view identifier, UNIT_PARAMS, NAVY_PARAMS
-) : Unit { identifier, type_t::NAVAL, UNIT_ARGS }, naval_icon { naval_icon }, sail { sail }, transport { transport },
-	capital { capital }, colonial_points { colonial_points }, build_overseas { build_overseas },
-	min_port_level { min_port_level }, limit_per_port { limit_per_port },
-	supply_consumption_score { supply_consumption_score }, hull { hull }, gun_power { gun_power }, fire_range { fire_range },
-	evasion { evasion }, torpedo_attack { torpedo_attack } {};
-
-bool UnitManager::_check_shared_parameters(std::string_view identifier, UNIT_PARAMS) {
+	std::string_view new_identifier, unit_args_t& unit_args, naval_unit_args_t const& naval_unit_args
+) : Unit { new_identifier, NAVAL, unit_args },
+	naval_icon { naval_unit_args.naval_icon },
+	sail { naval_unit_args.sail },
+	transport { naval_unit_args.transport },
+	capital { naval_unit_args.capital },
+	colonial_points { naval_unit_args.colonial_points },
+	build_overseas { naval_unit_args.build_overseas },
+	min_port_level { naval_unit_args.min_port_level },
+	limit_per_port { naval_unit_args.limit_per_port },
+	supply_consumption_score { naval_unit_args.supply_consumption_score },
+	hull { naval_unit_args.hull },
+	gun_power { naval_unit_args.gun_power },
+	fire_range { naval_unit_args.fire_range },
+	evasion { naval_unit_args.evasion },
+	torpedo_attack { naval_unit_args.torpedo_attack } {}
+
+void UnitManager::reserve_all_units(size_t size) {
+	reserve_more_units(size);
+	reserve_more_land_units(size);
+	reserve_more_naval_units(size);
+}
+
+void UnitManager::lock_all_units() {
+	units.lock();
+	land_units.lock();
+	naval_units.lock();
+}
+
+static bool _check_shared_parameters(std::string_view identifier, Unit::unit_args_t const& unit_args) {
 	if (identifier.empty()) {
-		Logger::error("Invalid religion identifier - empty!");
+		Logger::error("Invalid unit identifier - empty!");
 		return false;
 	}
 
-	if (sprite.empty()) {
-		Logger::error("Invalid sprite identifier - empty!");
+	if (unit_args.icon <= 0) {
+		Logger::error("Invalid icon for unit ", identifier, " - ", unit_args.icon, " (must be positive)");
 		return false;
 	}
 
-	if (unit_type.empty()) {
-		Logger::error("Invalid unit type - empty!");
+	if (unit_args.unit_type == INVALID_UNIT_TYPE) {
+		Logger::error("Invalid unit type for unit ", identifier, "!");
 		return false;
 	}
 
-	// TODO check that icon and sprite exist
+	// TODO check that sprite, move_sound and select_sound exist
 
 	return true;
 }
 
-bool UnitManager::add_land_unit(std::string_view identifier, UNIT_PARAMS, LAND_PARAMS) {
-	if (!_check_shared_parameters(identifier, UNIT_ARGS)) {
+bool UnitManager::add_land_unit(
+	std::string_view identifier, Unit::unit_args_t& unit_args, LandUnit::land_unit_args_t const& land_unit_args
+) {
+	if (!_check_shared_parameters(identifier, unit_args)) {
+		return false;
+	}
+
+	// TODO check that sprite_override, sprite_mount, and sprite_mount_attach_node exist
+
+	if (naval_units.has_identifier(identifier)) {
+		Logger::error("Land unit ", identifier, " already exists as a naval unit!");
 		return false;
 	}
 
-	return units.add_item(LandUnit { identifier, UNIT_ARGS, LAND_ARGS });
+	bool ret = land_units.add_item({ identifier, unit_args, std::move(land_unit_args) });
+	if (ret) {
+		ret &= units.add_item(&land_units.get_items().back());
+	}
+	return ret;
 }
 
-bool UnitManager::add_naval_unit(std::string_view identifier, UNIT_PARAMS, NAVY_PARAMS) {
-	if (!_check_shared_parameters(identifier, UNIT_ARGS)) {
+bool UnitManager::add_naval_unit(
+	std::string_view identifier, Unit::unit_args_t& unit_args, NavalUnit::naval_unit_args_t const& naval_unit_args
+) {
+	if (!_check_shared_parameters(identifier, unit_args)) {
 		return false;
 	}
 
-	// TODO: check that icon and sounds exist
+	if (naval_unit_args.naval_icon <= 0) {
+		Logger::error("Invalid naval icon identifier - ", naval_unit_args.naval_icon, " (must be positive)");
+		return false;
+	}
 
-	return units.add_item(NavalUnit { identifier, UNIT_ARGS, NAVY_ARGS });
-}
+	if (naval_unit_args.supply_consumption_score <= 0) {
+		Logger::warning("Supply consumption score for ", identifier, " is not positive!");
+	}
 
-callback_t<std::string_view> UnitManager::expect_type_str(Callback<Unit::type_t> auto callback) {
-	using enum Unit::type_t;
-	static const string_map_t<Unit::type_t> type_map = { { "land", LAND }, { "naval", NAVAL }, { "sea", NAVAL } };
-	return expect_mapped_string(type_map, callback);
+	if (land_units.has_identifier(identifier)) {
+		Logger::error("Naval unit ", identifier, " already exists as a land unit!");
+		return false;
+	}
+
+	bool ret = naval_units.add_item({ identifier, unit_args, naval_unit_args });
+	if (ret) {
+		ret &= units.add_item(&naval_units.get_items().back());
+	}
+	return ret;
 }
 
-bool UnitManager::load_unit_file(GoodManager const& good_manager, ast::NodeCPtr root) {
-	return expect_dictionary([this, &good_manager](std::string_view key, ast::NodeCPtr value) -> bool {
-		Unit::type_t type;
-		Unit::icon_t icon = 0;
-		std::string_view unit_type, sprite, move_sound, select_sound; // TODO defaults for move_sound and select_sound
-		bool active = true, floating_flag = false;
-		uint32_t priority = 0;
-		Timespan build_time;
-		fixed_point_t maximum_speed = 0, max_strength = 0, default_organisation = 0;
-		fixed_point_t weighted_value = 0, supply_consumption = 0;
-		Good::good_map_t build_cost, supply_cost;
-
-		bool ret = expect_key("type", expect_identifier(expect_type_str(assign_variable_callback(type))))(value);
-
-		if (!ret) {
-			Logger::error("Failed to read type for unit: ", key);
+bool UnitManager::load_unit_file(
+	GoodManager const& good_manager, TerrainTypeManager const& terrain_type_manager, ModifierManager const& modifier_manager,
+	ast::NodeCPtr root
+) {
+	return expect_dictionary([this, &good_manager, &terrain_type_manager, &modifier_manager](
+		std::string_view key, ast::NodeCPtr value
+	) -> bool {
+
+		Unit::branch_t branch = INVALID_BRANCH;
+
+		bool ret = expect_key("type", expect_branch_identifier(assign_variable_callback(branch)))(value);
+
+		/* We shouldn't just check ret as it can be false even if branch was successfully parsed,
+		 * but more than one instance of the key was found. */
+		if (branch != LAND && branch != NAVAL) {
+			Logger::error("Failed to read branch for unit: ", key);
 			return false;
 		}
 
-		key_map_t key_map;
+		Unit::unit_args_t unit_args {};
+
+		static const string_map_t<Unit::unit_type_t> unit_type_map {
+			{ "infantry", INFANTRY },
+			{ "cavalry", CAVALRY },
+			{ "support", SUPPORT },
+			{ "special", SPECIAL },
+			{ "big_ship", BIG_SHIP },
+			{ "light_ship", LIGHT_SHIP },
+			{ "transport", TRANSPORT }
+		};
+
+		key_map_t key_map {};
 		/* Shared dictionary entries */
 		ret &= add_key_map_entries(key_map,
-			"icon", ONE_EXACTLY, expect_uint(assign_variable_callback(icon)),
+			"icon", ONE_EXACTLY, expect_uint(assign_variable_callback(unit_args.icon)),
 			"type", ONE_EXACTLY, success_callback, /* Already loaded above using expect_key */
-			"sprite", ONE_EXACTLY, expect_identifier(assign_variable_callback(sprite)),
-			"active", ZERO_OR_ONE, expect_bool(assign_variable_callback(active)),
-			"unit_type", ONE_EXACTLY, expect_identifier(assign_variable_callback(unit_type)),
-			"floating_flag", ONE_EXACTLY, expect_bool(assign_variable_callback(floating_flag)),
-			"priority", ONE_EXACTLY, expect_uint(assign_variable_callback(priority)),
-			"max_strength", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(max_strength)),
-			"default_organisation", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(default_organisation)),
-			"maximum_speed", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(maximum_speed)),
-			"weighted_value", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(weighted_value)),
-			"move_sound", ZERO_OR_ONE, expect_identifier(assign_variable_callback(move_sound)),
-			"select_sound", ZERO_OR_ONE, expect_identifier(assign_variable_callback(select_sound)),
-			"build_time", ONE_EXACTLY, expect_days(assign_variable_callback(build_time)),
-			"build_cost", ONE_EXACTLY, good_manager.expect_good_decimal_map(move_variable_callback(build_cost)),
-			"supply_consumption", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(supply_consumption)),
-			"supply_cost", ONE_EXACTLY, good_manager.expect_good_decimal_map(move_variable_callback(supply_cost))
+			"sprite", ONE_EXACTLY, expect_identifier(assign_variable_callback(unit_args.sprite)),
+			"active", ZERO_OR_ONE, expect_bool(assign_variable_callback(unit_args.active)),
+			"unit_type", ONE_EXACTLY,
+				expect_identifier(expect_mapped_string(unit_type_map, assign_variable_callback(unit_args.unit_type))),
+			"floating_flag", ONE_EXACTLY, expect_bool(assign_variable_callback(unit_args.floating_flag)),
+			"priority", ONE_EXACTLY, expect_uint(assign_variable_callback(unit_args.priority)),
+			"max_strength", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(unit_args.max_strength)),
+			"default_organisation", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(unit_args.default_organisation)),
+			"maximum_speed", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(unit_args.maximum_speed)),
+			"weighted_value", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(unit_args.weighted_value)),
+			"move_sound", ZERO_OR_ONE, expect_identifier(assign_variable_callback(unit_args.move_sound)),
+			"select_sound", ZERO_OR_ONE, expect_identifier(assign_variable_callback(unit_args.select_sound)),
+			"build_time", ONE_EXACTLY, expect_days(assign_variable_callback(unit_args.build_time)),
+			"build_cost", ONE_EXACTLY, good_manager.expect_good_decimal_map(move_variable_callback(unit_args.build_cost)),
+			"supply_consumption", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(unit_args.supply_consumption)),
+			"supply_cost", ONE_EXACTLY, good_manager.expect_good_decimal_map(move_variable_callback(unit_args.supply_cost))
 		);
 
-		switch (type) {
-		case Unit::type_t::LAND: {
+		const auto add_terrain_modifier = [&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) {
+				// TODO - restrict what modifier effects can be used here
+				return modifier_manager.expect_modifier_value(
+					map_callback(unit_args.terrain_modifiers, terrain_type)
+				)(default_value);
+			}
+			return key_value_invalid_callback(default_key, default_value);
+		};
+
+		switch (branch) {
+		case LAND: {
+			LandUnit::land_unit_args_t land_unit_args {};
 			bool is_restricted_to_primary_culture = false;
 			bool is_restricted_to_accepted_cultures = false;
-			std::string_view sprite_override {}, sprite_mount {}, sprite_mount_attach_node {};
-			fixed_point_t reconnaissance = 0, attack = 0, defence = 0, discipline = 0, support = 0, maneuver = 0, siege = 0;
 
 			ret &= add_key_map_entries(key_map,
 				"primary_culture", ZERO_OR_ONE, expect_bool(assign_variable_callback(is_restricted_to_primary_culture)),
 				"accepted_culture", ZERO_OR_ONE, expect_bool(assign_variable_callback(is_restricted_to_accepted_cultures)),
-				"sprite_override", ZERO_OR_ONE, expect_identifier(assign_variable_callback(sprite_override)),
-				"sprite_mount", ZERO_OR_ONE, expect_identifier(assign_variable_callback(sprite_mount)),
-				"sprite_mount_attach_node", ZERO_OR_ONE, expect_identifier(assign_variable_callback(sprite_mount_attach_node)),
-				"reconnaissance", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(reconnaissance)),
-				"attack", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(attack)),
-				"defence", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(defence)),
-				"discipline", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(discipline)),
-				"support", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(support)),
-				"maneuver", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(maneuver)),
-				"siege", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(siege))
+				"sprite_override", ZERO_OR_ONE, expect_identifier(assign_variable_callback(land_unit_args.sprite_override)),
+				"sprite_mount", ZERO_OR_ONE, expect_identifier(assign_variable_callback(land_unit_args.sprite_mount)),
+				"sprite_mount_attach_node", ZERO_OR_ONE,
+					expect_identifier(assign_variable_callback(land_unit_args.sprite_mount_attach_node)),
+				"reconnaissance", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(land_unit_args.reconnaissance)),
+				"attack", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(land_unit_args.attack)),
+				"defence", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(land_unit_args.defence)),
+				"discipline", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(land_unit_args.discipline)),
+				"support", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(land_unit_args.support)),
+				"maneuver", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(land_unit_args.maneuver)),
+				"siege", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(land_unit_args.siege))
 			);
 
-			LandUnit::allowed_cultures_t allowed_cultures;
 			if (is_restricted_to_accepted_cultures) {
-				allowed_cultures = LandUnit::allowed_cultures_t::ACCEPTED_CULTURES;
+				land_unit_args.allowed_cultures = LandUnit::allowed_cultures_t::ACCEPTED_CULTURES;
 			} else if (is_restricted_to_primary_culture) {
-				allowed_cultures = LandUnit::allowed_cultures_t::PRIMARY_CULTURE;
+				land_unit_args.allowed_cultures = LandUnit::allowed_cultures_t::PRIMARY_CULTURE;
 			} else {
-				allowed_cultures = LandUnit::allowed_cultures_t::ALL_CULTURES;
+				land_unit_args.allowed_cultures = LandUnit::allowed_cultures_t::ALL_CULTURES;
 			}
 
-			ret &= expect_dictionary_key_map(key_map)(value);
+			ret &= expect_dictionary_key_map_and_default(key_map, add_terrain_modifier)(value);
 
-			ret &= add_land_unit(key, UNIT_ARGS, LAND_ARGS);
+			ret &= add_land_unit(key, unit_args, land_unit_args);
 
 			return ret;
 		}
-		case Unit::type_t::NAVAL: {
-			Unit::icon_t naval_icon = 0;
-			bool sail = false, transport = false, capital = false, build_overseas = false;
-			uint32_t min_port_level = 0;
-			int32_t limit_per_port = 0;
-			fixed_point_t fire_range = 0, evasion = 0, supply_consumption_score = 0, hull = 0;
-			fixed_point_t gun_power = 0, colonial_points = 0, torpedo_attack = 0;
+		case NAVAL: {
+			NavalUnit::naval_unit_args_t naval_unit_args {};
 
 			ret &= add_key_map_entries(key_map,
-				"naval_icon", ONE_EXACTLY, expect_uint(assign_variable_callback(naval_icon)),
-				"sail", ZERO_OR_ONE, expect_bool(assign_variable_callback(sail)),
-				"transport", ZERO_OR_ONE, expect_bool(assign_variable_callback(transport)),
-				"capital", ZERO_OR_ONE, expect_bool(assign_variable_callback(capital)),
-				"colonial_points", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(colonial_points)),
-				"can_build_overseas", ZERO_OR_ONE, expect_bool(assign_variable_callback(build_overseas)),
-				"min_port_level", ONE_EXACTLY, expect_uint(assign_variable_callback(min_port_level)),
-				"limit_per_port", ONE_EXACTLY, expect_int(assign_variable_callback(limit_per_port)),
-				"supply_consumption_score", ONE_EXACTLY,
-					expect_fixed_point(assign_variable_callback(supply_consumption_score)),
-				"hull", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(hull)),
-				"gun_power", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(gun_power)),
-				"fire_range", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(fire_range)),
-				"evasion", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(evasion)),
-				"torpedo_attack", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(torpedo_attack))
+				"naval_icon", ONE_EXACTLY, expect_uint(assign_variable_callback(naval_unit_args.naval_icon)),
+				"sail", ZERO_OR_ONE, expect_bool(assign_variable_callback(naval_unit_args.sail)),
+				"transport", ZERO_OR_ONE, expect_bool(assign_variable_callback(naval_unit_args.transport)),
+				"capital", ZERO_OR_ONE, expect_bool(assign_variable_callback(naval_unit_args.capital)),
+				"colonial_points", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(naval_unit_args.colonial_points)),
+				"can_build_overseas", ZERO_OR_ONE, expect_bool(assign_variable_callback(naval_unit_args.build_overseas)),
+				"min_port_level", ONE_EXACTLY, expect_uint(assign_variable_callback(naval_unit_args.min_port_level)),
+				"limit_per_port", ONE_EXACTLY, expect_int(assign_variable_callback(naval_unit_args.limit_per_port)),
+				"supply_consumption_score", ZERO_OR_ONE,
+					expect_fixed_point(assign_variable_callback(naval_unit_args.supply_consumption_score)),
+				"hull", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(naval_unit_args.hull)),
+				"gun_power", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(naval_unit_args.gun_power)),
+				"fire_range", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(naval_unit_args.fire_range)),
+				"evasion", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(naval_unit_args.evasion)),
+				"torpedo_attack", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(naval_unit_args.torpedo_attack))
 			);
 
-			ret &= expect_dictionary_key_map(key_map)(value);
+			ret &= expect_dictionary_key_map_and_default(key_map, add_terrain_modifier)(value);
 
-			ret &= add_naval_unit(key, UNIT_ARGS, NAVY_ARGS);
+			ret &= add_naval_unit(key, unit_args, naval_unit_args);
 
 			return ret;
 		}
-			default: Logger::error("Unknown unit type for ", key, ": ", static_cast<int>(type)); return false;
+		default:
+			/* Unreachable - an earlier check terminates the function if branch isn't LAND or NAVAL. */
+			Logger::error("Unknown branch for unit ", key, ": ", static_cast<int>(branch));
+			return false;
 		}
 	})(root);
 }
@@ -202,7 +286,7 @@ bool UnitManager::load_unit_file(GoodManager const& good_manager, ast::NodeCPtr
 bool UnitManager::generate_modifiers(ModifierManager& modifier_manager) const {
 	bool ret = true;
 
-	const auto generate_stat_modifiers = [&modifier_manager, &ret](std::string_view identifier, Unit::type_t type) -> void {
+	const auto generate_stat_modifiers = [&modifier_manager, &ret](std::string_view identifier, Unit::branch_t branch) -> void {
 		const auto stat_modifier = [&modifier_manager, &ret, &identifier](
 			std::string_view suffix, bool is_positive_good, ModifierEffect::format_t format
 		) -> void {
@@ -222,15 +306,15 @@ bool UnitManager::generate_modifiers(ModifierManager& modifier_manager) const {
 		stat_modifier("build_time", false, INT);
 		stat_modifier("supply_consumption", false, PROPORTION_DECIMAL);
 
-		switch (type) {
-		case Unit::type_t::LAND:
+		switch (branch) {
+		case LAND:
 			stat_modifier("reconnaissance", true, RAW_DECIMAL);
 			stat_modifier("discipline", true, PROPORTION_DECIMAL);
 			stat_modifier("support", true, PROPORTION_DECIMAL);
 			stat_modifier("maneuver", true, INT);
 			stat_modifier("siege", true, RAW_DECIMAL);
 			break;
-		case Unit::type_t::NAVAL:
+		case NAVAL:
 			stat_modifier("colonial_points", true, INT);
 			stat_modifier("supply_consumption_score", false, INT);
 			stat_modifier("hull", true, RAW_DECIMAL);
@@ -239,13 +323,21 @@ bool UnitManager::generate_modifiers(ModifierManager& modifier_manager) const {
 			stat_modifier("evasion", true, PROPORTION_DECIMAL);
 			stat_modifier("torpedo_attack", true, RAW_DECIMAL);
 			break;
+		default:
+			/* Unreachable - units are only added via add_land_unit or add_naval_unit which set branch to LAND or NAVAL. */
+			Logger::error("Invalid branch for unit ", identifier, ": ", static_cast<int>(branch));
 		}
 	};
 
-	generate_stat_modifiers("army_base", Unit::type_t::LAND);
-	generate_stat_modifiers("navy_base", Unit::type_t::NAVAL);
-	for (Unit const& unit : get_units()) {
-		generate_stat_modifiers(unit.get_identifier(), unit.get_type());
+	generate_stat_modifiers("army_base", LAND);
+	for (LandUnit const& land_unit : get_land_units()) {
+		generate_stat_modifiers(land_unit.get_identifier(), LAND);
+	}
+
+	generate_stat_modifiers("navy_base", NAVAL);
+	for (NavalUnit const& naval_unit : get_naval_units()) {
+		generate_stat_modifiers(naval_unit.get_identifier(), NAVAL);
 	}
+
 	return ret;
 }
diff --git a/src/openvic-simulation/military/Unit.hpp b/src/openvic-simulation/military/Unit.hpp
index c791aca..d1ee3e3 100644
--- a/src/openvic-simulation/military/Unit.hpp
+++ b/src/openvic-simulation/military/Unit.hpp
@@ -10,34 +10,42 @@
 #include "openvic-simulation/types/IdentifierRegistry.hpp"
 #include "openvic-simulation/types/fixed_point/FixedPoint.hpp"
 
-#define UNIT_PARAMS \
-	Unit::icon_t icon, std::string_view sprite, bool active, std::string_view unit_type, bool floating_flag, \
-	uint32_t priority, fixed_point_t max_strength, fixed_point_t default_organisation, fixed_point_t maximum_speed, \
-	fixed_point_t weighted_value, std::string_view move_sound, std::string_view select_sound, Timespan build_time, \
-	Good::good_map_t&& build_cost, fixed_point_t supply_consumption, Good::good_map_t&& supply_cost
-
-#define LAND_PARAMS \
-	LandUnit::allowed_cultures_t allowed_cultures, std::string_view sprite_override, std::string_view sprite_mount, \
-	std::string_view sprite_mount_attach_node, fixed_point_t reconnaissance, fixed_point_t attack, fixed_point_t defence, \
-	fixed_point_t discipline, fixed_point_t support, fixed_point_t maneuver, fixed_point_t siege
-
-#define NAVY_PARAMS \
-	Unit::icon_t naval_icon, bool sail, bool transport, bool capital, fixed_point_t colonial_points, bool build_overseas, \
-	uint32_t min_port_level, int32_t limit_per_port, fixed_point_t supply_consumption_score, fixed_point_t hull, \
-	fixed_point_t gun_power, fixed_point_t fire_range, fixed_point_t evasion, fixed_point_t torpedo_attack
-
 namespace OpenVic {
+	struct TerrainType;
+	struct TerrainTypeManager;
+
 	struct Unit : HasIdentifier {
 		using icon_t = uint32_t;
-
-		enum struct type_t { LAND, NAVAL };
+		using terrain_modifiers_t = ordered_map<TerrainType const*, ModifierValue>;
+
+		enum struct branch_t : uint8_t { INVALID_BRANCH, LAND, NAVAL };
+		enum struct unit_type_t : uint8_t {
+			INVALID_UNIT_TYPE, INFANTRY, CAVALRY, SUPPORT, SPECIAL, BIG_SHIP, LIGHT_SHIP, TRANSPORT
+		};
+
+		struct unit_args_t {
+			icon_t icon = 0;
+			unit_type_t unit_type = unit_type_t::INVALID_UNIT_TYPE;
+			// TODO defaults for move_sound and select_sound
+			std::string_view sprite, move_sound, select_sound;
+			bool active = true, floating_flag = false;
+			uint32_t priority = 0;
+			fixed_point_t max_strength = 0, default_organisation = 0, maximum_speed = 0, weighted_value = 0,
+				supply_consumption = 0;
+			Timespan build_time;
+			Good::good_map_t build_cost, supply_cost;
+			terrain_modifiers_t terrain_modifiers;
+
+			unit_args_t() = default;
+			unit_args_t(unit_args_t&&) = default;
+		};
 
 	private:
-		const type_t PROPERTY(type);
+		const branch_t PROPERTY(branch); /* type in defines */
 		const icon_t PROPERTY(icon);
-		const std::string PROPERTY(sprite);
+		std::string PROPERTY(sprite);
 		const bool PROPERTY_CUSTOM_PREFIX(active, is);
-		const std::string PROPERTY(unit_type);
+		unit_type_t PROPERTY(unit_type);
 		const bool PROPERTY_CUSTOM_PREFIX(floating_flag, has);
 
 		const uint32_t PROPERTY(priority);
@@ -46,16 +54,19 @@ namespace OpenVic {
 		const fixed_point_t PROPERTY(maximum_speed);
 		const fixed_point_t PROPERTY(weighted_value);
 
-		const std::string PROPERTY(move_sound);
-		const std::string PROPERTY(select_sound);
+		std::string PROPERTY(move_sound);
+		std::string PROPERTY(select_sound);
 
 		const Timespan PROPERTY(build_time);
-		const Good::good_map_t PROPERTY(build_cost);
+		Good::good_map_t PROPERTY(build_cost);
 		const fixed_point_t PROPERTY(supply_consumption);
-		const Good::good_map_t PROPERTY(supply_cost);
+		Good::good_map_t PROPERTY(supply_cost);
+
+		terrain_modifiers_t PROPERTY(terrain_modifiers);
 
 	protected:
-		Unit(std::string_view identifier, type_t type, UNIT_PARAMS);
+		/* Non-const reference unit_args so variables can be moved from it. */
+		Unit(std::string_view new_identifier, branch_t new_branch, unit_args_t& unit_args);
 
 	public:
 		Unit(Unit&&) = default;
@@ -66,11 +77,23 @@ namespace OpenVic {
 
 		enum struct allowed_cultures_t { ALL_CULTURES, ACCEPTED_CULTURES, PRIMARY_CULTURE };
 
+		struct land_unit_args_t {
+			allowed_cultures_t allowed_cultures = allowed_cultures_t::ALL_CULTURES;
+			std::string_view sprite_override, sprite_mount, sprite_mount_attach_node;
+			// TODO - represent these as modifier effects, so that they can be combined with tech, inventions,
+			// leader bonuses, etc. and applied to unit instances all in one go (same for NavalUnits below)
+			fixed_point_t reconnaissance = 0, attack = 0, defence = 0, discipline = 0, support = 0, maneuver = 0,
+				siege = 0;
+
+			land_unit_args_t() = default;
+			land_unit_args_t(land_unit_args_t&&) = default;
+		};
+
 	private:
 		const allowed_cultures_t PROPERTY(allowed_cultures);
-		const std::string PROPERTY(sprite_override);
-		const std::string PROPERTY(sprite_mount);
-		const std::string PROPERTY(sprite_mount_attach_node);
+		std::string PROPERTY(sprite_override);
+		std::string PROPERTY(sprite_mount);
+		std::string PROPERTY(sprite_mount_attach_node);
 		const fixed_point_t PROPERTY(reconnaissance);
 		const fixed_point_t PROPERTY(attack);
 		const fixed_point_t PROPERTY(defence);
@@ -79,7 +102,7 @@ namespace OpenVic {
 		const fixed_point_t PROPERTY(maneuver);
 		const fixed_point_t PROPERTY(siege);
 
-		LandUnit(std::string_view identifier, UNIT_PARAMS, LAND_PARAMS);
+		LandUnit(std::string_view new_identifier, unit_args_t& unit_args, land_unit_args_t const& land_unit_args);
 
 	public:
 		LandUnit(LandUnit&&) = default;
@@ -88,6 +111,18 @@ namespace OpenVic {
 	struct NavalUnit : Unit {
 		friend struct UnitManager;
 
+		struct naval_unit_args_t {
+			Unit::icon_t naval_icon = 0;
+			bool sail = false, transport = false, capital = false, build_overseas = false;
+			uint32_t min_port_level = 0;
+			int32_t limit_per_port = 0;
+			fixed_point_t colonial_points = 0, supply_consumption_score = 0, hull = 0, gun_power = 0, fire_range = 0,
+				evasion = 0, torpedo_attack = 0;
+
+			naval_unit_args_t() = default;
+			naval_unit_args_t(naval_unit_args_t&&) = default;
+		};
+
 	private:
 		const icon_t PROPERTY(naval_icon);
 		const bool PROPERTY_CUSTOM_PREFIX(sail, can);
@@ -105,7 +140,7 @@ namespace OpenVic {
 		const fixed_point_t PROPERTY(evasion);
 		const fixed_point_t PROPERTY(torpedo_attack);
 
-		NavalUnit(std::string_view identifier, UNIT_PARAMS, NAVY_PARAMS);
+		NavalUnit(std::string_view new_identifier, unit_args_t& unit_args, naval_unit_args_t const& naval_unit_args);
 
 	public:
 		NavalUnit(NavalUnit&&) = default;
@@ -113,17 +148,34 @@ namespace OpenVic {
 
 	struct UnitManager {
 	private:
-		IdentifierRegistry<Unit> IDENTIFIER_REGISTRY(unit);
-
-		bool _check_shared_parameters(std::string_view identifier, UNIT_PARAMS);
+		IdentifierPointerRegistry<Unit> IDENTIFIER_REGISTRY(unit);
+		IdentifierRegistry<LandUnit> IDENTIFIER_REGISTRY(land_unit);
+		IdentifierRegistry<NavalUnit> IDENTIFIER_REGISTRY(naval_unit);
 
 	public:
-		bool add_land_unit(std::string_view identifier, UNIT_PARAMS, LAND_PARAMS);
-		bool add_naval_unit(std::string_view identifier, UNIT_PARAMS, NAVY_PARAMS);
-
-		static NodeTools::callback_t<std::string_view> expect_type_str(NodeTools::Callback<Unit::type_t> auto callback);
-
-		bool load_unit_file(GoodManager const& good_manager, ast::NodeCPtr root);
+		void reserve_all_units(size_t size);
+		void lock_all_units();
+
+		bool add_land_unit(
+			std::string_view identifier, Unit::unit_args_t& unit_args, LandUnit::land_unit_args_t const& land_unit_args
+		);
+		bool add_naval_unit(
+			std::string_view identifier, Unit::unit_args_t& unit_args, NavalUnit::naval_unit_args_t const& naval_unit_args
+		);
+
+		static NodeTools::Callback<std::string_view> auto expect_branch_str(NodeTools::Callback<Unit::branch_t> auto callback) {
+			using enum Unit::branch_t;
+			static const string_map_t<Unit::branch_t> branch_map { { "land", LAND }, { "naval", NAVAL }, { "sea", NAVAL } };
+			return NodeTools::expect_mapped_string(branch_map, callback);
+		}
+		static NodeTools::NodeCallback auto expect_branch_identifier(NodeTools::Callback<Unit::branch_t> auto callback) {
+			return NodeTools::expect_identifier(expect_branch_str(callback));
+		}
+
+		bool load_unit_file(
+			GoodManager const& good_manager, TerrainTypeManager const& terrain_type_manager,
+			ModifierManager const& modifier_manager, ast::NodeCPtr root
+		);
 		bool generate_modifiers(ModifierManager& modifier_manager) const;
 	};
 }