Directory lookup refactor+suppressed wargoal error

10 files changed, 166 insertions, 220 deletions

diff --git a/src/openvic-simulation/dataloader/Dataloader.cpp b/src/openvic-simulation/dataloader/Dataloader.cpp
index f50aa39..011b524 100644
--- a/src/openvic-simulation/dataloader/Dataloader.cpp
+++ b/src/openvic-simulation/dataloader/Dataloader.cpp
@@ -36,8 +36,13 @@ using namespace ovdl;
 
 using StringUtils::append_string_views;
 
-#define FILESYSTEM_CASE_INSENSITIVE (defined(_WIN32) || (defined(__APPLE__) && defined(__MACH__)))
-#define FILESYSTEM_NEEDS_FORWARD_SLASHES (!defined(_WIN32))
+#if defined(_WIN32) || (defined(__APPLE__) && defined(__MACH__))
+#define FILESYSTEM_CASE_INSENSITIVE
+#endif
+
+#if !defined(_WIN32)
+#define FILESYSTEM_NEEDS_FORWARD_SLASHES
+#endif
 
 static constexpr bool path_equals_case_insensitive(std::string_view lhs, std::string_view rhs) {
 	constexpr auto ichar_equals = [](unsigned char l, unsigned char r) {
@@ -48,7 +53,7 @@ static constexpr bool path_equals_case_insensitive(std::string_view lhs, std::st
 
 // Windows and Mac by default act like case insensitive filesystems
 static constexpr bool path_equals(std::string_view lhs, std::string_view rhs) {
-#if FILESYSTEM_CASE_INSENSITIVE
+#if defined(FILESYSTEM_CASE_INSENSITIVE)
 	return path_equals_case_insensitive(lhs, rhs);
 #else
 	return std::equal(lhs.begin(), lhs.end(), rhs.begin(), rhs.end());
@@ -369,7 +374,7 @@ bool Dataloader::set_roots(path_vector_t const& new_roots) {
 }
 
 fs::path Dataloader::lookup_file(std::string_view path, bool print_error) const {
-#if FILESYSTEM_NEEDS_FORWARD_SLASHES
+#if defined(FILESYSTEM_NEEDS_FORWARD_SLASHES)
 	/* Back-slashes need to be converted into forward-slashes */
 	const std::string forward_slash_path { StringUtils::make_forward_slash_path(StringUtils::remove_leading_slashes(path)) };
 	path = forward_slash_path;
@@ -377,7 +382,7 @@ fs::path Dataloader::lookup_file(std::string_view path, bool print_error) const
 
 	const fs::path filepath { path };
 
-#if FILESYSTEM_CASE_INSENSITIVE
+#if defined(FILESYSTEM_CASE_INSENSITIVE)
 	/* Case-insensitive filesystem */
 	for (fs::path const& root : roots) {
 		const fs::path composed = root / filepath;
@@ -411,7 +416,7 @@ fs::path Dataloader::lookup_file(std::string_view path, bool print_error) const
 	return {};
 }
 
-fs::path Dataloader::lookup_image_file(std::string_view path) const {
+fs::path Dataloader::lookup_image_file_or_dds(std::string_view path) const {
 	fs::path ret = lookup_file(path, false);
 	if (ret.empty()) {
 		// TODO - change search order so root order takes priority over extension replacement order
@@ -424,34 +429,49 @@ fs::path Dataloader::lookup_image_file(std::string_view path) const {
 	return ret;
 }
 
-template<typename _DirIterator, std::predicate<fs::path const&, fs::path const&> _Equiv>
-Dataloader::path_vector_t Dataloader::_lookup_files_in_dir(std::string_view path, fs::path const& extension) const {
-#if FILESYSTEM_NEEDS_FORWARD_SLASHES
+template<typename _DirIterator, typename _UniqueKey>
+requires requires (_UniqueKey const& unique_key, std::string_view path) {
+	{ unique_key(path) } -> std::convertible_to<std::string_view>;
+}
+Dataloader::path_vector_t Dataloader::_lookup_files_in_dir(
+	std::string_view path, fs::path const& extension, _UniqueKey const& unique_key
+) const {
+#if defined(FILESYSTEM_NEEDS_FORWARD_SLASHES)
 	/* Back-slashes need to be converted into forward-slashes */
 	const std::string forward_slash_path { StringUtils::make_forward_slash_path(StringUtils::remove_leading_slashes(path)) };
 	path = forward_slash_path;
 #endif
-	const fs::path filepath { path };
-	static constexpr _Equiv Equiv {};
+	const fs::path dirpath { path };
 	path_vector_t ret;
-	size_t start_of_current_root_entries;
+	struct file_entry_t {
+		fs::path file;
+		fs::path const* root;
+	};
+	string_map_t<file_entry_t> found_files;
 	for (fs::path const& root : roots) {
-		start_of_current_root_entries = ret.size();
-		const fs::path composed = root / filepath;
+		const size_t root_len = root.string().size();
+		const fs::path composed = root / dirpath;
 		std::error_code ec;
 		for (fs::directory_entry const& entry : _DirIterator { composed, ec }) {
 			if (entry.is_regular_file()) {
-				const fs::path file = entry;
-				if ((extension.empty() || file.extension() == extension) && !Equiv(file, {})) {
-					size_t index = 0;
-					for (; index < ret.size() && !Equiv(file, ret[index]); ++index) {}
-					if (index >= ret.size()) {
-						ret.push_back(file);
-					} else if (start_of_current_root_entries <= index) {
-						Logger::warning(
-							"Files in the same directory with conflicting names: ", ret[index], " (accepted) and ", file,
-							" (rejected)"
-						);
+				fs::path file = entry;
+				if ((extension.empty() || file.extension() == extension)) {
+					const std::string full_path = file.string();
+					std::string_view relative_path = full_path;
+					relative_path.remove_prefix(root_len);
+					relative_path = StringUtils::remove_leading_slashes(relative_path);
+					const std::string_view key = unique_key(relative_path);
+					if (!key.empty()) {
+						const typename decltype(found_files)::const_iterator it = found_files.find(key);
+						if (it == found_files.end()) {
+							found_files.emplace(key, file_entry_t { file, &root });
+							ret.emplace_back(std::move(file));
+						} else if (it->second.root == &root) {
+							Logger::warning(
+								"Files in the same directory with conflicting keys: ", it->first, " - ", it->second.file,
+								" (accepted) and ", key, " - ", file, " (rejected)"
+							);
+						}
 					}
 				}
 			}
@@ -460,38 +480,28 @@ Dataloader::path_vector_t Dataloader::_lookup_files_in_dir(std::string_view path
 	return ret;
 }
 
-struct EquivFilename {
-	bool operator()(fs::path const& lhs, fs::path const& rhs) const {
-		return lhs.filename() == rhs.filename();
-	}
-};
-
 Dataloader::path_vector_t Dataloader::lookup_files_in_dir(std::string_view path, fs::path const& extension) const {
-	return _lookup_files_in_dir<fs::directory_iterator, EquivFilename>(path, extension);
+	return _lookup_files_in_dir<fs::directory_iterator>(path, extension, std::identity {});
 }
 
 Dataloader::path_vector_t Dataloader::lookup_files_in_dir_recursive(std::string_view path, fs::path const& extension) const {
-	return _lookup_files_in_dir<fs::recursive_directory_iterator, EquivFilename>(path, extension);
+	return _lookup_files_in_dir<fs::recursive_directory_iterator>(path, extension, std::identity {});
 }
 
-struct EquivBasicIdentifierPrefix {
-	bool operator()(fs::path const& lhs, fs::path const& rhs) const {
-		const std::string lhs_str = lhs.stem().string();
-		const std::string rhs_str = rhs.stem().string();
-		return extract_basic_identifier_prefix(lhs_str) == extract_basic_identifier_prefix(rhs_str);
-	}
+static std::string_view _extract_basic_identifier_prefix_from_path(std::string_view path) {
+	return extract_basic_identifier_prefix(StringUtils::get_filename(path));
 };
 
 Dataloader::path_vector_t Dataloader::lookup_basic_indentifier_prefixed_files_in_dir(
 	std::string_view path, fs::path const& extension
 ) const {
-	return _lookup_files_in_dir<fs::directory_iterator, EquivBasicIdentifierPrefix>(path, extension);
+	return _lookup_files_in_dir<fs::directory_iterator>(path, extension, _extract_basic_identifier_prefix_from_path);
 }
 
 Dataloader::path_vector_t Dataloader::lookup_basic_indentifier_prefixed_files_in_dir_recursive(
 	std::string_view path, fs::path const& extension
 ) const {
-	return _lookup_files_in_dir<fs::recursive_directory_iterator, EquivBasicIdentifierPrefix>(path, extension);
+	return _lookup_files_in_dir<fs::recursive_directory_iterator>(path, extension, _extract_basic_identifier_prefix_from_path);
 }
 
 bool Dataloader::apply_to_files(path_vector_t const& files, callback_t<fs::path const&> callback) const {
@@ -936,7 +946,9 @@ bool Dataloader::load_defines(GameManager& game_manager) const {
 		Logger::error("Failed to load leader traits!");
 		ret = false;
 	}
-	if (!game_manager.get_military_manager().get_wargoal_manager().load_wargoal_file(parse_defines(lookup_file(cb_types_file)).get_file_node())) {
+	if (!game_manager.get_military_manager().get_wargoal_manager().load_wargoal_file(
+		parse_defines(lookup_file(cb_types_file)).get_file_node()
+	)) {
 		Logger::error("Failed to load wargoals!");
 		ret = false;
 	}
diff --git a/src/openvic-simulation/dataloader/Dataloader.hpp b/src/openvic-simulation/dataloader/Dataloader.hpp
index 7c71763..5039582 100644
--- a/src/openvic-simulation/dataloader/Dataloader.hpp
+++ b/src/openvic-simulation/dataloader/Dataloader.hpp
@@ -30,11 +30,16 @@ namespace OpenVic {
 		bool _load_map_dir(GameManager& game_manager) const;
 		bool _load_history(GameManager& game_manager, bool unused_history_file_warnings) const;
 
-		/* _DirIterator is fs::directory_iterator or fs::recursive_directory_iterator.
-		 * _Equiv is an equivalence relation with respect to which every found file shall be unique.
-		 * If a file is equivalent to the empty path then it is not included. */
-		template<typename _DirIterator, std::predicate<fs::path const&, fs::path const&> _Equiv>
-		path_vector_t _lookup_files_in_dir(std::string_view path, fs::path const& extension) const;
+		/* _DirIterator is fs::directory_iterator or fs::recursive_directory_iterator. _UniqueKey is the type of a callable
+		 * which converts a string_view filepath with root removed into a string_view unique key. Any path whose key is empty
+		 * or matches an earlier found path's key is discarded, ensuring each looked up path's key is non-empty and unique. */
+		template<typename _DirIterator, typename _UniqueKey>
+		requires requires (_UniqueKey const& unique_key, std::string_view path) {
+			{ unique_key(path) } -> std::convertible_to<std::string_view>;
+		}
+		path_vector_t _lookup_files_in_dir(
+			std::string_view path, fs::path const& extension, _UniqueKey const& unique_key
+		) const;
 
 	public:
 		static ovdl::v2script::Parser parse_defines(fs::path const& path);
@@ -73,7 +78,7 @@ namespace OpenVic {
 		 */
 		fs::path lookup_file(std::string_view path, bool print_error = true) const;
 		/* Checks alternate file endings, e.g. if "*.tga" doesn't exist then try "*.dds" */
-		fs::path lookup_image_file(std::string_view path) const;
+		fs::path lookup_image_file_or_dds(std::string_view path) const;
 		path_vector_t lookup_files_in_dir(std::string_view path, fs::path const& extension) const;
 		path_vector_t lookup_files_in_dir_recursive(std::string_view path, fs::path const& extension) const;
 		path_vector_t lookup_basic_indentifier_prefixed_files_in_dir(std::string_view path, fs::path const& extension) const;
diff --git a/src/openvic-simulation/history/DiplomaticHistory.cpp b/src/openvic-simulation/history/DiplomaticHistory.cpp
index 6f9d73e..5cbafe7 100644
--- a/src/openvic-simulation/history/DiplomaticHistory.cpp
+++ b/src/openvic-simulation/history/DiplomaticHistory.cpp
@@ -164,7 +164,7 @@ bool DiplomaticHistoryManager::load_diplomacy_history_file(GameManager& game_man
 				"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);
-			
+
 			alliances.push_back({ first, second, start, end });
 			return ret;
 		},
diff --git a/src/openvic-simulation/history/DiplomaticHistory.hpp b/src/openvic-simulation/history/DiplomaticHistory.hpp
index 85e2654..64529c2 100644
--- a/src/openvic-simulation/history/DiplomaticHistory.hpp
+++ b/src/openvic-simulation/history/DiplomaticHistory.hpp
@@ -46,7 +46,7 @@ namespace OpenVic {
 			std::optional<Date> exited;
 
 			war_participant_t(Country const* new_country, Date new_joined, std::optional<Date> new_exited);
-		
+
 		public:
 			Country const* get_country() const;
 			Date get_date_joined() const;
@@ -105,7 +105,7 @@ namespace OpenVic {
 	public:
 		Country const* get_overlord() const;
 		Country const* get_subject() const;
-		const type_t get_subject_type() const; 
+		const type_t get_subject_type() const;
 	};
 
 	struct DiplomaticHistoryManager {
diff --git a/src/openvic-simulation/interface/GFX.cpp b/src/openvic-simulation/interface/GFX.cpp
index 24f4b21..f4e2074 100644
--- a/src/openvic-simulation/interface/GFX.cpp
+++ b/src/openvic-simulation/interface/GFX.cpp
@@ -15,6 +15,7 @@ node_callback_t Sprite::expect_sprite(callback_t<std::unique_ptr<Sprite>&&> call
 		"LineChartType", ZERO_OR_MORE, _expect_instance<Sprite, LineChart>(callback),
 		"textSpriteType", ZERO_OR_MORE, _expect_instance<Sprite, TextureSprite>(callback),
 		"maskedShieldType", ZERO_OR_MORE, _expect_instance<Sprite, MaskedFlag>(callback),
+		// TODO - add the rest of the sprite types
 		"corneredTileSpriteType", ZERO_OR_MORE, success_callback,
 		"tileSpriteType", ZERO_OR_MORE, success_callback,
 		"BarChartType", ZERO_OR_MORE, success_callback,
diff --git a/src/openvic-simulation/interface/GFX.hpp b/src/openvic-simulation/interface/GFX.hpp
index e155486..2422e24 100644
--- a/src/openvic-simulation/interface/GFX.hpp
+++ b/src/openvic-simulation/interface/GFX.hpp
@@ -22,7 +22,7 @@ namespace OpenVic::GFX {
 		Font(Font&&) = default;
 	};
 
-	using frame_t = int32_t;
+	using frame_t = int32_t; /* Keep this as int32_t to simplify interfacing with Godot */
 	static constexpr frame_t NO_FRAMES = 0;
 
 	class Sprite : public Named<> {
diff --git a/src/openvic-simulation/interface/GUI.cpp b/src/openvic-simulation/interface/GUI.cpp
index 534f552..aeec136 100644
--- a/src/openvic-simulation/interface/GUI.cpp
+++ b/src/openvic-simulation/interface/GUI.cpp
@@ -37,7 +37,7 @@ bool Element::_fill_elements_key_map(
 		"OverlappingElementsBoxType", ZERO_OR_MORE, _expect_instance<Element, OverlappingElementsBox>(callback, ui_manager),
 		"listboxType", ZERO_OR_MORE, _expect_instance<Element, ListBox>(callback, ui_manager),
 		"windowType", ZERO_OR_MORE, _expect_instance<Element, Window>(callback, ui_manager),
-		"positionType", ZERO_OR_MORE, success_callback
+		"positionType", ZERO_OR_MORE, success_callback // TODO - load this as a marker for placing sub-scenes
 	);
 	return ret;
 }
@@ -67,10 +67,10 @@ bool Window::_fill_key_map(NodeTools::key_map_t& key_map, UIManager const& ui_ma
 	}, ui_manager);
 	ret &= Element::_fill_key_map(key_map, ui_manager);
 	ret &= add_key_map_entries(key_map,
-		"backGround", ZERO_OR_ONE, success_callback,
+		"backGround", ZERO_OR_ONE, success_callback, // TODO - load as potential panel texture (almost always empty)
 		"size", ONE_EXACTLY, expect_ivec2(assign_variable_callback(size)),
 		"moveable", ONE_EXACTLY, expect_int_bool(assign_variable_callback(moveable)),
-		"dontRender", ZERO_OR_ONE, success_callback,
+		"dontRender", ZERO_OR_ONE, success_callback, // always empty string?
 		"horizontalBorder", ZERO_OR_ONE, success_callback,
 		"verticalBorder", ZERO_OR_ONE, success_callback,
 		"fullScreen", ZERO_OR_ONE, expect_bool(assign_variable_callback(fullscreen))
@@ -96,8 +96,8 @@ bool BaseButton::_fill_key_map(NodeTools::key_map_t& key_map, UIManager const& u
 	// look up sprite registry for texture sprite with name...
 	ret &= add_key_map_entries(key_map,
 		"quadTextureSprite", ONE_EXACTLY,
-			expect_string(ui_manager.expect_sprite_str(assign_variable_callback_pointer(sprite))),
-		"shortcut", ZERO_OR_ONE, success_callback
+			expect_string(ui_manager.expect_sprite_str(assign_variable_callback_pointer(sprite)), true),
+		"shortcut", ZERO_OR_ONE, success_callback // TODO - load and use shortcuts (how to integrate with custom keybinds?)
 	);
 	return ret;
 }
@@ -175,7 +175,7 @@ bool ListBox::_fill_key_map(NodeTools::key_map_t& key_map, UIManager const& ui_m
 		"backGround", ZERO_OR_ONE, success_callback,
 		"size", ONE_EXACTLY, expect_ivec2(assign_variable_callback(size)),
 		"spacing", ZERO_OR_ONE, success_callback,
-		"scrollbartype", ZERO_OR_ONE, success_callback,
+		"scrollbartype", ZERO_OR_ONE, success_callback, // TODO - implement modable listbox scrollbars
 		"borderSize", ZERO_OR_ONE, success_callback
 	);
 	return ret;
diff --git a/src/openvic-simulation/military/Wargoal.cpp b/src/openvic-simulation/military/Wargoal.cpp
index 8bc4446..b493f22 100644
--- a/src/openvic-simulation/military/Wargoal.cpp
+++ b/src/openvic-simulation/military/Wargoal.cpp
@@ -33,54 +33,6 @@ WargoalType::WargoalType(
 	modifiers { std::move(new_modifiers) },
 	peace_options { new_peace_options } {}
 
-std::string_view WargoalType::get_sprite() const {
-	return sprite;
-}
-
-std::string_view WargoalType::get_war_name() const {
-	return war_name;
-}
-
-const Timespan WargoalType::get_available_length() const {
-	return available_length;
-}
-
-const Timespan WargoalType::get_truce_length() const {
-	return truce_length;
-}
-
-const bool WargoalType::is_triggered_only() const {
-	return triggered_only;
-}
-
-const bool WargoalType::is_civil_war() const {
-	return civil_war;
-}
-
-const bool WargoalType::is_constructing() const {
-	return constructing;
-}
-
-const bool WargoalType::is_crisis() const {
-	return crisis;
-}
-
-const bool WargoalType::is_great_war() const {
-	return great_war;
-}
-
-const bool WargoalType::is_mutual() const {
-	return mutual;
-}
-
-WargoalType::peace_modifiers_t const& WargoalType::get_modifiers() const {
-	return modifiers;
-}
-
-const peace_options_t WargoalType::get_peace_options() const {
-	return peace_options;
-}
-
 WargoalTypeManager::WargoalTypeManager() : wargoal_types { "wargoal types" } {}
 
 const std::vector<WargoalType const*>& WargoalTypeManager::get_peace_priority_list() const {
@@ -106,7 +58,7 @@ bool WargoalTypeManager::add_wargoal_type(
 		Logger::error("Invalid wargoal identifier - empty!");
 		return false;
 	}
-	
+
 	if (sprite.empty()) {
 		Logger::error("Invalid sprite for wargoal ", identifier, " - empty!");
 		return false;
@@ -117,72 +69,47 @@ bool WargoalTypeManager::add_wargoal_type(
 		return false;
 	}
 
-	return wargoal_types.add_item({ identifier, sprite, war_name, available_length, truce_length, triggered_only, civil_war, constructing, crisis, great_war, mutual, std::move(modifiers), peace_options });
+	return wargoal_types.add_item({
+		identifier, sprite, war_name, available_length, truce_length, triggered_only, civil_war, constructing, crisis,
+		great_war, mutual, std::move(modifiers), peace_options
+	});
 }
 
 bool WargoalTypeManager::load_wargoal_file(ast::NodeCPtr root) {
 	bool ret = expect_dictionary(
 		[this](std::string_view identifier, ast::NodeCPtr value) -> bool {
 			if (identifier == "peace_order") return true;
-		
+
 			std::string_view sprite, war_name;
 			Timespan available, truce;
-			bool triggered_only = false, civil_war = false, constructing = false, crisis = false, great_war = false, mutual = false;
-			peace_options_t peace_options;
+			bool triggered_only = false, civil_war = false, constructing = true, crisis = true, great_war = false,
+				mutual = false;
+			peace_options_t peace_options {};
 			WargoalType::peace_modifiers_t modifiers;
 
 			bool ret = expect_dictionary_keys_and_default(
 				[&modifiers, &identifier](std::string_view key, ast::NodeCPtr value) -> bool {
-					fixed_point_t modifier;
-					expect_fixed_point(assign_variable_callback(modifier))(value);
-
-					if (key == "badboy_factor") {
-						modifiers[WargoalType::PEACE_MODIFIERS::BADBOY_FACTOR] += modifier;
-						return true;
-					}
-					if (key == "prestige_factor") {
-						modifiers[WargoalType::PEACE_MODIFIERS::PRESTIGE_FACTOR] += modifier;
-						return true;
-					}
-					if (key == "peace_cost_factor") {
-						modifiers[WargoalType::PEACE_MODIFIERS::PEACE_COST_FACTOR] += modifier;
-						return true;
-					}
-					if (key == "penalty_factor") {
-						modifiers[WargoalType::PEACE_MODIFIERS::PENALTY_FACTOR] += modifier;
-						return true;
-					}
-					if (key == "break_truce_prestige_factor") {
-						modifiers[WargoalType::PEACE_MODIFIERS::BREAK_TRUCE_PRESTIGE_FACTOR] += modifier;
-						return true;
-					}
-					if (key == "break_truce_infamy_factor") {
-						modifiers[WargoalType::PEACE_MODIFIERS::BREAK_TRUCE_INFAMY_FACTOR] += modifier;
-						return true;
-					}
-					if (key == "break_truce_militancy_factor") {
-						modifiers[WargoalType::PEACE_MODIFIERS::BREAK_TRUCE_MILITANCY_FACTOR] += modifier;
-						return true;
-					}
-					if (key == "good_relation_prestige_factor") {
-						modifiers[WargoalType::PEACE_MODIFIERS::GOOD_RELATION_PRESTIGE_FACTOR] += modifier;
-						return true;
-					}
-					if (key == "good_relation_infamy_factor") {
-						modifiers[WargoalType::PEACE_MODIFIERS::GOOD_RELATION_INFAMY_FACTOR] += modifier;
-						return true;
-					}
-					if (key == "good_relation_militancy_factor") {
-						modifiers[WargoalType::PEACE_MODIFIERS::GOOD_RELATION_MILITANCY_FACTOR] += modifier;
-						return true;
-					}
-					if (key == "tws_battle_factor") {
-						modifiers[WargoalType::PEACE_MODIFIERS::WAR_SCORE_BATTLE_FACTOR] += modifier;
-						return true;
-					}
-					if (key == "construction_speed") {
-						modifiers[WargoalType::PEACE_MODIFIERS::CONSTRUCTION_SPEED] += modifier;
-						return true;
+					using enum WargoalType::PEACE_MODIFIERS;
+					static const string_map_t<WargoalType::PEACE_MODIFIERS> peace_modifier_map {
+						{ "badboy_factor", BADBOY_FACTOR },
+						{ "prestige_factor", PRESTIGE_FACTOR },
+						{ "peace_cost_factor", PEACE_COST_FACTOR },
+						{ "penalty_factor", PENALTY_FACTOR },
+						{ "break_truce_prestige_factor", BREAK_TRUCE_PRESTIGE_FACTOR },
+						{ "break_truce_infamy_factor", BREAK_TRUCE_INFAMY_FACTOR },
+						{ "break_truce_militancy_factor", BREAK_TRUCE_MILITANCY_FACTOR },
+						{ "good_relation_prestige_factor", GOOD_RELATION_PRESTIGE_FACTOR },
+						{ "good_relation_infamy_factor", GOOD_RELATION_INFAMY_FACTOR },
+						{ "good_relation_militancy_factor", GOOD_RELATION_MILITANCY_FACTOR },
+						{ "tws_battle_factor", WAR_SCORE_BATTLE_FACTOR },
+						{ "construction_speed", CONSTRUCTION_SPEED }
+					};
+					const decltype(peace_modifier_map)::const_iterator it = peace_modifier_map.find(key);
+					if (it != peace_modifier_map.end()) {
+						return expect_fixed_point([&modifiers, peace_modifier = it->second](fixed_point_t val) -> bool {
+							modifiers[peace_modifier] = val;
+							return true;
+						})(value);
 					}
 
 					Logger::error("Modifier ", key, " in wargoal ", identifier, " is invalid.");
@@ -284,24 +211,30 @@ bool WargoalTypeManager::load_wargoal_file(ast::NodeCPtr root) {
 				"always", ZERO_OR_ONE, success_callback // usage unknown / quirk
 			)(value);
 
-			add_wargoal_type(identifier, sprite, war_name, available, truce, triggered_only, civil_war, constructing, crisis, great_war, mutual, std::move(modifiers), peace_options);
+			add_wargoal_type(
+				identifier, sprite, war_name, available, truce, triggered_only, civil_war, constructing, crisis, great_war,
+				mutual, std::move(modifiers), peace_options
+			);
 			return ret;
 		}
 	)(root);
 
 	/* load order in which CBs are prioritised by AI */
-	ret &= expect_dictionary_keys_and_default(
-		key_value_success_callback,
-		"peace_order", ONE_EXACTLY, expect_list([this](ast::NodeCPtr value) -> bool {
-			WargoalType const* wargoal;
-			bool ret = expect_wargoal_type_identifier(assign_variable_callback_pointer(wargoal))(value);
-			if (ret) {
-				peace_priorities.push_back(wargoal);
-			} else {
-				Logger::warning("Attempted to add invalid wargoal type to AI peace order!");
-			}
-			return true;
-		})
+	ret &= expect_key(
+		"peace_order",
+		expect_list(
+			expect_wargoal_type_identifier(
+				[this](WargoalType const& wargoal) -> bool {
+					if (std::find(peace_priorities.begin(), peace_priorities.end(), &wargoal) == peace_priorities.end()) {
+						peace_priorities.push_back(&wargoal);
+					} else {
+						Logger::warning("Wargoal ", wargoal.get_identifier(), " is already in the peace priority list!");
+					}
+					return true;
+				},
+				true // warn instead of error
+			)
+		)
 	)(root);
 
 	lock_wargoal_types();
diff --git a/src/openvic-simulation/military/Wargoal.hpp b/src/openvic-simulation/military/Wargoal.hpp
index dbb8d67..3700347 100644
--- a/src/openvic-simulation/military/Wargoal.hpp
+++ b/src/openvic-simulation/military/Wargoal.hpp
@@ -1,8 +1,9 @@
 #pragma once
 
-#include "openvic-simulation/types/IdentifierRegistry.hpp"
-#include "openvic-simulation/types/EnumBitfield.hpp"
 #include "openvic-simulation/Modifier.hpp"
+#include "openvic-simulation/types/EnumBitfield.hpp"
+#include "openvic-simulation/types/IdentifierRegistry.hpp"
+#include "openvic-simulation/utility/Getters.hpp"
 
 namespace OpenVic {
 	struct WargoalTypeManager;
@@ -46,22 +47,22 @@ namespace OpenVic {
 			WAR_SCORE_BATTLE_FACTOR,
 			CONSTRUCTION_SPEED
 		};
-		using peace_modifiers_t = std::map<PEACE_MODIFIERS, fixed_point_t>;
-		
+		using peace_modifiers_t = decimal_map_t<PEACE_MODIFIERS>;
+
 	private:
-		const std::string sprite;
-		const std::string war_name;
-		const Timespan available_length;
-		const Timespan truce_length;
-		const bool triggered_only; // only able to be added via effects (or within the code)
-		const bool civil_war;
-		const bool constructing; // can be added to existing wars or justified
-		const bool crisis; // able to be added to crises
-		const bool great_war; // automatically add to great wars
-		const bool mutual; // attacked and defender share wargoal
-		const peace_modifiers_t modifiers;
-		const peace_options_t peace_options;
-		
+		const std::string PROPERTY(sprite);
+		const std::string PROPERTY(war_name);
+		const Timespan PROPERTY(available_length);
+		const Timespan PROPERTY(truce_length);
+		const bool PROPERTY(triggered_only); // only able to be added via effects (or within the code)
+		const bool PROPERTY(civil_war);
+		const bool PROPERTY(constructing); // can be added to existing wars or justified
+		const bool PROPERTY(crisis); // able to be added to crises
+		const bool PROPERTY(great_war); // automatically add to great wars
+		const bool PROPERTY(mutual); // attacked and defender share wargoal
+		const peace_modifiers_t PROPERTY(modifiers);
+		const peace_options_t PROPERTY(peace_options);
+
 		// TODO: can_use, prerequisites, on_add, on_po_accepted
 
 		WargoalType(
@@ -82,25 +83,12 @@ namespace OpenVic {
 
 	public:
 		WargoalType(WargoalType&&) = default;
-
-		std::string_view get_sprite() const;
-		std::string_view get_war_name() const;
-		const Timespan get_available_length() const;
-		const Timespan get_truce_length() const;
-		const bool is_triggered_only() const;
-		const bool is_civil_war() const;
-		const bool is_constructing() const;
-		const bool is_crisis() const;
-		const bool is_great_war() const;
-		const bool is_mutual() const;
-		peace_modifiers_t const& get_modifiers() const;
-		const peace_options_t get_peace_options() const;
 	};
 
 	struct WargoalTypeManager {
 	private:
 		IdentifierRegistry<WargoalType> wargoal_types;
-		std::vector<WargoalType const*> peace_priorities; 
+		std::vector<WargoalType const*> PROPERTY(peace_priorities);
 
 	public:
 		WargoalTypeManager();
diff --git a/src/openvic-simulation/types/IdentifierRegistry.hpp b/src/openvic-simulation/types/IdentifierRegistry.hpp
index 1121956..662815e 100644
--- a/src/openvic-simulation/types/IdentifierRegistry.hpp
+++ b/src/openvic-simulation/types/IdentifierRegistry.hpp
@@ -214,24 +214,31 @@ namespace OpenVic {
 	value_type CONST* get_item_by_index(size_t index) CONST { \
 		return index < items.size() ? &items[index] : nullptr; \
 	} \
-	NodeTools::callback_t<std::string_view> expect_item_str(NodeTools::callback_t<value_type CONST&> callback) CONST { \
-		return [this, callback](std::string_view identifier) -> bool { \
+	NodeTools::callback_t<std::string_view> expect_item_str( \
+		NodeTools::callback_t<value_type CONST&> callback, bool warn \
+	) CONST { \
+		return [this, callback, warn](std::string_view identifier) -> bool { \
 			value_type CONST* item = get_item_by_identifier(identifier); \
 			if (item != nullptr) { \
 				return callback(*item); \
 			} \
-			Logger::error("Invalid ", name, ": ", identifier); \
-			return false; \
+			if (!warn) { \
+				Logger::error("Invalid ", name, ": ", identifier); \
+				return false; \
+			} else { \
+				Logger::warning("Invalid ", name, ": ", identifier); \
+				return true; \
+			} \
 		}; \
 	} \
-	NodeTools::node_callback_t expect_item_identifier(NodeTools::callback_t<value_type CONST&> callback) CONST { \
-		return NodeTools::expect_identifier(expect_item_str(callback)); \
+	NodeTools::node_callback_t expect_item_identifier(NodeTools::callback_t<value_type CONST&> callback, bool warn) CONST { \
+		return NodeTools::expect_identifier(expect_item_str(callback, warn)); \
 	} \
 	NodeTools::node_callback_t expect_item_dictionary( \
 		NodeTools::callback_t<value_type CONST&, ast::NodeCPtr> callback \
 	) CONST { \
 		return NodeTools::expect_dictionary([this, callback](std::string_view key, ast::NodeCPtr value) -> bool { \
-			return expect_item_str(std::bind(callback, std::placeholders::_1, value))(key); \
+			return expect_item_str(std::bind(callback, std::placeholders::_1, value), false)(key); \
 		}); \
 	}
 
@@ -348,14 +355,14 @@ namespace OpenVic {
 		return plural.get_item_identifiers(); \
 	} \
 	NodeTools::callback_t<std::string_view> expect_##singular##_str( \
-		NodeTools::callback_t<decltype(plural)::value_type const&> callback \
+		NodeTools::callback_t<decltype(plural)::value_type const&> callback, bool warn = false \
 	) const { \
-		return plural.expect_item_str(callback); \
+		return plural.expect_item_str(callback, warn); \
 	} \
 	NodeTools::node_callback_t expect_##singular##_identifier( \
-		NodeTools::callback_t<decltype(plural)::value_type const&> callback \
+		NodeTools::callback_t<decltype(plural)::value_type const&> callback, bool warn = false \
 	) const { \
-		return plural.expect_item_identifier(callback); \
+		return plural.expect_item_identifier(callback, warn); \
 	} \
 	NodeTools::node_callback_t expect_##singular##_dictionary( \
 		NodeTools::callback_t<decltype(plural)::value_type const&, ast::NodeCPtr> callback \
@@ -373,14 +380,14 @@ namespace OpenVic {
 		return plural.get_item_by_identifier(identifier); \
 	} \
 	NodeTools::callback_t<std::string_view> expect_##singular##_str( \
-		NodeTools::callback_t<decltype(plural)::value_type&> callback \
+		NodeTools::callback_t<decltype(plural)::value_type&> callback, bool warn = false \
 	) { \
-		return plural.expect_item_str(callback); \
+		return plural.expect_item_str(callback, warn); \
 	} \
 	NodeTools::node_callback_t expect_##singular##_identifier( \
-		NodeTools::callback_t<decltype(plural)::value_type&> callback \
+		NodeTools::callback_t<decltype(plural)::value_type&> callback, bool warn = false \
 	) { \
-		return plural.expect_item_identifier(callback); \
+		return plural.expect_item_identifier(callback, warn); \
 	} \
 	NodeTools::node_callback_t expect_##singular##_dictionary( \
 		NodeTools::callback_t<decltype(plural)::value_type&, ast::NodeCPtr> callback \