From 87b81ceb45fe26cf61ebd131f369f50e5323a075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20B=C3=A1lint=20Misius?= Date: Sat, 3 May 2025 16:42:53 +0200 Subject: [PATCH] Simplify event handler management Semantics are mostly the same: - event.register pushes a handler at the end of the handler chain, such that it has the lowest priority - event.unregister removes at most one handler, which is the first one that matches - a handler failing causes it to be removed from the chain But removing handlers from the chain doesn't mess up handler order for the frame anymore: until this commit, removing a handler from the chain would shift all handlers in the chain while it was being traversed, which meant that some handlers were always skipped whenever any were removed. Validate this with the following script: local t = {} for i = 1, 10 do table.insert(t, i) end for i = #t - 1, 1, -1 do local j = math.random(1, i) t[i], t[j] = t[j], t[i] end print(table.concat(t, " ")) event.register(event.MOUSEDOWN, function() print("=====") end) for i = 1, #t do local c = t[i] local function f() print(i) c = c - 1 if c == 0 then event.unregister(event.MOUSEDOWN, f) end end event.register(event.MOUSEDOWN, f) end The expected behaviour is that numbered event handlers are removed once they've been run as many times as the correspondingly numbered entry in t dictates. This, among other things, means that exactly one event handler is removed every MOUSEDOWN. This was absolutely not the case until now; removals were all over the place. --- src/lua/LuaEvent.cpp | 24 ++-------- src/lua/LuaScriptInterface.cpp | 87 +++++++++++++++++++++------------- src/lua/LuaScriptInterface.h | 11 ++++- 3 files changed, 69 insertions(+), 53 deletions(-) diff --git a/src/lua/LuaEvent.cpp b/src/lua/LuaEvent.cpp index c0ea3acb6..a72d80ad0 100644 --- a/src/lua/LuaEvent.cpp +++ b/src/lua/LuaEvent.cpp @@ -8,14 +8,11 @@ static int fregister(lua_State *L) lsi->AssertInterfaceEvent(); int eventType = luaL_checkinteger(L, 1); luaL_checktype(L, 2, LUA_TFUNCTION); - if (eventType < 0 || eventType >= int(lsi->gameControllerEventHandlers.size())) + if (eventType < 0 || eventType >= int(std::variant_size_v)) { luaL_error(L, "Invalid event type: %i", lua_tointeger(L, 1)); } - lsi->gameControllerEventHandlers[eventType].Push(L); - auto length = lua_objlen(L, -1); - lua_pushvalue(L, 2); - lua_rawseti(L, -2, length + 1); + lsi->AddEventHandler(eventType, 2); lua_pushvalue(L, 2); return 1; } @@ -26,24 +23,11 @@ static int unregister(lua_State *L) lsi->AssertInterfaceEvent(); int eventType = luaL_checkinteger(L, 1); luaL_checktype(L, 2, LUA_TFUNCTION); - if (eventType < 0 || eventType >= int(lsi->gameControllerEventHandlers.size())) + if (eventType < 0 || eventType >= int(std::variant_size_v)) { luaL_error(L, "Invalid event type: %i", lua_tointeger(L, 1)); } - lsi->gameControllerEventHandlers[eventType].Push(L); - auto length = lua_objlen(L, -1); - int skip = 0; - for (auto i = 1U; i <= length; ++i) - { - lua_rawgeti(L, -1, i); - if (!skip && lua_equal(L, -1, 2)) - { - skip = 1; - } - lua_pop(L, 1); - lua_rawgeti(L, -1, i + skip); - lua_rawseti(L, -2, i); - } + lsi->RemoveEventHandler(eventType, 2); return 0; } diff --git a/src/lua/LuaScriptInterface.cpp b/src/lua/LuaScriptInterface.cpp index 20afbddd6..3bddebbfc 100644 --- a/src/lua/LuaScriptInterface.cpp +++ b/src/lua/LuaScriptInterface.cpp @@ -170,12 +170,6 @@ LuaScriptInterface::LuaScriptInterface(GameController *newGameController, GameMo lua_setfield(L, -2, "randomseed"); lua_pop(L, 1); } - for (auto &ref : gameControllerEventHandlers) - { - lua_newtable(L); - ref.Assign(L, -1); - lua_pop(L, 1); - } auto compatSpan = compat_lua.AsCharSpan(); if (luaL_loadbuffer(L, compatSpan.data(), compatSpan.size(), "@[built-in compat.lua]") || tpt_lua_pcall(L, 0, 0, 0, eventTraitNone)) { @@ -411,62 +405,91 @@ bool CommandInterface::HandleEvent(const GameControllerEvent &event) { auto *lsi = static_cast(this); auto *L = lsi->L; + assert(!lsi->currentEventHandlerIt); + auto eventType = int(event.index()); + auto &list = lsi->gameControllerEventHandlers[eventType]; + auto it = list.begin(); + auto end = list.end(); + lsi->currentEventHandlerIt = ⁢ bool cont = true; - lsi->gameControllerEventHandlers[event.index()].Push(L); - int len = lua_objlen(L, -1); - for (int i = 1; i <= len && cont; i++) + while (it != end) { - lua_rawgeti(L, -1, i); + it->Push(L); + ++it; + lua_pushvalue(L, 1); int numArgs = pushGameControllerEvent(L, event); int callret = tpt_lua_pcall(L, numArgs, 1, 0, std::visit([](auto &event) { return event.traits; }, event)); if (callret) { - if (LuaGetError() == "Error: Script not responding") - { - for (int j = i; j <= len - 1; j++) - { - lua_rawgeti(L, -2, j + 1); - lua_rawseti(L, -3, j); - } - lua_pushnil(L); - lua_rawseti(L, -3, len); - i--; - } - Log(CommandInterface::LogError, LuaGetError()); + auto err = LuaGetError(); lua_pop(L, 1); + Log(CommandInterface::LogError, err); + if (err == "Error: Script not responding") + { + lsi->RemoveEventHandler(eventType, -1); + } } else { if (!lua_isnoneornil(L, -1)) + { cont = lua_toboolean(L, -1); + } lua_pop(L, 1); } - len = lua_objlen(L, -1); + lua_pop(L, 1); } - lua_pop(L, 1); + lsi->currentEventHandlerIt = nullptr; return cont; } +void LuaScriptInterface::AddEventHandler(int eventType, int stackIndex) +{ + gameControllerEventHandlers[eventType].emplace_back().Assign(L, stackIndex); +} + +void LuaScriptInterface::RemoveEventHandler(int eventType, int stackIndex) +{ + auto &list = gameControllerEventHandlers[eventType]; + auto it = list.begin(); + auto end = list.end(); + lua_pushvalue(L, stackIndex); + while (it != end) + { + it->Push(L); + auto equal = lua_equal(L, -1, -2); + lua_pop(L, 1); + if (equal) + { + if (currentEventHandlerIt && *currentEventHandlerIt == it) + { + ++*currentEventHandlerIt; + } + list.erase(it); + break; + } + ++it; + } + lua_pop(L, 1); +} + template -std::enable_if_t, bool> HaveSimGraphicsEventHandlersHelper(lua_State *L, std::vector &gameControllerEventHandlers) +std::enable_if_t, bool> HaveSimGraphicsEventHandlersHelper(const auto &gameControllerEventHandlers) { if (std::variant_alternative_t::traits & eventTraitHindersSrt) { - gameControllerEventHandlers[Index].Push(L); - auto have = lua_objlen(L, -1) > 0; - lua_pop(L, 1); - if (have) + if (!gameControllerEventHandlers[Index].empty()) { return true; } } - return HaveSimGraphicsEventHandlersHelper(L, gameControllerEventHandlers); + return HaveSimGraphicsEventHandlersHelper(gameControllerEventHandlers); } template -std::enable_if_t, bool> HaveSimGraphicsEventHandlersHelper(lua_State *L, std::vector &gameControllerEventHandlers) +std::enable_if_t, bool> HaveSimGraphicsEventHandlersHelper(const auto &gameControllerEventHandlers) { return false; } @@ -482,7 +505,7 @@ bool CommandInterface::HaveSimGraphicsEventHandlers() return true; } } - return HaveSimGraphicsEventHandlersHelper<0>(lsi->L, lsi->gameControllerEventHandlers); + return HaveSimGraphicsEventHandlersHelper<0>(lsi->gameControllerEventHandlers); } void CommandInterface::OnTick() diff --git a/src/lua/LuaScriptInterface.h b/src/lua/LuaScriptInterface.h index 96fc4df73..5d78522b7 100644 --- a/src/lua/LuaScriptInterface.h +++ b/src/lua/LuaScriptInterface.h @@ -8,6 +8,7 @@ #include #include #include +#include namespace http { @@ -105,7 +106,13 @@ public: int textInputRefcount = 0; long unsigned int luaExecutionStart = 0; - std::vector gameControllerEventHandlers; // must come after luaState +private: + std::vector> gameControllerEventHandlers; // must come after luaState + std::list::iterator *currentEventHandlerIt = nullptr; + +public: + void AddEventHandler(int eventType, int stackIndex); + void RemoveEventHandler(int eventType, int stackIndex); std::unique_ptr scriptManagerDownload; int luaHookTimeout; @@ -121,6 +128,8 @@ public: int Autorun(); void AssertInterfaceEvent(); + + friend class CommandInterface; }; void tpt_lua_pushByteString(lua_State *L, const ByteString &str);