Minor cleanup

13 files changed, 99 insertions, 137 deletions

diff --git a/src/openvic-simulation/Modifier.cpp b/src/openvic-simulation/Modifier.cpp
index 912b805..d0f3a69 100644
--- a/src/openvic-simulation/Modifier.cpp
+++ b/src/openvic-simulation/Modifier.cpp
@@ -299,13 +299,13 @@ node_callback_t ModifierManager::expect_modifier_value(callback_t<ModifierValue&
 	return expect_modifier_value_and_default(modifier_callback, key_value_invalid_callback);
 }
 
-node_callback_t ModifierManager::expect_whitelisted_modifier_value_and_default(callback_t<ModifierValue&&> modifier_callback, std::set<std::string, std::less<void>> const& whitelist, key_value_callback_t default_callback) const {
+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) const {
 	return expect_validated_modifier_value_and_default(modifier_callback, default_callback, [&whitelist](ModifierEffect const& effect) -> bool {
 		return whitelist.contains(effect.get_identifier());
 	});
 }
 
-node_callback_t ModifierManager::expect_whitelisted_modifier_value(callback_t<ModifierValue&&> modifier_callback, std::set<std::string, std::less<void>> const& whitelist) const {
+node_callback_t ModifierManager::expect_whitelisted_modifier_value(callback_t<ModifierValue&&> modifier_callback, string_set_t const& whitelist) const {
 	return expect_whitelisted_modifier_value_and_default(modifier_callback, whitelist, key_value_invalid_callback);
 }
 
diff --git a/src/openvic-simulation/Modifier.hpp b/src/openvic-simulation/Modifier.hpp
index c6b15a6..f936e5d 100644
--- a/src/openvic-simulation/Modifier.hpp
+++ b/src/openvic-simulation/Modifier.hpp
@@ -1,7 +1,5 @@
 #pragma once
 
-#include <set>
-
 #include "openvic-simulation/types/IdentifierRegistry.hpp"
 
 namespace OpenVic {
@@ -126,8 +124,8 @@ namespace OpenVic {
 		NodeTools::node_callback_t expect_modifier_value_and_default(NodeTools::callback_t<ModifierValue&&> modifier_callback, NodeTools::key_value_callback_t default_callback) 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, std::set<std::string, std::less<void>> const& whitelist, NodeTools::key_value_callback_t default_callback) const;
-		NodeTools::node_callback_t expect_whitelisted_modifier_value(NodeTools::callback_t<ModifierValue&&> modifier_callback, std::set<std::string, std::less<void>> const& whitelist) 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) const;
+		NodeTools::node_callback_t expect_whitelisted_modifier_value(NodeTools::callback_t<ModifierValue&&> modifier_callback, string_set_t const& whitelist) const;
 
 		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) const;
 		NodeTools::node_callback_t expect_modifier_value_and_key_map(NodeTools::callback_t<ModifierValue&&> modifier_callback, NodeTools::key_map_t&& key_map) const;
diff --git a/src/openvic-simulation/dataloader/Dataloader.cpp b/src/openvic-simulation/dataloader/Dataloader.cpp
index 475cdd1..cfc944d 100644
--- a/src/openvic-simulation/dataloader/Dataloader.cpp
+++ b/src/openvic-simulation/dataloader/Dataloader.cpp
@@ -48,7 +48,7 @@ static constexpr bool path_equals(std::string_view lhs, std::string_view rhs) {
 template<typename T>
 concept is_filename = std::same_as<T, std::filesystem::path> || std::convertible_to<T, std::string_view>;
 
-bool filename_equals(const is_filename auto& lhs, const is_filename auto& rhs) {
+static bool filename_equals(const is_filename auto& lhs, const is_filename auto& rhs) {
 	auto left = [&lhs] {
 		if constexpr (std::same_as<std::decay_t<decltype(lhs)>, std::filesystem::path>)
 			return lhs.filename().string();
diff --git a/src/openvic-simulation/dataloader/NodeTools.hpp b/src/openvic-simulation/dataloader/NodeTools.hpp
index f06a85f..692e2cb 100644
--- a/src/openvic-simulation/dataloader/NodeTools.hpp
+++ b/src/openvic-simulation/dataloader/NodeTools.hpp
@@ -4,6 +4,7 @@
 #include <cstdint>
 #include <functional>
 #include <map>
+#include <set>
 #include <type_traits>
 
 #include <openvic-dataloader/v2script/AbstractSyntaxTree.hpp>
@@ -16,10 +17,13 @@ namespace OpenVic {
 	namespace ast = ovdl::v2script::ast;
 
 	/* Template for map from strings to Ts, in which string_views can be
-	 * searched for without needing to be copied into a string, */
+	 * searched for without needing to be copied into a string */
 	template<typename T>
 	using string_map_t = std::map<std::string, T, std::less<void>>;
 
+	/* String set type supporting heterogeneous key lookup */
+	using string_set_t = std::set<std::string, std::less<void>>;
+
 	namespace NodeTools {
 
 		template<typename Fn, typename Return = void, typename ...Args>
diff --git a/src/openvic-simulation/map/Map.cpp b/src/openvic-simulation/map/Map.cpp
index 777579e..13d0933 100644
--- a/src/openvic-simulation/map/Map.cpp
+++ b/src/openvic-simulation/map/Map.cpp
@@ -200,14 +200,6 @@ std::vector<Map::shape_pixel_t> const& Map::get_province_shape_image() const {
 	return province_shape_image;
 }
 
-TerrainTypeManager& Map::get_terrain_type_manager() {
-	return terrain_type_manager;
-}
-
-TerrainTypeManager const& Map::get_terrain_type_manager() const {
-	return terrain_type_manager;
-}
-
 bool Map::add_mapmode(std::string_view identifier, Mapmode::colour_func_t colour_func) {
 	if (identifier.empty()) {
 		Logger::error("Invalid mapmode identifier - empty!");
diff --git a/src/openvic-simulation/map/Map.hpp b/src/openvic-simulation/map/Map.hpp
index 578b471..61b7b48 100644
--- a/src/openvic-simulation/map/Map.hpp
+++ b/src/openvic-simulation/map/Map.hpp
@@ -89,8 +89,7 @@ namespace OpenVic {
 		size_t get_width() const;
 		size_t get_height() const;
 		std::vector<shape_pixel_t> const& get_province_shape_image() const;
-		TerrainTypeManager& get_terrain_type_manager();
-		TerrainTypeManager const& get_terrain_type_manager() const;
+		REF_GETTERS(terrain_type_manager)
 
 		bool add_region(std::string_view identifier, std::vector<std::string_view> const& province_identifiers);
 		IDENTIFIER_REGISTRY_ACCESSORS(region)
diff --git a/src/openvic-simulation/military/LeaderTrait.cpp b/src/openvic-simulation/military/LeaderTrait.cpp
index 27f81c5..b90a399 100644
--- a/src/openvic-simulation/military/LeaderTrait.cpp
+++ b/src/openvic-simulation/military/LeaderTrait.cpp
@@ -3,58 +3,51 @@
 using namespace OpenVic;
 using namespace OpenVic::NodeTools;
 
-LeaderTrait::LeaderTrait(std::string_view new_identifier, trait_type_t new_type, ModifierValue new_modifiers)
-    : HasIdentifier { new_identifier }, type { new_type }, modifiers { new_modifiers } {}
+LeaderTrait::LeaderTrait(std::string_view new_identifier, trait_type_t new_type, ModifierValue&& new_modifiers)
+	: HasIdentifier { new_identifier }, type { new_type }, modifiers { std::move(new_modifiers) } {}
 
-trait_type_t LeaderTrait::get_trait_type() const {
-    return type;
+LeaderTrait::trait_type_t LeaderTrait::get_trait_type() const {
+	return type;
 }
 
 bool LeaderTrait::is_personality_trait() const {
-    return type == trait_type_t::PERSONALITY;
+	return type == trait_type_t::PERSONALITY;
 }
 
 bool LeaderTrait::is_background_trait() const {
-    return type == trait_type_t::BACKGROUND;
+	return type == trait_type_t::BACKGROUND;
 }
 
-ModifierValue LeaderTrait::get_modifiers() const {
-    return modifiers;
+ModifierValue const& LeaderTrait::get_modifiers() const {
+	return modifiers;
 }
 
 LeaderTraitManager::LeaderTraitManager() : leader_traits { "leader_traits" } {}
 
-bool LeaderTraitManager::add_leader_trait(std::string_view identifier, trait_type_t type, ModifierValue modifiers) {
-    if (identifier.empty()) {
-        Logger::error("Invalid leader trait identifier - empty!");
-        return false;
-    }
+bool LeaderTraitManager::add_leader_trait(std::string_view identifier, LeaderTrait::trait_type_t type, ModifierValue&& modifiers) {
+	if (identifier.empty()) {
+		Logger::error("Invalid leader trait identifier - empty!");
+		return false;
+	}
 
-    return leader_traits.add_item({ identifier, type, modifiers });
+	return leader_traits.add_item({ identifier, type, std::move(modifiers) });
 }
 
-bool LeaderTraitManager::load_leader_traits_file(ModifierManager& modifier_manager, ast::NodeCPtr root) {
-    bool ret = expect_dictionary_keys(
-        "personality", ONE_EXACTLY, expect_dictionary(
-            [this, &modifier_manager](std::string_view trait_identifier, ast::NodeCPtr value) -> bool {
-                ModifierValue modifiers;
-
-                bool ret = modifier_manager.expect_whitelisted_modifier_value(move_variable_callback(modifiers), allowed_modifiers)(value);
-                ret &= add_leader_trait(trait_identifier, trait_type_t::PERSONALITY, modifiers);
-                return ret;
-            }
-        ),
-        "background", ONE_EXACTLY, expect_dictionary(
-            [this, &modifier_manager](std::string_view trait_identifier, ast::NodeCPtr value) -> bool {
-                ModifierValue modifiers;
-
-                bool ret = modifier_manager.expect_whitelisted_modifier_value(move_variable_callback(modifiers), allowed_modifiers)(value);
-                ret &= add_leader_trait(trait_identifier, trait_type_t::BACKGROUND, modifiers);
-                return ret;
-            }
-        )
-    )(root);
-    lock_leader_traits();
-
-    return ret;
-}
\ No newline at end of file
+bool LeaderTraitManager::load_leader_traits_file(ModifierManager const& modifier_manager, ast::NodeCPtr root) {
+	using enum LeaderTrait::trait_type_t;
+	const auto trait_callback = [this, &modifier_manager](LeaderTrait::trait_type_t type) -> key_value_callback_t {
+		return [this, &modifier_manager, type](std::string_view trait_identifier, ast::NodeCPtr value) -> bool {
+			ModifierValue modifiers;
+			bool ret = modifier_manager.expect_whitelisted_modifier_value(move_variable_callback(modifiers), allowed_modifiers)(value);
+			ret &= add_leader_trait(trait_identifier, type, std::move(modifiers));
+			return ret;
+		};
+	};
+	const bool ret = expect_dictionary_keys(
+		"personality", ONE_EXACTLY, expect_dictionary(trait_callback(PERSONALITY)),
+		"background", ONE_EXACTLY, expect_dictionary(trait_callback(BACKGROUND))
+	)(root);
+	lock_leader_traits();
+
+	return ret;
+}
diff --git a/src/openvic-simulation/military/LeaderTrait.hpp b/src/openvic-simulation/military/LeaderTrait.hpp
index f4445f6..e525e23 100644
--- a/src/openvic-simulation/military/LeaderTrait.hpp
+++ b/src/openvic-simulation/military/LeaderTrait.hpp
@@ -8,65 +8,65 @@
 #include "openvic-simulation/Modifier.hpp"
 
 namespace OpenVic {
-    struct LeaderTraitManager;
+	struct LeaderTraitManager;
 
-    enum class trait_type_t {
-        PERSONALITY,
-        BACKGROUND
-    };
+	struct LeaderTrait : HasIdentifier {
+		friend struct LeaderTraitManager;
 
-    struct LeaderTrait : HasIdentifier {
-        friend struct LeaderTraitManager;
+		enum class trait_type_t {
+			PERSONALITY,
+			BACKGROUND
+		};
 
-    private:
+	private:
 
-        const trait_type_t type;
-        /* 
-         * Allowed modifiers for leaders:
-         * attack - integer
-         * defence - integer
-         * morale - %
-         * organisation - %
-         * reconnaissance - %
-         * speed - %
-         * attrition - %, negative is good
-         * experience - %
-         * reliability - decimal, mil gain or loss for associated POPs
-         */
-        const ModifierValue modifiers;
+		const trait_type_t type;
+		/*
+		 * Allowed modifiers for leaders:
+		 * attack - integer
+		 * defence - integer
+		 * morale - %
+		 * organisation - %
+		 * reconnaissance - %
+		 * speed - %
+		 * attrition - %, negative is good
+		 * experience - %
+		 * reliability - decimal, mil gain or loss for associated POPs
+		 */
+		const ModifierValue modifiers;
 
-        LeaderTrait(std::string_view new_identifier, trait_type_t new_type, ModifierValue new_modifiers);
-        
-    public:
-        LeaderTrait(LeaderTrait&&) = default;
+		LeaderTrait(std::string_view new_identifier, trait_type_t new_type, ModifierValue&& new_modifiers);
 
-        trait_type_t get_trait_type() const;
-        bool is_personality_trait() const;
-        bool is_background_trait() const;
-        ModifierValue get_modifiers() const;
-    };
+	public:
+		LeaderTrait(LeaderTrait&&) = default;
 
-    struct LeaderTraitManager {
-    private:
-        IdentifierRegistry<LeaderTrait> leader_traits;
-        const std::set<std::string, std::less<void>> allowed_modifiers = {
-            "attack",
-            "defence",
-            "morale",
-            "organisation",
-            "reconnaissance",
-            "speed",
-            "attrition",
-            "experience",
-            "reliability"
-        };
-    
-    public:
-        LeaderTraitManager();
+		trait_type_t get_trait_type() const;
+		bool is_personality_trait() const;
+		bool is_background_trait() const;
+		ModifierValue const& get_modifiers() const;
+	};
 
-        bool add_leader_trait(std::string_view identifier, trait_type_t type, ModifierValue modifiers);
-        IDENTIFIER_REGISTRY_ACCESSORS(leader_trait)
+	struct LeaderTraitManager {
+	private:
+		IdentifierRegistry<LeaderTrait> leader_traits;
+		inline static const string_set_t allowed_modifiers = {
+			"attack",
+			"defence",
+			"morale",
+			"organisation",
+			"reconnaissance",
+			"speed",
+			"attrition",
+			"experience",
+			"reliability"
+		};
 
-        bool load_leader_traits_file(ModifierManager& modifier_manager, ast::NodeCPtr root);
-    };
+	public:
+		LeaderTraitManager();
+
+		bool add_leader_trait(std::string_view identifier, LeaderTrait::trait_type_t type, ModifierValue&& modifiers);
+		IDENTIFIER_REGISTRY_ACCESSORS(leader_trait)
+
+		bool load_leader_traits_file(ModifierManager const& modifier_manager, ast::NodeCPtr root);
+	};
 } // namespace OpenVic
diff --git a/src/openvic-simulation/military/Unit.cpp b/src/openvic-simulation/military/Unit.cpp
index 8b010e3..294c77f 100644
--- a/src/openvic-simulation/military/Unit.cpp
+++ b/src/openvic-simulation/military/Unit.cpp
@@ -1,7 +1,5 @@
 #include "Unit.hpp"
 
-#include <set>
-
 #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)
diff --git a/src/openvic-simulation/politics/Government.cpp b/src/openvic-simulation/politics/Government.cpp
index c65abc6..927c773 100644
--- a/src/openvic-simulation/politics/Government.cpp
+++ b/src/openvic-simulation/politics/Government.cpp
@@ -1,7 +1,5 @@
 #include "Government.hpp"
 
-#include <set>
-
 #include "openvic-simulation/GameManager.hpp"
 
 using namespace OpenVic;
@@ -77,7 +75,7 @@ bool GovernmentTypeManager::load_government_types_file(IdeologyManager const& id
 
 			ret &= expect_dictionary(
 				[this, &ideology_manager, &ideologies, government_type_identifier](std::string_view key, ast::NodeCPtr value) -> bool {
-					static const std::set<std::string, std::less<void>> reserved_keys = {
+					static const string_set_t reserved_keys = {
 						"election", "duration", "appoint_ruling_party", "flagType"
 					};
 					if (reserved_keys.find(key) != reserved_keys.end()) return true;
diff --git a/src/openvic-simulation/pop/Culture.cpp b/src/openvic-simulation/pop/Culture.cpp
index bbfa35a..8a0ca37 100644
--- a/src/openvic-simulation/pop/Culture.cpp
+++ b/src/openvic-simulation/pop/Culture.cpp
@@ -1,7 +1,5 @@
 #include "Culture.hpp"
 
-#include <set>
-
 #include "openvic-simulation/dataloader/NodeTools.hpp"
 
 using namespace OpenVic;
@@ -193,7 +191,7 @@ bool CultureManager::load_culture_file(ast::NodeCPtr root) {
 			CultureGroup const* culture_group = get_culture_group_by_identifier(culture_group_key);
 			return expect_dictionary(
 				[this, culture_group](std::string_view key, ast::NodeCPtr value) -> bool {
-					static const std::set<std::string, std::less<void>> reserved_keys = {
+					static const string_set_t reserved_keys = {
 						"leader", "unit", "union", "is_overseas"
 					};
 					if (reserved_keys.find(key) != reserved_keys.end()) return true;
diff --git a/src/openvic-simulation/pop/Pop.cpp b/src/openvic-simulation/pop/Pop.cpp
index dc53201..5b1ce02 100644
--- a/src/openvic-simulation/pop/Pop.cpp
+++ b/src/openvic-simulation/pop/Pop.cpp
@@ -123,22 +123,6 @@ bool PopType::get_is_slave() const {
 
 PopManager::PopManager() : pop_types { "pop types" } {}
 
-CultureManager& PopManager::get_culture_manager() {
-	return culture_manager;
-}
-
-CultureManager const& PopManager::get_culture_manager() const {
-	return culture_manager;
-}
-
-ReligionManager& PopManager::get_religion_manager() {
-	return religion_manager;
-}
-
-ReligionManager const& PopManager::get_religion_manager() const {
-	return religion_manager;
-}
-
 bool PopManager::add_pop_type(std::string_view identifier, colour_t colour, PopType::strata_t strata,
 	PopType::sprite_t sprite, Good::good_map_t&& life_needs, Good::good_map_t&& everyday_needs,
 	Good::good_map_t&& luxury_needs, PopType::rebel_units_t&& rebel_units, Pop::pop_size_t max_size,
diff --git a/src/openvic-simulation/pop/Pop.hpp b/src/openvic-simulation/pop/Pop.hpp
index 26f89db..7261831 100644
--- a/src/openvic-simulation/pop/Pop.hpp
+++ b/src/openvic-simulation/pop/Pop.hpp
@@ -99,10 +99,8 @@ namespace OpenVic {
 	public:
 		PopManager();
 
-		CultureManager& get_culture_manager();
-		CultureManager const& get_culture_manager() const;
-		ReligionManager& get_religion_manager();
-		ReligionManager const& get_religion_manager() const;
+		REF_GETTERS(culture_manager)
+		REF_GETTERS(religion_manager)
 
 		bool add_pop_type(std::string_view identifier, colour_t new_colour, PopType::strata_t strata, PopType::sprite_t sprite,
 			Good::good_map_t&& life_needs, Good::good_map_t&& everyday_needs, Good::good_map_t&& luxury_needs,