fix(serialization): no longer aborts if wrong type is parsed

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);
 }

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();
 };

core/src/error/fallible.h

@@ -1,9 +0,0 @@
-#pragma once
-
-#include "result.h"
-
-struct DUMMY_VALUE;
-
-template <typename ...E>
-class fallible : public result<DUMMY_VALUE, E...> {
-};

core/src/error/result.cpp

@@ -1,36 +0,0 @@
-#include "result.h"
-#include <optional>
-#include <variant>
-
-template <typename Result, typename ...E>
-result<Result, E...>::result(Result value) : value(SuccessContainer { value }) {
-}
-
-template <typename Result, typename ...E>
-result<Result, E...>::result(std::variant<E...> value) : value(ErrorContainer { value }) {
-}
-
-template <typename Result, typename ...E>
-bool result<Result, E...>::is_success() {
-    return std::holds_alternative<SuccessContainer>(value);
-}
-
-template <typename Result, typename ...E>
-bool result<Result, E...>::is_error() {
-    return std::holds_alternative<ErrorContainer>(value);
-}
-
-template <typename Result, typename ...E>
-std::optional<Result> result<Result, E...>::success() {
-    return std::holds_alternative<SuccessContainer>(value) ? std::make_optional(std::get<SuccessContainer>(value).success) : std::nullopt;
-}
-
-template <typename Result, typename ...E>
-std::optional<std::variant<E...>> result<Result, E...>::error() {
-    return std::holds_alternative<ErrorContainer>(value) ? std::make_optional(std::get<ErrorContainer>(value).error) : std::nullopt;
-}
-
-template <typename Result, typename ...E>
-result<Result, E...>::operator std::optional<Result>() {
-    return this->success();
-}

core/src/error/result.h

@@ -1,10 +1,14 @@
 #pragma once
 
 #include "error.h"
+#include "logger.h"
+#include "panic.h"
 #include <optional>
 #include <string>
 #include <variant>
 
+struct DUMMY_VALUE {};
+
 template <typename Result, typename ...E>
 class [[nodiscard]] result {
     struct ErrorContainer {
@@ -17,18 +21,39 @@ class [[nodiscard]] result {
 
     std::variant<SuccessContainer, ErrorContainer> value;
 public:
-    result(Result value);
+    result(Result success) : value(SuccessContainer { success }) {}
-    result(std::variant<E...> value);
+    result(std::variant<E...> 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<SuccessContainer>(value).success;
+        Logger::fatalError(errMsg);
+        panic();
+    }
 
-    bool is_success();
+    bool is_success() { return std::holds_alternative<SuccessContainer>(value); }
-    bool is_error();
+    bool is_error() { return std::holds_alternative<ErrorContainer>(value); }
 
-    std::optional<Result> success();
+    std::optional<Result> success() { return is_success() ? std::get<SuccessContainer>(value).success : std::nullopt; }
-    std::optional<std::variant<E...>> error();
+    std::optional<std::variant<E...>> error() { return is_error() ? std::make_optional(std::get<ErrorContainer>(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<Result>();
+    operator std::optional<Result>() { return success(); }
+    operator bool() { return is_success(); }
+    bool operator !() { return is_error(); }
+};
+
+template <typename ...E>
+class fallible : public result<DUMMY_VALUE, E...> {
+public:
+    fallible() : result<DUMMY_VALUE, E...>(DUMMY_VALUE {}) {}
+    fallible(std::variant<E...> error) : result<DUMMY_VALUE, E...>(error) {}
 };

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<InstanceRef, NoSuchInstance> 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());
+        return std::variant<NoSuchInstance>(NoSuchInstance(className));
-        panic();
     }
     // 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);
+        result<InstanceRef, NoSuchInstance> child = Instance::Deserialize(childNode);
-        object->AddChild(child);
+        if (child.is_error()) {
+            std::get<NoSuchInstance>(child.error().value()).logMessage();
+            continue;
+        }
+        object->AddChild(child.expect());
     }
 
     return object;

core/src/objects/base/instance.h

@@ -15,6 +15,8 @@
 #include <expected.hpp>
 #include <pugixml.hpp>
 
+#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<Instance> Deserialize(pugi::xml_node node);
+    static result<std::shared_ptr<Instance>, NoSuchInstance> Deserialize(pugi::xml_node node);
 };
 
 typedef std::shared_ptr<Instance> InstanceRef;

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);
+        result<InstanceRef, NoSuchInstance> child = Instance::Deserialize(childNode);
-        object->AddChild(child);
+        if (child.is_error()) {
+            std::get<NoSuchInstance>(child.error().value()).logMessage();
+            continue;
+        }
+        object->AddChild(child.expect());
     }
 
     // We add the service to the list

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 <memory>
+#include <variant>
 
 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<ServiceAlreadyExists, NoSuchService> InsertService(std::string name) {
         if (services.count(name) != 0)
-            return false;
+            return fallible<ServiceAlreadyExists, NoSuchService>(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<Service>(INSTANCE_MAP[name]->constructor());
         AddChild(std::dynamic_pointer_cast<Instance>(services[name]));
 
-        return true;
+        return {};
     }
 
     template <typename T>
-    std::shared_ptr<T> GetService(std::string name) {
+    result<std::shared_ptr<T>, 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);
+            return NoSuchService(name);
-            panic();
         }
 
         services[name] = std::dynamic_pointer_cast<Service>(INSTANCE_MAP[name]->constructor());

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();
+[[noreturn]] void panic();

editor/mainwindow.cpp

@@ -10,6 +10,7 @@
 #include <QWidget>
 #include <QTreeView>
 #include <QAbstractItemView>
+#include <cstdio>
 #include <functional>
 #include <memory>
 #include <optional>
@@ -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);
+            result<InstanceRef, NoSuchInstance> inst = Instance::Deserialize(instNode);
-            gWorkspace()->AddChild(inst);
+            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);
+            result<InstanceRef, NoSuchInstance> inst = Instance::Deserialize(instNode);
-            selectedParent->AddChild(inst);
+            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);
+            result<InstanceRef, NoSuchInstance> inst = Instance::Deserialize(instNode);
-            selectedParent->AddChild(inst);
+            if (!inst) { inst.logError(); continue; }
+            selectedParent->AddChild(inst.expect());
         }
     });