[MIRROR] Fixes SS13.register_signal throwing unclear errors when called on deleted datums (#27293)

* Cleans up the SS13_base lua file and adds a new lua file for easily handling multiple signals on different objects. (#82458)

## About The Pull Request
Cleaned up the SS13.register_signal and SS13.unregister_signal, removing
the weird list shifting.
Also adds a new lua file that can be included for the use of registering
different signals on various datums and being able to clear them all in
1 function.
Removed the make_easy_clear_function option when registering a signal
via lua because I don't think it's used by anyone and it lacks any sort
of versatility. Users can just create their own function for clearing
signals from a datum.

Also updates the documentation for HARDDELETES.md as
COMSIG_PARENT_QDELETING was renamed to COMSIG_QDELETING

## Why It's Good For The Game
New handler file makes registering signals in batches a lot easier if
you want to clear them in one go without clearing unrelated callbacks on
the same datum. The list shifting in SS13.register_signal had pretty
significant performance problems, so removing that will make registering
and unregistering signals faster.

## Changelog
🆑
admin: LUA - Adds a new library called handler_group. Include it in your
files by doing require('handler_group')
/🆑

---------

Co-authored-by: Watermelon914 <3052169-Watermelon914@users.noreply.gitlab.com>

* Fixes SS13.register_signal throwing unclear errors when called on deleted datums (#82597)

## About The Pull Request
See title

## Why It's Good For The Game
More descriptive error message

## Changelog
🆑
fix: LUA: Registering a signal on a deleted datum will throw a more
descriptive error message.
/🆑

---------

Co-authored-by: Watermelon914 <3052169-Watermelon914@users.noreply.gitlab.com>

---------

Co-authored-by: Watermelon914 <37270891+Watermelon914@users.noreply.github.com>
Co-authored-by: Watermelon914 <3052169-Watermelon914@users.noreply.gitlab.com>

code/modules/admin/verbs/lua/lua_state.dm

@@ -109,14 +109,7 @@ GLOBAL_PROTECT(lua_usr)
 	if(islist(function))
 		var/list/new_function_path = list()
 		for(var/path_element in function)
-			if(isweakref(path_element))
-				var/datum/weakref/weak_ref = path_element
-				var/resolved = weak_ref.hard_resolve()
-				if(!resolved)
-					return list("status" = "errored", "param" = "Weakref in function path ([weak_ref] [text_ref(weak_ref)]) resolved to null.", "name" = jointext(function, "."))
-				new_function_path += resolved
-			else
-				new_function_path += path_element
+			new_function_path += path_element
 		function = new_function_path
 	var/msg = "[key_name(usr)] called the lua function \"[function]\" with arguments: [english_list(call_args)]"
 	log_lua(msg)

lua/SS13_base.lua

@@ -1,3 +1,6 @@
+local timer = require("timer")
+local state = require("state")
+
 local SS13 = {}
 
 __SS13_signal_handlers = __SS13_signal_handlers or {}
@@ -6,12 +9,7 @@ SS13.SSlua = dm.global_vars.vars.SSlua
 
 SS13.global_proc = "some_magic_bullshit"
 
-for _, state in SS13.SSlua.vars.states do
-	if state.vars.internal_id == dm.state_id then
-		SS13.state = state
-		break
-	end
-end
+SS13.state = state.state
 
 function SS13.get_runner_ckey()
 	return SS13.state:get_var("ckey_last_runner")
@@ -25,12 +23,16 @@ function SS13.istype(thing, type)
 	return dm.global_proc("_istype", thing, dm.global_proc("_text2path", type)) == 1
 end
 
+function SS13.start_tracking(datum)
+	local references = SS13.state.vars.references
+	references:add(datum)
+	SS13.state:call_proc("clear_on_delete", datum)
+end
+
 function SS13.new(type, ...)
-	local datum = SS13.new_untracked(type, table.unpack({...}))
+	local datum = SS13.new_untracked(type, ...)
 	if datum then
-		local references = SS13.state.vars.references
-		references:add(datum)
-		SS13.state:call_proc("clear_on_delete", datum)
+		SS13.start_tracking(datum)
 		return datum
 	end
 end
@@ -75,58 +77,44 @@ function SS13.await(thing_to_call, proc_to_call, ...)
 	return return_value, runtime_message
 end
 
-function SS13.register_signal(datum, signal, func, make_easy_clear_function)
+function SS13.register_signal(datum, signal, func)
 	if not SS13.istype(datum, "/datum") then
 		return
 	end
-	if not __SS13_signal_handlers[datum] then
-		__SS13_signal_handlers[datum] = {}
+	if not SS13.is_valid(datum) then
+		error("Tried to register a signal on a deleted datum!", 2)
+		return
+	end
+	local datumWeakRef = dm.global_proc("WEAKREF", datum)
+	if not __SS13_signal_handlers[datumWeakRef] then
+		__SS13_signal_handlers[datumWeakRef] = {}
 	end
 	if signal == "_cleanup" then
 		return
 	end
-	if not __SS13_signal_handlers[datum][signal] then
-		__SS13_signal_handlers[datum][signal] = {}
+	if not __SS13_signal_handlers[datumWeakRef][signal] then
+		__SS13_signal_handlers[datumWeakRef][signal] = {}
 	end
 	local callback = SS13.new("/datum/callback", SS13.state, "call_function_return_first")
+	local callbackWeakRef = dm.global_proc("WEAKREF", callback)
 	callback:call_proc("RegisterSignal", datum, signal, "Invoke")
-	local path = { "__SS13_signal_handlers", dm.global_proc("WEAKREF", datum), signal, dm.global_proc("WEAKREF", callback), "func" }
+	local path = { "__SS13_signal_handlers", datumWeakRef, signal, callbackWeakRef, "func" }
 	callback.vars.arguments = { path }
-	if not __SS13_signal_handlers[datum]["_cleanup"] then
-		local cleanup_path = { "__SS13_signal_handlers", dm.global_proc("WEAKREF", datum), "_cleanup", "func" }
-		local cleanup_callback = SS13.new("/datum/callback", SS13.state, "call_function_return_first", cleanup_path)
-		cleanup_callback:call_proc("RegisterSignal", datum, "parent_qdeleting", "Invoke")
-		__SS13_signal_handlers[datum]["_cleanup"] = {
-			func = function(datum)
-				SS13.signal_handler_cleanup(datum)
-				SS13.stop_tracking(cleanup_callback)
-			end,
-			callback = cleanup_callback,
-		}
-	end
-	if signal == "parent_qdeleting" then --We want to make sure that the cleanup function is the very last signal handler called.
-		local comp_lookup = datum.vars._listen_lookup
-		if comp_lookup then
-			local lookup_for_signal = comp_lookup.entries.parent_qdeleting
-			if lookup_for_signal and not SS13.istype(lookup_for_signal, "/datum") then
-				local cleanup_callback_index =
-					dm.global_proc("_list_find", lookup_for_signal, __SS13_signal_handlers[datum]["_cleanup"].callback)
-				if cleanup_callback_index ~= 0 and cleanup_callback_index ~= #comp_lookup then
-					dm.global_proc("_list_swap", lookup_for_signal, cleanup_callback_index, #lookup_for_signal)
-				end
-			end
-		end
-	end
-	__SS13_signal_handlers[datum][signal][callback] = { func = func, callback = callback }
-	if make_easy_clear_function then
-		local clear_function_name = "clear_signal_" .. tostring(datum) .. "_" .. signal .. "_" .. tostring(callback)
-		SS13[clear_function_name] = function()
-			if callback then
-				SS13.unregister_signal(datum, signal, callback)
-			end
-			SS13[clear_function_name] = nil
+	if not __SS13_signal_handlers[datumWeakRef]._cleanup then
+		local cleanupCallback = SS13.new("/datum/callback", SS13.state, "call_function_return_first")
+		local cleanupPath = { "__SS13_signal_handlers", datumWeakRef, "_cleanup"}
+		cleanupCallback.vars.arguments = { cleanupPath }
+		cleanupCallback:call_proc("RegisterSignal", datum, "parent_qdeleting", "Invoke")
+		__SS13_signal_handlers[datumWeakRef]._cleanup = function(datum)
+			SS13.start_tracking(datumWeakRef)
+			timer.set_timeout(0, function()
+				SS13.signal_handler_cleanup(datumWeakRef)
+				SS13.stop_tracking(cleanupCallback)
+				SS13.stop_tracking(datumWeakRef)
+			end)
 		end
 	end
+	__SS13_signal_handlers[datumWeakRef][signal][callbackWeakRef] = { func = func, callback = callback }
 	return callback
 end
 
@@ -143,51 +131,53 @@ function SS13.unregister_signal(datum, signal, callback)
 			return
 		end
 		local handler_callback = handler_info.callback
-		handler_callback:call_proc("UnregisterSignal", datum, signal)
+		local callbackWeakRef = dm.global_proc("WEAKREF", handler_callback)
+		if not SS13.istype(datum, "/datum/weakref") then
+			handler_callback:call_proc("UnregisterSignal", datum, signal)
+		end
 		SS13.stop_tracking(handler_callback)
 	end
 
-	local function clear_easy_clear_function(callback_to_clear)
-		local clear_function_name = "clear_signal_" .. tostring(datum) .. "_" .. signal .. "_" .. tostring(callback_to_clear)
-		SS13[clear_function_name] = nil
+	local datumWeakRef = datum
+	if not SS13.istype(datum, "/datum/weakref") then
+		datumWeakRef = dm.global_proc("WEAKREF", datum)
 	end
-
-	if not __SS13_signal_handlers[datum] then
+	if not __SS13_signal_handlers[datumWeakRef] then
 		return
 	end
+
 	if signal == "_cleanup" then
 		return
 	end
-	if not __SS13_signal_handlers[datum][signal] then
+
+	if not __SS13_signal_handlers[datumWeakRef][signal] then
 		return
 	end
 
 	if not callback then
-		for handler_key, handler_info in __SS13_signal_handlers[datum][signal] do
-			clear_easy_clear_function(handler_key)
+		for handler_key, handler_info in __SS13_signal_handlers[datumWeakRef][signal] do
 			clear_handler(handler_info)
 		end
-		__SS13_signal_handlers[datum][signal] = nil
+		__SS13_signal_handlers[datumWeakRef][signal] = nil
 	else
 		if not SS13.istype(callback, "/datum/callback") then
 			return
 		end
-		clear_easy_clear_function(callback)
-		clear_handler(__SS13_signal_handlers[datum][signal][callback])
-		__SS13_signal_handlers[datum][signal][callback] = nil
+		local callbackWeakRef = dm.global_proc("WEAKREF", callback)
+		clear_handler(__SS13_signal_handlers[datumWeakRef][signal][callbackWeakRef])
+		__SS13_signal_handlers[datumWeakRef][signal][callbackWeakRef] = nil
 	end
 end
 
-function SS13.signal_handler_cleanup(datum)
-	if not __SS13_signal_handlers[datum] then
+function SS13.signal_handler_cleanup(datumWeakRef)
+	if not __SS13_signal_handlers[datumWeakRef] then
 		return
 	end
 
-	for signal, _ in __SS13_signal_handlers[datum] do
-		SS13.unregister_signal(datum, signal)
+	for signal, _ in __SS13_signal_handlers[datumWeakRef] do
+		SS13.unregister_signal(datumWeakRef, signal)
 	end
-
-	__SS13_signal_handlers[datum] = nil
+	__SS13_signal_handlers[datumWeakRef] = nil
 end
 
 return SS13

lua/docs/handler_group.md

@@ -0,0 +1,66 @@
+# Handler Group
+
+This module is for registering signals on a datum or several datums and being able to clear them all at once without having to unregister them manually. This is particularly useful if you register signals on a datum and need to clear them later without accidentally unregistering unrelated signals
+
+## Functions
+
+### HandlerGroup.new()
+Creates a new handler group instance
+
+### HandlerGroup:register_signal(datum, signal, func)
+Registers a signal on a datum, exactly the same as `SS13.register_signal`
+
+### HandlerGroup:clear()
+Clears all registered signals that have been registered by this handler group.
+
+### HandlerGroup:clear_on(datum, signal, func)
+Clears all registered signals that have been registered by this handler group when a signal is called on the specified datum. Additionally, a function can be ran before it is cleared
+
+### HandlerGroup.register_once(datum, signal func)
+Identical to just creating a new HandlerGroup instance and calling `clear_on(datum, signal, func)`.
+
+The idea is to register a signal and clear it after it has been called once.
+
+## Examples
+
+The following examples showcase why using handler groups can make life easier in specific situations.
+
+### Explode when mob enters location
+This function creates a 1 tile-wide explosion at the specified location if a specific mob walks over it. The explosion won't happen if the mob dies. This function should be callable on the same mob for different locations. The function should be self-contained, it should not affect other registered signals that the mob may have registered.
+
+#### Without Handler Groups
+```lua
+local function explodeAtLocation(mobVar, position)
+	local deathCallback
+	local moveCallback
+	local function unlinkFromMob()
+		SS13.unregister_signal(mobVar, "living_death", deathCallback)
+		SS13.unregister_signal(mobVar, "movable_moved", moveCallback)
+	end
+	deathCallback = SS13.register_signal(mobVar, "living_death", function(_, gibbed)
+		unlinkFromMob()
+	end)
+	moveCallback = SS13.register_signal(mobVar, "movable_moved", function(_, oldLoc)
+		if mobVar:get_var("loc") == position then
+			-- Creates a 1 tile-wide explosion at the specified position
+			dm.global_proc("explosion", position, 1, 0, 0)
+			unlinkFromMob()
+		end
+	end)
+end
+```
+
+#### With Handler Groups
+```lua
+local function explodeAtLocation(mobVar, position)
+	local handler = handler_group.new()
+	handler:clear_on(mobVar, "living_death")
+	handler:register_signal(mobVar, "movable_moved", function(_, oldLoc)
+		if mobVar:get_var("loc") == position then
+			-- Creates a 1 tile-wide explosion at the specified position
+			dm.global_proc("explosion", position, 1, 0, 0)
+			handler:clear()
+		end
+	end)
+end
+```

lua/handler_group.lua

@@ -0,0 +1,49 @@
+local SS13 = require('SS13')
+local HandlerGroup = {}
+HandlerGroup.__index = HandlerGroup
+
+function HandlerGroup.new()
+	return setmetatable({
+		registered = {}
+	}, HandlerGroup)
+end
+
+-- Registers a signal on a datum for this handler group instance.
+function HandlerGroup:register_signal(datum, signal, func)
+	local callback = SS13.register_signal(datum, signal, func)
+	if not callback then
+		return
+	end
+	table.insert(self.registered, { datum = datum, signal = signal, callback = callback  })
+end
+
+-- Clears all the signals that have been registered on this HandlerGroup
+function HandlerGroup:clear()
+	for _, data in self.registered do
+		if not data.callback or not data.datum then
+			continue
+		end
+		SS13.unregister_signal(data.datum, data.signal, data.callback)
+	end
+	table.clear(self.registered)
+end
+
+-- Clears all the signals that have been registered on this HandlerGroup when a specific signal is sent on a datum.
+function HandlerGroup:clear_on(datum, signal, func)
+	SS13.register_signal(datum, signal, function(...)
+		if func then
+			func(...)
+		end
+		self:clear()
+	end)
+end
+
+-- Registers a signal on a datum and clears it after it is called once.
+function HandlerGroup.register_once(datum, signal, func)
+	local callback = HandlerGroup.new()
+	callback:clear_on(datum, signal, func)
+	return callback
+end
+
+
+return HandlerGroup

lua/state.lua

@@ -0,0 +1,9 @@
+local SSlua = dm.global_vars:get_var("SSlua")
+
+for _, state in SSlua:get_var("states") do
+	if state:get_var("internal_id") == dm.state_id then
+		return { state = state }
+	end
+end
+
+return { state = nil }

lua/timer.lua

@@ -1,6 +1,8 @@
-local SS13 = require("SS13_base")
+local state = require("state")
+
 local Timer = {}
 
+local SSlua = dm.global_vars:get_var("SSlua")
 __Timer_timers = __Timer_timers or {}
 __Timer_callbacks = __Timer_callbacks or {}
 
@@ -35,7 +37,7 @@ function __stop_internal_timer(func)
 end
 
 __Timer_timer_processing = __Timer_timer_processing or false
-SS13.state:set_var("timer_enabled", 1)
+state.state:set_var("timer_enabled", 1)
 __Timer_timer_process = function(seconds_per_tick)
 	if __Timer_timer_processing then
 		return 0
@@ -50,7 +52,7 @@ __Timer_timer_process = function(seconds_per_tick)
 			sleep()
 		end
 		if time >= timeData.executeTime then
-			SS13.state:get_var("functions_to_execute"):add(func)
+			state.state:get_var("functions_to_execute"):add(func)
 			timeData.executing = true
 		end
 	end
@@ -61,7 +63,7 @@ end
 function Timer.wait(time)
 	local next_yield_index = __next_yield_index
 	__add_internal_timer(function()
-		SS13.SSlua:call_proc("queue_resume", SS13.state, next_yield_index)
+		SSlua:call_proc("queue_resume", state.state, next_yield_index)
 	end, time * 10, false)
 	coroutine.yield()
 end