From b27b7e77bdc99aab9d859390e62f3e213d4067fa Mon Sep 17 00:00:00 2001 From: Jan Laupetin Date: Sat, 11 Oct 2025 15:28:19 +0100 Subject: [PATCH] chore: pass ZoneLoading error as result --- src/Linker/Linker.cpp | 10 +- src/ModMan/Context/FastFileContext.cpp | 6 +- src/ModMan/Context/FastFileContext.h | 5 +- src/ModMan/Web/Binds/FastFileBinds.cpp | 6 +- src/ModManUi/src/App.vue | 4 +- src/Unlinker/Unlinker.cpp | 22 ++- src/Utils/Utils/Result.h | 227 ++++++++++++++++--------- src/ZoneLoading/ZoneLoading.cpp | 18 +- src/ZoneLoading/ZoneLoading.h | 4 +- 9 files changed, 181 insertions(+), 121 deletions(-) diff --git a/src/Linker/Linker.cpp b/src/Linker/Linker.cpp index 61050cf0..3da46b23 100644 --- a/src/Linker/Linker.cpp +++ b/src/Linker/Linker.cpp @@ -364,14 +364,16 @@ class LinkerImpl final : public Linker zoneDirectory = fs::current_path(); auto absoluteZoneDirectory = absolute(zoneDirectory).string(); - auto zone = ZoneLoading::LoadZone(zonePath); - if (!zone) + auto maybeZone = ZoneLoading::LoadZone(zonePath); + if (!maybeZone) { - con::error("Failed to load zone \"{}\".", zonePath); + con::error("Failed to load zone \"{}\": {}", zonePath, maybeZone.error()); return false; } - con::debug("Load zone \"{}\"", zone->m_name); + auto zone = std::move(*maybeZone); + + con::debug("Loaded zone \"{}\"", zone->m_name); m_loaded_zones.emplace_back(std::move(zone)); } diff --git a/src/ModMan/Context/FastFileContext.cpp b/src/ModMan/Context/FastFileContext.cpp index 51d0814e..823d595c 100644 --- a/src/ModMan/Context/FastFileContext.cpp +++ b/src/ModMan/Context/FastFileContext.cpp @@ -2,11 +2,11 @@ #include "ZoneLoading.h" -std::optional FastFileContext::LoadFastFile(const std::string& path) +result::Expected FastFileContext::LoadFastFile(const std::string& path) { auto zone = ZoneLoading::LoadZone(path); if (!zone) - return std::nullopt; + return result::Unexpected(std::move(zone.error())); - return m_loaded_zones.emplace_back(std::move(zone)).get(); + return m_loaded_zones.emplace_back(std::move(*zone)).get(); } diff --git a/src/ModMan/Context/FastFileContext.h b/src/ModMan/Context/FastFileContext.h index 54fa0114..74b0b3af 100644 --- a/src/ModMan/Context/FastFileContext.h +++ b/src/ModMan/Context/FastFileContext.h @@ -1,14 +1,15 @@ #pragma once + +#include "Utils/Result.h" #include "Zone/Zone.h" #include -#include #include class FastFileContext { public: - std::optional LoadFastFile(const std::string& path); + result::Expected LoadFastFile(const std::string& path); std::vector> m_loaded_zones; }; diff --git a/src/ModMan/Web/Binds/FastFileBinds.cpp b/src/ModMan/Web/Binds/FastFileBinds.cpp index a86ea0bf..fab40843 100644 --- a/src/ModMan/Web/Binds/FastFileBinds.cpp +++ b/src/ModMan/Web/Binds/FastFileBinds.cpp @@ -10,7 +10,7 @@ namespace ModManContext::Get().m_db_thread.Dispatch( [&wv, id, path] { - const auto maybeZone = ModManContext::Get().m_fast_file.LoadFastFile(path); + auto maybeZone = ModManContext::Get().m_fast_file.LoadFastFile(path); if (maybeZone) { @@ -19,8 +19,8 @@ namespace } else { - con::warn("Failed to load zone \"{}\"", path); - ui::PromiseReject(wv, id, false); + con::warn("Failed to load zone \"{}\": {}", path, maybeZone.error()); + ui::PromiseReject(wv, id, std::move(maybeZone).error()); } }); } diff --git a/src/ModManUi/src/App.vue b/src/ModManUi/src/App.vue index 3e15c414..85054db9 100644 --- a/src/ModManUi/src/App.vue +++ b/src/ModManUi/src/App.vue @@ -12,8 +12,8 @@ async function onOpenFastfileClick() { loadingFastFile.value = true; webviewBinds.loadFastFile(lastPath.value) - .catch((e) => { - console.error("Failed to load fastfile", e); + .catch((e: string) => { + console.error("Failed to load fastfile:", e); }) .finally(() => { loadingFastFile.value = false; diff --git a/src/Unlinker/Unlinker.cpp b/src/Unlinker/Unlinker.cpp index 790e75c0..1503d215 100644 --- a/src/Unlinker/Unlinker.cpp +++ b/src/Unlinker/Unlinker.cpp @@ -227,13 +227,15 @@ private: auto absoluteZoneDirectory = absolute(std::filesystem::path(zonePath).remove_filename()).string(); auto searchPathsForZone = paths.GetSearchPathsForZone(absoluteZoneDirectory); - auto zone = ZoneLoading::LoadZone(zonePath); - if (zone == nullptr) + auto maybeZone = ZoneLoading::LoadZone(zonePath); + if (!maybeZone) { - con::error("Failed to load zone \"{}\".", zonePath); + con::error("Failed to load zone \"{}\": {}", zonePath, maybeZone.error()); return false; } + auto zone = std::move(*maybeZone); + con::debug("Loaded zone \"{}\"", zone->m_name); if (ShouldLoadObj()) @@ -287,16 +289,16 @@ private: auto searchPathsForZone = paths.GetSearchPathsForZone(absoluteZoneDirectory); - std::string zoneName; - auto zone = ZoneLoading::LoadZone(zonePath); - if (zone == nullptr) + auto maybeZone = ZoneLoading::LoadZone(zonePath); + if (!maybeZone) { - con::error("Failed to load zone \"{}\".", zonePath); + con::error("Failed to load zone \"{}\": {}", zonePath, maybeZone.error()); return false; } - zoneName = zone->m_name; - con::debug("Loaded zone \"{}\"", zoneName); + auto zone = std::move(*maybeZone); + + con::debug("Loaded zone \"{}\"", zone->m_name); const auto* objLoader = IObjLoader::GetObjLoaderForGame(zone->m_game_id); if (ShouldLoadObj()) @@ -308,6 +310,8 @@ private: if (ShouldLoadObj()) objLoader->UnloadContainersOfZone(*zone); + // Copy zone name for using it after freeing the zone + std::string zoneName = zone->m_name; zone.reset(); con::debug("Unloaded zone \"{}\"", zoneName); } diff --git a/src/Utils/Utils/Result.h b/src/Utils/Utils/Result.h index 2e932393..b5d870c0 100644 --- a/src/Utils/Utils/Result.h +++ b/src/Utils/Utils/Result.h @@ -6,112 +6,171 @@ using NoResult = std::monostate; // Can be replaced by std::expected with c++23 -template class Result +namespace result { -public: - Result(TResult result) - : m_data(std::variant(std::in_place_index<0>, std::move(result))) + template class Unexpected { - } + public: + Unexpected(TError result) + : m_data(std::move(result)) + { + } - static Result Ok(TResult result) + constexpr std::add_lvalue_reference_t value() & + { + return m_data; + } + + constexpr std::add_const_t> value() const& + { + return m_data; + } + + constexpr std::add_rvalue_reference_t value() && + { + return std::move(m_data); + } + + constexpr std::add_const_t> value() const&& + { + return std::move(m_data); + } + + constexpr std::add_lvalue_reference_t operator*() & + { + return m_data; + } + + constexpr std::add_const_t> operator*() const& + { + return m_data; + } + + constexpr std::add_rvalue_reference_t operator*() && + { + return std::move(m_data); + } + + constexpr std::add_const_t> operator*() const&& + { + return std::move(m_data); + } + + constexpr std::add_pointer_t operator->() + { + return m_data; + } + + constexpr std::add_const_t> operator->() const + { + return m_data; + } + + private: + TError m_data; + }; + + template class Expected { - return Result(std::in_place_index<0>, std::move(result)); - } + public: + Expected(TResult result) + : m_data(std::variant(std::in_place_index<0>, std::move(result))) + { + } - static Result Bad(TError error) - { - return Result(std::in_place_index<1>, std::move(error)); - } + Expected(Unexpected unexpected) + : m_data(std::variant(std::in_place_index<1>, std::move(*unexpected))) + { + } - constexpr operator bool() const noexcept - { - return m_data.index() == 0; - } + constexpr operator bool() const noexcept + { + return m_data.index() == 0; + } - constexpr bool has_value() const noexcept - { - return m_data.index() == 0; - } + constexpr bool has_value() const noexcept + { + return m_data.index() == 0; + } - constexpr std::add_lvalue_reference_t value() & - { - return std::get<0, TResult>(m_data); - } + constexpr std::add_lvalue_reference_t value() & + { + return std::get<0>(m_data); + } - constexpr std::add_const_t> value() const& - { - return std::get<0, TResult>(m_data); - } + constexpr std::add_const_t> value() const& + { + return std::get<0>(m_data); + } - constexpr std::add_rvalue_reference_t value() && - { - return std::move(std::get<0, TResult>(m_data)); - } + constexpr std::add_rvalue_reference_t value() && + { + return std::move(std::get<0>(m_data)); + } - constexpr std::add_const_t> value() const&& - { - return std::move(std::get<0, TResult>(m_data)); - } + constexpr std::add_const_t> value() const&& + { + return std::move(std::get<0>(m_data)); + } - constexpr std::add_lvalue_reference_t operator*() & - { - return std::get<0, TResult>(m_data); - } + constexpr std::add_lvalue_reference_t operator*() & + { + return std::get<0>(m_data); + } - constexpr std::add_const_t> operator*() const& - { - return std::get<0, TResult>(m_data); - } + constexpr std::add_const_t> operator*() const& + { + return std::get<0>(m_data); + } - constexpr std::add_rvalue_reference_t operator*() && - { - return std::move(std::get<0, TResult>(m_data)); - } + constexpr std::add_rvalue_reference_t operator*() && + { + return std::move(std::get<0>(m_data)); + } - constexpr std::add_const_t> operator*() const&& - { - return std::move(std::get<0, TResult>(m_data)); - } + constexpr std::add_const_t> operator*() const&& + { + return std::move(std::get<0>(m_data)); + } - constexpr std::add_pointer_t operator->() - { - return std::get<0, TResult>(m_data); - } + constexpr std::add_pointer_t operator->() + { + return std::get<0>(m_data); + } - constexpr std::add_const_t> operator->() const - { - return std::get<0, TResult>(m_data); - } + constexpr std::add_const_t> operator->() const + { + return std::get<0>(m_data); + } - constexpr std::add_lvalue_reference_t error() & - { - return std::get<1, TError>(m_data); - } + constexpr std::add_lvalue_reference_t error() & + { + return std::get<1>(m_data); + } - constexpr std::add_const_t> error() const& - { - return std::get<1, TError>(m_data); - } + constexpr std::add_const_t> error() const& + { + return std::get<1>(m_data); + } - constexpr std::add_rvalue_reference_t error() && - { - return std::move(std::get<1, TError>(m_data)); - } + constexpr std::add_rvalue_reference_t error() && + { + return std::move(std::get<1>(m_data)); + } - constexpr std::add_const_t> error() const&& - { - return std::move(std::get<1, TError>(m_data)); - } + constexpr std::add_const_t> error() const&& + { + return std::move(std::get<1>(m_data)); + } -private: - explicit Result(std::variant data) - : m_data(std::move(data)) - { - } + private: + explicit Expected(std::variant data) + : m_data(std::move(data)) + { + } - std::variant m_data; -}; + std::variant m_data; + }; #define ENSURE_RESULT_VAR(var) \ if (!(var)) \ @@ -122,3 +181,5 @@ private: if (!result) \ return result; \ } + +} // namespace result diff --git a/src/ZoneLoading/ZoneLoading.cpp b/src/ZoneLoading/ZoneLoading.cpp index 3d15b80c..36e2a2ed 100644 --- a/src/ZoneLoading/ZoneLoading.cpp +++ b/src/ZoneLoading/ZoneLoading.cpp @@ -2,7 +2,6 @@ #include "Loading/IZoneLoaderFactory.h" #include "Loading/ZoneLoader.h" -#include "Utils/Logging/Log.h" #include "Utils/ObjFileStream.h" #include @@ -12,24 +11,18 @@ namespace fs = std::filesystem; -std::unique_ptr ZoneLoading::LoadZone(const std::string& path) +result::Expected, std::string> ZoneLoading::LoadZone(const std::string& path) { auto zoneName = fs::path(path).filename().replace_extension().string(); std::ifstream file(path, std::fstream::in | std::fstream::binary); if (!file.is_open()) - { - con::error("Could not open file '{}'.", path); - return nullptr; - } + return result::Unexpected(std::format("Could not open file '{}'.", path)); ZoneHeader header{}; file.read(reinterpret_cast(&header), sizeof(header)); if (file.gcount() != sizeof(header)) - { - con::error("Failed to read zone header from file '{}'.", path); - return nullptr; - } + return result::Unexpected(std::format("Failed to read zone header from file '{}'.", path)); std::unique_ptr zoneLoader; for (auto game = 0u; game < static_cast(GameId::COUNT); game++) @@ -42,10 +35,7 @@ std::unique_ptr ZoneLoading::LoadZone(const std::string& path) } if (!zoneLoader) - { - con::error("Could not create factory for zone '{}'.", zoneName); - return nullptr; - } + return result::Unexpected(std::format("Could not create factory for zone '{}'.", zoneName)); auto loadedZone = zoneLoader->LoadZone(file); diff --git a/src/ZoneLoading/ZoneLoading.h b/src/ZoneLoading/ZoneLoading.h index 749db050..330b3421 100644 --- a/src/ZoneLoading/ZoneLoading.h +++ b/src/ZoneLoading/ZoneLoading.h @@ -1,4 +1,6 @@ #pragma once + +#include "Utils/Result.h" #include "Zone/Zone.h" #include @@ -6,5 +8,5 @@ class ZoneLoading { public: - static std::unique_ptr LoadZone(const std::string& path); + static result::Expected, std::string> LoadZone(const std::string& path); };