From 7772f8871348b7b52cb0a478bb76df68d8799a07 Mon Sep 17 00:00:00 2001 From: Hop311 Date: Fri, 8 Sep 2023 17:12:22 +0100 Subject: More refactoring and duplicate code removal --- src/openvic-simulation/utility/Logger.hpp | 87 +++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 src/openvic-simulation/utility/Logger.hpp (limited to 'src/openvic-simulation/utility/Logger.hpp') diff --git a/src/openvic-simulation/utility/Logger.hpp b/src/openvic-simulation/utility/Logger.hpp new file mode 100644 index 0000000..417aba7 --- /dev/null +++ b/src/openvic-simulation/utility/Logger.hpp @@ -0,0 +1,87 @@ +#pragma once + +#include +#include +#include +#ifdef __cpp_lib_source_location +#include +#endif + +namespace OpenVic { + +#ifndef __cpp_lib_source_location +#include + // Implementation of std::source_location for compilers that do not support it + // Note: uses non-standard extensions that are supported by Clang, GCC, and MSVC + // https://clang.llvm.org/docs/LanguageExtensions.html#source-location-builtins + // https://stackoverflow.com/a/67970107 + class source_location { + std::string _file; + int _line; + std::string _function; + + public: + source_location(std::string f, int l, std::string n) : _file(f), _line(l), _function(n) {} + static source_location current(std::string f = __builtin_FILE(), int l = __builtin_LINE(), std::string n = __builtin_FUNCTION()) { + return source_location(f, l, n); + } + + inline char const* file_name() const { return _file.c_str(); } + inline int line() const { return _line; } + inline char const* function_name() const { return _function.c_str(); } + }; +#endif + + class Logger { + using log_func_t = std::function; + using log_queue_t = std::queue; + +#ifdef __cpp_lib_source_location + using source_location = std::source_location; +#else + using source_location = OpenVic::source_location; +#endif + + static char const* get_filename(char const* filepath); + + template + struct log { + log(log_func_t log_func, log_queue_t& log_queue, Ts&&... ts, source_location const& location) { + std::stringstream stream; + stream << "\n" << get_filename(location.file_name()) << "(" + << location.line() << ") `" << location.function_name() << "`: "; + ((stream << std::forward(ts)), ...); + stream << std::endl; + log_queue.push(stream.str()); + if (log_func) { + do { + log_func(std::move(log_queue.front())); + log_queue.pop(); + } while (!log_queue.empty()); + } + } + }; + +#define LOG_FUNC(name) \ + private: \ + static log_func_t name##_func; \ + static log_queue_t name##_queue; \ + public: \ + static void set_##name##_func(log_func_t log_func) { \ + name##_func = log_func; \ + } \ + template \ + struct name { \ + name(Ts&&... ts, source_location const& location = source_location::current()) { \ + log{ name##_func, name##_queue, std::forward(ts)..., location }; \ + } \ + }; \ + template \ + name(Ts&&...) -> name; + + LOG_FUNC(info) + LOG_FUNC(error) + +#undef LOG_FUNC + }; +} -- cgit v1.2.3-56-ga3b1 From b4220ad73e14e3b497b2fdeb83d76a6633664764 Mon Sep 17 00:00:00 2001 From: Hop311 Date: Sat, 9 Sep 2023 15:00:23 +0100 Subject: PR feedback + Logger::warning --- src/headless/main.cpp | 7 +- src/openvic-simulation/GameManager.cpp | 11 ++-- src/openvic-simulation/dataloader/Dataloader.cpp | 76 +++++++++++----------- src/openvic-simulation/dataloader/Dataloader.hpp | 29 +++++---- src/openvic-simulation/map/Map.cpp | 14 ++-- src/openvic-simulation/map/Province.cpp | 6 +- src/openvic-simulation/pop/Pop.cpp | 25 +++---- src/openvic-simulation/pop/Pop.hpp | 6 +- .../types/IdentifierRegistry.cpp | 8 +++ .../types/IdentifierRegistry.hpp | 3 + src/openvic-simulation/utility/Logger.cpp | 2 + src/openvic-simulation/utility/Logger.hpp | 1 + 12 files changed, 99 insertions(+), 89 deletions(-) (limited to 'src/openvic-simulation/utility/Logger.hpp') diff --git a/src/headless/main.cpp b/src/headless/main.cpp index 3185001..02eeb48 100644 --- a/src/headless/main.cpp +++ b/src/headless/main.cpp @@ -18,10 +18,11 @@ static char const* get_program_name(char const* name) { return last_separator; } -static bool headless_load(std::vector const& roots) { +static bool headless_load(Dataloader::path_vector_t const& roots) { bool ret = true; Logger::set_info_func([](std::string&& str) { std::cout << str; }); + Logger::set_warning_func([](std::string&& str) { std::cerr << str; }); Logger::set_error_func([](std::string&& str) { std::cerr << str; }); GameManager game_manager { []() { @@ -46,10 +47,10 @@ static bool headless_load(std::vector const& roots) { } int main(int argc, char const* argv[]) { - std::vector roots; + Dataloader::path_vector_t roots; if (argc < 2) { // TODO - non-Windows default paths - static const std::filesystem::path default_path = "C:/Program Files (x86)/Steam/steamapps/common/Victoria 2"; + static const fs::path default_path = "C:/Program Files (x86)/Steam/steamapps/common/Victoria 2"; std::cout << "Usage: " << get_program_name(argc > 0 ? argv[0] : nullptr) << " [[mod defines dir]+]\n" << "Requires defines path(s) as arguments, starting with the base defines and continuing with mods.\n" diff --git a/src/openvic-simulation/GameManager.cpp b/src/openvic-simulation/GameManager.cpp index ee5050d..0b42230 100644 --- a/src/openvic-simulation/GameManager.cpp +++ b/src/openvic-simulation/GameManager.cpp @@ -13,12 +13,11 @@ void GameManager::set_needs_update() { } void GameManager::update_state() { - if (needs_update) { - Logger::info("Update: ", today); - map.update_state(today); - if (state_updated) state_updated(); - needs_update = false; - } + if (!needs_update) return; + Logger::info("Update: ", today); + map.update_state(today); + if (state_updated) state_updated(); + needs_update = false; } /* REQUIREMENTS: diff --git a/src/openvic-simulation/dataloader/Dataloader.cpp b/src/openvic-simulation/dataloader/Dataloader.cpp index 334d5b8..25641e2 100644 --- a/src/openvic-simulation/dataloader/Dataloader.cpp +++ b/src/openvic-simulation/dataloader/Dataloader.cpp @@ -11,15 +11,15 @@ using namespace OpenVic; using namespace OpenVic::NodeTools; using namespace ovdl; -bool Dataloader::set_roots(std::vector new_roots) { +bool Dataloader::set_roots(path_vector_t new_roots) { if (!roots.empty()) { Logger::error("Overriding existing dataloader roots!"); roots.clear(); } bool ret = true; - for (std::reverse_iterator::const_iterator> it = new_roots.crbegin(); it != new_roots.crend(); ++it) { + for (std::reverse_iterator it = new_roots.crbegin(); it != new_roots.crend(); ++it) { if (std::find(roots.begin(), roots.end(), *it) == roots.end()) { - if (std::filesystem::is_directory(*it)) { + if (fs::is_directory(*it)) { Logger::info("Adding dataloader root: ", *it); roots.push_back(*it); } else { @@ -38,10 +38,10 @@ bool Dataloader::set_roots(std::vector new_roots) { return ret; } -std::filesystem::path Dataloader::lookup_file(std::filesystem::path const& path) const { - for (std::filesystem::path const& root : roots) { - const std::filesystem::path composed = root / path; - if (std::filesystem::is_regular_file(composed)) { +fs::path Dataloader::lookup_file(fs::path const& path) const { + for (fs::path const& root : roots) { + const fs::path composed = root / path; + if (fs::is_regular_file(composed)) { return composed; } } @@ -49,27 +49,27 @@ std::filesystem::path Dataloader::lookup_file(std::filesystem::path const& path) return {}; } -const std::filesystem::path Dataloader::TXT = ".txt"; +const fs::path Dataloader::TXT = ".txt"; -static bool contains_file_with_name(std::vector const& paths, - std::filesystem::path const& name) { +static bool contains_file_with_name(Dataloader::path_vector_t const& paths, + fs::path const& name) { - for (std::filesystem::path const& path : paths) { + for (fs::path const& path : paths) { if (path.filename() == name) return true; } return false; } -std::vector Dataloader::lookup_files_in_dir(std::filesystem::path const& path, - std::filesystem::path const* extension) const { +Dataloader::path_vector_t Dataloader::lookup_files_in_dir(fs::path const& path, + fs::path const* extension) const { - std::vector ret; - for (std::filesystem::path const& root : roots) { - const std::filesystem::path composed = root / path; + path_vector_t ret; + for (fs::path const& root : roots) { + const fs::path composed = root / path; std::error_code ec; - for (std::filesystem::directory_entry const& entry : std::filesystem::directory_iterator { composed, ec }) { + for (fs::directory_entry const& entry : fs::directory_iterator { composed, ec }) { if (entry.is_regular_file()) { - const std::filesystem::path file = entry; + const fs::path file = entry; if (extension == nullptr || file.extension() == *extension) { if (!contains_file_with_name(ret, file.filename())) { ret.push_back(file); @@ -81,19 +81,19 @@ std::vector Dataloader::lookup_files_in_dir(std::filesyst return ret; } -bool Dataloader::apply_to_files_in_dir(std::filesystem::path const& path, - std::function callback, - std::filesystem::path const* extension) const { +bool Dataloader::apply_to_files_in_dir(fs::path const& path, + std::function callback, + fs::path const* extension) const { bool ret = true; - for (std::filesystem::path const& file : lookup_files_in_dir(path, extension)) { + for (fs::path const& file : lookup_files_in_dir(path, extension)) { ret &= callback(file); } return ret; } template Parser, bool(Parser::*parse_func)()> -static Parser _run_ovdl_parser(std::filesystem::path const& path) { +static Parser _run_ovdl_parser(fs::path const& path) { Parser parser; std::string buffer; auto error_log_stream = ovdl::detail::CallbackStream { @@ -130,18 +130,18 @@ static Parser _run_ovdl_parser(std::filesystem::path const& path) { return parser; } -static v2script::Parser _parse_defines(std::filesystem::path const& path) { +static v2script::Parser _parse_defines(fs::path const& path) { return _run_ovdl_parser(path); } -static csv::Windows1252Parser _parse_csv(std::filesystem::path const& path) { +static csv::Windows1252Parser _parse_csv(fs::path const& path) { return _run_ovdl_parser(path); } -bool Dataloader::_load_pop_types(PopManager& pop_manager, std::filesystem::path const& pop_type_directory) const { +bool Dataloader::_load_pop_types(PopManager& pop_manager, fs::path const& pop_type_directory) const { const bool ret = apply_to_files_in_dir(pop_type_directory, - [&pop_manager](std::filesystem::path const& file) -> bool { - return pop_manager.load_pop_type_file(file, _parse_defines(file).get_file_node()); + [&pop_manager](fs::path const& file) -> bool { + return pop_manager.load_pop_type_file(file.stem().string(), _parse_defines(file).get_file_node()); } ); if (!ret) { @@ -151,8 +151,8 @@ bool Dataloader::_load_pop_types(PopManager& pop_manager, std::filesystem::path return ret; } -bool Dataloader::_load_map_dir(Map& map, std::filesystem::path const& map_directory) const { - static const std::filesystem::path defaults_filename = "default.map"; +bool Dataloader::_load_map_dir(Map& map, fs::path const& map_directory) const { + static const fs::path defaults_filename = "default.map"; static const std::string default_definitions = "definition.csv"; static const std::string default_provinces = "provinces.bmp"; static const std::string default_positions = "positions.txt"; @@ -233,12 +233,12 @@ bool Dataloader::_load_map_dir(Map& map, std::filesystem::path const& map_direct } bool Dataloader::load_defines(GameManager& game_manager) const { - static const std::filesystem::path good_file = "common/goods.txt"; - static const std::filesystem::path pop_type_directory = "poptypes"; - static const std::filesystem::path graphical_culture_type_file = "common/graphicalculturetype.txt"; - static const std::filesystem::path culture_file = "common/cultures.txt"; - static const std::filesystem::path religion_file = "common/religion.txt"; - static const std::filesystem::path map_directory = "map"; + static const fs::path good_file = "common/goods.txt"; + static const fs::path pop_type_directory = "poptypes"; + static const fs::path graphical_culture_type_file = "common/graphicalculturetype.txt"; + static const fs::path culture_file = "common/cultures.txt"; + static const fs::path religion_file = "common/religion.txt"; + static const fs::path map_directory = "map"; bool ret = true; @@ -270,9 +270,9 @@ bool Dataloader::load_defines(GameManager& game_manager) const { return ret; } -bool Dataloader::load_pop_history(GameManager& game_manager, std::filesystem::path const& path) const { +bool Dataloader::load_pop_history(GameManager& game_manager, fs::path const& path) const { return apply_to_files_in_dir(path, - [&game_manager](std::filesystem::path const& file) -> bool { + [&game_manager](fs::path const& file) -> bool { return expect_dictionary( [&game_manager](std::string_view province_key, ast::NodeCPtr province_node) -> bool { Province* province = game_manager.map.get_province_by_identifier(province_key); diff --git a/src/openvic-simulation/dataloader/Dataloader.hpp b/src/openvic-simulation/dataloader/Dataloader.hpp index f723803..20538a6 100644 --- a/src/openvic-simulation/dataloader/Dataloader.hpp +++ b/src/openvic-simulation/dataloader/Dataloader.hpp @@ -5,31 +5,36 @@ #include namespace OpenVic { + namespace fs = std::filesystem; + struct GameManager; struct PopManager; struct Map; class Dataloader { - std::vector roots; + public: + using path_vector_t = std::vector; + private: + path_vector_t roots; - bool _load_pop_types(PopManager& pop_manager, std::filesystem::path const& pop_type_directory) const; - bool _load_map_dir(Map& map, std::filesystem::path const& map_directory) const; + bool _load_pop_types(PopManager& pop_manager, fs::path const& pop_type_directory) const; + bool _load_map_dir(Map& map, fs::path const& map_directory) const; public: Dataloader() = default; /* In reverse-load order, so base defines first and final loaded mod last */ - bool set_roots(std::vector new_roots); + bool set_roots(path_vector_t new_roots); - std::filesystem::path lookup_file(std::filesystem::path const& path) const; - static const std::filesystem::path TXT; - std::vector lookup_files_in_dir(std::filesystem::path const& path, - std::filesystem::path const* extension = &TXT) const; - bool apply_to_files_in_dir(std::filesystem::path const& path, - std::function callback, - std::filesystem::path const* extension = &TXT) const; + fs::path lookup_file(fs::path const& path) const; + static const fs::path TXT; + path_vector_t lookup_files_in_dir(fs::path const& path, + fs::path const* extension = &TXT) const; + bool apply_to_files_in_dir(fs::path const& path, + std::function callback, + fs::path const* extension = &TXT) const; bool load_defines(GameManager& game_manager) const; - bool load_pop_history(GameManager& game_manager, std::filesystem::path const& path) const; + bool load_pop_history(GameManager& game_manager, fs::path const& path) const; }; } diff --git a/src/openvic-simulation/map/Map.cpp b/src/openvic-simulation/map/Map.cpp index 3f86ccc..8f10410 100644 --- a/src/openvic-simulation/map/Map.cpp +++ b/src/openvic-simulation/map/Map.cpp @@ -68,8 +68,8 @@ bool Map::set_water_province(const std::string_view identifier) { return false; } if (province->is_water()) { - Logger::error("Province ", identifier, " is already a water province!"); - return false; + Logger::warning("Province ", identifier, " is already a water province!"); + return true; } if (!water_provinces.add_province(province)) { Logger::error("Failed to add province ", identifier, " to water province set!"); @@ -278,7 +278,7 @@ bool Map::generate_province_shape_image(size_t new_width, size_t new_height, uin if (unrecognised_terrain_colours.find(terrain_colour) == unrecognised_terrain_colours.end()) { unrecognised_terrain_colours.insert(terrain_colour); if (detailed_errors) { - Logger::error("Unrecognised terrain colour ", colour_to_hex_string(terrain_colour), + Logger::warning("Unrecognised terrain colour ", colour_to_hex_string(terrain_colour), " at (", x, ", ", y, ")"); } } @@ -309,7 +309,7 @@ bool Map::generate_province_shape_image(size_t new_width, size_t new_height, uin if (unrecognised_province_colours.find(province_colour) == unrecognised_province_colours.end()) { unrecognised_province_colours.insert(province_colour); if (detailed_errors) { - Logger::error("Unrecognised province colour ", colour_to_hex_string(province_colour), + Logger::warning("Unrecognised province colour ", colour_to_hex_string(province_colour), " at (", x, ", ", y, ")"); } } @@ -317,12 +317,10 @@ bool Map::generate_province_shape_image(size_t new_width, size_t new_height, uin } } if (!unrecognised_province_colours.empty()) { - Logger::error("Province image contains ", unrecognised_province_colours.size(), " unrecognised province colours"); - ret = false; + Logger::warning("Province image contains ", unrecognised_province_colours.size(), " unrecognised province colours"); } if (!unrecognised_terrain_colours.empty()) { - Logger::error("Terrain image contains ", unrecognised_terrain_colours.size(), " unrecognised terrain colours"); - ret = false; + Logger::warning("Terrain image contains ", unrecognised_terrain_colours.size(), " unrecognised terrain colours"); } size_t missing = 0; diff --git a/src/openvic-simulation/map/Province.cpp b/src/openvic-simulation/map/Province.cpp index 43eddd1..8d4abcb 100644 --- a/src/openvic-simulation/map/Province.cpp +++ b/src/openvic-simulation/map/Province.cpp @@ -55,10 +55,10 @@ std::string Province::to_string() const { } bool Province::load_pop_list(PopManager const& pop_manager, ast::NodeCPtr root) { - return expect_list_reserve_length( + return expect_dictionary_reserve_length( pops, - [this, &pop_manager](ast::NodeCPtr pop_node) -> bool { - return pop_manager.load_pop_into_province(*this, pop_node); + [this, &pop_manager](std::string_view pop_type_identifier, ast::NodeCPtr pop_node) -> bool { + return pop_manager.load_pop_into_province(*this, pop_type_identifier, pop_node); } )(root); } diff --git a/src/openvic-simulation/pop/Pop.cpp b/src/openvic-simulation/pop/Pop.cpp index 7b4dd60..f8e7254 100644 --- a/src/openvic-simulation/pop/Pop.cpp +++ b/src/openvic-simulation/pop/Pop.cpp @@ -126,7 +126,7 @@ bool PopManager::add_pop_type(const std::string_view identifier, colour_t colour return pop_types.add_item({ identifier, colour, strata, sprite, max_size, merge_max_size, state_capital_only, demote_migrant, is_artisan, is_slave }); } -bool PopManager::load_pop_type_file(std::filesystem::path const& path, ast::NodeCPtr root) { +bool PopManager::load_pop_type_file(const std::string_view filestem, ast::NodeCPtr root) { // TODO - pop type loading @@ -135,29 +135,24 @@ bool PopManager::load_pop_type_file(std::filesystem::path const& path, ast::Node return true; } -bool PopManager::load_pop_into_province(Province& province, ast::NodeCPtr root) const { +bool PopManager::load_pop_into_province(Province& province, const std::string_view pop_type_identifier, ast::NodeCPtr pop_node) const { static PopType const* type = get_pop_type_by_identifier("test_pop_type"); Culture const* culture = nullptr; Religion const* religion = nullptr; Pop::pop_size_t size = 0; - bool ret = expect_assign( - [this, &culture, &religion, &size](std::string_view, ast::NodeCPtr pop_node) -> bool { - return expect_dictionary_keys( - "culture", ONE_EXACTLY, culture_manager.expect_culture(culture), - "religion", ONE_EXACTLY, religion_manager.expect_religion(religion), - "size", ONE_EXACTLY, expect_uint(assign_variable_callback_uint("pop size", size)), - "militancy", ZERO_OR_ONE, success_callback, - "rebel_type", ZERO_OR_ONE, success_callback - )(pop_node); - } - )(root); + bool ret = expect_dictionary_keys( + "culture", ONE_EXACTLY, culture_manager.expect_culture(culture), + "religion", ONE_EXACTLY, religion_manager.expect_religion(religion), + "size", ONE_EXACTLY, expect_uint(assign_variable_callback_uint("pop size", size)), + "militancy", ZERO_OR_ONE, success_callback, + "rebel_type", ZERO_OR_ONE, success_callback + )(pop_node); if (type != nullptr && culture != nullptr && religion != nullptr && size > 0) { ret &= province.add_pop({ *type, *culture, *religion, size }); } else { - Logger::error("Some pop arguments are invalid: type = ", type, ", culture = ", culture, ", religion = ", religion, ", size = ", size); - ret = false; + Logger::warning("Some pop arguments are invalid: province = ", province, ", type = ", type, ", culture = ", culture, ", religion = ", religion, ", size = ", size); } return ret; } diff --git a/src/openvic-simulation/pop/Pop.hpp b/src/openvic-simulation/pop/Pop.hpp index 3c635a4..d01eb97 100644 --- a/src/openvic-simulation/pop/Pop.hpp +++ b/src/openvic-simulation/pop/Pop.hpp @@ -1,7 +1,5 @@ #pragma once -#include - #include "openvic-simulation/pop/Culture.hpp" #include "openvic-simulation/pop/Religion.hpp" @@ -95,7 +93,7 @@ namespace OpenVic { bool is_artisan, bool is_slave); IDENTIFIER_REGISTRY_ACCESSORS(PopType, pop_type) - bool load_pop_type_file(std::filesystem::path const& path, ast::NodeCPtr root); - bool load_pop_into_province(Province& province, ast::NodeCPtr root) const; + bool load_pop_type_file(const std::string_view filestem, ast::NodeCPtr root); + bool load_pop_into_province(Province& province, const std::string_view pop_type_identifier, ast::NodeCPtr pop_node) const; }; } diff --git a/src/openvic-simulation/types/IdentifierRegistry.cpp b/src/openvic-simulation/types/IdentifierRegistry.cpp index 6d5221b..e33bc29 100644 --- a/src/openvic-simulation/types/IdentifierRegistry.cpp +++ b/src/openvic-simulation/types/IdentifierRegistry.cpp @@ -13,6 +13,14 @@ std::string const& HasIdentifier::get_identifier() const { return identifier; } +std::ostream& OpenVic::operator<<(std::ostream& stream, HasIdentifier const& obj) { + return stream << obj.get_identifier(); +} + +std::ostream& OpenVic::operator<<(std::ostream& stream, HasIdentifier const* obj) { + return obj != nullptr ? stream << *obj : stream << ""; +} + HasColour::HasColour(colour_t const new_colour, bool can_be_null) : colour(new_colour) { assert((can_be_null || colour != NULL_COLOUR) && colour <= MAX_COLOUR_RGB); } diff --git a/src/openvic-simulation/types/IdentifierRegistry.hpp b/src/openvic-simulation/types/IdentifierRegistry.hpp index 494ff3e..20eebb9 100644 --- a/src/openvic-simulation/types/IdentifierRegistry.hpp +++ b/src/openvic-simulation/types/IdentifierRegistry.hpp @@ -27,6 +27,9 @@ namespace OpenVic { std::string const& get_identifier() const; }; + std::ostream& operator<<(std::ostream& stream, HasIdentifier const& obj); + std::ostream& operator<<(std::ostream& stream, HasIdentifier const* obj); + /* * Base class for objects with associated colour information. */ diff --git a/src/openvic-simulation/utility/Logger.cpp b/src/openvic-simulation/utility/Logger.cpp index bf9a67c..fca08a5 100644 --- a/src/openvic-simulation/utility/Logger.cpp +++ b/src/openvic-simulation/utility/Logger.cpp @@ -6,6 +6,8 @@ using namespace OpenVic; Logger::log_func_t Logger::info_func {}; Logger::log_queue_t Logger::info_queue {}; +Logger::log_func_t Logger::warning_func {}; +Logger::log_queue_t Logger::warning_queue {}; Logger::log_func_t Logger::error_func {}; Logger::log_queue_t Logger::error_queue {}; diff --git a/src/openvic-simulation/utility/Logger.hpp b/src/openvic-simulation/utility/Logger.hpp index 417aba7..f9ebd5d 100644 --- a/src/openvic-simulation/utility/Logger.hpp +++ b/src/openvic-simulation/utility/Logger.hpp @@ -80,6 +80,7 @@ namespace OpenVic { name(Ts&&...) -> name; LOG_FUNC(info) + LOG_FUNC(warning) LOG_FUNC(error) #undef LOG_FUNC -- cgit v1.2.3-56-ga3b1