refactor: update various methods to return Result type

EndstoneMC · Sep 29, 2024 · e492667 · e492667
1 parent 9514e7c
commit e492667
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 100 deletions.
diff --git a/include/endstone/detail/scoreboard/scoreboard.h b/include/endstone/detail/scoreboard/scoreboard.h
@@ -28,11 +28,11 @@ class EndstoneScoreboard : public Scoreboard {
     explicit EndstoneScoreboard(std::unique_ptr<::Scoreboard> board);
     void init();
 
-    std::unique_ptr<Objective> addObjective(std::string name, Criteria::Type criteria) override;
-    std::unique_ptr<Objective> addObjective(std::string name, Criteria::Type criteria,
-                                            std::string display_name) override;
-    std::unique_ptr<Objective> addObjective(std::string name, Criteria::Type criteria, std::string display_name,
-                                            RenderType render_type) override;
+    Result<std::unique_ptr<Objective>> addObjective(std::string name, Criteria::Type criteria) override;
+    Result<std::unique_ptr<Objective>> addObjective(std::string name, Criteria::Type criteria,
+                                                    std::string display_name) override;
+    Result<std::unique_ptr<Objective>> addObjective(std::string name, Criteria::Type criteria, std::string display_name,
+                                                    RenderType render_type) override;
     [[nodiscard]] std::unique_ptr<Objective> getObjective(std::string name) const override;
     [[nodiscard]] std::unique_ptr<Objective> getObjective(DisplaySlot slot) const override;
     [[nodiscard]] std::vector<std::unique_ptr<Objective>> getObjectives() const override;

diff --git a/include/endstone/permissions/permissible.h b/include/endstone/permissions/permissible.h
@@ -18,6 +18,7 @@
 #include <unordered_set>
 
 #include "endstone/permissions/permission_attachment_info.h"
+#include "endstone/util/error.h"
 
 namespace endstone {
 

diff --git a/include/endstone/scoreboard/scoreboard.h b/include/endstone/scoreboard/scoreboard.h
@@ -20,6 +20,7 @@
 #include "endstone/scoreboard/display_slot.h"
 #include "endstone/scoreboard/objective.h"
 #include "endstone/scoreboard/score_entry.h"
+#include "endstone/util/result.h"
 
 namespace endstone {
 
@@ -35,22 +36,20 @@ class Scoreboard : public std::enable_shared_from_this<Scoreboard> {
      *
      * @param name Name of the Objective
      * @param criteria Criteria for the Objective
-     * @return A reference to the newly registered Objective,
-     * or <code>nullptr</code> if an objective by that name already exists.
+     * @return A reference to the newly registered Objective.
      */
-    virtual std::unique_ptr<Objective> addObjective(std::string name, Criteria::Type criteria) = 0;
+    virtual Result<std::unique_ptr<Objective>> addObjective(std::string name, Criteria::Type criteria) = 0;
 
     /**
      * @brief Registers an Objective on this Scoreboard
      *
      * @param name Name of the Objective
      * @param criteria Criteria type for the Objective
      * @param display_name Name displayed to players for the Objective.
-     * @return A reference to the newly registered Objective,
-     * or <code>nullptr</code> if an objective by that name already exists.
+     * @return A reference to the newly registered Objective.
      */
-    virtual std::unique_ptr<Objective> addObjective(std::string name, Criteria::Type criteria,
-                                                    std::string display_name) = 0;
+    virtual Result<std::unique_ptr<Objective>> addObjective(std::string name, Criteria::Type criteria,
+                                                            std::string display_name) = 0;
 
     /**
      * @brief Registers an Objective on this Scoreboard
@@ -59,11 +58,10 @@ class Scoreboard : public std::enable_shared_from_this<Scoreboard> {
      * @param criteria Criteria type for the Objective
      * @param display_name Name displayed to players for the Objective.
      * @param render_type Manner of rendering the Objective
-     * @return A reference to the newly registered Objective,
-     * or <code>nullptr</code> if an objective by that name already exists.
+     * @return A reference to the newly registered Objective.
      */
-    virtual std::unique_ptr<Objective> addObjective(std::string name, Criteria::Type criteria, std::string display_name,
-                                                    RenderType render_type) = 0;
+    virtual Result<std::unique_ptr<Objective>> addObjective(std::string name, Criteria::Type criteria,
+                                                            std::string display_name, RenderType render_type) = 0;
 
     /**
      * @brief Gets an Objective on this Scoreboard by name

diff --git a/src/endstone_core/scheduler/scheduler.cpp b/src/endstone_core/scheduler/scheduler.cpp
@@ -15,9 +15,25 @@
 #include "endstone/detail/scheduler/scheduler.h"
 
 #include "endstone/detail/scheduler/async_task.h"
+#include "endstone/detail/util/error.h"
 
 namespace endstone::detail {
 
+namespace {
+Result<void> validate(const Plugin &plugin, const std::function<void()> &task)
+{
+    if (!task) {
+        return nonstd::make_unexpected(make_error("Plugin {} attempted to register an empty task", plugin.getName()));
+    }
+
+    if (!plugin.isEnabled()) {
+        return nonstd::make_unexpected(
+            make_error("Plugin {} attempted to register task while disabled", plugin.getName()));
+    }
+    return {};
+}
+}  // namespace
+
 EndstoneScheduler::EndstoneScheduler(Server &server) : server_(server) {}
 
 std::shared_ptr<Task> EndstoneScheduler::runTask(Plugin &plugin, std::function<void()> task)
@@ -33,13 +49,7 @@ std::shared_ptr<Task> EndstoneScheduler::runTaskLater(Plugin &plugin, std::funct
 std::shared_ptr<Task> EndstoneScheduler::runTaskTimer(Plugin &plugin, std::function<void()> task, std::uint64_t delay,
                                                       std::uint64_t period)
 {
-    if (!task) {
-        server_.getLogger().error("Plugin {} attempted to register an empty task", plugin.getName());
-        return nullptr;
-    }
-
-    if (!plugin.isEnabled()) {
-        server_.getLogger().error("Plugin {} attempted to register task while disabled", plugin.getName());
+    if (!validate(plugin, task)) {
         return nullptr;
     }
 
@@ -63,13 +73,7 @@ std::shared_ptr<Task> EndstoneScheduler::runTaskLaterAsync(Plugin &plugin, std::
 std::shared_ptr<Task> EndstoneScheduler::runTaskTimerAsync(Plugin &plugin, std::function<void()> task,
                                                            std::uint64_t delay, std::uint64_t period)
 {
-    if (!task) {
-        server_.getLogger().error("Plugin {} attempted to register an empty task", plugin.getName());
-        return nullptr;
-    }
-
-    if (!plugin.isEnabled()) {
-        server_.getLogger().error("Plugin {} attempted to register task while disabled", plugin.getName());
+    if (!validate(plugin, task)) {
         return nullptr;
     }
 

diff --git a/src/endstone_core/scoreboard/objective.cpp b/src/endstone_core/scoreboard/objective.cpp
@@ -33,40 +33,31 @@ EndstoneObjective::EndstoneObjective(EndstoneScoreboard &scoreboard, ::Objective
 
 Result<std::string> EndstoneObjective::getName() const
 {
-    return checkState()
-        .and_then([](const auto *self) -> Result<std::string> { return self->objective_.getName(); })
-        .or_else([](const auto &err) -> Result<std::string> { return nonstd::make_unexpected(err); });
+    return checkState().and_then([](const auto *self) -> Result<std::string> { return self->objective_.getName(); });
 }
 
 Result<std::string> EndstoneObjective::getDisplayName() const
 {
-    return checkState()
-        .and_then([](const auto *self) -> Result<std::string> { return self->objective_.getDisplayName(); })
-        .or_else([](const auto &err) -> Result<std::string> { return nonstd::make_unexpected(err); });
+    return checkState().and_then(
+        [](const auto *self) -> Result<std::string> { return self->objective_.getDisplayName(); });
 }
 
 Result<void> EndstoneObjective::setDisplayName(std::string display_name)
 {
-    return checkState()
-        .and_then([&display_name](const auto *self) -> Result<void> {
-            self->objective_.setDisplayName(display_name);
-            return {};
-        })
-        .or_else([](const auto &err) -> Result<void> { return nonstd::make_unexpected(err); });
+    return checkState().and_then([&display_name](const auto *self) -> Result<void> {
+        self->objective_.setDisplayName(display_name);
+        return {};
+    });
 }
 
 Result<const Criteria *> EndstoneObjective::getCriteria() const
 {
-    return checkState()
-        .and_then([](const auto *self) -> Result<const Criteria *> { return &self->criteria_; })
-        .or_else([](const auto &err) -> Result<const Criteria *> { return nonstd::make_unexpected(err); });
+    return checkState().and_then([](const auto *self) -> Result<const Criteria *> { return &self->criteria_; });
 }
 
 Result<bool> EndstoneObjective::isModifiable() const
 {
-    return checkState()
-        .and_then([](const auto *self) -> Result<bool> { return !self->criteria_.isReadOnly(); })
-        .or_else([](const auto &err) -> Result<bool> { return nonstd::make_unexpected(err); });
+    return checkState().and_then([](const auto *self) -> Result<bool> { return !self->criteria_.isReadOnly(); });
 }
 
 Scoreboard &EndstoneObjective::getScoreboard() const
@@ -76,12 +67,10 @@ Scoreboard &EndstoneObjective::getScoreboard() const
 
 Result<void> EndstoneObjective::unregister() const
 {
-    return checkState()
-        .and_then([](const auto *self) -> Result<void> {
-            self->scoreboard_.board_.removeObjective(&self->objective_);
-            return {};
-        })
-        .or_else([](const auto &err) -> Result<void> { return nonstd::make_unexpected(err); });
+    return checkState().and_then([](const auto *self) -> Result<void> {
+        self->scoreboard_.board_.removeObjective(&self->objective_);
+        return {};
+    });
 }
 
 Result<bool> EndstoneObjective::isDisplayed() const
@@ -94,8 +83,7 @@ Result<bool> EndstoneObjective::isDisplayed() const
                }
                return true;
            })
-        .and_then([&displayed]() -> Result<bool> { return displayed; })
-        .or_else([](const auto &err) -> Result<bool> { return nonstd::make_unexpected(err); });
+        .and_then([&displayed]() -> Result<bool> { return displayed; });
 }
 
 Result<DisplaySlot> EndstoneObjective::getDisplaySlot() const
@@ -113,8 +101,7 @@ Result<DisplaySlot> EndstoneObjective::getDisplaySlot() const
                 return result.value();
             }
             return nonstd::make_unexpected(make_error("Object is not displayed."));
-        })
-        .or_else([](const auto &err) -> Result<DisplaySlot> { return nonstd::make_unexpected(err); });
+        });
 }
 
 Result<ObjectiveSortOrder> EndstoneObjective::getSortOrder() const
@@ -132,8 +119,7 @@ Result<ObjectiveSortOrder> EndstoneObjective::getSortOrder() const
                 return result.value();
             }
             return nonstd::make_unexpected(make_error("Object is not displayed."));
-        })
-        .or_else([](const auto &err) -> Result<ObjectiveSortOrder> { return nonstd::make_unexpected(err); });
+        });
 }
 
 Result<void> EndstoneObjective::setDisplay(std::optional<DisplaySlot> slot)
@@ -155,35 +141,28 @@ Result<void> EndstoneObjective::setDisplay(std::optional<DisplaySlot> slot, Obje
                                                        static_cast<::ObjectiveSortOrder>(order));
             }
             return {};
-        })
-        .or_else([](const auto &err) -> Result<void> { return nonstd::make_unexpected(err); });
+        });
 }
 
 Result<RenderType> EndstoneObjective::getRenderType() const
 {
-    return checkState()
-        .and_then([](const auto *self) -> Result<RenderType> {
-            return static_cast<RenderType>(self->objective_.getRenderType());
-        })
-        .or_else([](const auto &err) -> Result<RenderType> { return nonstd::make_unexpected(err); });
+    return checkState().and_then([](const auto *self) -> Result<RenderType> {
+        return static_cast<RenderType>(self->objective_.getRenderType());
+    });
 }
 
 Result<void> EndstoneObjective::setRenderType(RenderType render_type)
 {
-    return checkState()
-        .and_then([](const auto * /*self*/) -> Result<void> {
-            return nonstd::make_unexpected(make_error("setRenderType is not supported."));
-        })
-        .or_else([](const auto &err) -> Result<void> { return nonstd::make_unexpected(err); });
+    return checkState().and_then([](const auto * /*self*/) -> Result<void> {
+        return nonstd::make_unexpected(make_error("setRenderType is not supported."));
+    });
 }
 
 Result<std::unique_ptr<Score>> EndstoneObjective::getScore(ScoreEntry entry) const
 {
-    return checkState()
-        .and_then([entry](const auto *self) -> Result<std::unique_ptr<Score>> {
-            return std::make_unique<EndstoneScore>(self->copy(), entry);
-        })
-        .or_else([](const auto &err) -> Result<std::unique_ptr<Score>> { return nonstd::make_unexpected(err); });
+    return checkState().and_then([entry](const auto *self) -> Result<std::unique_ptr<Score>> {
+        return std::make_unique<EndstoneScore>(self->copy(), entry);
+    });
 }
 
 Result<const EndstoneObjective *> EndstoneObjective::checkState() const
@@ -197,19 +176,17 @@ Result<const EndstoneObjective *> EndstoneObjective::checkState() const
 Result<void> EndstoneObjective::forEachDisplayObjective(
     const std::function<bool(DisplaySlot, const DisplayObjective &)> &callback) const
 {
-    return checkState()
-        .and_then([&callback](const EndstoneObjective *self) -> Result<void> {
-            for (auto const &slot : magic_enum::enum_values<DisplaySlot>()) {
-                const auto slot_name = EndstoneScoreboard::getDisplaySlotName(slot);
-                if (const auto *display = self->scoreboard_.board_.getDisplayObjective(slot_name)) {
-                    if (!callback(slot, *display)) {
-                        return {};
-                    }
+    return checkState().and_then([&callback](const EndstoneObjective *self) -> Result<void> {
+        for (auto const &slot : magic_enum::enum_values<DisplaySlot>()) {
+            const auto slot_name = EndstoneScoreboard::getDisplaySlotName(slot);
+            if (const auto *display = self->scoreboard_.board_.getDisplayObjective(slot_name)) {
+                if (!callback(slot, *display)) {
+                    return {};
                 }
             }
-            return {};
-        })
-        .or_else([](const auto &err) -> Result<void> { return nonstd::make_unexpected(err); });
+        }
+        return {};
+    });
 }
 
 std::unique_ptr<EndstoneObjective> EndstoneObjective::copy() const

diff --git a/src/endstone_core/scoreboard/scoreboard.cpp b/src/endstone_core/scoreboard/scoreboard.cpp
@@ -26,6 +26,7 @@
 #include "endstone/detail/scoreboard/objective.h"
 #include "endstone/detail/scoreboard/score.h"
 #include "endstone/detail/server.h"
+#include "endstone/detail/util/error.h"
 
 namespace endstone::detail {
 
@@ -48,33 +49,29 @@ void EndstoneScoreboard::init()
     board_.setPacketSender(packet_sender_.get());
 }
 
-std::unique_ptr<Objective> EndstoneScoreboard::addObjective(std::string name, Criteria::Type criteria)
+Result<std::unique_ptr<Objective>> EndstoneScoreboard::addObjective(std::string name, Criteria::Type criteria)
 {
     return addObjective(name, criteria, name);
 }
 
-std::unique_ptr<Objective> EndstoneScoreboard::addObjective(std::string name, Criteria::Type criteria,
-                                                            std::string display_name)
+Result<std::unique_ptr<Objective>> EndstoneScoreboard::addObjective(std::string name, Criteria::Type criteria,
+                                                                    std::string display_name)
 {
     return addObjective(name, criteria, name, RenderType::Integer);
 }
 
-std::unique_ptr<Objective> EndstoneScoreboard::addObjective(std::string name, Criteria::Type criteria,
-                                                            std::string display_name, RenderType render_type)
+Result<std::unique_ptr<Objective>> EndstoneScoreboard::addObjective(std::string name, Criteria::Type criteria,
+                                                                    std::string display_name, RenderType render_type)
 {
-    auto &server = entt::locator<EndstoneServer>::value();
-    auto *cr = board_.getCriteria(getCriteriaName(criteria));
+    const auto *cr = board_.getCriteria(getCriteriaName(criteria));
     if (!cr) {
-        server.getLogger().error("Unknown criteria: {}.", getCriteriaName(criteria));
-        return nullptr;
+        throw std::runtime_error("getCriteria returns null!");
     }
 
     auto *objective = board_.addObjective(name, display_name, *cr);
     if (!objective) {
-        server.getLogger().error("Objective {} already exists.", name);
-        return nullptr;
+        return nonstd::make_unexpected(make_error("Objective {} already exists.", name));
     }
-
     return std::make_unique<EndstoneObjective>(*this, *objective);
 }