refactor: optimize ItemAPI

fix: fix potential memory leak in `_extractValue(RemoteCall::ItemType&& v)` of RemoteCallAPI

Author: ShrBox

src/legacy/api/CommandAPI.cpp

@@ -93,13 +93,10 @@ Local<Value> convertResult(ParamStorageType const& result, CommandOrigin const&
         );
     } else if (result.hold(ParamKind::Kind::Item)) {
         return ItemClass::newItem(
-            new ItemStack(
-                ll::service::getLevel()->getItemRegistry().getNameFromLegacyID(
-                    std::get<CommandItem>(result.value()).mId
-                )
-            ),
-            false
-        ); // Not managed by BDS, pointer will be saved as unique_ptr
+            std::make_unique<ItemStack>(ll::service::getLevel()->getItemRegistry().getNameFromLegacyID(
+                std::get<CommandItem>(result.value()).mId
+            ))
+        );
     } else if (result.hold(ParamKind::Kind::Actor)) {
         auto arr = Array::newArray();
         for (auto i : std::get<CommandSelector<Actor>>(result.value()).results(origin)) {

src/legacy/api/ItemAPI.cpp

@@ -73,19 +73,20 @@ ClassDefine<ItemClass> ItemClassBuilder = defineClass<ItemClass>("LLSE_Item")
 
 //////////////////// Classes ////////////////////
 
-ItemClass::ItemClass(ItemStack* itemStack, bool isManagedByBDS)
+ItemClass::ItemClass(std::variant<std::monostate, std::unique_ptr<ItemStack>, ItemStack*> itemStack)
 : ScriptClass(ScriptClass::ConstructFromCpp<ItemClass>{}) {
-    if (isManagedByBDS) {
-        item = itemStack;
-    } else {
-        item = std::unique_ptr<ItemStack>(itemStack);
-    }
+    item = std::move(itemStack);
     preloadData();
 }
 
 // 生成函数
-Local<Object> ItemClass::newItem(ItemStack* itemStack, bool isManagedByBDS) {
-    auto newp = new ItemClass(itemStack, isManagedByBDS);
+Local<Object> ItemClass::newItem(ItemStack* itemStack) {
+    auto newp = new ItemClass(itemStack);
+    return newp->getScriptObject();
+}
+
+Local<Object> ItemClass::newItem(std::unique_ptr<ItemStack> itemStack) {
+    auto newp = new ItemClass(std::move(itemStack));
     return newp->getScriptObject();
 }
 
@@ -358,9 +359,8 @@ Local<Value> ItemClass::set(const Arguments& args) {
 Local<Value> ItemClass::clone(const Arguments&) {
     try {
         auto itemStack = get();
-        if (!itemStack) return Local<Value>(); // Null
-        auto itemNew = new ItemStack(*itemStack);
-        return ItemClass::newItem(itemNew, false);
+        if (!itemStack) return Local<Value>();
+        return ItemClass::newItem(std::make_unique<ItemStack>(*itemStack));
     }
     CATCH("Fail in cloneItem!");
 }
@@ -469,20 +469,17 @@ Local<Value> McClass::newItem(const Arguments& args) {
                 std::string type = args[0].asString().toString();
                 int         cnt  = args[1].asNumber().toInt32();
 
-                ItemStack* item = new ItemStack{type, cnt, 0, nullptr};
-                if (!item) return Local<Value>();            // Null
-                else return ItemClass::newItem(item, false); // Not managed by BDS, pointer will be saved as unique_ptr
+                return ItemClass::newItem(std::make_unique<ItemStack>(type, cnt, 0, nullptr));
             } else {
                 LOG_TOO_FEW_ARGS(__FUNCTION__);
                 return Local<Value>();
             }
         } else {
             auto nbt = NbtCompoundClass::extract(args[0]);
             if (nbt) {
-                auto newItem = new ItemStack{ItemStack::EMPTY_ITEM()};
+                auto newItem = std::make_unique<ItemStack>(ItemStack::EMPTY_ITEM());
                 ItemHelper::load(*newItem, *nbt);
-                return ItemClass::newItem(newItem,
-                                          false); // Not managed by BDS, pointer will be saved as unique_ptr
+                return ItemClass::newItem(std::move(newItem));
             } else {
                 LOG_WRONG_ARG_TYPE(__FUNCTION__);
                 return Local<Value>();

src/legacy/api/ItemAPI.h

@@ -10,26 +10,26 @@ class ItemStack;
 
 class ItemClass : public ScriptClass {
 private:
-    // ItemStack* is managed by BDS// ItemStack* is managed by BDS
-    std::variant<ItemStack*, std::unique_ptr<ItemStack>> item;
+    // ItemStack* is managed by BDS
+    std::variant<std::monostate, std::unique_ptr<ItemStack>, ItemStack*> item;
 
     // Pre data
     std::string name, type;
     int         id, maxCount, count, aux;
 
 public:
-    explicit ItemClass(ItemStack* itemStack, bool isManagedByBDS = true);
+    explicit ItemClass(std::variant<std::monostate, std::unique_ptr<ItemStack>, ItemStack*> itemStack);
     void preloadData();
 
     ItemStack* get() {
-        if (std::holds_alternative<std::unique_ptr<ItemStack>>(item)) {
+        if (std::holds_alternative<std::monostate>(item)) return nullptr;
+        if (std::holds_alternative<std::unique_ptr<ItemStack>>(item))
             return std::get<std::unique_ptr<ItemStack>>(item).get();
-        } else {
-            return std::get<ItemStack*>(item);
-        }
+        return std::get<ItemStack*>(item);
     }
 
-    static Local<Object> newItem(ItemStack* itemStack, bool isManagedByBDS = true);
+    static Local<Object> newItem(ItemStack* itemStack);
+    static Local<Object> newItem(std::unique_ptr<ItemStack> itemStack);
     static ItemStack*    extract(Local<Value> v);
 
     Local<Value> getName();

src/legacy/api/RemoteCallAPI.cpp

@@ -111,7 +111,7 @@ Local<Value> _extractValue(RemoteCall::BlockType v) {
 };
 Local<Value> _extractValue(RemoteCall::NumberType v) { return Number::newNumber(v.get<double>()); };
 Local<Value> _extractValue(RemoteCall::ItemType&& v) {
-    if (v.own) return ItemClass::newItem(v.tryGetUniquePtr().release());
+    if (v.own) return ItemClass::newItem(std::move(v.tryGetUniquePtr()));
     else return ItemClass::newItem(const_cast<ItemStack*>(v.ptr));
 };
 Local<Value> _extractValue(RemoteCall::NbtType&& v) {