Country: datatypes + TGC compat changes

9 files changed, 247 insertions, 221 deletions

diff --git a/deps/openvic-dataloader b/deps/openvic-dataloader
-Subproject 0c5ecbc9b693a11548b55c2e0cc0c0a50a0f5f7
+Subproject efc11f571794acd64884680834c6636a03e8322
diff --git a/src/openvic-simulation/country/Country.cpp b/src/openvic-simulation/country/Country.cpp
index fc867d3..885340d 100644
--- a/src/openvic-simulation/country/Country.cpp
+++ b/src/openvic-simulation/country/Country.cpp
@@ -21,180 +21,216 @@
 using namespace OpenVic;
 using namespace OpenVic::NodeTools;
 
-std::string_view CountryParty::get_name() const {
-	return name;
-}
+CountryParty::CountryParty(
+	std::string_view new_identifier, Date new_start_date, Date new_end_date, Ideology const& new_ideology,
+	policy_map_t&& new_policies
+) : HasIdentifier { new_identifier }, start_date { new_start_date }, end_date { new_end_date }, ideology { new_ideology },
+	policies { std::move(new_policies) } {}
 
-const Date& CountryParty::get_start_date() const {
+Date const& CountryParty::get_start_date() const {
 	return start_date;
 }
 
-const Date& CountryParty::get_end_date() const {
+Date const& CountryParty::get_end_date() const {
 	return end_date;
 }
 
-const Ideology& CountryParty::get_ideology() const {
+Ideology const& CountryParty::get_ideology() const {
 	return ideology;
 }
 
-const std::vector<const Issue*>& CountryParty::get_policies() const {
+CountryParty::policy_map_t const& CountryParty::get_policies() const {
 	return policies;
 }
 
-CountryParty::CountryParty(
-	std::string_view new_name, Date new_start_date, Date new_end_date, const Ideology& new_ideology,
-	std::vector<const Issue*>&& new_policies
-) : name { new_name }, start_date { new_start_date }, end_date { new_end_date }, ideology { new_ideology },
-	policies { std::move(new_policies) } {}
-
-std::string_view UnitNames::get_identifier() const {
-	return identifier;
-}
-
-const std::vector<std::string>& UnitNames::get_names() const {
-	return names;
-}
-
-UnitNames::UnitNames(std::string_view new_identifier, std::vector<std::string>&& new_names)
-	: identifier { new_identifier }, names { std::move(new_names) } {}
+Country::Country(
+	std::string_view new_identifier, colour_t new_colour, GraphicalCultureType const& new_graphical_culture,
+	std::vector<CountryParty>&& new_parties, unit_names_map_t&& new_unit_names, bool new_dynamic_tag,
+	government_colour_map_t&& new_alternative_colours
+) : HasIdentifierAndColour { new_identifier, new_colour, true, false }, graphical_culture { new_graphical_culture },
+	parties { std::move(new_parties) }, unit_names { std::move(new_unit_names) }, dynamic_tag { new_dynamic_tag },
+	alternative_colours { std::move(new_alternative_colours) } {}
 
-const GraphicalCultureType& Country::get_graphical_culture() const {
+GraphicalCultureType const& Country::get_graphical_culture() const {
 	return graphical_culture;
 }
 
-const std::vector<CountryParty>& Country::get_parties() const {
+std::vector<CountryParty> const& Country::get_parties() const {
 	return parties;
 }
 
-const std::vector<UnitNames>& Country::get_unit_names() const {
+Country::unit_names_map_t const& Country::get_unit_names() const {
 	return unit_names;
 }
 
-const std::map<const GovernmentType*, colour_t>& Country::get_alternative_colours() const {
-	return alternative_colours;
-}
-
-const bool Country::is_dynamic_tag() const {
+bool Country::is_dynamic_tag() const {
 	return dynamic_tag;
 }
 
-Country::Country(
-	std::string_view new_identifier, colour_t new_color, const GraphicalCultureType& new_graphical_culture,
-	std::vector<CountryParty>&& new_parties, std::vector<UnitNames>&& new_unit_names, bool new_dynamic_tag,
-	std::map<const GovernmentType*, colour_t>&& new_alternative_colours
-) : HasIdentifierAndColour(new_identifier, new_color, false, false), graphical_culture { new_graphical_culture },
-	parties { std::move(new_parties) }, unit_names { std::move(new_unit_names) }, dynamic_tag { new_dynamic_tag },
-	alternative_colours { std::move(new_alternative_colours) } {}
+Country::government_colour_map_t const& Country::get_alternative_colours() const {
+	return alternative_colours;
+}
 
 CountryManager::CountryManager() : countries { "countries" } {}
 
 bool CountryManager::add_country(
-	std::string_view identifier, colour_t color, const GraphicalCultureType& graphical_culture,
-	std::vector<CountryParty>&& parties, std::vector<UnitNames>&& unit_names, bool dynamic_tag,
-	std::map<const GovernmentType*, colour_t>&& alternative_colours
+	std::string_view identifier, colour_t colour, GraphicalCultureType const* graphical_culture,
+	std::vector<CountryParty>&& parties, Country::unit_names_map_t&& unit_names, bool dynamic_tag,
+	Country::government_colour_map_t&& alternative_colours
 ) {
 	if (identifier.empty()) {
+		Logger::error("Invalid country identifier - empty!");
+		return false;
+	}
+	if (colour > MAX_COLOUR_RGB) {
+		Logger::error("Invalid country colour for ", identifier, ": ", colour_to_hex_string(colour));
+		return false;
+	}
+	if (graphical_culture == nullptr) {
+		Logger::error("Null graphical culture for country ", identifier);
 		return false;
 	}
 
 	return countries.add_item({
-		identifier, color, graphical_culture, std::move(parties), std::move(unit_names),
-		dynamic_tag, std::move(alternative_colours)
+		identifier, colour, *graphical_culture, std::move(parties), std::move(unit_names), dynamic_tag,
+		std::move(alternative_colours)
 	});
 }
 
-bool CountryManager::load_country_data_file(
-	GameManager& game_manager, std::string_view name, bool is_dynamic, ast::NodeCPtr root
+bool CountryManager::load_countries(
+	GameManager const& game_manager, Dataloader const& dataloader, fs::path const& countries_dir, ast::NodeCPtr root
 ) {
-	colour_t color;
-	const GraphicalCultureType* graphical_culture;
-	std::vector<CountryParty> country_parties;
-	std::vector<UnitNames> unit_names;
-	std::map<const GovernmentType*, colour_t> alternative_colours;
-
-	bool ret = expect_dictionary_keys_and_default(
-		[&game_manager, &alternative_colours, &name](std::string_view key, ast::NodeCPtr value) -> bool {
-			const GovernmentType* colour_gov_type;
-			bool ret = game_manager.get_politics_manager().get_government_type_manager()
-				.expect_government_type_str(assign_variable_callback_pointer(colour_gov_type))(key);
-
-			if (!ret) {
-				return false;
-			}
-
-			colour_t alternative_colour;
-			ret &= expect_colour(assign_variable_callback(alternative_colour))(value);
-
-			if (!ret) {
-				return false;
-			}
-
-			return alternative_colours.emplace(std::move(colour_gov_type), std::move(alternative_colour)).second;
-		},
-		"color", ONE_EXACTLY, expect_colour(assign_variable_callback(color)),
-		"graphical_culture", ONE_EXACTLY, expect_identifier_or_string(
-			[&game_manager, &graphical_culture, &name](std::string_view value) -> bool {
-				graphical_culture = game_manager.get_pop_manager().get_culture_manager()
-					.get_graphical_culture_type_by_identifier(value);
-				if (graphical_culture == nullptr) {
-					Logger::error("When loading country ", name, ", specified graphical culture ", value,
-						" is invalid!\nCheck that CultureManager has loaded before CountryManager.");
-				}
-
-				return graphical_culture != nullptr;
+	bool is_dynamic = false;
+
+	const bool ret = expect_dictionary_reserve_length(
+		countries,
+		[this, &game_manager, &is_dynamic, &dataloader, &countries_dir](std::string_view key, ast::NodeCPtr value) -> bool {
+			if (key == "dynamic_tags") {
+				return expect_bool([&is_dynamic](bool val) -> bool {
+					if (val == is_dynamic) {
+						Logger::warning("Redundant \"is_dynamic\", already ", val ? "true" : "false");
+					} else {
+						if (is_dynamic) {
+							Logger::warning("Changing \"is_dynamic\" back to false");
+						}
+						is_dynamic = val;
+					}
+					return true;
+				})(value);
 			}
-		),
-		"party", ZERO_OR_MORE, [&game_manager, &country_parties, &name](ast::NodeCPtr value) -> bool {
-			std::string_view party_name;
-			Date start_date, end_date;
-			const Ideology* ideology;
-			std::vector<const Issue*> policies;
-
-			bool ret = expect_dictionary_keys_and_default(
-				[&game_manager, &policies](std::string_view key, ast::NodeCPtr value) -> bool {
-					const Issue* policy;
-					bool ret = expect_identifier_or_string(
-						game_manager.get_politics_manager().get_issue_manager().expect_issue_str(
-							assign_variable_callback_pointer(policy)
-						)
-					)(value);
-
-					if (ret && policy->get_group().get_identifier() == key) {
-						policies.push_back(policy);
+			if (expect_string(
+				[this, &game_manager, is_dynamic, &dataloader, &countries_dir, &key](std::string_view filepath) -> bool {
+					if (load_country_data_file(
+						game_manager, key, is_dynamic,
+						Dataloader::parse_defines(dataloader.lookup_file(countries_dir / filepath)).get_file_node()
+					)) {
 						return true;
 					}
-
+					Logger::error("Failed to load country data file: ", filepath);
 					return false;
-				},
-				"name", ONE_EXACTLY, expect_identifier_or_string(assign_variable_callback(party_name)),
-				"start_date", ONE_EXACTLY, expect_date(assign_variable_callback(start_date)),
-				"end_date", ONE_EXACTLY, expect_date(assign_variable_callback(end_date)),
-				"ideology", ONE_EXACTLY, expect_identifier_or_string(game_manager.get_politics_manager()
-					.get_ideology_manager().expect_ideology_str(assign_variable_callback_pointer(ideology)))
-			)(value);
+				}
+			)(value)) {
+				return true;
+			}
+			Logger::error("Failed to load country: ", key);
+			return false;
+		}
+	)(root);
+	lock_countries();
+	return ret;
+}
 
-			country_parties.push_back({ party_name, start_date, end_date, *ideology, std::move(policies) });
+node_callback_t CountryManager::load_country_party(
+	PoliticsManager const& politics_manager, std::vector<CountryParty>& country_parties
+) const {
+	return [&politics_manager, &country_parties](ast::NodeCPtr value) -> bool {
+		std::string_view party_name;
+		Date start_date, end_date;
+		Ideology const* ideology;
+		CountryParty::policy_map_t policies;
+
+		const bool ret = expect_dictionary_keys_and_default(
+			[&politics_manager, &policies, &party_name](std::string_view key, ast::NodeCPtr value) -> bool {
+				return politics_manager.get_issue_manager().expect_issue_group_str(
+					[&politics_manager, &policies, value, &party_name](IssueGroup const& group) -> bool {
+						if (policies.contains(&group)) {
+							Logger::error("Country party ", party_name, " has duplicate entry for ", group.get_identifier());
+							return false;
+						}
+						return politics_manager.get_issue_manager().expect_issue_identifier(
+							[&policies, &group](Issue const& issue) -> bool {
+								if (&issue.get_group() == &group) {
+									policies.emplace(&group, &issue);
+									return true;
+								}
+								Logger::error("Invalid policy ", issue.get_identifier(), ", group is ",
+									issue.get_group().get_identifier(), " when ", group.get_identifier(), " was expected");
+								return false;
+							}
+						)(value);
+					}
+				)(key);
+			},
+			"name", ONE_EXACTLY, expect_string(assign_variable_callback(party_name)),
+			"start_date", ONE_EXACTLY, expect_date(assign_variable_callback(start_date)),
+			"end_date", ONE_EXACTLY, expect_date(assign_variable_callback(end_date)),
+			"ideology", ONE_EXACTLY,
+				politics_manager.get_ideology_manager().expect_ideology_identifier(assign_variable_callback_pointer(ideology))
+		)(value);
+
+		country_parties.emplace_back(CountryParty { party_name, start_date, end_date, *ideology, std::move(policies) });
+
+		return ret;
+	};
+}
 
-			return ret;
+bool CountryManager::load_country_data_file(
+	GameManager const& game_manager, std::string_view name, bool is_dynamic, ast::NodeCPtr root
+) {
+	colour_t colour;
+	GraphicalCultureType const* graphical_culture;
+	std::vector<CountryParty> country_parties;
+	Country::unit_names_map_t unit_names;
+	Country::government_colour_map_t alternative_colours;
+	bool ret = expect_dictionary_keys_and_default(
+		[&game_manager, &alternative_colours, &name](std::string_view key, ast::NodeCPtr value) -> bool {
+			return game_manager.get_politics_manager().get_government_type_manager().expect_government_type_str(
+				[&alternative_colours, &name, &value](GovernmentType const& government_type) -> bool {
+					if (alternative_colours.contains(&government_type)) {
+						Logger::error(
+							"Country ", name, " has duplicate entry for ", government_type.get_identifier(),
+							" alternative colour"
+						);
+						return false;
+					}
+					return expect_colour([&alternative_colours, &government_type](colour_t colour) -> bool {
+						return alternative_colours.emplace(&government_type, std::move(colour)).second;
+					})(value);
+				}
+			)(key);
 		},
-		"unit_names", ZERO_OR_ONE, expect_dictionary([&unit_names](std::string_view key, ast::NodeCPtr value) -> bool {
-			std::vector<std::string> names;
-
-			bool ret = expect_list(expect_identifier_or_string(
-				[&names](std::string_view value) -> bool {
-					names.push_back(std::string(value));
-					return true;
+		"color", ONE_EXACTLY, expect_colour(assign_variable_callback(colour)),
+		"graphical_culture", ONE_EXACTLY,
+			game_manager.get_pop_manager().get_culture_manager().expect_graphical_culture_type_identifier(
+				assign_variable_callback_pointer(graphical_culture)
+			),
+		"party", ZERO_OR_MORE, load_country_party(game_manager.get_politics_manager(), country_parties),
+		"unit_names", ZERO_OR_ONE,
+			game_manager.get_military_manager().get_unit_manager().expect_unit_dictionary(
+				[&unit_names, &name](Unit const& unit, ast::NodeCPtr value) -> bool {
+					if (unit_names.contains(&unit)) {
+						Logger::error("Country ", name, " has duplicate entry for ", unit.get_identifier(), " name list");
+						return false;
+					}
+					return name_list_callback([&unit_names, &unit](std::vector<std::string>&& list) -> bool {
+						return unit_names.emplace(&unit, std::move(list)).second;
+					})(value);
 				}
-			))(value);
-
-			unit_names.push_back({ key, std::move(names) });
-
-			return ret;
-		})
+			)
 	)(root);
 
 	ret &= add_country(
-		name, color, *graphical_culture, std::move(country_parties), std::move(unit_names), is_dynamic,
+		name, colour, graphical_culture, std::move(country_parties), std::move(unit_names), is_dynamic,
 		std::move(alternative_colours)
 	);
 	return ret;
diff --git a/src/openvic-simulation/country/Country.hpp b/src/openvic-simulation/country/Country.hpp
index 66ce2e6..b1bbf2b 100644
--- a/src/openvic-simulation/country/Country.hpp
+++ b/src/openvic-simulation/country/Country.hpp
@@ -12,10 +12,12 @@
 
 #include "openvic-simulation/dataloader/Dataloader.hpp"
 #include "openvic-simulation/map/Province.hpp"
+#include "openvic-simulation/military/Unit.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/PoliticsManager.hpp"
 #include "openvic-simulation/pop/Culture.hpp"
 #include "openvic-simulation/pop/Religion.hpp"
 #include "openvic-simulation/types/Colour.hpp"
@@ -26,81 +28,86 @@ namespace OpenVic {
 	struct GameManager;
 	struct CountryManager;
 
-	struct CountryParty {
+	struct CountryParty : HasIdentifier {
 		friend struct CountryManager;
 
+		using policy_map_t = std::map<IssueGroup const*, Issue const*>;
+
 	private:
-		const std::string name;
 		const Date start_date;
 		const Date end_date;
-		const Ideology& ideology;
-		const std::vector<Issue const*> policies;
+		Ideology const& ideology;
+		const policy_map_t policies;
 
 		CountryParty(
-			std::string_view new_name, Date new_start_date, Date new_end_date, const Ideology& new_ideology,
-			std::vector<const Issue*>&& new_policies
+			std::string_view new_identifier, Date new_start_date, Date new_end_date, Ideology const& new_ideology,
+			policy_map_t&& new_policies
 		);
 
 	public:
-		std::string_view get_name() const;
+		CountryParty(CountryParty&&) = default;
+
 		const Date& get_start_date() const;
 		const Date& get_end_date() const;
-		const Ideology& get_ideology() const;
-		const std::vector<const Issue*>& get_policies() const;
-	};
-
-	struct UnitNames {
-		friend struct CountryManager;
-
-	private:
-		const std::string identifier;
-		const std::vector<std::string> names;
-
-		UnitNames(std::string_view new_identifier, std::vector<std::string>&& new_names);
-
-	public:
-		std::string_view get_identifier() const;
-		const std::vector<std::string>& get_names() const;
+		Ideology const& get_ideology() const;
+		policy_map_t const& get_policies() const;
 	};
 
 	struct Country : HasIdentifierAndColour {
 		friend struct CountryManager;
 
+		using unit_names_map_t = std::map<Unit const*, std::vector<std::string>>;
+		using government_colour_map_t = std::map<GovernmentType const*, colour_t>;
+
 	private:
-		const GraphicalCultureType& graphical_culture;
-		const std::vector<CountryParty> parties;
-		const std::vector<UnitNames> unit_names;
+		GraphicalCultureType const& graphical_culture;
+		/* Not const to allow elements to be moved, otherwise a copy is forced
+		 * which causes a compile error as the copy constructor has been deleted.
+		 */
+		std::vector<CountryParty> parties;
+		const unit_names_map_t unit_names;
 		const bool dynamic_tag;
-		const std::map<const GovernmentType*, colour_t> alternative_colours;
+		const government_colour_map_t alternative_colours;
 
 		Country(
-			std::string_view new_identifier, colour_t new_color, const GraphicalCultureType& new_graphical_culture,
-			std::vector<CountryParty>&& new_parties, std::vector<UnitNames>&& new_unit_names, const bool new_dynamic_tag,
-			std::map<const GovernmentType*, colour_t>&& new_alternative_colours
+			std::string_view new_identifier, colour_t new_colour, GraphicalCultureType const& new_graphical_culture,
+			std::vector<CountryParty>&& new_parties, unit_names_map_t&& new_unit_names, bool new_dynamic_tag,
+			government_colour_map_t&& new_alternative_colours
 		);
 
 	public:
-		const GraphicalCultureType& get_graphical_culture() const;
-		const std::vector<CountryParty>& get_parties() const;
-		const std::vector<UnitNames>& get_unit_names() const;
-		const bool is_dynamic_tag() const;
-		const std::map<const GovernmentType*, colour_t>& get_alternative_colours() const;
+		Country(Country&&) = default;
+
+		GraphicalCultureType const& get_graphical_culture() const;
+		std::vector<CountryParty> const& get_parties() const;
+		unit_names_map_t const& get_unit_names() const;
+		bool is_dynamic_tag() const;
+		government_colour_map_t const& get_alternative_colours() const;
 	};
 
 	struct CountryManager {
 	private:
 		IdentifierRegistry<Country> countries;
 
+		NodeTools::node_callback_t load_country_party(
+			PoliticsManager const& politics_manager, std::vector<CountryParty>& country_parties
+		) const;
+
 	public:
 		CountryManager();
 
 		bool add_country(
-			std::string_view identifier, colour_t color, const GraphicalCultureType& graphical_culture,
-			std::vector<CountryParty>&& parties, std::vector<UnitNames>&& unit_names, bool dynamic_tag,
-			std::map<const GovernmentType*, colour_t>&& alternative_colours
+			std::string_view identifier, colour_t colour, GraphicalCultureType const* graphical_culture,
+			std::vector<CountryParty>&& parties, Country::unit_names_map_t&& unit_names, bool dynamic_tag,
+			Country::government_colour_map_t&& alternative_colours
 		);
 		IDENTIFIER_REGISTRY_ACCESSORS_CUSTOM_PLURAL(country, countries);
 
-		bool load_country_data_file(GameManager& game_manager, std::string_view name, bool is_dynamic, ast::NodeCPtr root);
+		bool load_countries(
+			GameManager const& game_manager, Dataloader const& dataloader, fs::path const& countries_dir, ast::NodeCPtr root
+		);
+		bool load_country_data_file(
+			GameManager const& game_manager, std::string_view name, bool is_dynamic, ast::NodeCPtr root
+		);
 	};
 }
diff --git a/src/openvic-simulation/dataloader/Dataloader.cpp b/src/openvic-simulation/dataloader/Dataloader.cpp
index 5301aa1..0094253 100644
--- a/src/openvic-simulation/dataloader/Dataloader.cpp
+++ b/src/openvic-simulation/dataloader/Dataloader.cpp
@@ -534,31 +534,6 @@ bool Dataloader::_load_oobs(GameManager& game_manager) const {
 	return ret;
 }
 
-bool Dataloader::_load_countries(GameManager& game_manager) const {
-	static const fs::path countries_file = "common/countries.txt";
-
-	bool is_dynamic = false;
-
-	bool ret = expect_dictionary([this, &game_manager, &is_dynamic](std::string_view key, ast::NodeCPtr value) -> bool {
-		if (key == "dynamic_tags") {
-			return expect_bool(assign_variable_callback(is_dynamic))(value);
-		}
-
-		std::string_view data_path;
-
-		if (!expect_string(assign_variable_callback(data_path))(value)) {
-			return false;
-		}
-
-		return game_manager.get_country_manager().load_country_data_file(
-			game_manager, key, is_dynamic, parse_defines(lookup_file(countries_file.parent_path() / data_path)).get_file_node()
-		);
-	})(parse_defines(lookup_file(countries_file)).get_file_node());
-	game_manager.get_country_manager().lock_countries();
-
-	return ret;
-}
-
 bool Dataloader::_load_history(GameManager& game_manager) const {
 	static const fs::path country_history_directory = "history/countries";
 	static const fs::path province_history_directory = "history/provinces";
@@ -724,6 +699,7 @@ bool Dataloader::load_defines(GameManager& game_manager) const {
 	static const fs::path defines_file = "common/defines.lua";
 	static const fs::path buildings_file = "common/buildings.txt";
 	static const fs::path bookmark_file = "common/bookmarks.txt";
+	static const fs::path countries_file = "common/countries.txt";
 	static const fs::path culture_file = "common/cultures.txt";
 	static const fs::path goods_file = "common/goods.txt";
 	static const fs::path governments_file = "common/governments.txt";
@@ -836,7 +812,9 @@ bool Dataloader::load_defines(GameManager& game_manager) const {
 		Logger::error("Failed to load orders of battle!");
 		ret = false;
 	}
-	if (!_load_countries(game_manager)) {
+	if (!game_manager.get_country_manager().load_countries(
+		game_manager, *this, countries_file.parent_path(), parse_defines(lookup_file(countries_file)).get_file_node()
+	)) {
 		Logger::error("Failed to load countries!");
 		ret = false;
 	}
diff --git a/src/openvic-simulation/dataloader/Dataloader.hpp b/src/openvic-simulation/dataloader/Dataloader.hpp
index 0bf3086..508bf13 100644
--- a/src/openvic-simulation/dataloader/Dataloader.hpp
+++ b/src/openvic-simulation/dataloader/Dataloader.hpp
@@ -23,13 +23,10 @@ namespace OpenVic {
 	private:
 		path_vector_t roots;
 
-		bool _load_pop_types(
-			PopManager& pop_manager, UnitManager const& unit_manager, GoodManager const& good_manager
-		) const;
+		bool _load_pop_types(PopManager& pop_manager, UnitManager const& unit_manager, GoodManager const& good_manager) const;
 		bool _load_units(UnitManager& unit_manager, GoodManager const& good_manager) const;
 		bool _load_map_dir(GameManager& game_manager) const;
 		bool _load_oobs(GameManager& game_manager) const;
-		bool _load_countries(GameManager& game_manager) const;
 		bool _load_history(GameManager& game_manager) const;
 
 	public:
diff --git a/src/openvic-simulation/history/CountryHistory.cpp b/src/openvic-simulation/history/CountryHistory.cpp
index 2dfc171..d0764be 100644
--- a/src/openvic-simulation/history/CountryHistory.cpp
+++ b/src/openvic-simulation/history/CountryHistory.cpp
@@ -272,7 +272,7 @@ inline bool CountryHistoryManager::_load_country_history_entry(
 				const std::vector<CountryParty>* parties =
 					&game_manager.get_country_manager().get_country_by_identifier(name)->get_parties();
 				for (auto& party : *parties) {
-					if (party.get_name() == identifier) {
+					if (party.get_identifier() == identifier) {
 						if (party.get_start_date() <= date && date <= party.get_end_date()) {
 							ruling_party = &party;
 							return true;
@@ -337,9 +337,10 @@ inline bool CountryHistoryManager::_load_country_history_entry(
 	)(root);
 
 	ret &= add_country_history_entry(
-		game_manager.get_country_manager().get_country_by_identifier(name), date, primary_culture, std::move(accepted_cultures),
-		religion, ruling_party, last_election, std::move(upper_house), capital, government_type, plurality, national_value,
-		civilised, prestige, std::move(reforms), initial_oob, updated_accepted_cultures, updated_upper_house, updated_reforms
+		game_manager.get_country_manager().get_country_by_identifier(name), date, primary_culture,
+		std::move(accepted_cultures), religion, ruling_party, last_election, std::move(upper_house), capital, government_type,
+		plurality, national_value, civilised, prestige, std::move(reforms), initial_oob, updated_accepted_cultures,
+		updated_upper_house, updated_reforms
 	);
 	return ret;
 }
diff --git a/src/openvic-simulation/types/IdentifierRegistry.hpp b/src/openvic-simulation/types/IdentifierRegistry.hpp
index bcb8e33..ebdedce 100644
--- a/src/openvic-simulation/types/IdentifierRegistry.hpp
+++ b/src/openvic-simulation/types/IdentifierRegistry.hpp
@@ -49,10 +49,12 @@ namespace OpenVic {
 
 #define HASID_PROPERTY(NAME) \
 	const NAME; \
+\
 public: \
-	auto get_##NAME() const->decltype(get_property(NAME)) { \
+	auto get_##NAME() const -> decltype(get_property(NAME)) { \
 		return get_property(NAME); \
 	} \
+\
 private:
 	};
 
@@ -180,9 +182,10 @@ private:
 					return duplicate_callback(name, new_identifier);
 				}
 			}
-			identifier_index_map.emplace(new_identifier, items.size());
-			items.push_back(std::move(item));
-			return true;
+			const std::pair<string_map_t<size_t>::iterator, bool> ret =
+				identifier_index_map.emplace(std::move(new_identifier), items.size());
+			items.emplace_back(std::move(item));
+			return ret.second && ret.first->second + 1 == items.size();
 		}
 
 		void lock() {
@@ -280,8 +283,9 @@ private:
 			return identifiers;
 		}
 
-		NodeTools::node_callback_t expect_item_decimal_map(NodeTools::callback_t<decimal_map_t<value_type const*>&&> callback)
-			const {
+		NodeTools::node_callback_t expect_item_decimal_map(
+			NodeTools::callback_t<decimal_map_t<value_type const*>&&> callback
+		) const {
 			return [this, callback](ast::NodeCPtr node) -> bool {
 				decimal_map_t<value_type const*> map;
 				bool ret = expect_item_dictionary([&map](value_type const& key, ast::NodeCPtr value) -> bool {
@@ -390,7 +394,9 @@ private:
 	) { \
 		return plural.expect_item_str(callback); \
 	} \
-	NodeTools::node_callback_t expect_##singular##_identifier(NodeTools::callback_t<decltype(plural)::value_type&> callback) { \
+	NodeTools::node_callback_t expect_##singular##_identifier( \
+		NodeTools::callback_t<decltype(plural)::value_type&> callback \
+	) { \
 		return plural.expect_item_identifier(callback); \
 	} \
 	NodeTools::node_callback_t expect_##singular##_dictionary( \
diff --git a/src/openvic-simulation/utility/Logger.cpp b/src/openvic-simulation/utility/Logger.cpp
index 5e25c98..63dfd6c 100644
--- a/src/openvic-simulation/utility/Logger.cpp
+++ b/src/openvic-simulation/utility/Logger.cpp
@@ -6,13 +6,13 @@ using namespace OpenVic;
 
 void Logger::set_logger_funcs() {
 	Logger::set_info_func([](std::string&& str) {
-		std::cout << str;
+		std::cout << "[INFO] " << str;
 	});
 	Logger::set_warning_func([](std::string&& str) {
-		std::cerr << str;
+		std::cerr << "[WARNING] " << str;
 	});
 	Logger::set_error_func([](std::string&& str) {
-		std::cerr << str;
+		std::cerr << "[ERROR] " << str;
 	});
 }
 
diff --git a/src/openvic-simulation/utility/Logger.hpp b/src/openvic-simulation/utility/Logger.hpp
index a1c599d..04306f7 100644
--- a/src/openvic-simulation/utility/Logger.hpp
+++ b/src/openvic-simulation/utility/Logger.hpp
@@ -65,13 +65,13 @@ namespace OpenVic {
 		struct log {
 			log(log_channel_t& log_channel, Ts&&... ts, source_location const& location) {
 				std::stringstream stream;
-				stream << "\n" << get_filename(location.file_name()) << "("
+				stream << get_filename(location.file_name()) << "("
 					/* Function name removed to reduce clutter. It is already included
 					* in Godot's print functions, so this was repeating it. */
 					//<< location.line() << ") `" << location.function_name() << "`: ";
 					<< location.line() << "): ";
 				((stream << std::forward<Ts>(ts)), ...);
-				stream << std::endl;
+				stream << "\n" << std::endl;
 				log_channel.queue.push(stream.str());
 				if (log_channel.func) {
 					do {
@@ -83,20 +83,21 @@ namespace OpenVic {
 		};
 
 #define LOG_FUNC(name) \
-	private: \
-		static inline log_channel_t name##_channel {}; \
-	public: \
-		static inline void set_##name##_func(log_func_t log_func) { \
-			name##_channel.func = log_func; \
+private: \
+	static inline log_channel_t name##_channel {}; \
+\
+public: \
+	static inline void set_##name##_func(log_func_t log_func) { \
+		name##_channel.func = log_func; \
+	} \
+	template<typename... Ts> \
+	struct name { \
+		name(Ts&&... ts, source_location const& location = source_location::current()) { \
+			log<Ts...> { name##_channel, std::forward<Ts>(ts)..., location }; \
 		} \
-		template<typename... Ts> \
-		struct name { \
-			name(Ts&&... ts, source_location const& location = source_location::current()) { \
-				log<Ts...> { name##_channel, std::forward<Ts>(ts)..., location }; \
-			} \
-		}; \
-		template<typename... Ts> \
-		name(Ts&&...) -> name<Ts...>;
+	}; \
+	template<typename... Ts> \
+	name(Ts&&...) -> name<Ts...>;
 
 		LOG_FUNC(info)
 		LOG_FUNC(warning)