From eeaaef8c889b78c761f55901c4f87d6aee4c259d Mon Sep 17 00:00:00 2001 From: maelstrom Date: Thu, 10 Apr 2025 18:50:36 +0200 Subject: [PATCH] fix(serialization): no longer aborts if wrong type is parsed --- core/src/error/error.cpp | 4 +-- core/src/error/error.h | 2 +- core/src/error/fallible.h | 9 ------- core/src/error/result.cpp | 36 -------------------------- core/src/error/result.h | 41 ++++++++++++++++++++++++------ core/src/objects/base/instance.cpp | 13 ++++++---- core/src/objects/base/instance.h | 10 +++++++- core/src/objects/datamodel.cpp | 10 +++++--- core/src/objects/datamodel.h | 23 ++++++++++++----- core/src/panic.h | 2 +- editor/mainwindow.cpp | 16 +++++++----- 11 files changed, 88 insertions(+), 78 deletions(-) delete mode 100644 core/src/error/fallible.h delete mode 100644 core/src/error/result.cpp diff --git a/core/src/error/error.cpp b/core/src/error/error.cpp index 1872963..99bed01 100644 --- a/core/src/error/error.cpp +++ b/core/src/error/error.cpp @@ -9,9 +9,9 @@ std::string Error::getMessage() { } void Error::logMessage(Logger::LogLevel logLevel) { - Logger::logf("%s", logLevel, this->message); + Logger::log(this->message, logLevel); } void Error::logMessageFatal() { - Logger::fatalErrorf("%s", this->message); + Logger::fatalError(this->message); } \ No newline at end of file diff --git a/core/src/error/error.h b/core/src/error/error.h index 6422fd3..bb7275d 100644 --- a/core/src/error/error.h +++ b/core/src/error/error.h @@ -12,6 +12,6 @@ protected: public: std::string getMessage(); - void logMessage(Logger::LogLevel logLevel = Logger::LogLevel::INFO); + void logMessage(Logger::LogLevel logLevel = Logger::LogLevel::ERROR); void logMessageFatal(); }; \ No newline at end of file diff --git a/core/src/error/fallible.h b/core/src/error/fallible.h deleted file mode 100644 index ef6068c..0000000 --- a/core/src/error/fallible.h +++ /dev/null @@ -1,9 +0,0 @@ -#pragma once - -#include "result.h" - -struct DUMMY_VALUE; - -template -class fallible : public result { -}; \ No newline at end of file diff --git a/core/src/error/result.cpp b/core/src/error/result.cpp deleted file mode 100644 index 8871e51..0000000 --- a/core/src/error/result.cpp +++ /dev/null @@ -1,36 +0,0 @@ -#include "result.h" -#include -#include - -template -result::result(Result value) : value(SuccessContainer { value }) { -} - -template -result::result(std::variant value) : value(ErrorContainer { value }) { -} - -template -bool result::is_success() { - return std::holds_alternative(value); -} - -template -bool result::is_error() { - return std::holds_alternative(value); -} - -template -std::optional result::success() { - return std::holds_alternative(value) ? std::make_optional(std::get(value).success) : std::nullopt; -} - -template -std::optional> result::error() { - return std::holds_alternative(value) ? std::make_optional(std::get(value).error) : std::nullopt; -} - -template -result::operator std::optional() { - return this->success(); -} diff --git a/core/src/error/result.h b/core/src/error/result.h index c18fd0a..48fc73f 100644 --- a/core/src/error/result.h +++ b/core/src/error/result.h @@ -1,10 +1,14 @@ #pragma once #include "error.h" +#include "logger.h" +#include "panic.h" #include #include #include +struct DUMMY_VALUE {}; + template class [[nodiscard]] result { struct ErrorContainer { @@ -17,18 +21,39 @@ class [[nodiscard]] result { std::variant value; public: - result(Result value); - result(std::variant value); + result(Result success) : value(SuccessContainer { success }) {} + result(std::variant error) : value(ErrorContainer { error }) {} // Expects the result to be successful, otherwise panic with error message - Result expect(std::string errMsg = "Unwrapped a result with failure value"); + Result expect(std::string errMsg = "Unwrapped a result with failure value") { + if (is_success()) + return std::get(value).success; + Logger::fatalError(errMsg); + panic(); + } - bool is_success(); - bool is_error(); + bool is_success() { return std::holds_alternative(value); } + bool is_error() { return std::holds_alternative(value); } - std::optional success(); - std::optional> error(); + std::optional success() { return is_success() ? std::get(value).success : std::nullopt; } + std::optional> error() { return is_error() ? std::make_optional(std::get(value).error) : std::nullopt; } + + void logError(Logger::LogLevel logLevel = Logger::LogLevel::ERROR) { + if (is_success()) return; + std::visit([&](auto&& it) { + it.logMessage(logLevel); + }, error().value()); + } // Equivalent to .success - operator std::optional(); + operator std::optional() { return success(); } + operator bool() { return is_success(); } + bool operator !() { return is_error(); } +}; + +template +class fallible : public result { +public: + fallible() : result(DUMMY_VALUE {}) {} + fallible(std::variant error) : result(error) {} }; \ No newline at end of file diff --git a/core/src/objects/base/instance.cpp b/core/src/objects/base/instance.cpp index df768d2..5216d43 100644 --- a/core/src/objects/base/instance.cpp +++ b/core/src/objects/base/instance.cpp @@ -207,11 +207,10 @@ void Instance::Serialize(pugi::xml_node parent) { } } -InstanceRef Instance::Deserialize(pugi::xml_node node) { +result Instance::Deserialize(pugi::xml_node node) { std::string className = node.attribute("class").value(); if (INSTANCE_MAP.count(className) == 0) { - Logger::fatalErrorf("Unknown type for instance: '%s'", className.c_str()); - panic(); + return std::variant(NoSuchInstance(className)); } // This will error if an abstract instance is used in the file. Oh well, not my prob rn. // printf("What are you? A %s sandwich\n", className.c_str()); @@ -235,8 +234,12 @@ InstanceRef Instance::Deserialize(pugi::xml_node node) { // Read children for (pugi::xml_node childNode : node.children("Item")) { - InstanceRef child = Instance::Deserialize(childNode); - object->AddChild(child); + result child = Instance::Deserialize(childNode); + if (child.is_error()) { + std::get(child.error().value()).logMessage(); + continue; + } + object->AddChild(child.expect()); } return object; diff --git a/core/src/objects/base/instance.h b/core/src/objects/base/instance.h index 9828f40..b7ef341 100644 --- a/core/src/objects/base/instance.h +++ b/core/src/objects/base/instance.h @@ -15,6 +15,8 @@ #include #include +#include "error/error.h" +#include "error/result.h" #include "member.h" class Instance; @@ -38,6 +40,12 @@ struct InstanceType { InstanceFlags flags; }; +// Errors +class NoSuchInstance : public Error { + public: + inline NoSuchInstance(std::string className) : Error("Cannot create instance of unknown type " + className) {} +}; + class DescendantsIterator; // Base class for all instances in the data model @@ -95,7 +103,7 @@ public: // Serialization void Serialize(pugi::xml_node parent); - static std::shared_ptr Deserialize(pugi::xml_node node); + static result, NoSuchInstance> Deserialize(pugi::xml_node node); }; typedef std::shared_ptr InstanceRef; diff --git a/core/src/objects/datamodel.cpp b/core/src/objects/datamodel.cpp index f6a008c..fb9559d 100644 --- a/core/src/objects/datamodel.cpp +++ b/core/src/objects/datamodel.cpp @@ -66,7 +66,7 @@ void DataModel::DeserializeService(pugi::xml_node node) { std::string className = node.attribute("class").value(); if (INSTANCE_MAP.count(className) == 0) { Logger::fatalErrorf("Unknown service: '%s'", className.c_str()); - panic(); + return; } if (services.count(className) != 0) { @@ -93,8 +93,12 @@ void DataModel::DeserializeService(pugi::xml_node node) { // Add children for (pugi::xml_node childNode : node.children("Item")) { - InstanceRef child = Instance::Deserialize(childNode); - object->AddChild(child); + result child = Instance::Deserialize(childNode); + if (child.is_error()) { + std::get(child.error().value()).logMessage(); + continue; + } + object->AddChild(child.expect()); } // We add the service to the list diff --git a/core/src/objects/datamodel.h b/core/src/objects/datamodel.h index ec9832e..8393da3 100644 --- a/core/src/objects/datamodel.h +++ b/core/src/objects/datamodel.h @@ -1,17 +1,29 @@ #pragma once #include "base.h" +#include "error/result.h" #include "logger.h" #include "objects/base/instance.h" #include "objects/meta.h" #include "panic.h" #include +#include class Workspace; class DataModel; class Service; +class NoSuchService : public Error { + public: + inline NoSuchService(std::string className) : Error("Cannot insert service of unknown type " + className) {} +}; + +class ServiceAlreadyExists : public Error { + public: + inline ServiceAlreadyExists(std::string className) : Error("Service " + className + " is already inserted") {} +}; + // The root instance to all objects in the hierarchy class DataModel : public Instance { private: @@ -30,9 +42,9 @@ public: virtual const InstanceType* GetClass() override; // Inserts a service if it doesn't already exist - bool InsertService(std::string name) { + fallible InsertService(std::string name) { if (services.count(name) != 0) - return false; + return fallible(ServiceAlreadyExists(name)); if (!INSTANCE_MAP[name] || (INSTANCE_MAP[name]->flags ^ (INSTANCE_NOTCREATABLE | INSTANCE_SERVICE)) != 0) { Logger::fatalErrorf("Attempt to create instance of unknown type %s", name); @@ -42,18 +54,17 @@ public: services[name] = std::dynamic_pointer_cast(INSTANCE_MAP[name]->constructor()); AddChild(std::dynamic_pointer_cast(services[name])); - return true; + return {}; } template - std::shared_ptr GetService(std::string name) { + result, NoSuchService> GetService(std::string name) { if (services.count(name) != 0) return services[name]; // TODO: Replace this with a result return type if (!INSTANCE_MAP[name] || (INSTANCE_MAP[name]->flags ^ (INSTANCE_NOTCREATABLE | INSTANCE_SERVICE)) != 0) { - Logger::fatalErrorf("Attempt to create instance of unknown type %s", name); - panic(); + return NoSuchService(name); } services[name] = std::dynamic_pointer_cast(INSTANCE_MAP[name]->constructor()); diff --git a/core/src/panic.h b/core/src/panic.h index ca448a4..23775fd 100644 --- a/core/src/panic.h +++ b/core/src/panic.h @@ -5,4 +5,4 @@ // before shutting down. // If this process fails, or the panic function is called within itself, it will hard-abort -void panic(); \ No newline at end of file +[[noreturn]] void panic(); \ No newline at end of file diff --git a/editor/mainwindow.cpp b/editor/mainwindow.cpp index b3e259c..07a526d 100644 --- a/editor/mainwindow.cpp +++ b/editor/mainwindow.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -284,8 +285,9 @@ MainWindow::MainWindow(QWidget *parent) rootDoc.load_string(encoded.c_str()); for (pugi::xml_node instNode : rootDoc.children()) { - InstanceRef inst = Instance::Deserialize(instNode); - gWorkspace()->AddChild(inst); + result inst = Instance::Deserialize(instNode); + if (!inst) { inst.logError(); continue; } + gWorkspace()->AddChild(inst.expect()); } }); @@ -303,8 +305,9 @@ MainWindow::MainWindow(QWidget *parent) rootDoc.load_string(encoded.c_str()); for (pugi::xml_node instNode : rootDoc.children()) { - InstanceRef inst = Instance::Deserialize(instNode); - selectedParent->AddChild(inst); + result inst = Instance::Deserialize(instNode); + if (!inst) { inst.logError(); continue; } + selectedParent->AddChild(inst.expect()); } }); @@ -337,8 +340,9 @@ MainWindow::MainWindow(QWidget *parent) modelDoc.load(inStream); for (pugi::xml_node instNode : modelDoc.child("openblocks").children("Item")) { - InstanceRef inst = Instance::Deserialize(instNode); - selectedParent->AddChild(inst); + result inst = Instance::Deserialize(instNode); + if (!inst) { inst.logError(); continue; } + selectedParent->AddChild(inst.expect()); } });