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

@@ -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<GameControllerEvent>))
 	{
 		luaL_error(L, "Invalid event type: %i", lua_tointeger(L, 1));
 	}
-	lsi->gameControllerEventHandlers[eventType].Push(L);
+	lsi->AddEventHandler(eventType, 2);
-	auto length = lua_objlen(L, -1);
-	lua_pushvalue(L, 2);
-	lua_rawseti(L, -2, length + 1);
 	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<GameControllerEvent>))
 	{
 		luaL_error(L, "Invalid event type: %i", lua_tointeger(L, 1));
 	}
-	lsi->gameControllerEventHandlers[eventType].Push(L);
+	lsi->RemoveEventHandler(eventType, 2);
-	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);
-	}
 	return 0;
 }
 

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<LuaScriptInterface *>(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 = &it;
 	bool cont = true;
-	lsi->gameControllerEventHandlers[event.index()].Push(L);
+	while (it != end)
-	int len = lua_objlen(L, -1);
-	for (int i = 1; i <= len && cont; i++)
 	{
-		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")
+			auto err = LuaGetError();
-			{
-				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());
 			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<size_t Index>
-std::enable_if_t<Index != std::variant_size_v<GameControllerEvent>, bool> HaveSimGraphicsEventHandlersHelper(lua_State *L, std::vector<LuaSmartRef> &gameControllerEventHandlers)
+std::enable_if_t<Index != std::variant_size_v<GameControllerEvent>, bool> HaveSimGraphicsEventHandlersHelper(const auto &gameControllerEventHandlers)
 {
 	if (std::variant_alternative_t<Index, GameControllerEvent>::traits & eventTraitHindersSrt)
 	{
-		gameControllerEventHandlers[Index].Push(L);
+		if (!gameControllerEventHandlers[Index].empty())
-		auto have = lua_objlen(L, -1) > 0;
-		lua_pop(L, 1);
-		if (have)
 		{
 			return true;
 		}
 	}
-	return HaveSimGraphicsEventHandlersHelper<Index + 1>(L, gameControllerEventHandlers);
+	return HaveSimGraphicsEventHandlersHelper<Index + 1>(gameControllerEventHandlers);
 }
 
 template<size_t Index>
-std::enable_if_t<Index == std::variant_size_v<GameControllerEvent>, bool> HaveSimGraphicsEventHandlersHelper(lua_State *L, std::vector<LuaSmartRef> &gameControllerEventHandlers)
+std::enable_if_t<Index == std::variant_size_v<GameControllerEvent>, 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()

src/lua/LuaScriptInterface.h

@@ -8,6 +8,7 @@
 #include <cstdint>
 #include <map>
 #include <memory>
+#include <list>
 
 namespace http
 {
@@ -105,7 +106,13 @@ public:
 	int textInputRefcount = 0;
 	long unsigned int luaExecutionStart = 0;
 
-	std::vector<LuaSmartRef> gameControllerEventHandlers; // must come after luaState
+private:
+	std::vector<std::list<LuaSmartRef>> gameControllerEventHandlers; // must come after luaState
+	std::list<LuaSmartRef>::iterator *currentEventHandlerIt = nullptr;
+
+public:
+	void AddEventHandler(int eventType, int stackIndex);
+	void RemoveEventHandler(int eventType, int stackIndex);
 	std::unique_ptr<http::Request> 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);