Mod Compatibility Fixes

Author: BrickPi

src/headless/main.cpp

@@ -112,7 +112,7 @@ static std::chrono::nanoseconds run_pathing_test(AStarPathing& pathing) {
 	return end_time - start_time;
 }
 
-static bool run_headless(const fs::path root, std::vector<std::string> mods, bool run_tests) {
+static bool run_headless(fs::path const& root, std::vector<std::string>& mods, bool run_tests) {
 	bool ret = true;
 	Dataloader::path_vector_t roots = { root };
 	Dataloader::path_vector_t replace_paths = {};
@@ -122,7 +122,7 @@ static bool run_headless(const fs::path root, std::vector<std::string> mods, boo
 	} };
 
 	Logger::info("===== Loading mod descriptors... =====");
-	ret &= game_manager.load_mod_descriptors(std::move(mods));
+	ret &= game_manager.load_mod_descriptors(mods);
 	for (auto const& mod : game_manager.get_mod_manager().get_mods()) {
 		roots.emplace_back(root / mod.get_dataloader_root_path());
 		for (std::string_view path : mod.get_replace_paths()) {
@@ -296,7 +296,7 @@ int main(int argc, char const* argv[]) {
 
 	std::cout << "!!! HEADLESS SIMULATION START !!!" << std::endl;
 
-	const bool ret = run_headless(root, std::move(mods), run_tests);
+	const bool ret = run_headless(root, mods, run_tests);
 
 	std::cout << "!!! HEADLESS SIMULATION END !!!" << std::endl;
 

src/openvic-simulation/GameManager.cpp

@@ -6,17 +6,15 @@ GameManager::GameManager(
 	InstanceManager::gamestate_updated_func_t new_gamestate_updated_callback
 ) : gamestate_updated_callback {
 		new_gamestate_updated_callback ? std::move(new_gamestate_updated_callback) : []() {}
-	}, clock_state_changed_callback {
-		new_clock_state_changed_callback ? std::move(new_clock_state_changed_callback) : []() {}
 	}, definitions_loaded { false }, mod_descriptors_loaded { false } {}
 
-bool GameManager::load_mod_descriptors(std::vector<std::string> descriptors) {
+bool GameManager::load_mod_descriptors(std::vector<std::string> const& descriptors) {
 	if (mod_descriptors_loaded) {
 		Logger::error("Cannot load mod descriptors - already loaded!");
 		return false;
 	}
 
-	if (!dataloader.load_mod_descriptors(std::move(descriptors), mod_manager)) {
+	if (!dataloader.load_mod_descriptors(descriptors, mod_manager)) {
 		Logger::error("Failed to load mod descriptors!");
 		return false;
 	}

src/openvic-simulation/GameManager.hpp

@@ -36,7 +36,7 @@ namespace OpenVic {
 
 		bool set_roots(Dataloader::path_vector_t const& roots, Dataloader::path_vector_t const& replace_paths);
 
-		bool load_mod_descriptors(std::vector<std::string> descriptors);
+		bool load_mod_descriptors(std::vector<std::string> const& descriptors);
 
 		bool load_definitions(Dataloader::localisation_callback_t localisation_callback);
 

src/openvic-simulation/country/CountryDefinition.cpp

@@ -2,6 +2,8 @@
 
 #include <string_view>
 
+#include <openvic-dataloader/v2script/AbstractSyntaxTree.hpp>
+
 #include "openvic-simulation/dataloader/Dataloader.hpp"
 #include "openvic-simulation/dataloader/NodeTools.hpp"
 #include "openvic-simulation/DefinitionManager.hpp"
@@ -11,7 +13,7 @@
 #include "openvic-simulation/pop/Culture.hpp"
 #include "openvic-simulation/types/Colour.hpp"
 #include "openvic-simulation/types/IdentifierRegistry.hpp"
-#include <openvic-dataloader/v2script/AbstractSyntaxTree.hpp>
+#include "openvic-simulation/utility/Logger.hpp"
 
 using namespace OpenVic;
 using namespace OpenVic::NodeTools;
@@ -129,13 +131,19 @@ bool CountryDefinitionManager::load_countries(
 }
 
 bool CountryDefinitionManager::load_country_colours(ast::NodeCPtr root) {
-	return country_definitions.expect_item_dictionary([](CountryDefinition& country, ast::NodeCPtr colour_node) -> bool {
-		return expect_dictionary_keys(
-			"color1", ONE_EXACTLY, expect_colour(assign_variable_callback(country.primary_unit_colour)),
-			"color2", ONE_EXACTLY, expect_colour(assign_variable_callback(country.secondary_unit_colour)),
-			"color3", ONE_EXACTLY, expect_colour(assign_variable_callback(country.tertiary_unit_colour))
-		)(colour_node);
-	})(root);
+	return country_definitions.expect_item_dictionary_and_default(
+		[](std::string_view key, ast::NodeCPtr value) -> bool {
+			Logger::warning("country_colors.txt references country tag ", key, " which is not defined!");
+			return true;
+		},
+		[](CountryDefinition& country, ast::NodeCPtr colour_node) -> bool {
+			return expect_dictionary_keys(
+				"color1", ONE_EXACTLY, expect_colour(assign_variable_callback(country.primary_unit_colour)),
+				"color2", ONE_EXACTLY, expect_colour(assign_variable_callback(country.secondary_unit_colour)),
+				"color3", ONE_EXACTLY, expect_colour(assign_variable_callback(country.tertiary_unit_colour))
+			)(colour_node);
+		}
+	)(root);
 }
 
 node_callback_t CountryDefinitionManager::load_country_party(

src/openvic-simulation/dataloader/Dataloader.cpp

@@ -42,7 +42,7 @@ bool Dataloader::set_roots(path_vector_t const& new_roots, path_vector_t const&
 		replace_paths.clear();
 	}
 	bool ret = true;
-	for (const fs::path& replace_path : new_replace_paths) {
+	for (fs::path const& replace_path : new_replace_paths) {
 		Logger::info("Adding replace path: ", replace_path);
 		replace_paths.push_back(replace_path);
 	}
@@ -70,36 +70,13 @@ bool Dataloader::set_roots(path_vector_t const& new_roots, path_vector_t const&
 fs::path Dataloader::lookup_file(std::string_view path, bool print_error) const {
 	const fs::path filepath { ensure_forward_slash_path(path) };
 
-#if defined(FILESYSTEM_CASE_INSENSITIVE)
-	/* Case-insensitive filesystem */
-	for (fs::path const& root : roots) {
-		const fs::path composed = root / filepath;
-		if (fs::is_regular_file(composed)) {
-			if (root == roots.back()) {
-				bool ignore = false;
-				for (auto const& replace_path : replace_paths) {
-					if (filepath.string().starts_with(replace_path.string())) {
-						ignore = true;
-						break;
-					}
-				}
-				if (!ignore) {
-					return composed;
-				}
-			} else {
-				return composed;
-			}
-		}
-	}
-#else
-	/* Case-sensitive filesystem */
 	const std::string_view filename = StringUtils::get_filename(path);
 	for (fs::path const& root : roots) {
 		const fs::path composed = root / filepath;
 		if (fs::is_regular_file(composed)) {
 			if (root == roots.back()) {
 				bool ignore = false;
-				for (auto const& replace_path : replace_paths) {
+				for (fs::path const& replace_path : replace_paths) {
 					if (filepath.string().starts_with(replace_path.string())) {
 						ignore = true;
 						break;
@@ -119,7 +96,7 @@ fs::path Dataloader::lookup_file(std::string_view path, bool print_error) const
 				if (StringUtils::strings_equal_case_insensitive(file.filename().string(), filename)) {
 					if (root == roots.back()) {
 						bool ignore = false;
-						for (auto const& replace_path : replace_paths) {
+						for (fs::path const& replace_path : replace_paths) {
 							if (filepath.string().starts_with(replace_path.string())) {
 								ignore = true;
 								break;
@@ -135,7 +112,6 @@ fs::path Dataloader::lookup_file(std::string_view path, bool print_error) const
 			}
 		}
 	}
-#endif
 
 	if (print_error) {
 		Logger::error("Lookup for \"", path, "\" failed!");
@@ -154,6 +130,17 @@ fs::path Dataloader::lookup_image_file(std::string_view path) const {
 	return lookup_file(path);
 }
 
+bool Dataloader::should_ignore_path(fs::path const& path, path_vector_t const& replace_paths) const {
+	bool ignore = false;
+	for (fs::path const& replace_path : replace_paths) {
+		if (path.string().starts_with(replace_path.string())) {
+			ignore = true;
+			break;
+		}
+	}
+	return ignore;
+}
+
 template<typename _DirIterator, UniqueFileKey _UniqueKey>
 Dataloader::path_vector_t Dataloader::_lookup_files_in_dir(
 	std::string_view path, fs::path const& extension, _UniqueKey const& unique_key
@@ -168,17 +155,8 @@ Dataloader::path_vector_t Dataloader::_lookup_files_in_dir(
 	for (fs::path const& root : roots) {
 		const size_t root_len = root.string().size();
 		std::error_code ec;
-		if (root == roots.back()) {
-			bool ignore = false;
-			for (auto const& replace_path : replace_paths) {
-				if (dirpath.string().starts_with(replace_path.string())) {
-					ignore = true;
-					break;
-				}
-			}
-			if (ignore) {
-				continue;
-			}
+		if (root == roots.back() && should_ignore_path(dirpath, replace_paths)) {
+			continue;
 		}
 		for (fs::directory_entry const& entry : _DirIterator { root / dirpath, ec }) {
 			if (entry.is_regular_file()) {
@@ -328,7 +306,7 @@ void Dataloader::free_cache() {
 	cached_parsers.clear();
 }
 
-bool Dataloader::load_mod_descriptors(std::vector<std::string> descriptors, ModManager& mod_manager) {
+bool Dataloader::load_mod_descriptors(std::vector<std::string> const& descriptors, ModManager& mod_manager) {
 	bool ret = true;
 
 	for (std::string_view descriptor_path : descriptors) {
@@ -824,6 +802,11 @@ bool Dataloader::_load_map_dir(DefinitionManager& definition_manager) const {
 		ret = false;
 	}
 
+	if (!map_definition.set_water_province_list(water_province_identifiers)) {
+		Logger::error("Failed to set water provinces!");
+		ret = false;
+	}
+
 	{
 		std::vector<colour_t> colours;
 		if (!MapDefinition::load_region_colours(parse_defines(lookup_file(region_colours)).get_file_node(), colours)) {
@@ -841,11 +824,6 @@ bool Dataloader::_load_map_dir(DefinitionManager& definition_manager) const {
 		}
 	}
 
-	if (!map_definition.set_water_province_list(water_province_identifiers)) {
-		Logger::error("Failed to set water provinces!");
-		ret = false;
-	}
-
 	if (!map_definition.get_terrain_type_manager().load_terrain_types(
 		definition_manager.get_modifier_manager(),
 		parse_defines(lookup_file(append_string_views(map_directory, terrain_definition))).get_file_node()

src/openvic-simulation/dataloader/Dataloader.hpp

@@ -46,6 +46,8 @@ namespace OpenVic {
 		bool _load_decisions(DefinitionManager& definition_manager);
 		bool _load_history(DefinitionManager& definition_manager, bool unused_history_file_warnings) const;
 
+		bool should_ignore_path(fs::path const& path, path_vector_t const& replace_paths) 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. */
@@ -123,7 +125,7 @@ namespace OpenVic {
 		string_set_t lookup_dirs_in_dir(std::string_view path) const;
 
 		/* Load all mod descriptors passed by the user. Importantly, loads dependencies and replace_paths for us to check. */
-		bool load_mod_descriptors(std::vector<std::string> descriptors, ModManager& mod_manager);
+		bool load_mod_descriptors(std::vector<std::string> const& descriptors, ModManager& mod_manager);
 
 		/* Load and parse all of the text defines data, including parsing cached condition and effect scripts after all the
 		 * static data is loaded. Paths to the base and mod defines must have been supplied with set_roots.*/

src/openvic-simulation/dataloader/ModManager.cpp

@@ -5,45 +5,44 @@
 
 #include "openvic-simulation/dataloader/NodeTools.hpp"
 #include "openvic-simulation/types/HasIdentifier.hpp"
-#include "types/IdentifierRegistry.hpp"
+#include "openvic-simulation/types/IdentifierRegistry.hpp"
 
 using namespace OpenVic;
 using namespace OpenVic::NodeTools;
 
 Mod::Mod(std::string_view new_identifier, std::string_view new_path, std::optional<std::string_view> new_user_dir, std::vector<std::string> new_replace_paths, std::vector<std::string> new_dependencies)
-    : HasIdentifier { new_identifier }, dataloader_root_path { new_path }, user_dir { new_user_dir }, replace_paths { new_replace_paths }, dependencies { new_dependencies } {}
+	: HasIdentifier { new_identifier }, dataloader_root_path { new_path }, user_dir { new_user_dir }, replace_paths { new_replace_paths }, dependencies { new_dependencies } {}
 
 ModManager::ModManager() {}
 
 bool ModManager::load_mod_file(ast::NodeCPtr root) {
-    std::string_view identifier;
-    std::string_view path;
-    std::optional<std::string_view> user_dir;
-    std::vector<std::string> replace_paths;
-    std::vector<std::string> dependencies;
-
-    bool ret = NodeTools::expect_dictionary_keys(
-        "name", ONE_EXACTLY, expect_string(assign_variable_callback(identifier)),
-        "path", ONE_EXACTLY, expect_string(assign_variable_callback(path)),
-        "user_dir", ZERO_OR_ONE, expect_string(assign_variable_callback_opt(user_dir)),
-        "replace_path", ZERO_OR_MORE, expect_string(vector_callback_string(replace_paths)),
-        "dependencies", ZERO_OR_ONE, expect_list_reserve_length(dependencies, expect_string(vector_callback_string(dependencies)))
-    )(root);
-
-    std::vector<std::string_view> previous_mods = mods.get_item_identifiers();
-    for (std::string_view dependency : dependencies) {
+	std::string_view identifier;
+	std::string_view path;
+	std::optional<std::string_view> user_dir;
+	std::vector<std::string> replace_paths;
+	std::vector<std::string> dependencies;
+
+	bool ret = NodeTools::expect_dictionary_keys(
+		"name", ONE_EXACTLY, expect_string(assign_variable_callback(identifier)),
+		"path", ONE_EXACTLY, expect_string(assign_variable_callback(path)),
+		"user_dir", ZERO_OR_ONE, expect_string(assign_variable_callback_opt(user_dir)),
+		"replace_path", ZERO_OR_MORE, expect_string(vector_callback_string(replace_paths)),
+		"dependencies", ZERO_OR_ONE, expect_list_reserve_length(dependencies, expect_string(vector_callback_string(dependencies)))
+	)(root);
+
+	std::vector<std::string_view> previous_mods = mods.get_item_identifiers();
+	for (std::string_view dependency : dependencies) {
 		if (std::find(previous_mods.begin(), previous_mods.end(), dependency) == previous_mods.end()) {
-            ret = false;
+			ret = false;
 			Logger::error("Mod ", identifier, " has unmet dependency ", dependency);
 		}
 	}
 
-    if (ret) {
-        ret &= mods.add_item(
-            { identifier, path, user_dir, std::move(replace_paths), std::move(dependencies) },
-            duplicate_fail_callback
-        );
-    }
+	if (ret) {
+		ret &= mods.add_item(
+			{ identifier, path, user_dir, std::move(replace_paths), std::move(dependencies) }
+		);
+	}
 
-    return ret;
+	return ret;
 }

src/openvic-simulation/dataloader/ModManager.hpp

@@ -5,33 +5,33 @@
 
 #include "openvic-simulation/types/HasIdentifier.hpp"
 #include "openvic-simulation/types/IdentifierRegistry.hpp"
-#include "dataloader/NodeTools.hpp"
+#include "openvic-simulation/dataloader/NodeTools.hpp"
 
 namespace OpenVic {
 
-    struct Mod : HasIdentifier {
-        friend struct ModManager;
+	struct Mod : HasIdentifier {
+		friend struct ModManager;
 
-    private:
-        const std::string PROPERTY(dataloader_root_path);
-        const std::optional<std::string> PROPERTY(user_dir);
-        const std::vector<std::string> PROPERTY(replace_paths);
-        const std::vector<std::string> PROPERTY(dependencies);
+	private:
+		const std::string PROPERTY(dataloader_root_path);
+		const std::optional<std::string> PROPERTY(user_dir);
+		const std::vector<std::string> PROPERTY(replace_paths);
+		const std::vector<std::string> PROPERTY(dependencies);
 
-        Mod(std::string_view new_identifier, std::string_view new_path, std::optional<std::string_view> new_user_dir, std::vector<std::string> new_replace_paths, std::vector<std::string> new_dependencies);
+		Mod(std::string_view new_identifier, std::string_view new_path, std::optional<std::string_view> new_user_dir, std::vector<std::string> new_replace_paths, std::vector<std::string> new_dependencies);
 
-    public:
-        Mod(Mod&&) = default;
-    };
+	public:
+		Mod(Mod&&) = default;
+	};
 
-    struct ModManager {
-    
-    private:
-        IdentifierRegistry<Mod> IDENTIFIER_REGISTRY(mod);
+	struct ModManager {
+	
+	private:
+		IdentifierRegistry<Mod> IDENTIFIER_REGISTRY(mod);
 
-    public:
-        ModManager();
+	public:
+		ModManager();
 
-        bool load_mod_file(ast::NodeCPtr root);
-    };
+		bool load_mod_file(ast::NodeCPtr root);
+	};
 } // namespace OpenVic

src/openvic-simulation/dataloader/NodeTools.hpp

@@ -121,7 +121,10 @@ using namespace std::string_view_literals;
 		constexpr bool key_value_success_callback(std::string_view, ast::NodeCPtr) {
 			return true;
 		}
-
+		inline bool key_value_warn_callback(std::string_view key, ast::NodeCPtr) {
+			Logger::warning("Invalid dictionary key: ", key);
+			return true;
+		}
 		inline bool key_value_invalid_callback(std::string_view key, ast::NodeCPtr) {
 			Logger::error("Invalid dictionary key: ", key);
 			return false;