refactor(error): replaced tl::expected with result

core/src/error/error.cpp

@@ -1,17 +0,0 @@
-#include "error.h"
-#include "logger.h"
-
-Error::Error(std::string message) : message(message) {
-}
-
-std::string Error::getMessage() {
-    return this->message;
-}
-
-void Error::logMessage(Logger::LogLevel logLevel) {
-    Logger::log(this->message, logLevel);
-}
-
-void Error::logMessageFatal() {
-    Logger::fatalError(this->message);
-}

core/src/error/error.h

@@ -5,13 +5,15 @@
 
 // Base class for all errors
 class Error {
-    std::string message;
+    std::string _errorType;
+    std::string _message;
 
 protected:
-    Error(std::string message);
+    inline Error(std::string errorType, std::string message) : _errorType(errorType), _message(message) {}
 
 public:
-    std::string getMessage();
+    inline std::string errorType() { return this->_errorType; }
-    void logMessage(Logger::LogLevel logLevel = Logger::LogLevel::ERROR);
+    inline std::string message() { return this->_message; }
-    void logMessageFatal();
+    inline void logMessage(Logger::LogLevel logLevel = Logger::LogLevel::ERROR) { Logger::log(this->_message, logLevel); }
+    void logMessageFatal() { Logger::fatalError(this->_message); }
 };

core/src/error/instance.h

@@ -0,0 +1,28 @@
+#pragma once
+
+#include "error.h"
+
+class NoSuchInstance : public Error {
+    public:
+    inline NoSuchInstance(std::string className) : Error("NoSuchInstance", "Cannot create instance of unknown type " + className) {}
+};
+
+class NoSuchService : public Error {
+    public:
+    inline NoSuchService(std::string className) : Error("NoSuchService", "Cannot insert service of unknown type " + className) {}
+};
+
+class ServiceAlreadyExists : public Error {
+    public:
+    inline ServiceAlreadyExists(std::string className) : Error("ServiceAlreadyExists", "Service " + className + " is already inserted") {}
+};
+
+class MemberNotFound : public Error {
+    public:
+    inline MemberNotFound(std::string className, std::string memberName) : Error("MemberNotFound", "Could not find member '" + memberName + "' in class " + className) {}
+};
+
+class AssignToReadOnlyMember : public Error {
+    public:
+    inline AssignToReadOnlyMember(std::string className, std::string memberName) : Error("AssignToReadOnlyMember", "Attempt to assign value to read-only member '" + memberName + "' in class " + className) {}
+};

core/src/error/result.h

@@ -1,59 +1,68 @@
 #pragma once
 
-#include "error.h"
+#include "error/error.h"
 #include "logger.h"
 #include "panic.h"
 #include <optional>
 #include <string>
+#include <type_traits>
 #include <variant>
 
 struct DUMMY_VALUE {};
 
-template <typename Result, typename ...E>
+template <typename T_Result, typename ...T_Errors>
 class [[nodiscard]] result {
-    struct ErrorContainer {
+    static_assert(std::conjunction_v<std::is_base_of<Error, T_Errors>...>, "result<T_Errors...> requires T_Errors to derive from Error");
-        std::variant<E...> error;
+
+    struct error_state {
+        std::variant<T_Errors...> error;
     };
 
-    struct SuccessContainer {
+    struct success_state {
-        Result success;
+        T_Result success;
     };
 
-    std::variant<SuccessContainer, ErrorContainer> value;
+    std::variant<success_state, error_state> value;
 public:
-    result(Result success) : value(SuccessContainer { success }) {}
+    result(T_Result success) : value(success_state { success }) {}
-    result(std::variant<E...> error) : value(ErrorContainer { error }) {}
+    result(std::variant<T_Errors...> error) : value(error_state { error }) {}
+    template <typename T_Error, std::enable_if_t<std::disjunction_v<std::is_same<T_Error, T_Errors>...>, int> = 0>
+    result(T_Error error) : value(error_state { error }) {}
 
     // Expects the result to be successful, otherwise panic with error message
-    Result expect(std::string errMsg = "Unwrapped a result with failure value") {
+    T_Result expect(std::string errMsg = "Expected result to contain success") {
-        if (is_success())
+        if (isSuccess())
-            return std::get<SuccessContainer>(value).success;
+            return std::get<success_state>(value).success;
-        Logger::fatalError(errMsg);
+        std::visit([&](auto&& it) {
+            Logger::fatalErrorf("Unwrapped a result with error value: [%s] %s\n\t%s", it.errorType(), it.message(), errMsg);
+        }, error().value());
         panic();
     }
 
-    bool is_success() { return std::holds_alternative<SuccessContainer>(value); }
+    bool isSuccess() { return std::holds_alternative<success_state>(value); }
-    bool is_error() { return std::holds_alternative<ErrorContainer>(value); }
+    bool isError() { return std::holds_alternative<error_state>(value); }
 
-    std::optional<Result> success() { return is_success() ? std::get<SuccessContainer>(value).success : std::nullopt; }
+    std::optional<T_Result> success() { return isSuccess() ? std::get<success_state>(value).success : std::nullopt; }
-    std::optional<std::variant<E...>> error() { return is_error() ? std::make_optional(std::get<ErrorContainer>(value).error) : std::nullopt; }
+    std::optional<std::variant<T_Errors...>> error() { return isError() ? std::make_optional(std::get<error_state>(value).error) : std::nullopt; }
 
     void logError(Logger::LogLevel logLevel = Logger::LogLevel::ERROR) {
-        if (is_success()) return;
+        if (isSuccess()) return;
         std::visit([&](auto&& it) {
             it.logMessage(logLevel);
         }, error().value());
     }
 
     // Equivalent to .success
-    operator std::optional<Result>() { return success(); }
+    operator std::optional<T_Result>() { return success(); }
-    operator bool() { return is_success(); }
+    operator bool() { return isSuccess(); }
-    bool operator !() { return is_error(); }
+    bool operator !() { return isError(); }
 };
 
-template <typename ...E>
+template <typename ...T_Errors>
-class fallible : public result<DUMMY_VALUE, E...> {
+class [[nodiscard]] fallible : public result<DUMMY_VALUE, T_Errors...> {
 public:
-    fallible() : result<DUMMY_VALUE, E...>(DUMMY_VALUE {}) {}
+    fallible() : result<DUMMY_VALUE, T_Errors...>(DUMMY_VALUE {}) {}
-    fallible(std::variant<E...> error) : result<DUMMY_VALUE, E...>(error) {}
+    fallible(std::variant<T_Errors...> error) : result<DUMMY_VALUE, T_Errors...>(error) {}
+    template <typename T_Error, std::enable_if_t<std::disjunction_v<std::is_same<T_Error, T_Errors>...>, int> = 0>
+    fallible(T_Error error) : result<DUMMY_VALUE, T_Errors...>(error) {}
 };

core/src/objects/base/instance.cpp

@@ -2,6 +2,7 @@
 #include "common.h"
 #include "datatypes/meta.h"
 #include "datatypes/base.h"
+#include "error/instance.h"
 #include "objects/base/member.h"
 #include "objects/meta.h"
 #include "logger.h"
@@ -157,24 +158,27 @@ void Instance::OnAncestryChanged(std::optional<std::shared_ptr<Instance>> child,
 
 // Properties
 
-tl::expected<Data::Variant, MemberNotFound> Instance::GetPropertyValue(std::string name) {
+result<Data::Variant, MemberNotFound> Instance::GetPropertyValue(std::string name) {
-auto meta = GetPropertyMeta(name);
+    auto meta_ = GetPropertyMeta(name);
-    if (!meta) return tl::make_unexpected(MemberNotFound());
+    if (!meta_) return MemberNotFound(GetClass()->className, name);
+    auto meta = meta_.expect();
 
-    return meta->codec.read(meta->backingField);
+    return meta.codec.read(meta.backingField);
 }
 
-tl::expected<void, MemberNotFound> Instance::SetPropertyValue(std::string name, Data::Variant value) {
+fallible<MemberNotFound, AssignToReadOnlyMember> Instance::SetPropertyValue(std::string name, Data::Variant value) {
-    auto meta = GetPropertyMeta(name);
+    auto meta_ = GetPropertyMeta(name);
-    if (!meta || meta->flags & PROP_READONLY) return tl::make_unexpected(MemberNotFound());
+    if (!meta_) return MemberNotFound(GetClass()->className, name);
+    auto meta = meta_.expect();
+    if (meta.flags & PROP_READONLY) AssignToReadOnlyMember(GetClass()->className, name);
 
-    meta->codec.write(value, meta->backingField);
+    meta.codec.write(value, meta.backingField);
-    if (meta->updateCallback) meta->updateCallback.value()(name);
+    if (meta.updateCallback) meta.updateCallback.value()(name);
 
     return {};
 }
 
-tl::expected<PropertyMeta, MemberNotFound> Instance::GetPropertyMeta(std::string name) {
+result<PropertyMeta, MemberNotFound> Instance::GetPropertyMeta(std::string name) {
     MemberMap* current = &*memberMap;
     while (true) {
         // We look for the property in current member map
@@ -186,7 +190,7 @@ tl::expected<PropertyMeta, MemberNotFound> Instance::GetPropertyMeta(std::string
 
         // It is not found, If there are no other maps to search, return null
         if (!current->super.has_value())
-            return tl::make_unexpected(MemberNotFound());
+            return MemberNotFound(GetClass()->className, name);
 
         // Search in the parent
         current = current->super->get();
@@ -223,12 +227,12 @@ void Instance::Serialize(pugi::xml_node parent) {
     // Add properties
     pugi::xml_node propertiesNode = node.append_child("Properties");
     for (std::string name : GetProperties()) {
-        PropertyMeta meta = GetPropertyMeta(name).value();
+        PropertyMeta meta = GetPropertyMeta(name).expect("Meta of declared property is missing");
         if (meta.flags & PropertyFlags::PROP_NOSAVE) continue; // This property should not be serialized. Skip...
 
         pugi::xml_node propertyNode = propertiesNode.append_child(meta.type->name);
         propertyNode.append_attribute("name").set_value(name);
-        GetPropertyValue(name)->Serialize(propertyNode);
+        GetPropertyValue(name).expect("Declared property is missing").Serialize(propertyNode);
     }
 
     // Add children
@@ -240,7 +244,7 @@ void Instance::Serialize(pugi::xml_node parent) {
 result<InstanceRef, NoSuchInstance> Instance::Deserialize(pugi::xml_node node) {
     std::string className = node.attribute("class").value();
     if (INSTANCE_MAP.count(className) == 0) {
-        return std::variant<NoSuchInstance>(NoSuchInstance(className));
+        return 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());
@@ -254,18 +258,18 @@ result<InstanceRef, NoSuchInstance> Instance::Deserialize(pugi::xml_node node) {
     for (pugi::xml_node propertyNode : propertiesNode) {
         std::string propertyName = propertyNode.attribute("name").value();
         auto meta_ = object->GetPropertyMeta(propertyName);
-        if (!meta_.has_value()) {
+        if (!meta_) {
             Logger::fatalErrorf("Attempt to set unknown property '%s' of %s", propertyName.c_str(), object->GetClass()->className.c_str());
             continue;
         }
         Data::Variant value = Data::Variant::Deserialize(propertyNode);
-        object->SetPropertyValue(propertyName, value);
+        object->SetPropertyValue(propertyName, value).expect("Declared property was missing");
     }
 
     // Read children
     for (pugi::xml_node childNode : node.children("Item")) {
         result<InstanceRef, NoSuchInstance> child = Instance::Deserialize(childNode);
-        if (child.is_error()) {
+        if (child.isError()) {
             std::get<NoSuchInstance>(child.error().value()).logMessage();
             continue;
         }

core/src/objects/base/instance.h

@@ -13,7 +13,7 @@
 #include <expected.hpp>
 #include <pugixml.hpp>
 
-#include "error/error.h"
+#include "error/instance.h"
 #include "error/result.h"
 #include "member.h"
 
@@ -38,11 +38,6 @@ 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;
 
@@ -97,10 +92,9 @@ public:
     inline void AddChild(std::shared_ptr<Instance> object) { object->SetParent(this->shared_from_this()); }
 
     // Properties
-    // Do I like using expected?
+    result<Data::Variant, MemberNotFound> GetPropertyValue(std::string name);
-    tl::expected<Data::Variant, MemberNotFound> GetPropertyValue(std::string name);
+    fallible<MemberNotFound, AssignToReadOnlyMember> SetPropertyValue(std::string name, Data::Variant value);
-    tl::expected<void, MemberNotFound> SetPropertyValue(std::string name, Data::Variant value);
+    result<PropertyMeta, MemberNotFound> GetPropertyMeta(std::string name);
-    tl::expected<PropertyMeta, MemberNotFound> GetPropertyMeta(std::string name);
     // Returning a list of property names feels kinda janky. Is this really the way to go?
     std::vector<std::string> GetProperties();
 

core/src/objects/base/member.h

@@ -75,6 +75,4 @@ typedef std::variant<PropertyMeta> MemberMeta;
 struct MemberMap {
     std::optional<std::unique_ptr<MemberMap>> super;
     std::map<std::string, PropertyMeta> members;
-};
+};
-
-struct MemberNotFound {};

core/src/objects/datamodel.cpp

@@ -82,18 +82,18 @@ void DataModel::DeserializeService(pugi::xml_node node) {
     for (pugi::xml_node propertyNode : propertiesNode) {
         std::string propertyName = propertyNode.attribute("name").value();
         auto meta_ = object->GetPropertyMeta(propertyName);
-        if (!meta_.has_value()) {
+        if (!meta_) {
             Logger::fatalErrorf("Attempt to set unknown property '%s' of %s", propertyName.c_str(), object->GetClass()->className.c_str());
             continue;
         }
         Data::Variant value = Data::Variant::Deserialize(propertyNode);
-        object->SetPropertyValue(propertyName, value);
+        object->SetPropertyValue(propertyName, value).expect();
     }
 
     // Add children
     for (pugi::xml_node childNode : node.children("Item")) {
         result<InstanceRef, NoSuchInstance> child = Instance::Deserialize(childNode);
-        if (child.is_error()) {
+        if (child.isError()) {
             std::get<NoSuchInstance>(child.error().value()).logMessage();
             continue;
         }

core/src/objects/datamodel.h

@@ -1,6 +1,5 @@
 #pragma once
 
-#include "base.h"
 #include "error/result.h"
 #include "logger.h"
 #include "objects/base/instance.h"
@@ -14,16 +13,6 @@ 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:

editor/panes/propertiesview.cpp

@@ -41,8 +41,8 @@ public:
 
         std::string propertyName = !isComposite ? view->itemFromIndex(index)->data(0, Qt::DisplayRole).toString().toStdString()
             : view->itemFromIndex(index.parent())->data(0, Qt::DisplayRole).toString().toStdString();
-        PropertyMeta meta = inst->GetPropertyMeta(propertyName).value();
+        PropertyMeta meta = inst->GetPropertyMeta(propertyName).expect();
-        Data::Variant currentValue = inst->GetPropertyValue(propertyName).value();
+        Data::Variant currentValue = inst->GetPropertyValue(propertyName).expect();
 
         if (isComposite) {
             if (meta.type == &Data::Vector3::TYPE) {
@@ -107,8 +107,8 @@ public:
 
         std::string propertyName = !index.parent().parent().isValid() ? view->itemFromIndex(index)->data(0, Qt::DisplayRole).toString().toStdString()
             : view->itemFromIndex(index.parent())->data(0, Qt::DisplayRole).toString().toStdString();
-        PropertyMeta meta = inst->GetPropertyMeta(propertyName).value();
+        PropertyMeta meta = inst->GetPropertyMeta(propertyName).expect();
-        Data::Variant currentValue = inst->GetPropertyValue(propertyName).value();
+        Data::Variant currentValue = inst->GetPropertyValue(propertyName).expect();
         
         if (isComposite) {
             if (meta.type == &Data::Vector3::TYPE) {
@@ -159,19 +159,19 @@ public:
 
         std::string propertyName = !index.parent().parent().isValid() ? view->itemFromIndex(index)->data(0, Qt::DisplayRole).toString().toStdString()
             : view->itemFromIndex(index.parent())->data(0, Qt::DisplayRole).toString().toStdString();
-        PropertyMeta meta = inst->GetPropertyMeta(propertyName).value();
+        PropertyMeta meta = inst->GetPropertyMeta(propertyName).expect();
 
         if (isComposite) {
             if (meta.type == &Data::Vector3::TYPE) {
                 QDoubleSpinBox* spinBox = dynamic_cast<QDoubleSpinBox*>(editor);
                 float value = spinBox->value();
 
-                Data::Vector3 prev = inst->GetPropertyValue(propertyName).value().get<Data::Vector3>();
+                Data::Vector3 prev = inst->GetPropertyValue(propertyName).expect().get<Data::Vector3>();
                 Data::Vector3 newVector = componentName == "X" ? Data::Vector3(value, prev.Y(), prev.Z())
                 : componentName == "Y" ? Data::Vector3(prev.X(), value, prev.Z())
                 : componentName == "Z" ? Data::Vector3(prev.X(), prev.Y(), value) : prev;
 
-                inst->SetPropertyValue(propertyName, newVector);
+                inst->SetPropertyValue(propertyName, newVector).expect();
                 view->rebuildCompositeProperty(view->itemFromIndex(index.parent()), &Data::Vector3::TYPE, newVector);
                 return;
             }
@@ -182,24 +182,24 @@ public:
         if (meta.type == &Data::Float::TYPE) {
             QDoubleSpinBox* spinBox = dynamic_cast<QDoubleSpinBox*>(editor);
 
-            inst->SetPropertyValue(propertyName, Data::Float((float)spinBox->value()));
+            inst->SetPropertyValue(propertyName, Data::Float((float)spinBox->value())).expect();
             model->setData(index, spinBox->value());
         } else if (meta.type == &Data::Int::TYPE) {
             QSpinBox* spinBox = dynamic_cast<QSpinBox*>(editor);
 
-            inst->SetPropertyValue(propertyName, Data::Int((float)spinBox->value()));
+            inst->SetPropertyValue(propertyName, Data::Int((float)spinBox->value())).expect();
             model->setData(index, spinBox->value());
         } else if (meta.type == &Data::String::TYPE) {
             QLineEdit* lineEdit = dynamic_cast<QLineEdit*>(editor);
 
-            inst->SetPropertyValue(propertyName, Data::String(lineEdit->text().toStdString()));
+            inst->SetPropertyValue(propertyName, Data::String(lineEdit->text().toStdString())).expect();
             model->setData(index, lineEdit->text());
         } else if (meta.type == &Data::Color3::TYPE) {
             QColorDialog* colorDialog = dynamic_cast<QColorDialog*>(editor);
 
             QColor color = colorDialog->currentColor();
             Data::Color3 color3(color.redF(), color.greenF(), color.blueF());
-            inst->SetPropertyValue(propertyName, color3);
+            inst->SetPropertyValue(propertyName, color3).expect();
             model->setData(index, QString::fromStdString(color3.ToString()), Qt::DisplayRole);
             model->setData(index, color, Qt::DecorationRole);
         } else if (meta.type->fromString) {
@@ -207,7 +207,7 @@ public:
 
             std::optional<Data::Variant> parsedResult = meta.type->fromString(lineEdit->text().toStdString());
             if (!parsedResult) return;
-            inst->SetPropertyValue(propertyName, parsedResult.value());
+            inst->SetPropertyValue(propertyName, parsedResult.value()).expect();
             model->setData(index, QString::fromStdString(parsedResult.value().ToString()));
             view->rebuildCompositeProperty(view->itemFromIndex(index), meta.type, parsedResult.value());
         }
@@ -283,8 +283,8 @@ void PropertiesView::setSelected(std::optional<InstanceRef> instance) {
     std::vector<std::string> properties = inst->GetProperties();
 
     for (std::string property : properties) {
-        PropertyMeta meta = inst->GetPropertyMeta(property).value();
+        PropertyMeta meta = inst->GetPropertyMeta(property).expect();
-        Data::Variant currentValue = inst->GetPropertyValue(property).value();
+        Data::Variant currentValue = inst->GetPropertyValue(property).expect();
 
         if (meta.type == &Data::CFrame::TYPE) continue;
 
@@ -330,10 +330,10 @@ void PropertiesView::propertyChanged(QTreeWidgetItem *item, int column) {
     InstanceRef inst = currentInstance->lock();
 
     std::string propertyName = item->data(0, Qt::DisplayRole).toString().toStdString();
-    PropertyMeta meta = inst->GetPropertyMeta(propertyName).value();
+    PropertyMeta meta = inst->GetPropertyMeta(propertyName).expect();
 
     if (meta.type == &Data::Bool::TYPE) {
-        inst->SetPropertyValue(propertyName, Data::Bool(item->checkState(1)));
+        inst->SetPropertyValue(propertyName, Data::Bool(item->checkState(1))).expect();
     }
 }