fix: fix ll.onUnload #303

Author: xiaoqch

src/legacy/api/EventAPI.cpp

@@ -40,6 +40,7 @@
 #include "ll/api/event/world/FireSpreadEvent.h"
 #include "ll/api/event/world/SpawnMobEvent.h"
 #include "ll/api/service/Bedrock.h"
+#include "ll/api/service/GamingStatus.h"
 #include "ll/api/thread/ServerThreadExecutor.h"
 #include "lse/Entry.h"
 #include "lse/events/BlockEvents.h"
@@ -133,13 +134,19 @@ bool LLSECallEventsOnHotLoad(ScriptEngine* engine) {
     return true;
 }
 
-bool LLSECallEventsOnHotUnload(ScriptEngine* engine) {
-    ll::service::getLevel()->forEachPlayer([&](Player& pl) -> bool {
-        FakeCallEvent(engine, EVENT_TYPES::onLeft, PlayerClass::newPlayer(&pl));
-        return true;
-    });
+bool LLSECallEventsOnUnload(ScriptEngine* engine) {
+    if (ll::getGamingStatus() == ll::GamingStatus::Running) {
+        ll::service::getLevel()->forEachPlayer([&](Player& pl) -> bool {
+            FakeCallEvent(engine, EVENT_TYPES::onLeft, PlayerClass::newPlayer(&pl));
+            return true;
+        });
+    }
     for (auto& [index, cb] : getEngineData(engine)->unloadCallbacks) {
-        cb(engine);
+        EngineScope scope(engine);
+        try {
+            cb(engine);
+        }
+        CATCH_IN_CALLBACK("onUnload")
     }
     getEngineData(engine)->unloadCallbacks.clear();
     return true;

src/legacy/api/EventAPI.h

@@ -10,7 +10,7 @@ void EnableEventListener(int eventId);
 bool LLSEAddEventListener(ScriptEngine* engine, const std::string& eventName, const Local<Function>& func);
 bool LLSERemoveAllEventListeners(ScriptEngine* engine);
 bool LLSECallEventsOnHotLoad(ScriptEngine* engine);
-bool LLSECallEventsOnHotUnload(ScriptEngine* engine);
+bool LLSECallEventsOnUnload(ScriptEngine* engine);
 
 //////////////////// Callback ////////////////////
 

src/legacy/engine/EngineOwnData.h

@@ -1,16 +1,12 @@
 #pragma once
 #include "legacy/main/Global.h"
-#include "ll/api/base/Macro.h"
-#include "ll/api/mod/Mod.h"
-#include "lse/Entry.h"
 #include "lse/Plugin.h"
 #include "utils/UsingScriptX.h"
 
 #include <ll/api/Expected.h>
 #include <ll/api/i18n/I18n.h>
 #include <ll/api/io/Logger.h>
 #include <ll/api/io/LoggerRegistry.h>
-#include <map>
 #include <string>
 #include <unordered_map>
 
@@ -25,15 +21,6 @@ struct RemoteCallData {
     script::Global<Function> callback;
 };
 
-/*
-struct SimpleCallbackData
-{
-    ScriptEngine* engine;
-    script::Global<script::Function> func;
-    std::vector<script::Global<Value>> values;
-};
-*/
-
 // It is similar to ll::mod::Mod, it stores data of an engine(usually a plugin).
 struct EngineOwnData {
     // Basic information
@@ -43,28 +30,6 @@ struct EngineOwnData {
     // RemoteCall Exported Functions: unordered_map<nameSpace, funcName>
     std::unordered_map<std::string, RemoteCallData> exportFuncs;
 
-    /*
-    uint64_t simpleCallbackIndex = 0;
-    std::unordered_map<uint64_t, SimpleCallbackData> simpleCallbacks;
-
-    inline uint64_t addSimpleCallback(script::Local<Function> func,
-    std::vector<script::Local<Value>> values)
-    {
-        auto index = ++simpleCallbackIndex;
-        std::vector<script::Global<Value>> globalValues;
-        for (auto& value : values)
-            globalValues.emplace_back(value);
-        SimpleCallbackData data{EngineScope::currentEngine(),
-    script::Global<Function>(func), std::move(globalValues)};
-        simpleCallbacks.emplace(index, std::move(data));
-        return index;
-    }
-    inline bool removeSimpleCallback(uint64_t index)
-    {
-        return simpleCallbacks.erase(index);
-    }
-    */
-
     // I18nAPI
     ll::i18n::I18n i18n;
     std::string    defaultLocaleName;
@@ -90,7 +55,9 @@ struct EngineOwnData {
         EngineScope scope(engine);
         auto        data = std::static_pointer_cast<EngineOwnData>(engine->getData());
         data->playerDataDB.clear();
+        data->unloadCallbacks.clear();
         // LLSERemoveAllExportedFuncs(engine);
+        assert(data->exportFuncs.empty());
     }
 };
 

src/lse/PluginManager.cpp

@@ -6,7 +6,7 @@
 #include "legacy/engine/EngineOwnData.h"
 #include "ll/api/chrono/GameChrono.h"
 #include "ll/api/coro/CoroTask.h"
-#include "ll/api/io/FileUtils.h"
+#include "ll/api/io/FileUtils.h" // IWYU pragma: keep
 #include "ll/api/mod/Mod.h"
 #include "ll/api/mod/ModManager.h"
 #include "ll/api/service/GamingStatus.h"
@@ -56,14 +56,15 @@ bool LLSERemoveCmdRegister(script::ScriptEngine* engine);
 bool LLSERemoveCmdCallback(script::ScriptEngine* engine);
 bool LLSERemoveAllExportedFuncs(script::ScriptEngine* engine);
 bool LLSECallEventsOnHotLoad(ScriptEngine* engine);
-bool LLSECallEventsOnHotUnload(ScriptEngine* engine);
+bool LLSECallEventsOnUnload(ScriptEngine* engine);
 
 namespace lse {
 
 PluginManager::PluginManager() : ll::mod::ModManager(PluginManagerName) {}
 PluginManager::~PluginManager() = default;
 
 ll::Expected<> PluginManager::load(ll::mod::Manifest manifest) {
+    auto& logger = lse::LegacyScriptEngine::getInstance().getSelf().getLogger();
 #ifdef LEGACY_SCRIPT_ENGINE_BACKEND_PYTHON
     std::filesystem::path dirPath = ll::mod::getModsRoot() / manifest.name; // Plugin path
     std::string           entryPath =
@@ -78,22 +79,17 @@ ll::Expected<> PluginManager::load(ll::mod::Manifest manifest) {
             PythonHelper::getPluginPackDependencyFilePath(ll::string_utils::u8str2str(dirPath.u8string()));
         if (!dependTmpFilePath.empty()) {
             int exitCode = 0;
-            lse::LegacyScriptEngine::getInstance().getSelf().getLogger().info(
-                "Executing \"pip install\" for plugin {name}..."_tr(
-                    fmt::arg("name", ll::string_utils::u8str2str(dirPath.filename().u8string()))
-                )
-            );
+            logger.info("Executing \"pip install\" for plugin {name}..."_tr(
+                fmt::arg("name", ll::string_utils::u8str2str(dirPath.filename().u8string()))
+            ));
 
             if ((exitCode = PythonHelper::executePipCommand(
                      "pip install -r \"" + dependTmpFilePath + "\" -t \""
                      + ll::string_utils::u8str2str(realPackageInstallDir.u8string()) + "\" --disable-pip-version-check "
                  ))
                 == 0) {
-                lse::LegacyScriptEngine::getInstance().getSelf().getLogger().info("Pip finished successfully."_tr());
-            } else
-                lse::LegacyScriptEngine::getInstance().getSelf().getLogger().error(
-                    "Error occurred. Exit code: {code}"_tr(fmt::arg("code", exitCode))
-                );
+                logger.info("Pip finished successfully."_tr());
+            } else logger.error("Error occurred. Exit code: {code}"_tr(fmt::arg("code", exitCode)));
 
             // remove temp dependency file after installation
             std::error_code ec;
@@ -111,19 +107,15 @@ ll::Expected<> PluginManager::load(ll::mod::Manifest manifest) {
     if (NodeJsHelper::doesPluginPackHasDependency(ll::string_utils::u8str2str(dirPath.u8string()))
         && !std::filesystem::exists(std::filesystem::path(dirPath) / "node_modules")) {
         int exitCode = 0;
-        lse::LegacyScriptEngine::getInstance().getSelf().getLogger().info(
-            "Executing \"npm install\" for plugin {name}..."_tr(
-                fmt::arg("name", ll::string_utils::u8str2str(dirPath.filename().u8string()))
-            )
-        );
+        logger.info("Executing \"npm install\" for plugin {name}..."_tr(
+            fmt::arg("name", ll::string_utils::u8str2str(dirPath.filename().u8string()))
+        ));
         if ((exitCode = NodeJsHelper::executeNpmCommand(
                  {"install", "--omit=dev", "--no-fund"},
                  ll::string_utils::u8str2str(dirPath.u8string())
              ))
             != 0) {
-            lse::LegacyScriptEngine::getInstance().getSelf().getLogger().error(
-                "Error occurred. Exit code: {code}"_tr(fmt::arg("code", exitCode))
-            );
+            logger.error("Error occurred. Exit code: {code}"_tr(fmt::arg("code", exitCode)));
         }
     }
 #endif
@@ -260,9 +252,7 @@ ll::Expected<> PluginManager::unload(std::string_view name) {
 #ifndef LEGACY_SCRIPT_ENGINE_BACKEND_NODEJS
         LLSERemoveTimeTaskData(scriptEngine);
 #endif
-        if (ll::getGamingStatus() == ll::GamingStatus::Running) {
-            LLSECallEventsOnHotUnload(scriptEngine);
-        }
+        LLSECallEventsOnUnload(scriptEngine);
         LLSERemoveAllEventListeners(scriptEngine);
         LLSERemoveCmdRegister(scriptEngine);
         LLSERemoveCmdCallback(scriptEngine);
@@ -296,6 +286,8 @@ ll::Expected<> PluginManager::unload(std::string_view name) {
         }
 
         return {};
+    } catch (const script::Exception& e) {
+        return ll::makeStringError("Failed to unload plugin {0}: {1}"_tr(name, "Unknown script exception"));
     } catch (const std::exception& e) {
         return ll::makeStringError("Failed to unload plugin {0}: {1}"_tr(name, e.what()));
     }