From d504fe76f4935518b45f3ba1ec6d935af5f16b55 Mon Sep 17 00:00:00 2001
From: SkyratBot <59378654+SkyratBot@users.noreply.github.com>
Date: Sun, 6 Feb 2022 21:30:12 +0100
Subject: [PATCH] [MIRROR] Fixes harddels in pinned module code, cleans up a
 musty pattern that I want to die [MDB IGNORE] (#11319)

* Fixes harddels in pinned module code, cleans up a musty pattern that I want to die (#64674)

* Please stop typecasting target, noooooooooooooooooo

* Fixes harddels in pinned module code

The logic for pinned modules was intentionally hanging references to the
mob that pinned the action button. I have depression.

The pinned_to list also was never fully cleared, but that would have
just exasperated the issue. I've converted its use of mobs to refs, and
its use of the module var into something better managed

(Friendly reminder that actions will persist in your nightmares forever
unless they are manually qdel'd, this code wasn't doing that.

Also cleaned up how the pinned_to list is managed, hopefully it's a bit
more action centered now

* Fixes harddels in pinned module code, cleans up a musty pattern that I want to die

Co-authored-by: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com>
---
 code/datums/action.dm                         | 12 +++----
 code/modules/mod/mod_actions.dm               | 31 ++++++++++++++-----
 code/modules/mod/mod_control.dm               | 15 ++-------
 code/modules/mod/modules/_module.dm           |  6 ++--
 code/modules/mod/modules/module_pathfinder.dm |  7 +++--
 5 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/code/datums/action.dm b/code/datums/action.dm
index bce56831548..0e31fcc7957 100644
--- a/code/datums/action.dm
+++ b/code/datums/action.dm
@@ -400,16 +400,12 @@
 
 /datum/action/item_action/nano_picket_sign
 	name = "Retext Nano Picket Sign"
-	var/obj/item/picket_sign/S
-
-/datum/action/item_action/nano_picket_sign/New(Target)
-	..()
-	if(istype(Target, /obj/item/picket_sign))
-		S = Target
 
 /datum/action/item_action/nano_picket_sign/Trigger(trigger_flags)
-	if(istype(S))
-		S.retext(owner)
+	if(!istype(target, /obj/item/picket_sign))
+		return
+	var/obj/item/picket_sign/sign = target
+	sign.retext(owner)
 
 /datum/action/item_action/adjust
 
diff --git a/code/modules/mod/mod_actions.dm b/code/modules/mod/mod_actions.dm
index fa5caec72f8..70138d5959c 100644
--- a/code/modules/mod/mod_actions.dm
+++ b/code/modules/mod/mod_actions.dm
@@ -4,16 +4,17 @@
 	check_flags = AB_CHECK_CONSCIOUS
 	/// Whether this action is intended for the AI. Stuff breaks a lot if this is done differently.
 	var/ai_action = FALSE
-	/// The MODsuit linked to this action
-	var/obj/item/mod/control/mod
 
 /datum/action/item_action/mod/New(Target)
 	..()
-	mod = Target
+	if(!istype(Target, /obj/item/mod/control))
+		qdel(src)
+		return
 	if(ai_action)
 		background_icon_state = ACTION_BUTTON_DEFAULT_BACKGROUND
 
 /datum/action/item_action/mod/Grant(mob/user)
+	var/obj/item/mod/control/mod = target
 	if(ai_action && user != mod.ai)
 		return
 	else if(!ai_action && user == mod.ai)
@@ -21,6 +22,7 @@
 	return ..()
 
 /datum/action/item_action/mod/Remove(mob/user)
+	var/obj/item/mod/control/mod = target
 	if(ai_action && user != mod.ai)
 		return
 	else if(!ai_action && user == mod.ai)
@@ -30,6 +32,7 @@
 /datum/action/item_action/mod/Trigger(trigger_flags)
 	if(!IsAvailable())
 		return FALSE
+	var/obj/item/mod/control/mod = target
 	if(mod.malfunctioning && prob(75))
 		mod.balloon_alert(usr, "button malfunctions!")
 		return FALSE
@@ -44,6 +47,7 @@
 	. = ..()
 	if(!.)
 		return
+	var/obj/item/mod/control/mod = target
 	if(trigger_flags & TRIGGER_SECONDARY_ACTION)
 		mod.quick_deploy(usr)
 	else
@@ -71,6 +75,7 @@
 		UpdateButtonIcon()
 		addtimer(CALLBACK(src, .proc/reset_ready), 3 SECONDS)
 		return
+	var/obj/item/mod/control/mod = target
 	reset_ready()
 	mod.toggle_activate(usr)
 
@@ -94,6 +99,7 @@
 	. = ..()
 	if(!.)
 		return
+	var/obj/item/mod/control/mod = target
 	mod.quick_module(usr)
 
 /datum/action/item_action/mod/module/ai
@@ -108,6 +114,7 @@
 	. = ..()
 	if(!.)
 		return
+	var/obj/item/mod/control/mod = target
 	mod.ui_interact(usr)
 
 /datum/action/item_action/mod/panel/ai
@@ -119,8 +126,8 @@
 	var/override = FALSE
 	/// Module we are linked to.
 	var/obj/item/mod/module/module
-	/// Mob we are pinned to.
-	var/mob/pinner
+	/// A ref to the mob we are pinned to.
+	var/pinner_ref
 
 /datum/action/item_action/mod/pinned_module/New(Target, obj/item/mod/module/linked_module, mob/user)
 	if(isAI(user))
@@ -131,13 +138,21 @@
 	desc = "Quickly activate [linked_module]."
 	icon_icon = linked_module.icon
 	button_icon_state = linked_module.icon_state
-	pinner = user
 	RegisterSignal(linked_module, COMSIG_MODULE_ACTIVATED, .proc/on_module_activate)
 	RegisterSignal(linked_module, COMSIG_MODULE_DEACTIVATED, .proc/on_module_deactivate)
 	RegisterSignal(linked_module, COMSIG_MODULE_USED, .proc/on_module_use)
 
+/datum/action/item_action/mod/pinned_module/Destroy()
+	module.pinned_to -= pinner_ref
+	module = null
+	return ..()
+
 /datum/action/item_action/mod/pinned_module/Grant(mob/user)
-	if(user != pinner)
+	var/user_ref = REF(user)
+	if(!pinner_ref)
+		pinner_ref = user_ref
+		module.pinned_to[pinner_ref] = src
+	else if(pinner_ref != user_ref)
 		return
 	return ..()
 
@@ -145,6 +160,7 @@
 	. = ..()
 	if(!.)
 		return
+	var/obj/item/mod/control/mod = target
 	if(!mod.active)
 		mod.balloon_alert(usr, "suit not on!")
 	module.on_select()
@@ -153,6 +169,7 @@
 	. = ..(current_button, force = TRUE)
 	if(override)
 		return
+	var/obj/item/mod/control/mod = target
 	if(module == mod.selected_module)
 		current_button.add_overlay(image(icon = 'icons/hud/radial.dmi', icon_state = "module_selected", layer = FLOAT_LAYER-0.1))
 	else if(module.active)
diff --git a/code/modules/mod/mod_control.dm b/code/modules/mod/mod_control.dm
index 0dd1c923ff7..43646be6e9b 100644
--- a/code/modules/mod/mod_control.dm
+++ b/code/modules/mod/mod_control.dm
@@ -566,11 +566,11 @@
 	new_module.on_install()
 	if(wearer)
 		new_module.on_equip()
-		var/datum/action/item_action/mod/pinned_module/action = new_module.pinned_to[wearer]
+		var/datum/action/item_action/mod/pinned_module/action = new_module.pinned_to[REF(wearer)]
 		if(action)
 			action.Grant(wearer)
 	if(ai)
-		var/datum/action/item_action/mod/pinned_module/action = new_module.pinned_to[ai]
+		var/datum/action/item_action/mod/pinned_module/action = new_module.pinned_to[REF(ai)]
 		if(action)
 			action.Grant(ai)
 	if(user)
@@ -585,16 +585,7 @@
 		old_module.on_suit_deactivation()
 		if(old_module.active)
 			old_module.on_deactivation(display_message = TRUE)
-	if(wearer)
-		old_module.on_unequip()
-		var/datum/action/item_action/mod/pinned_module/action = old_module.pinned_to[wearer]
-		if(action)
-			action.Remove(wearer)
-	if(ai)
-		var/datum/action/item_action/mod/pinned_module/action = old_module.pinned_to[ai]
-		if(action)
-			action.Remove(ai)
-	old_module.pinned_to.Cut()
+	QDEL_LIST(old_module.pinned_to)
 	old_module.on_uninstall()
 	old_module.mod = null
 
diff --git a/code/modules/mod/modules/_module.dm b/code/modules/mod/modules/_module.dm
index 2060a668cb5..86cb365b1f5 100644
--- a/code/modules/mod/modules/_module.dm
+++ b/code/modules/mod/modules/_module.dm
@@ -35,7 +35,7 @@
 	var/cooldown_time = 0
 	/// The mouse button needed to use this module
 	var/used_signal
-	/// List of mobs we are pinned to, linked with their action buttons
+	/// List of REF()s mobs we are pinned to, linked with their action buttons
 	var/list/pinned_to = list()
 	/// If we're allowed to use this module while phased out.
 	var/allowed_in_phaseout = FALSE
@@ -289,14 +289,12 @@
 
 /// Pins the module to the user's action buttons
 /obj/item/mod/module/proc/pin(mob/user)
-	var/datum/action/item_action/mod/pinned_module/action = pinned_to[user]
+	var/datum/action/item_action/mod/pinned_module/action = pinned_to[REF(user)]
 	if(action)
 		qdel(action)
-		pinned_to[user] = null
 	else
 		action = new(mod, src, user)
 		action.Grant(user)
-		pinned_to[user] = action
 
 ///Anomaly Locked - Causes the module to not function without an anomaly.
 /obj/item/mod/module/anomaly_locked
diff --git a/code/modules/mod/modules/module_pathfinder.dm b/code/modules/mod/modules/module_pathfinder.dm
index f37ee7e1b8e..35a288c540e 100644
--- a/code/modules/mod/modules/module_pathfinder.dm
+++ b/code/modules/mod/modules/module_pathfinder.dm
@@ -150,17 +150,18 @@
 	button_icon_state = "recall"
 	/// The cooldown for the recall.
 	COOLDOWN_DECLARE(recall_cooldown)
-	/// The implant we are linked to.
-	var/obj/item/implant/mod/implant
 
 /datum/action/item_action/mod_recall/New(Target)
 	..()
-	implant = Target
+	if(!istype(Target, /obj/item/implant/mod))
+		qdel(src)
+		return
 
 /datum/action/item_action/mod_recall/Trigger(trigger_flags)
 	. = ..()
 	if(!.)
 		return
+	var/obj/item/implant/mod/implant = target
 	if(!COOLDOWN_FINISHED(src, recall_cooldown))
 		implant.balloon_alert(implant.imp_in, "on cooldown!")
 		return