Merge pull request #85 from OpenVicProject/container-callbacks

Container (pointer) callbacks + format cleanup

34 files changed, 290 insertions, 281 deletions

diff --git a/src/openvic-simulation/GameManager.cpp b/src/openvic-simulation/GameManager.cpp
index 5b74ec4..5142c66 100644
--- a/src/openvic-simulation/GameManager.cpp
+++ b/src/openvic-simulation/GameManager.cpp
@@ -42,7 +42,7 @@ bool GameManager::reset() {
 	clock.reset();
 	today = {};
 	economy_manager.get_good_manager().reset_to_defaults();
-	bool ret = map.reset(economy_manager.get_building_manager());
+	bool ret = map.reset(economy_manager.get_building_type_manager());
 	set_needs_update();
 	return ret;
 }
diff --git a/src/openvic-simulation/GameManager.hpp b/src/openvic-simulation/GameManager.hpp
index 0e8eb2b..e9a9284 100644
--- a/src/openvic-simulation/GameManager.hpp
+++ b/src/openvic-simulation/GameManager.hpp
@@ -1,18 +1,19 @@
 #pragma once
 
-#include "openvic-simulation/misc/GameAdvancementHook.hpp"
-#include "openvic-simulation/misc/Modifier.hpp"
 #include "openvic-simulation/country/Country.hpp"
 #include "openvic-simulation/economy/EconomyManager.hpp"
 #include "openvic-simulation/history/HistoryManager.hpp"
 #include "openvic-simulation/interface/UI.hpp"
+#include "openvic-simulation/map/Crime.hpp"
 #include "openvic-simulation/map/Map.hpp"
 #include "openvic-simulation/military/MilitaryManager.hpp"
 #include "openvic-simulation/misc/Define.hpp"
+#include "openvic-simulation/misc/GameAdvancementHook.hpp"
+#include "openvic-simulation/misc/Modifier.hpp"
 #include "openvic-simulation/politics/PoliticsManager.hpp"
 #include "openvic-simulation/pop/Pop.hpp"
+#include "openvic-simulation/research/ResearchManager.hpp"
 #include "openvic-simulation/research/Technology.hpp"
-#include "research/ResearchManager.hpp"
 
 namespace OpenVic {
 	struct GameManager {
@@ -29,6 +30,7 @@ namespace OpenVic {
 		ResearchManager PROPERTY_REF(research_manager);
 		PopManager PROPERTY_REF(pop_manager);
 		CountryManager PROPERTY_REF(country_manager);
+		CrimeManager PROPERTY_REF(crime_manager);
 		UIManager PROPERTY_REF(ui_manager);
 		GameAdvancementHook PROPERTY_REF(clock);
 
diff --git a/src/openvic-simulation/dataloader/Dataloader.cpp b/src/openvic-simulation/dataloader/Dataloader.cpp
index 11b4332..c986a78 100644
--- a/src/openvic-simulation/dataloader/Dataloader.cpp
+++ b/src/openvic-simulation/dataloader/Dataloader.cpp
@@ -349,17 +349,13 @@ bool Dataloader::_load_rebel_types(GameManager& game_manager) const {
 	PoliticsManager& politics_manager = game_manager.get_politics_manager();
 	RebelManager& rebel_manager = politics_manager.get_rebel_manager();
 
-	bool ret = rebel_manager.load_rebels_file(
-		politics_manager.get_ideology_manager(),
-		politics_manager.get_government_type_manager(),
-		parse_defines(lookup_file(rebel_types_file)).get_file_node()
-	);
+	bool ret = politics_manager.load_rebels_file(parse_defines(lookup_file(rebel_types_file)).get_file_node());
 
 	if(!rebel_manager.generate_modifiers(game_manager.get_modifier_manager())) {
 		Logger::error("Failed to generate rebel type-based modifiers!");
 		ret &= false;
 	}
-	
+
 	return ret;
 }
 
@@ -396,7 +392,7 @@ bool Dataloader::_load_technologies(GameManager& game_manager) const {
 			return technology_manager.load_technologies_file(
 				modifier_manager,
 				game_manager.get_military_manager().get_unit_manager(),
-				game_manager.get_economy_manager().get_building_manager(),
+				game_manager.get_economy_manager().get_building_type_manager(),
 				parse_defines(file).get_file_node()
 			);
 		}
@@ -420,12 +416,13 @@ bool Dataloader::_load_inventions(GameManager& game_manager) const {
 			return invention_manager.load_inventions_file(
 				game_manager.get_modifier_manager(),
 				game_manager.get_military_manager().get_unit_manager(),
-				game_manager.get_economy_manager().get_building_manager(),
+				game_manager.get_economy_manager().get_building_type_manager(),
+				game_manager.get_crime_manager(),
 				parse_defines(file).get_file_node()
 			);
 		}
 	);
-	
+
 	invention_manager.lock_inventions();
 
 	return ret;
@@ -491,7 +488,9 @@ bool Dataloader::_load_history(GameManager& game_manager, bool unused_history_fi
 	ret &= apply_to_files(
 		lookup_files_in_dir(diplomacy_history_directory, ".txt"),
 		[this, &game_manager](fs::path const& file) -> bool {
-			return game_manager.get_history_manager().get_diplomacy_manager().load_diplomacy_history_file(game_manager, parse_defines(file).get_file_node());
+			return game_manager.get_history_manager().get_diplomacy_manager().load_diplomacy_history_file(
+				game_manager.get_country_manager(), parse_defines(file).get_file_node()
+			);
 		}
 	);
 	static constexpr std::string_view war_history_directory = "history/wars";
@@ -548,18 +547,10 @@ bool Dataloader::_load_map_dir(GameManager& game_manager) const {
 
 	bool ret = expect_dictionary_keys(
 		"max_provinces", ONE_EXACTLY,
-			expect_uint<Province::index_t>(
-				std::bind(&Map::set_max_provinces, &map, std::placeholders::_1)
-			),
+			expect_uint<Province::index_t>(std::bind_front(&Map::set_max_provinces, &map)),
 		"sea_starts", ONE_EXACTLY,
 			expect_list_reserve_length(
-				water_province_identifiers,
-				expect_identifier(
-					[&water_province_identifiers](std::string_view identifier) -> bool {
-						water_province_identifiers.push_back(identifier);
-						return true;
-					}
-				)
+				water_province_identifiers, expect_identifier(vector_callback(water_province_identifiers))
 			),
 
 #define MAP_PATH_DICT_ENTRY(X) #X, ONE_EXACTLY, expect_string(assign_variable_callback(X)),
@@ -584,7 +575,7 @@ bool Dataloader::_load_map_dir(GameManager& game_manager) const {
 	}
 
 	if (!map.load_province_positions(
-		game_manager.get_economy_manager().get_building_manager(),
+		game_manager.get_economy_manager().get_building_type_manager(),
 		parse_defines(lookup_file(append_string_views(map_directory, positions))).get_file_node()
 	)) {
 		Logger::error("Failed to load province positions file!");
@@ -744,8 +735,8 @@ bool Dataloader::load_defines(GameManager& game_manager) const {
 	if (!_load_technologies(game_manager)) {
 		ret = false;
 	}
-	if (!game_manager.get_modifier_manager().load_crime_modifiers(
-		parse_defines(lookup_file(crime_modifiers_file)).get_file_node()
+	if (!game_manager.get_crime_manager().load_crime_modifiers(
+		game_manager.get_modifier_manager(), parse_defines(lookup_file(crime_modifiers_file)).get_file_node()
 	)) {
 		Logger::error("Failed to load crime modifiers!");
 		ret = false;
diff --git a/src/openvic-simulation/dataloader/NodeTools.cpp b/src/openvic-simulation/dataloader/NodeTools.cpp
index 12e51a4..c4addb7 100644
--- a/src/openvic-simulation/dataloader/NodeTools.cpp
+++ b/src/openvic-simulation/dataloader/NodeTools.cpp
@@ -380,17 +380,7 @@ node_callback_t NodeTools::name_list_callback(callback_t<std::vector<std::string
 	return [callback](ast::NodeCPtr node) -> bool {
 		std::vector<std::string> list;
 		bool ret = expect_list_reserve_length(
-			list,
-			expect_identifier_or_string(
-				[&list](std::string_view str) -> bool {
-					if (!str.empty()) {
-						list.push_back(std::string { str });
-						return true;
-					}
-					Logger::error("Empty identifier or string");
-					return false;
-				}
-			)
+			list, expect_identifier_or_string(vector_callback<std::string_view>(list))
 		)(node);
 		ret &= callback(std::move(list));
 		return ret;
diff --git a/src/openvic-simulation/dataloader/NodeTools.hpp b/src/openvic-simulation/dataloader/NodeTools.hpp
index 1ca6cf0..45e6d4f 100644
--- a/src/openvic-simulation/dataloader/NodeTools.hpp
+++ b/src/openvic-simulation/dataloader/NodeTools.hpp
@@ -7,6 +7,7 @@
 #include <optional>
 #include <set>
 #include <type_traits>
+#include <unordered_set>
 
 #include <openvic-dataloader/v2script/AbstractSyntaxTree.hpp>
 
@@ -320,5 +321,34 @@ namespace OpenVic {
 				return true;
 			};
 		}
+
+		template<typename T, typename U>
+		Callback<T> auto vector_callback(std::vector<U>& vec) {
+			return [&vec](T val) -> bool {
+				vec.emplace_back(std::move(val));
+				return true;
+			};
+		}
+
+		template<typename T>
+		Callback<T> auto vector_callback(std::vector<T>& vec) {
+			return vector_callback<T, T>(vec);
+		}
+
+		template<typename T>
+		Callback<T const&> auto vector_callback_pointer(std::vector<T const*>& vec) {
+			return [&vec](T const& val) -> bool {
+				vec.emplace_back(&val);
+				return true;
+			};
+		}
+
+		template<typename T>
+		Callback<T const&> auto set_callback_pointer(std::unordered_set<T const*>& set) {
+			return [&set](T const& val) -> bool {
+				set.insert(&val);
+				return true;
+			};
+		}
 	}
 }
diff --git a/src/openvic-simulation/economy/BuildingType.cpp b/src/openvic-simulation/economy/BuildingType.cpp
index b80999a..876afd9 100644
--- a/src/openvic-simulation/economy/BuildingType.cpp
+++ b/src/openvic-simulation/economy/BuildingType.cpp
@@ -14,9 +14,9 @@ BuildingType::BuildingType(
 	colonial_range { colonial_range }, infrastructure { infrastructure }, spawn_railway_track { spawn_railway_track },
 	sail { sail }, steam { steam }, capital { capital }, port { port } {}
 
-BuildingManager::BuildingManager() : building_types { "building types" } {}
+BuildingTypeManager::BuildingTypeManager() : building_types { "building types" } {}
 
-bool BuildingManager::add_building_type(std::string_view identifier, ARGS) {
+bool BuildingTypeManager::add_building_type(std::string_view identifier, ARGS) {
 	if (identifier.empty()) {
 		Logger::error("Invalid building identifier - empty!");
 		return false;
@@ -30,7 +30,7 @@ bool BuildingManager::add_building_type(std::string_view identifier, ARGS) {
 	});
 }
 
-bool BuildingManager::load_buildings_file(
+bool BuildingTypeManager::load_buildings_file(
 	GoodManager const& good_manager, ProductionTypeManager const& production_type_manager, ModifierManager& modifier_manager,
 	ast::NodeCPtr root
 ) {
@@ -70,12 +70,7 @@ bool BuildingManager::load_buildings_file(
 				"advanced_factory", ZERO_OR_ONE, expect_bool(assign_variable_callback(advanced_factory)),
 				"fort_level", ZERO_OR_ONE, expect_uint(assign_variable_callback(fort_level)),
 				"naval_capacity", ZERO_OR_ONE, expect_uint(assign_variable_callback(naval_capacity)),
-				"colonial_points", ZERO_OR_ONE, expect_list(expect_fixed_point(
-					[&colonial_points](fixed_point_t points) -> bool {
-						colonial_points.push_back(points);
-						return true;
-					}
-				)),
+				"colonial_points", ZERO_OR_ONE, expect_list(expect_fixed_point(vector_callback(colonial_points))),
 				"province", ZERO_OR_ONE, expect_bool(assign_variable_callback(in_province)),
 				"one_per_state", ZERO_OR_ONE, expect_bool(assign_variable_callback(one_per_state)),
 				"colonial_range", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(colonial_range)),
diff --git a/src/openvic-simulation/economy/BuildingType.hpp b/src/openvic-simulation/economy/BuildingType.hpp
index a49c461..deba77a 100644
--- a/src/openvic-simulation/economy/BuildingType.hpp
+++ b/src/openvic-simulation/economy/BuildingType.hpp
@@ -17,7 +17,7 @@
 
 namespace OpenVic {
 
-	struct BuildingManager;
+	struct BuildingTypeManager;
 
 	/* REQUIREMENTS:
 	 * MAP-11, MAP-72, MAP-73
@@ -25,7 +25,7 @@ namespace OpenVic {
 	 * MAP-13, MAP-78, MAP-79
 	 */
 	struct BuildingType : HasIdentifier {
-		friend struct BuildingManager;
+		friend struct BuildingTypeManager;
 
 		using level_t = int16_t;
 
@@ -69,14 +69,14 @@ namespace OpenVic {
 		BuildingType(BuildingType&&) = default;
 	};
 
-	struct BuildingManager {
+	struct BuildingTypeManager {
 		using level_t = BuildingType::level_t; // this is getting ridiculous
 
 	private:
 		IdentifierRegistry<BuildingType> building_types;
 
 	public:
-		BuildingManager();
+		BuildingTypeManager();
 
 		bool add_building_type(std::string_view identifier, ARGS);
 		IDENTIFIER_REGISTRY_ACCESSORS(building_type)
diff --git a/src/openvic-simulation/economy/EconomyManager.hpp b/src/openvic-simulation/economy/EconomyManager.hpp
index fcde094..adeabd9 100644
--- a/src/openvic-simulation/economy/EconomyManager.hpp
+++ b/src/openvic-simulation/economy/EconomyManager.hpp
@@ -7,7 +7,7 @@
 namespace OpenVic {
 	struct EconomyManager {
 	private:
-		BuildingManager PROPERTY_REF(building_manager);
+		BuildingTypeManager PROPERTY_REF(building_type_manager);
 		GoodManager PROPERTY_REF(good_manager);
 		ProductionTypeManager PROPERTY_REF(production_type_manager);
 
@@ -17,7 +17,7 @@ namespace OpenVic {
 		}
 
 		inline bool load_buildings_file(ModifierManager& modifier_manager, ast::NodeCPtr root) {
-			return building_manager.load_buildings_file(good_manager, production_type_manager, modifier_manager, root);
+			return building_type_manager.load_buildings_file(good_manager, production_type_manager, modifier_manager, root);
 		}
 	};
 }
diff --git a/src/openvic-simulation/economy/Good.cpp b/src/openvic-simulation/economy/Good.cpp
index 02c4f0b..ac3c8c7 100644
--- a/src/openvic-simulation/economy/Good.cpp
+++ b/src/openvic-simulation/economy/Good.cpp
@@ -101,8 +101,7 @@ bool GoodManager::load_goods_file(ast::NodeCPtr root) {
 	for (Good const& good : this->get_goods()) { \
 		ret &= modifier_manager.add_modifier_effect( \
 			StringUtils::append_string_views(name, "_", good.get_identifier()), \
-			true, \
-			ModifierEffect::format_t::PROPORTION_DECIMAL \
+			true \
 		); \
 	}
 
diff --git a/src/openvic-simulation/economy/ProductionType.cpp b/src/openvic-simulation/economy/ProductionType.cpp
index a862b12..d0b90fa 100644
--- a/src/openvic-simulation/economy/ProductionType.cpp
+++ b/src/openvic-simulation/economy/ProductionType.cpp
@@ -55,16 +55,11 @@ node_callback_t ProductionTypeManager::_expect_employed_pop(
 node_callback_t ProductionTypeManager::_expect_employed_pop_list(
 	GoodManager const& good_manager, PopManager const& pop_manager, callback_t<std::vector<EmployedPop>&&> cb
 ) {
-
 	return [this, &good_manager, &pop_manager, cb](ast::NodeCPtr node) -> bool {
 		std::vector<EmployedPop> employed_pops;
-		bool res = expect_list([this, &good_manager, &pop_manager, &employed_pops](ast::NodeCPtr node) -> bool {
-			EmployedPop owner;
-			bool res_partial = _expect_employed_pop(good_manager, pop_manager, assign_variable_callback(owner))(node);
-			employed_pops.push_back(owner);
-			return res_partial;
-		})(node);
-		return res & cb(std::move(employed_pops));
+		bool ret = expect_list(_expect_employed_pop(good_manager, pop_manager, vector_callback(employed_pops)))(node);
+		ret &= cb(std::move(employed_pops));
+		return ret;
 	};
 }
 
diff --git a/src/openvic-simulation/history/Bookmark.cpp b/src/openvic-simulation/history/Bookmark.cpp
index 483b49b..e9543be 100644
--- a/src/openvic-simulation/history/Bookmark.cpp
+++ b/src/openvic-simulation/history/Bookmark.cpp
@@ -3,10 +3,9 @@
 #include <openvic-dataloader/v2script/AbstractSyntaxTree.hpp>
 
 #include "openvic-simulation/dataloader/NodeTools.hpp"
-
-#include "types/Date.hpp"
-#include "types/IdentifierRegistry.hpp"
-#include "utility/Logger.hpp"
+#include "openvic-simulation/types/Date.hpp"
+#include "openvic-simulation/types/IdentifierRegistry.hpp"
+#include "openvic-simulation/utility/Logger.hpp"
 
 using namespace OpenVic;
 using namespace OpenVic::NodeTools;
diff --git a/src/openvic-simulation/history/CountryHistory.cpp b/src/openvic-simulation/history/CountryHistory.cpp
index 04a5e09..d312261 100644
--- a/src/openvic-simulation/history/CountryHistory.cpp
+++ b/src/openvic-simulation/history/CountryHistory.cpp
@@ -59,10 +59,7 @@ bool CountryHistoryMap::_load_history_entry(
 		"primary_culture", ZERO_OR_ONE,
 			culture_manager.expect_culture_identifier(assign_variable_callback_pointer(entry.primary_culture)),
 		"culture", ZERO_OR_MORE, culture_manager.expect_culture_identifier(
-			[&entry](Culture const& culture) -> bool {
-				entry.accepted_cultures.push_back(&culture);
-				return true;
-			}
+			vector_callback_pointer(entry.accepted_cultures)
 		),
 		"religion", ZERO_OR_ONE, game_manager.get_pop_manager().get_religion_manager().expect_religion_identifier(
 			assign_variable_callback_pointer(entry.religion)
diff --git a/src/openvic-simulation/history/DiplomaticHistory.cpp b/src/openvic-simulation/history/DiplomaticHistory.cpp
index 79ac907..ce55001 100644
--- a/src/openvic-simulation/history/DiplomaticHistory.cpp
+++ b/src/openvic-simulation/history/DiplomaticHistory.cpp
@@ -79,16 +79,16 @@ std::vector<WarHistory const*> DiplomaticHistoryManager::get_wars(Date date) con
 	return ret;
 }
 
-bool DiplomaticHistoryManager::load_diplomacy_history_file(GameManager& game_manager, ast::NodeCPtr root) {
+bool DiplomaticHistoryManager::load_diplomacy_history_file(CountryManager const& country_manager, ast::NodeCPtr root) {
 	return expect_dictionary_keys(
-		"alliance", ZERO_OR_MORE, [this, &game_manager](ast::NodeCPtr node) -> bool {
+		"alliance", ZERO_OR_MORE, [this, &country_manager](ast::NodeCPtr node) -> bool {
 			Country const* first;
 			Country const* second;
 			Date start, end;
 
 			bool ret = expect_dictionary_keys(
-				"first", ONE_EXACTLY, expect_identifier_or_string(game_manager.get_country_manager().expect_country_str(assign_variable_callback_pointer(first))),
-				"second", ONE_EXACTLY, expect_identifier_or_string(game_manager.get_country_manager().expect_country_str(assign_variable_callback_pointer(second))),
+				"first", ONE_EXACTLY, expect_identifier_or_string(country_manager.expect_country_str(assign_variable_callback_pointer(first))),
+				"second", ONE_EXACTLY, expect_identifier_or_string(country_manager.expect_country_str(assign_variable_callback_pointer(second))),
 				"start_date", ONE_EXACTLY, expect_identifier_or_string(expect_date_str(assign_variable_callback(start))),
 				"end_date", ONE_EXACTLY, expect_identifier_or_string(expect_date_str(assign_variable_callback(end)))
 			)(node);
@@ -96,14 +96,14 @@ bool DiplomaticHistoryManager::load_diplomacy_history_file(GameManager& game_man
 			alliances.push_back({ first, second, start, end });
 			return ret;
 		},
-		"vassal", ZERO_OR_MORE, [this, &game_manager](ast::NodeCPtr node) -> bool {
+		"vassal", ZERO_OR_MORE, [this, &country_manager](ast::NodeCPtr node) -> bool {
 			Country const* overlord;
 			Country const* subject;
 			Date start, end;
 
 			bool ret = expect_dictionary_keys(
-				"first", ONE_EXACTLY, expect_identifier_or_string(game_manager.get_country_manager().expect_country_str(assign_variable_callback_pointer(overlord))),
-				"second", ONE_EXACTLY, expect_identifier_or_string(game_manager.get_country_manager().expect_country_str(assign_variable_callback_pointer(subject))),
+				"first", ONE_EXACTLY, expect_identifier_or_string(country_manager.expect_country_str(assign_variable_callback_pointer(overlord))),
+				"second", ONE_EXACTLY, expect_identifier_or_string(country_manager.expect_country_str(assign_variable_callback_pointer(subject))),
 				"start_date", ONE_EXACTLY, expect_identifier_or_string(expect_date_str(assign_variable_callback(start))),
 				"end_date", ONE_EXACTLY, expect_identifier_or_string(expect_date_str(assign_variable_callback(end)))
 			)(node);
@@ -111,14 +111,14 @@ bool DiplomaticHistoryManager::load_diplomacy_history_file(GameManager& game_man
 			subjects.push_back({ overlord, subject, SubjectHistory::type_t::VASSAL, start, end });
 			return ret;
 		},
-		"union", ZERO_OR_MORE, [this, &game_manager](ast::NodeCPtr node) -> bool {
+		"union", ZERO_OR_MORE, [this, &country_manager](ast::NodeCPtr node) -> bool {
 			Country const* overlord;
 			Country const* subject;
 			Date start, end;
 
 			bool ret = expect_dictionary_keys(
-				"first", ONE_EXACTLY, game_manager.get_country_manager().expect_country_identifier(assign_variable_callback_pointer(overlord)),
-				"second", ONE_EXACTLY, game_manager.get_country_manager().expect_country_identifier(assign_variable_callback_pointer(subject)),
+				"first", ONE_EXACTLY, country_manager.expect_country_identifier(assign_variable_callback_pointer(overlord)),
+				"second", ONE_EXACTLY, country_manager.expect_country_identifier(assign_variable_callback_pointer(subject)),
 				"start_date", ONE_EXACTLY, expect_date(assign_variable_callback(start)),
 				"end_date", ONE_EXACTLY, expect_date(assign_variable_callback(end))
 			)(node);
@@ -126,14 +126,14 @@ bool DiplomaticHistoryManager::load_diplomacy_history_file(GameManager& game_man
 			subjects.push_back({ overlord, subject, SubjectHistory::type_t::UNION, start, end });
 			return ret;
 		},
-		"substate", ZERO_OR_MORE, [this, &game_manager](ast::NodeCPtr node) -> bool {
+		"substate", ZERO_OR_MORE, [this, &country_manager](ast::NodeCPtr node) -> bool {
 			Country const* overlord;
 			Country const* subject;
 			Date start, end;
 
 			bool ret = expect_dictionary_keys(
-				"first", ONE_EXACTLY, game_manager.get_country_manager().expect_country_identifier(assign_variable_callback_pointer(overlord)),
-				"second", ONE_EXACTLY, game_manager.get_country_manager().expect_country_identifier(assign_variable_callback_pointer(subject)),
+				"first", ONE_EXACTLY, country_manager.expect_country_identifier(assign_variable_callback_pointer(overlord)),
+				"second", ONE_EXACTLY, country_manager.expect_country_identifier(assign_variable_callback_pointer(subject)),
 				"start_date", ONE_EXACTLY, expect_date(assign_variable_callback(start)),
 				"end_date", ONE_EXACTLY, expect_date(assign_variable_callback(end))
 			)(node);
@@ -144,7 +144,7 @@ bool DiplomaticHistoryManager::load_diplomacy_history_file(GameManager& game_man
 	)(root);
 }
 
-bool DiplomaticHistoryManager::load_war_history_file(GameManager& game_manager, ast::NodeCPtr root) {
+bool DiplomaticHistoryManager::load_war_history_file(GameManager const& game_manager, ast::NodeCPtr root) {
 	std::string name = "";
 	std::vector<WarHistory::war_participant_t> attackers;
 	std::vector<WarHistory::war_participant_t> defenders;
diff --git a/src/openvic-simulation/history/DiplomaticHistory.hpp b/src/openvic-simulation/history/DiplomaticHistory.hpp
index 4d620b5..3e877eb 100644
--- a/src/openvic-simulation/history/DiplomaticHistory.hpp
+++ b/src/openvic-simulation/history/DiplomaticHistory.hpp
@@ -98,7 +98,7 @@ namespace OpenVic {
 		/* Returns all wars that begin before date. NOTE: Some wargoals may be added or countries may join after date, should be checked for by functions that use get_wars() */
 		std::vector<WarHistory const*> get_wars(Date date) const;
 
-		bool load_diplomacy_history_file(GameManager& game_manager, ast::NodeCPtr root);
-		bool load_war_history_file(GameManager& game_manager, ast::NodeCPtr root);
+		bool load_diplomacy_history_file(CountryManager const& country_manager, ast::NodeCPtr root);
+		bool load_war_history_file(GameManager const& game_manager, ast::NodeCPtr root);
 	};
 } // namespace OpenVic
\ No newline at end of file
diff --git a/src/openvic-simulation/history/ProvinceHistory.cpp b/src/openvic-simulation/history/ProvinceHistory.cpp
index 991dfee..d9d3ef6 100644
--- a/src/openvic-simulation/history/ProvinceHistory.cpp
+++ b/src/openvic-simulation/history/ProvinceHistory.cpp
@@ -18,7 +18,7 @@ std::unique_ptr<ProvinceHistoryEntry> ProvinceHistoryMap::_make_entry(Date date)
 bool ProvinceHistoryMap::_load_history_entry(
 	GameManager const& game_manager, ProvinceHistoryEntry& entry, ast::NodeCPtr root
 ) {
-	BuildingManager const& building_manager = game_manager.get_economy_manager().get_building_manager();
+	BuildingTypeManager const& building_type_manager = game_manager.get_economy_manager().get_building_type_manager();
 	CountryManager const& country_manager = game_manager.get_country_manager();
 	GoodManager const& good_manager = game_manager.get_economy_manager().get_good_manager();
 	IdeologyManager const& ideology_manager = game_manager.get_politics_manager().get_ideology_manager();
@@ -30,10 +30,10 @@ bool ProvinceHistoryMap::_load_history_entry(
 	};
 
 	return expect_dictionary_keys_and_default(
-		[this, &game_manager, &building_manager, &entry](
+		[this, &game_manager, &building_type_manager, &entry](
 			std::string_view key, ast::NodeCPtr value) -> bool {
 			// used for province buildings like forts or railroads
-			BuildingType const* building_type = building_manager.get_building_type_by_identifier(key);
+			BuildingType const* building_type = building_type_manager.get_building_type_by_identifier(key);
 			if (building_type != nullptr) {
 				return expect_uint<BuildingType::level_t>([&entry, building_type](BuildingType::level_t level) -> bool {
 					entry.province_buildings[building_type] = level;
@@ -47,17 +47,9 @@ bool ProvinceHistoryMap::_load_history_entry(
 			country_manager.expect_country_identifier(assign_variable_callback_pointer(entry.owner)),
 		"controller", ZERO_OR_ONE,
 			country_manager.expect_country_identifier(assign_variable_callback_pointer(entry.controller)),
-		"add_core", ZERO_OR_MORE, country_manager.expect_country_identifier(
-			[&entry](Country const& core) -> bool {
-				entry.add_cores.push_back(&core);
-				return true;
-			}
-		),
+		"add_core", ZERO_OR_MORE, country_manager.expect_country_identifier(vector_callback_pointer(entry.add_cores)),
 		"remove_core", ZERO_OR_MORE, country_manager.expect_country_identifier(
-			[&entry](Country const& core) -> bool {
-				entry.remove_cores.push_back(&core);
-				return true;
-			}
+			vector_callback_pointer(entry.remove_cores)
 		),
 		"colonial", ZERO_OR_ONE,
 			expect_identifier(expect_mapped_string(colony_status_map, assign_variable_callback(entry.colonial))),
@@ -82,13 +74,13 @@ bool ProvinceHistoryMap::_load_history_entry(
 			entry.party_loyalties[ideology] = amount;
 			return ret;
 		},
-		"state_building", ZERO_OR_MORE, [&building_manager, &entry](ast::NodeCPtr node) -> bool {
+		"state_building", ZERO_OR_MORE, [&building_type_manager, &entry](ast::NodeCPtr node) -> bool {
 			BuildingType const* building_type = nullptr;
 			uint8_t level = 0;
 
 			const bool ret = expect_dictionary_keys(
 				"level", ONE_EXACTLY, expect_uint(assign_variable_callback(level)),
-				"building", ONE_EXACTLY, building_manager.expect_building_type_identifier(
+				"building", ONE_EXACTLY, building_type_manager.expect_building_type_identifier(
 					assign_variable_callback_pointer(building_type)
 				),
 				"upgrade", ZERO_OR_ONE, success_callback // doesn't appear to have an effect
diff --git a/src/openvic-simulation/map/Crime.cpp b/src/openvic-simulation/map/Crime.cpp
new file mode 100644
index 0000000..3a9ae64
--- /dev/null
+++ b/src/openvic-simulation/map/Crime.cpp
@@ -0,0 +1,40 @@
+#include "Crime.hpp"
+
+using namespace OpenVic;
+using namespace OpenVic::NodeTools;
+
+Crime::Crime(std::string_view new_identifier, ModifierValue&& new_values, icon_t new_icon, bool new_default_active)
+  : TriggeredModifier { new_identifier, std::move(new_values), new_icon }, default_active { new_default_active } {}
+
+CrimeManager::CrimeManager() : crime_modifiers { "crime modifiers" } {}
+
+bool CrimeManager::add_crime_modifier(
+	std::string_view identifier, ModifierValue&& values, Modifier::icon_t icon, bool default_active
+) {
+	if (identifier.empty()) {
+		Logger::error("Invalid crime modifier effect identifier - empty!");
+		return false;
+	}
+	return crime_modifiers.add_item({ identifier, std::move(values), icon, default_active }, duplicate_warning_callback);
+}
+
+bool CrimeManager::load_crime_modifiers(ModifierManager const& modifier_manager, ast::NodeCPtr root) {
+	const bool ret = expect_dictionary_reserve_length(
+		crime_modifiers,
+		[this, &modifier_manager](std::string_view key, ast::NodeCPtr value) -> bool {
+			ModifierValue modifier_value;
+			Modifier::icon_t icon = 0;
+			bool default_active = false;
+			bool ret = modifier_manager.expect_modifier_value_and_keys(
+				move_variable_callback(modifier_value),
+				"icon", ZERO_OR_ONE, expect_uint(assign_variable_callback(icon)),
+				"trigger", ONE_EXACTLY, success_callback, // TODO - load condition
+				"active", ZERO_OR_ONE, expect_bool(assign_variable_callback(default_active))
+			)(value);
+			ret &= add_crime_modifier(key, std::move(modifier_value), icon, default_active);
+			return ret;
+		}
+	)(root);
+	lock_crime_modifiers();
+	return ret;
+}
diff --git a/src/openvic-simulation/map/Crime.hpp b/src/openvic-simulation/map/Crime.hpp
new file mode 100644
index 0000000..d0dd844
--- /dev/null
+++ b/src/openvic-simulation/map/Crime.hpp
@@ -0,0 +1,32 @@
+#pragma once
+
+#include "openvic-simulation/misc/Modifier.hpp"
+
+namespace OpenVic {
+	struct Crime final : TriggeredModifier {
+		friend struct CrimeManager;
+
+	private:
+		const bool PROPERTY(default_active);
+
+		Crime(std::string_view new_identifier, ModifierValue&& new_values, icon_t new_icon, bool new_default_active);
+
+	public:
+		Crime(Crime&&) = default;
+	};
+
+	struct CrimeManager {
+	private:
+		IdentifierRegistry<Crime> crime_modifiers;
+
+	public:
+		CrimeManager();
+
+		bool add_crime_modifier(
+			std::string_view identifier, ModifierValue&& values, Modifier::icon_t icon, bool default_active
+		);
+		IDENTIFIER_REGISTRY_ACCESSORS(crime_modifier)
+
+		bool load_crime_modifiers(ModifierManager const& modifier_manager, ast::NodeCPtr root);
+	};
+}
diff --git a/src/openvic-simulation/map/Map.cpp b/src/openvic-simulation/map/Map.cpp
index f854cab..7bf5e7d 100644
--- a/src/openvic-simulation/map/Map.cpp
+++ b/src/openvic-simulation/map/Map.cpp
@@ -282,10 +282,10 @@ Pop::pop_size_t Map::get_total_map_population() const {
 	return total_map_population;
 }
 
-bool Map::reset(BuildingManager const& building_manager) {
+bool Map::reset(BuildingTypeManager const& building_type_manager) {
 	bool ret = true;
 	for (Province& province : provinces.get_items()) {
-		ret &= province.reset(building_manager);
+		ret &= province.reset(building_type_manager);
 	}
 	return ret;
 }
@@ -388,9 +388,9 @@ bool Map::load_province_definitions(std::vector<LineObject> const& lines) {
 	return ret;
 }
 
-bool Map::load_province_positions(BuildingManager const& building_manager, ast::NodeCPtr root) {
-	return expect_province_dictionary([&building_manager](Province& province, ast::NodeCPtr node) -> bool {
-		return province.load_positions(building_manager, node);
+bool Map::load_province_positions(BuildingTypeManager const& building_type_manager, ast::NodeCPtr root) {
+	return expect_province_dictionary([&building_type_manager](Province& province, ast::NodeCPtr node) -> bool {
+		return province.load_positions(building_type_manager, node);
 	})(root);
 }
 
@@ -399,11 +399,7 @@ bool Map::load_region_file(ast::NodeCPtr root) {
 		[this](std::string_view region_identifier, ast::NodeCPtr region_node) -> bool {
 			std::vector<std::string_view> province_identifiers;
 			bool ret = expect_list_reserve_length(
-				province_identifiers,
-				expect_identifier([&province_identifiers](std::string_view identifier) -> bool {
-					province_identifiers.push_back(identifier);
-					return true;
-				})
+				province_identifiers, expect_identifier(vector_callback(province_identifiers))
 			)(region_node);
 			ret &= add_region(region_identifier, province_identifiers);
 			return ret;
diff --git a/src/openvic-simulation/map/Map.hpp b/src/openvic-simulation/map/Map.hpp
index f286276..bb760e6 100644
--- a/src/openvic-simulation/map/Map.hpp
+++ b/src/openvic-simulation/map/Map.hpp
@@ -108,7 +108,7 @@ namespace OpenVic {
 		 * that is the maximum allowed number of provinces plus one for the index-zero "null province". */
 		bool generate_mapmode_colours(Mapmode::index_t index, uint8_t* target) const;
 
-		bool reset(BuildingManager const& building_manager);
+		bool reset(BuildingTypeManager const& building_type_manager);
 		bool apply_history_to_provinces(ProvinceHistoryManager const& history_manager, Date date);
 
 		void update_highest_province_population();
@@ -120,7 +120,7 @@ namespace OpenVic {
 		void tick(Date today);
 
 		bool load_province_definitions(std::vector<ovdl::csv::LineObject> const& lines);
-		bool load_province_positions(BuildingManager const& building_manager, ast::NodeCPtr root);
+		bool load_province_positions(BuildingTypeManager const& building_type_manager, ast::NodeCPtr root);
 		bool load_region_file(ast::NodeCPtr root);
 		bool load_map_images(fs::path const& province_path, fs::path const& terrain_path, bool detailed_errors);
 		bool generate_and_load_province_adjacencies(std::vector<ovdl::csv::LineObject> const& additional_adjacencies);
diff --git a/src/openvic-simulation/map/Province.cpp b/src/openvic-simulation/map/Province.cpp
index 201d3a0..1f9ca99 100644
--- a/src/openvic-simulation/map/Province.cpp
+++ b/src/openvic-simulation/map/Province.cpp
@@ -21,7 +21,7 @@ std::string Province::to_string() const {
 	return stream.str();
 }
 
-bool Province::load_positions(BuildingManager const& building_manager, ast::NodeCPtr root) {
+bool Province::load_positions(BuildingTypeManager const& building_type_manager, ast::NodeCPtr root) {
 	return expect_dictionary_keys(
 		"text_position", ZERO_OR_ONE, expect_fvec2(assign_variable_callback(positions.text)),
 		"text_rotation", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(positions.text_rotation)),
@@ -32,16 +32,16 @@ bool Province::load_positions(BuildingManager const& building_manager, ast::Node
 		"factory", ZERO_OR_ONE, expect_fvec2(assign_variable_callback(positions.factory)),
 		"building_construction", ZERO_OR_ONE, expect_fvec2(assign_variable_callback(positions.building_construction)),
 		"military_construction", ZERO_OR_ONE, expect_fvec2(assign_variable_callback(positions.military_construction)),
-		"building_position", ZERO_OR_ONE, expect_dictionary_keys( // TODO: for TGC etc are building positions available for modded-in buildings? needs testing
-			"fort", ZERO_OR_ONE, expect_fvec2(assign_variable_callback(positions.fort)),
-			"railroad", ZERO_OR_ONE, expect_fvec2(assign_variable_callback(positions.railroad)),
-			"naval_base", ZERO_OR_ONE, expect_fvec2(assign_variable_callback(positions.navalbase))
+		"building_position", ZERO_OR_ONE, building_type_manager.expect_building_type_dictionary(
+			[this](BuildingType const& type, ast::NodeCPtr value) -> bool {
+				return expect_fvec2([this, &type](fvec2_t position) -> bool {
+					positions.building_position.emplace(&type, std::move(position));
+					return true;
+				})(value);
+			}
 		),
-		"building_rotation", ZERO_OR_ONE, expect_dictionary_keys(
-			"fort", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(positions.fort_rotation)),
-			"railroad", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(positions.railroad_rotation)),
-			"naval_base", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(positions.navalbase_rotation)),
-			"aeroplane_factory", ZERO_OR_ONE, success_callback /* see below */
+		"building_rotation", ZERO_OR_ONE, building_type_manager.expect_building_type_decimal_map(
+			move_variable_callback(positions.building_rotation)
 		),
 		/* the below are esoteric clausewitz leftovers that either have no impact or whose functionality is lost to time */
 		"spawn_railway_track", ZERO_OR_ONE, success_callback,
@@ -137,7 +137,7 @@ bool Province::add_adjacency(Province const* province, distance_t distance, flag
 	return true;
 }
 
-bool Province::reset(BuildingManager const& building_manager) {
+bool Province::reset(BuildingTypeManager const& building_type_manager) {
 	terrain_type = default_terrain_type;
 	life_rating = 0;
 	colony_status = colony_status_t::STATE;
@@ -150,8 +150,8 @@ bool Province::reset(BuildingManager const& building_manager) {
 	buildings.reset();
 	bool ret = true;
 	if (!get_water()) {
-		if (building_manager.building_types_are_locked()) {
-			for (BuildingType const& building_type : building_manager.get_building_types()) {
+		if (building_type_manager.building_types_are_locked()) {
+			for (BuildingType const& building_type : building_type_manager.get_building_types()) {
 				if (building_type.get_in_province()) {
 					ret &= buildings.add_item({ building_type });
 				}
diff --git a/src/openvic-simulation/map/Province.hpp b/src/openvic-simulation/map/Province.hpp
index dd440fa..b45dd9c 100644
--- a/src/openvic-simulation/map/Province.hpp
+++ b/src/openvic-simulation/map/Province.hpp
@@ -11,6 +11,7 @@ namespace OpenVic {
 	struct Map;
 	struct Region;
 	struct State;
+	struct Crime;
 	struct Good;
 	struct TerrainType;
 	struct TerrainTypeMapping;
@@ -50,12 +51,8 @@ namespace OpenVic {
 			fvec2_t factory;
 			fvec2_t building_construction;
 			fvec2_t military_construction;
-			fvec2_t fort;
-			fixed_point_t fort_rotation;
-			fvec2_t railroad;
-			fixed_point_t railroad_rotation;
-			fvec2_t navalbase;
-			fixed_point_t navalbase_rotation;
+			std::map<BuildingType const*, fvec2_t> building_position;
+			fixed_point_map_t<BuildingType const*> building_rotation;
 		};
 
 		static constexpr index_t NULL_INDEX = 0, MAX_INDEX = std::numeric_limits<index_t>::max();
@@ -99,7 +96,7 @@ namespace OpenVic {
 
 		std::string to_string() const;
 
-		bool load_positions(BuildingManager const& building_manager, ast::NodeCPtr root);
+		bool load_positions(BuildingTypeManager const& building_type_manager, ast::NodeCPtr root);
 
 		IDENTIFIER_REGISTRY_ACCESSORS(building)
 		bool expand_building(std::string_view building_type_identifier);
@@ -115,7 +112,7 @@ namespace OpenVic {
 		bool is_adjacent_to(Province const* province);
 		bool add_adjacency(Province const* province, distance_t distance, flags_t flags);
 
-		bool reset(BuildingManager const& building_manager);
+		bool reset(BuildingTypeManager const& building_type_manager);
 		bool apply_history_to_province(ProvinceHistoryEntry const* entry);
 	};
 }
diff --git a/src/openvic-simulation/map/State.cpp b/src/openvic-simulation/map/State.cpp
index bc78469..8da1e85 100644
--- a/src/openvic-simulation/map/State.cpp
+++ b/src/openvic-simulation/map/State.cpp
@@ -1,6 +1,6 @@
 #include "State.hpp"
 
-#include "Map.hpp"
+#include "openvic-simulation/map/Map.hpp"
 
 using namespace OpenVic;
 
diff --git a/src/openvic-simulation/map/TerrainType.cpp b/src/openvic-simulation/map/TerrainType.cpp
index ae10474..4134662 100644
--- a/src/openvic-simulation/map/TerrainType.cpp
+++ b/src/openvic-simulation/map/TerrainType.cpp
@@ -140,7 +140,7 @@ bool TerrainTypeManager::load_terrain_types(ModifierManager const& modifier_mana
 			terrain_type_mappings.reserve(size - 2);
 			return size;
 		},
-		std::bind(&TerrainTypeManager::_load_terrain_type_mapping, this, std::placeholders::_1, std::placeholders::_2),
+		std::bind_front(&TerrainTypeManager::_load_terrain_type_mapping, this),
 		"terrain", ONE_EXACTLY, expect_uint(assign_variable_callback(terrain_texture_limit)),
 		"categories", ONE_EXACTLY, _load_terrain_type_categories(modifier_manager)
 	)(root);
diff --git a/src/openvic-simulation/misc/Define.cpp b/src/openvic-simulation/misc/Define.cpp
index cfd96a6..9e468d9 100644
--- a/src/openvic-simulation/misc/Define.cpp
+++ b/src/openvic-simulation/misc/Define.cpp
@@ -1,4 +1,3 @@
-
 #include "Define.hpp"
 
 #include <cassert>
diff --git a/src/openvic-simulation/misc/Modifier.cpp b/src/openvic-simulation/misc/Modifier.cpp
index 87a6b65..1b3e551 100644
--- a/src/openvic-simulation/misc/Modifier.cpp
+++ b/src/openvic-simulation/misc/Modifier.cpp
@@ -84,13 +84,9 @@ TriggeredModifier::TriggeredModifier(std::string_view new_identifier, ModifierVa
 ModifierInstance::ModifierInstance(Modifier const& modifier, Date expiry_date)
 	: modifier { modifier }, expiry_date { expiry_date } {}
 
-Crime::Crime(std::string_view new_identifier, ModifierValue&& new_values, icon_t new_icon, bool new_default_active)
-  : TriggeredModifier { new_identifier, std::move(new_values), new_icon }, default_active { new_default_active },
-	active { new_default_active } {}
-
 ModifierManager::ModifierManager()
-  : modifier_effects { "modifier effects" }, crime_modifiers { "crime modifiers" }, event_modifiers { "event modifiers" },
-	static_modifiers { "static modifiers" }, triggered_modifiers { "triggered modifiers" } {}
+  : modifier_effects { "modifier effects" }, event_modifiers { "event modifiers" }, static_modifiers { "static modifiers" },
+	triggered_modifiers { "triggered modifiers" } {}
 
 bool ModifierManager::add_modifier_effect(std::string_view identifier, bool positive_good, ModifierEffect::format_t format) {
 	if (identifier.empty()) {
@@ -261,38 +257,6 @@ void ModifierManager::register_complex_modifier(std::string_view identifier) {
 	complex_modifiers.emplace(identifier);
 }
 
-bool ModifierManager::add_crime_modifier(
-	std::string_view identifier, ModifierValue&& values, Modifier::icon_t icon, bool active
-) {
-	if (identifier.empty()) {
-		Logger::error("Invalid crime modifier effect identifier - empty!");
-		return false;
-	}
-	return crime_modifiers.add_item({ identifier, std::move(values), icon, active }, duplicate_warning_callback);
-}
-
-bool ModifierManager::load_crime_modifiers(ast::NodeCPtr root) {
-	const bool ret = expect_dictionary_reserve_length(
-		crime_modifiers,
-		[this](std::string_view key, ast::NodeCPtr value) -> bool {
-			ModifierValue modifier_value;
-			Modifier::icon_t icon = 0;
-			bool active = false;
-			bool ret = expect_modifier_value_and_keys(
-				move_variable_callback(modifier_value),
-				"icon", ZERO_OR_ONE, expect_uint(assign_variable_callback(icon)),
-				"trigger", ONE_EXACTLY, success_callback, // TODO - load condition
-				"active", ZERO_OR_ONE, expect_bool(assign_variable_callback(active))
-			)(value);
-			ret &= add_crime_modifier(key, std::move(modifier_value), icon, active);
-			return ret;
-		}
-	)(root);
-	lock_crime_modifiers();
-	return ret;
-	return true;
-}
-
 bool ModifierManager::add_event_modifier(std::string_view identifier, ModifierValue&& values, Modifier::icon_t icon) {
 	if (identifier.empty()) {
 		Logger::error("Invalid event modifier effect identifier - empty!");
@@ -410,8 +374,12 @@ key_value_callback_t ModifierManager::_modifier_effect_callback(
 				)(value);
 				ret &= add_flattened_modifier_cb(key, faction_identifier, value_node);
 				return ret;
-			} else return expect_dictionary(std::bind(add_flattened_modifier_cb, key, std::placeholders::_1, std::placeholders::_2))(value);
-		} else return default_callback(key, value);
+			} else {
+				return expect_dictionary(std::bind_front(add_flattened_modifier_cb, key))(value);
+			}
+		} else {
+			return default_callback(key, value);
+		}
 	};
 }
 
diff --git a/src/openvic-simulation/misc/Modifier.hpp b/src/openvic-simulation/misc/Modifier.hpp
index f3678d9..617159a 100644
--- a/src/openvic-simulation/misc/Modifier.hpp
+++ b/src/openvic-simulation/misc/Modifier.hpp
@@ -1,7 +1,6 @@
 #pragma once
 
 #include "openvic-simulation/types/IdentifierRegistry.hpp"
-#include <unordered_set>
 
 namespace OpenVic {
 	struct ModifierManager;
@@ -93,19 +92,6 @@ namespace OpenVic {
 		TriggeredModifier(TriggeredModifier&&) = default;
 	};
 
-	struct Crime final : TriggeredModifier {
-		friend struct ModifierManager;
-
-	private:
-		const bool PROPERTY(default_active);
-		bool PROPERTY_RW(active);
-
-		Crime(std::string_view new_identifier, ModifierValue&& new_values, icon_t new_icon, bool new_default_active);
-
-	public:
-		Crime(Crime&&) = default;
-	};
-
 	struct ModifierInstance {
 
 	private:
@@ -127,7 +113,6 @@ namespace OpenVic {
 		IdentifierInstanceRegistry<ModifierEffect> modifier_effects;
 		string_set_t complex_modifiers;
 
-		IdentifierRegistry<Crime> crime_modifiers;
 		IdentifierRegistry<Modifier> event_modifiers;
 		IdentifierRegistry<Modifier> static_modifiers;
 		IdentifierRegistry<TriggeredModifier> triggered_modifiers;
@@ -151,10 +136,6 @@ namespace OpenVic {
 
 		bool setup_modifier_effects();
 
-		bool add_crime_modifier(std::string_view identifier, ModifierValue&& values, Modifier::icon_t icon, bool active);
-		IDENTIFIER_REGISTRY_ACCESSORS(crime_modifier)
-		bool load_crime_modifiers(ast::NodeCPtr root);
-
 		bool add_event_modifier(std::string_view identifier, ModifierValue&& values, Modifier::icon_t icon);
 		IDENTIFIER_REGISTRY_ACCESSORS(event_modifier)
 		bool load_event_modifiers(ast::NodeCPtr root);
diff --git a/src/openvic-simulation/politics/Government.cpp b/src/openvic-simulation/politics/Government.cpp
index 609e75f..b18432c 100644
--- a/src/openvic-simulation/politics/Government.cpp
+++ b/src/openvic-simulation/politics/Government.cpp
@@ -41,7 +41,7 @@ bool GovernmentTypeManager::add_government_type(
 		identifier, std::move(ideologies), elections, appoint_ruling_party, term_duration, flag_type
 	});
 
-	/* flag_type can be empty here for default/non-ideological flag  */
+	/* flag_type can be empty here for default/non-ideological flag */
 	if (ret && std::find(flag_types.begin(), flag_types.end(), flag_type) == flag_types.end()) {
 		flag_types.emplace_back(flag_type);
 	}
diff --git a/src/openvic-simulation/politics/NationalFocus.hpp b/src/openvic-simulation/politics/NationalFocus.hpp
index 396f8ec..abf6deb 100644
--- a/src/openvic-simulation/politics/NationalFocus.hpp
+++ b/src/openvic-simulation/politics/NationalFocus.hpp
@@ -69,7 +69,7 @@ namespace OpenVic {
 			NationalFocus::party_loyalty_map_t&& encouraged_loyalty,
 			NationalFocus::production_map_t&& encouraged_production
 		);
-		IDENTIFIER_REGISTRY_ACCESSORS_CUSTOM_PLURAL(national_focus, national_foci);
+		IDENTIFIER_REGISTRY_ACCESSORS_CUSTOM_PLURAL(national_focus, national_foci)
 
 		bool load_national_foci_file(PopManager const& pop_manager, IdeologyManager const& ideology_manager, GoodManager const& good_manager, ModifierManager const& modifier_manager, ast::NodeCPtr root);
 	};
diff --git a/src/openvic-simulation/politics/PoliticsManager.hpp b/src/openvic-simulation/politics/PoliticsManager.hpp
index 0afe002..1111f0a 100644
--- a/src/openvic-simulation/politics/PoliticsManager.hpp
+++ b/src/openvic-simulation/politics/PoliticsManager.hpp
@@ -1,11 +1,11 @@
 #pragma once
 
-#include "Rebel.hpp"
 #include "openvic-simulation/politics/Government.hpp"
 #include "openvic-simulation/politics/Ideology.hpp"
 #include "openvic-simulation/politics/Issue.hpp"
-#include "openvic-simulation/politics/NationalValue.hpp"
 #include "openvic-simulation/politics/NationalFocus.hpp"
+#include "openvic-simulation/politics/NationalValue.hpp"
+#include "openvic-simulation/politics/Rebel.hpp"
 
 namespace OpenVic {
 	struct PoliticsManager {
@@ -21,7 +21,10 @@ namespace OpenVic {
 		inline bool load_government_types_file(ast::NodeCPtr root) {
 			return government_type_manager.load_government_types_file(ideology_manager, root);
 		}
-		inline bool load_national_foci_file(PopManager const& pop_manager, GoodManager const& good_manager, ModifierManager const& modifier_manager, ast::NodeCPtr root) {
+		inline bool load_national_foci_file(
+			PopManager const& pop_manager, GoodManager const& good_manager, ModifierManager const& modifier_manager,
+			ast::NodeCPtr root
+		) {
 			return national_focus_manager.load_national_foci_file(pop_manager, ideology_manager, good_manager, modifier_manager, root);
 		}
 		inline bool load_rebels_file(ast::NodeCPtr root) {
diff --git a/src/openvic-simulation/pop/Culture.cpp b/src/openvic-simulation/pop/Culture.cpp
index bc17290..faf05d3 100644
--- a/src/openvic-simulation/pop/Culture.cpp
+++ b/src/openvic-simulation/pop/Culture.cpp
@@ -73,9 +73,7 @@ bool CultureManager::add_culture(
 
 bool CultureManager::load_graphical_culture_type_file(ast::NodeCPtr root) {
 	const bool ret = expect_list_reserve_length(graphical_culture_types,
-		expect_identifier(
-			std::bind(&CultureManager::add_graphical_culture_type, this, std::placeholders::_1)
-		)
+		expect_identifier(std::bind_front(&CultureManager::add_graphical_culture_type, this))
 	)(root);
 	lock_graphical_culture_types();
 	return ret;
diff --git a/src/openvic-simulation/research/Invention.cpp b/src/openvic-simulation/research/Invention.cpp
index eb4e342..381a249 100644
--- a/src/openvic-simulation/research/Invention.cpp
+++ b/src/openvic-simulation/research/Invention.cpp
@@ -1,23 +1,26 @@
 #include "Invention.hpp"
-#include <algorithm>
-#include <cstring>
-#include <utility>
+
+#include "openvic-simulation/economy/BuildingType.hpp"
+#include "openvic-simulation/map/Crime.hpp"
+#include "openvic-simulation/military/Unit.hpp"
 
 using namespace OpenVic;
 using namespace OpenVic::NodeTools;
 
 Invention::Invention(
-	std::string_view identifier, ModifierValue&& values, bool news, unit_set_t activated_units, building_set_t activated_buildings,
-	crime_set_t enabled_crimes, bool unlock_gas_attack, bool unlock_gas_defence) 
-	: Modifier { identifier, std::move(values), 0 },
-		news { news }, activated_units { std::move(activated_units) }, activated_buildings { std::move(activated_buildings) },
-		enabled_crimes { std::move(enabled_crimes) }, unlock_gas_attack { unlock_gas_attack }, unlock_gas_defence { unlock_gas_defence } {} //TODO icon
+	std::string_view new_identifier, ModifierValue&& new_values, bool new_news, unit_set_t&& new_activated_units,
+	building_set_t&& new_activated_buildings, crime_set_t&& new_enabled_crimes, bool new_unlock_gas_attack,
+	bool new_unlock_gas_defence
+) : Modifier { new_identifier, std::move(new_values), 0 }, news { new_news },
+	activated_units { std::move(new_activated_units) }, activated_buildings { std::move(new_activated_buildings) },
+	enabled_crimes { std::move(new_enabled_crimes) }, unlock_gas_attack { new_unlock_gas_attack },
+	unlock_gas_defence { new_unlock_gas_defence } {} //TODO icon
 
 InventionManager::InventionManager() : inventions { "inventions" } {}
 
 bool InventionManager::add_invention(
-	std::string_view identifier, ModifierValue&& values, bool news, Invention::unit_set_t activated_units,
-	Invention::building_set_t activated_buildings, Invention::crime_set_t enabled_crimes,
+	std::string_view identifier, ModifierValue&& values, bool news, Invention::unit_set_t&& activated_units,
+	Invention::building_set_t&& activated_buildings, Invention::crime_set_t&& enabled_crimes,
 	bool unlock_gas_attack, bool unlock_gas_defence
 ) {
 	if (identifier.empty()) {
@@ -32,49 +35,49 @@ bool InventionManager::add_invention(
 }
 
 bool InventionManager::load_inventions_file(
-	ModifierManager const& modifier_manager, UnitManager const& unit_manager, BuildingManager const& building_manager, ast::NodeCPtr root
+	ModifierManager const& modifier_manager, UnitManager const& unit_manager, BuildingTypeManager const& building_type_manager,
+	CrimeManager const& crime_manager, ast::NodeCPtr root
 ) {
-	return expect_dictionary_reserve_length(inventions, [this, &modifier_manager, &unit_manager, &building_manager](std::string_view identifier, ast::NodeCPtr value) -> bool {
-		ModifierValue loose_modifiers;
-		ModifierValue modifiers;
+	return expect_dictionary_reserve_length(
+		inventions, [this, &modifier_manager, &unit_manager, &building_type_manager, &crime_manager](
+			std::string_view identifier, ast::NodeCPtr value) -> bool {
+			ModifierValue loose_modifiers;
+			ModifierValue modifiers;
 
-		Invention::unit_set_t activated_units;
-		Invention::building_set_t activated_buildings;
-		Invention::crime_set_t enabled_crimes;
+			Invention::unit_set_t activated_units;
+			Invention::building_set_t activated_buildings;
+			Invention::crime_set_t enabled_crimes;
 
-		bool unlock_gas_attack = false;
-		bool unlock_gas_defence = false;
-		bool news = true; //defaults to true!
+			bool unlock_gas_attack = false;
+			bool unlock_gas_defence = false;
+			bool news = true; //defaults to true!
 
-		bool ret = modifier_manager.expect_modifier_value_and_keys(move_variable_callback(loose_modifiers),
-			"news", ZERO_OR_ONE, expect_bool(assign_variable_callback(news)),
-			"limit", ONE_EXACTLY, success_callback,
-			"chance", ONE_EXACTLY, success_callback,
-			"effect", ZERO_OR_ONE, modifier_manager.expect_modifier_value_and_keys(
-				move_variable_callback(modifiers),
-				"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, unit_manager.expect_unit_identifier([this, &activated_units](Unit const& unit) -> bool {
-					activated_units.insert(&unit);
-					return true;
-				}),
-				"activate_building", ZERO_OR_MORE, building_manager.expect_building_type_identifier([this, &activated_buildings](BuildingType const& type) -> bool {
-					activated_buildings.insert(&type);
-					return true;
-				}),
-				"enable_crime", ZERO_OR_ONE, modifier_manager.expect_crime_modifier_identifier([this, &enabled_crimes](Crime const& crime) -> bool {
-					enabled_crimes.insert(&crime);
-					return true;
-				}))
-		)(value);
+			bool ret = modifier_manager.expect_modifier_value_and_keys(move_variable_callback(loose_modifiers),
+				"news", ZERO_OR_ONE, expect_bool(assign_variable_callback(news)),
+				"limit", ONE_EXACTLY, success_callback,
+				"chance", ONE_EXACTLY, success_callback,
+				"effect", ZERO_OR_ONE, modifier_manager.expect_modifier_value_and_keys(
+					move_variable_callback(modifiers),
+					"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, unit_manager.expect_unit_identifier(set_callback_pointer(activated_units)),
+					"activate_building", ZERO_OR_MORE, building_type_manager.expect_building_type_identifier(
+						set_callback_pointer(activated_buildings)
+					),
+					"enable_crime", ZERO_OR_ONE, crime_manager.expect_crime_modifier_identifier(
+						set_callback_pointer(enabled_crimes)
+					)
+				)
+			)(value);
 
-		modifiers += loose_modifiers;
+			modifiers += loose_modifiers;
 
-		ret &= add_invention(
-			identifier, std::move(modifiers), news, activated_units, activated_buildings, enabled_crimes,
-			unlock_gas_attack, unlock_gas_defence
-		);
+			ret &= add_invention(
+				identifier, std::move(modifiers), news, std::move(activated_units), std::move(activated_buildings),
+				std::move(enabled_crimes), unlock_gas_attack, unlock_gas_defence
+			);
 
-		return ret;
-	})(root);
+			return ret;
+		}
+	)(root);
 }
\ No newline at end of file
diff --git a/src/openvic-simulation/research/Invention.hpp b/src/openvic-simulation/research/Invention.hpp
index 321d982..9cc158c 100644
--- a/src/openvic-simulation/research/Invention.hpp
+++ b/src/openvic-simulation/research/Invention.hpp
@@ -1,13 +1,17 @@
 #pragma once
 
-#include "openvic-simulation/economy/BuildingType.hpp"
-#include "openvic-simulation/military/Unit.hpp"
 #include "openvic-simulation/misc/Modifier.hpp"
 #include "openvic-simulation/types/IdentifierRegistry.hpp"
-#include <string_view>
-#include <unordered_set>
 
 namespace OpenVic {
+	struct Unit;
+	struct BuildingType;
+	struct Crime;
+
+	struct UnitManager;
+	struct BuildingTypeManager;
+	struct CrimeManager;
+
 	struct Invention : Modifier {
 		friend struct InventionManager;
 		//TODO implement limit and chance
@@ -24,9 +28,9 @@ namespace OpenVic {
 		const bool PROPERTY_CUSTOM_PREFIX(unlock_gas_defence, will);
 
 		Invention(
-			std::string_view identifier, ModifierValue&& values, bool news, unit_set_t activated_units,
-			building_set_t activated_buildings, crime_set_t enabled_crimes, bool unlock_gas_attack,
-			bool unlock_gas_defence
+			std::string_view new_identifier, ModifierValue&& new_values, bool new_news, unit_set_t&& new_activated_units,
+			building_set_t&& new_activated_buildings, crime_set_t&& new_enabled_crimes, bool new_unlock_gas_attack,
+			bool new_unlock_gas_defence
 		);
 
 	public:
@@ -40,16 +44,16 @@ namespace OpenVic {
 		InventionManager();
 
 		bool add_invention(
-			std::string_view identifier, ModifierValue&& values, bool news, Invention::unit_set_t activated_units,
-			Invention::building_set_t activated_buildings, Invention::crime_set_t enabled_crimes, bool unlock_gas_attack,
+			std::string_view identifier, ModifierValue&& values, bool news, Invention::unit_set_t&& activated_units,
+			Invention::building_set_t&& activated_buildings, Invention::crime_set_t&& enabled_crimes, bool unlock_gas_attack,
 			bool unlock_gas_defence
 		);
 
-		IDENTIFIER_REGISTRY_ACCESSORS(invention);
+		IDENTIFIER_REGISTRY_ACCESSORS(invention)
 
 		bool load_inventions_file(
-			ModifierManager const& modifier_manager, UnitManager const& unit_manager, BuildingManager const& building_manager,
-			ast::NodeCPtr root
+			ModifierManager const& modifier_manager, UnitManager const& unit_manager,
+			BuildingTypeManager const& building_type_manager, CrimeManager const& crime_manager, ast::NodeCPtr root
 		); // inventions/*.txt
 	};
 }
\ No newline at end of file
diff --git a/src/openvic-simulation/research/Technology.cpp b/src/openvic-simulation/research/Technology.cpp
index 07faaa3..f8ebb4b 100644
--- a/src/openvic-simulation/research/Technology.cpp
+++ b/src/openvic-simulation/research/Technology.cpp
@@ -111,8 +111,12 @@ bool TechnologyManager::load_technology_file_schools(ModifierManager const& modi
 	})(root);
 }
 
-bool TechnologyManager::load_technologies_file(ModifierManager const& modifier_manager, UnitManager const& unit_manager, BuildingManager const& building_manager, ast::NodeCPtr root) {
-	return expect_dictionary_reserve_length(technologies, [this, &modifier_manager, &unit_manager, &building_manager](std::string_view tech_key, ast::NodeCPtr tech_value) -> bool {
+bool TechnologyManager::load_technologies_file(
+	ModifierManager const& modifier_manager, UnitManager const& unit_manager, BuildingTypeManager const& building_type_manager,
+	ast::NodeCPtr root
+) {
+	return expect_dictionary_reserve_length(technologies, [this, &modifier_manager, &unit_manager, &building_type_manager](
+		std::string_view tech_key, ast::NodeCPtr tech_value) -> bool {
 		ModifierValue modifiers;
 		TechnologyArea const* area = nullptr;
 		Date::year_t year = 0;
@@ -128,15 +132,9 @@ bool TechnologyManager::load_technologies_file(ModifierManager const& modifier_m
 			"cost", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(cost)),
 			"unciv_military", ZERO_OR_ONE, expect_bool(assign_variable_callback(unciv_military)),
 			"unit", ZERO_OR_ONE, expect_uint(assign_variable_callback(unit)),
-			"activate_unit", ZERO_OR_MORE, unit_manager.expect_unit_identifier([&activated_units](Unit const& unit) -> bool {
-				activated_units.insert(&unit);
-				return true;
-			}),
-			"activate_building", ZERO_OR_MORE, building_manager.expect_building_type_identifier(
-				[&activated_buildings](BuildingType const& building_type) -> bool {
-					activated_buildings.insert(&building_type);
-					return true;
-				}
+			"activate_unit", ZERO_OR_MORE, unit_manager.expect_unit_identifier(set_callback_pointer(activated_units)),
+			"activate_building", ZERO_OR_MORE, building_type_manager.expect_building_type_identifier(
+				set_callback_pointer(activated_buildings)
 			),
 			"ai_chance", ONE_EXACTLY, success_callback //TODO
 		)(tech_value);
@@ -149,7 +147,7 @@ bool TechnologyManager::load_technologies_file(ModifierManager const& modifier_m
 	})(root);
 }
 
-#define TECH_MODIFIER(NAME, POS) ret &= modifier_manager.add_modifier_effect(NAME, POS, ModifierEffect::format_t::PROPORTION_DECIMAL)
+#define TECH_MODIFIER(NAME, POS) ret &= modifier_manager.add_modifier_effect(NAME, POS)
 #define UNCIV_TECH_MODIFIER(NAME) TECH_MODIFIER(NAME, false); TECH_MODIFIER(StringUtils::append_string_views("self_", NAME), false);
 bool TechnologyManager::generate_modifiers(ModifierManager& modifier_manager) {
 	bool ret = true;
@@ -160,7 +158,7 @@ bool TechnologyManager::generate_modifiers(ModifierManager& modifier_manager) {
 	for (TechnologyFolder const& folder : get_technology_folders()) {
 		TECH_MODIFIER(StringUtils::append_string_views(folder.get_identifier(), "_research_bonus"), true);
 	}
-	
+
 	return ret;
 }
 #undef UNCIV_TECH_MODIFIER
diff --git a/src/openvic-simulation/research/Technology.hpp b/src/openvic-simulation/research/Technology.hpp
index 0939369..540d10f 100644
--- a/src/openvic-simulation/research/Technology.hpp
+++ b/src/openvic-simulation/research/Technology.hpp
@@ -85,13 +85,13 @@ namespace OpenVic {
 		IDENTIFIER_REGISTRY_ACCESSORS_CUSTOM_PLURAL(technology, technologies)
 
 		bool add_technology_school(std::string_view identifier, ModifierValue&& values);
-		IDENTIFIER_REGISTRY_ACCESSORS(technology_school);
+		IDENTIFIER_REGISTRY_ACCESSORS(technology_school)
 
 		bool load_technology_file_areas(ast::NodeCPtr root); // common/technology.txt
 		bool load_technology_file_schools(ModifierManager const& modifier_manager, ast::NodeCPtr root); // also common/technology.txt
 		bool load_technologies_file(
-			ModifierManager const& modifier_manager, UnitManager const& unit_manager, BuildingManager const& building_manager,
-			ast::NodeCPtr root
+			ModifierManager const& modifier_manager, UnitManager const& unit_manager,
+			BuildingTypeManager const& building_type_manager, ast::NodeCPtr root
 		); // technologies/*.txt
 		bool generate_modifiers(ModifierManager& modifier_manager);
 	};