Mob Ref Cleanup Part 3 (#22599)

Part 3 of the Mob Destroy Refactor, this time going through the entire
list of every ref stored up to a mob/human, and (attempting) to verify
and cleanup every possible circular ref. This PR also fixes some
mistakes made with cleaning up UI elements, namely that tgui's really
don't like it when you qdel them, and screen objects also weren't always
clearing their own references if Qdel'ed directly. There were several
niche situations too where circle refs might be retained by a mob. I
also found an issue where static lights were only cleaned up on
/atom/movable/ but actually existed farther up the chain on /atom. I
don't know if any /atoms that aren't /atom/movable ever get static
lights, but the fact that they can be needs to be correctly accounted
for.

I can't possibly have gotten all of them, but this is every single one I
could find after 3 hours of work.

code/ZAS/Atom.dm

@@ -74,6 +74,3 @@
 		result |= M.c_airblock(other)
 		if(result == BLOCKED) return BLOCKED
 	return result
-
-/atom/movable
-	var/atmos_canpass = CANPASS_ALWAYS

code/__HELPERS/icon_smoothing.dm

@@ -52,11 +52,6 @@
 	/// Icon state of the overlay this object uses to attach to other objects.
 	var/attach_overlay
 
-/atom/movable
-	var/can_be_unanchored = 0
-	var/obj/buckled_to
-	var/can_be_buckled = FALSE
-
 /turf
 	var/list/fixed_underlay
 	/// Determines if we should attempt to generate turf underlays for this type.

code/_onclick/hud/screen_objects.dm

@@ -28,7 +28,10 @@
 	screen_loc = null
 	if(length(hud?.mymob?.client?.screen))
 		hud.mymob.client.screen -= src
-	hud = null
+	// All register signals MUST be mirrored with Unregister signals included in a Destroy proc, even if other procs also call Unregister Signal.
+	if (hud)
+		UnregisterSignal(hud, COMSIG_QDELETING)
+		hud = null
 	return ..()
 
 /// Screen elements are always on top of the players screen and don't move so yes they are adjacent
@@ -41,6 +44,10 @@
 /atom/movable/screen/proc/handle_hud_destruction()
 	SIGNAL_HANDLER
 
+	// If we were QDEL'ed directly (probably by our owner), then there's no need to Unregister Signal and qdel ourselves twice.
+	if (QDELING(src))
+		return
+
 	UnregisterSignal(hud, COMSIG_QDELETING)
 	qdel(src)
 

code/controllers/subsystems/processing/airflow.dm

@@ -92,13 +92,6 @@ PROCESSING_SUBSYSTEM_DEF(airflow)
 
 #undef CLEAR_OBJECT
 
-/atom/movable
-	var/tmp/airflow_xo
-	var/tmp/airflow_yo
-	var/tmp/airflow_od
-	var/tmp/airflow_process_delay
-	var/tmp/airflow_skip_speedcheck
-
 /atom/movable/proc/prepare_airflow(n)
 	if (!airflow_dest || airflow_speed < 0 || last_airflow > world.time - GLOB.vsc.airflow_delay)
 		return FALSE

code/game/atom/_atom.dm

@@ -74,6 +74,10 @@
 	var/tmp/datum/dynamic_light_source/light
 	///Any light sources that are "inside" of us, for example, if src here was a mob that's carrying a flashlight, that flashlight's light source would be part of this list.
 	var/tmp/list/hybrid_light_sources
+	///The light source, datum. Dont fuck with this directly
+	var/tmp/datum/static_light_source/static_light
+	///Static light sources currently attached to this atom, this includes ones owned by atoms inside this atom
+	var/tmp/list/static_light_sources
 
 	//Values should avoid being close to -16, 16, -48, 48 etc.
 	//Best keep them within 10 units of a multiple of 32, as when the light is closer to a wall, the probability
@@ -152,6 +156,7 @@
 		overlays.Cut()
 
 	QDEL_NULL(light)
+	QDEL_NULL(static_light)
 
 	if(smoothing_flags & SMOOTH_QUEUED)
 		SSicon_smooth.remove_from_queues(src)

code/game/atoms_movable.dm

@@ -92,6 +92,26 @@
 	var/tmp/airflow_time = 0
 	var/tmp/last_airflow = 0
 
+	var/can_be_unanchored = 0
+	var/obj/buckled_to
+	var/can_be_buckled = FALSE
+
+	/** Used to check wether or not an atom is being handled by SSfalling. */
+	var/tmp/multiz_falling = 0
+
+	var/tmp/airflow_xo
+	var/tmp/airflow_yo
+	var/tmp/airflow_od
+	var/tmp/airflow_process_delay
+	var/tmp/airflow_skip_speedcheck
+
+	/// The mimic (if any) that's *directly* copying us.
+	var/tmp/atom/movable/openspace/mimic/bound_overlay
+	/// If TRUE, this atom is ignored by Z-Mimic.
+	var/z_flags
+
+	var/atmos_canpass = CANPASS_ALWAYS
+
 /atom/movable/Initialize(mapload, ...)
 	. = ..()
 	update_emissive_blocker()
@@ -144,16 +164,12 @@
 			M.pulling = null
 		pulledby = null
 
-	if (bound_overlay)
-		QDEL_NULL(bound_overlay)
-
-	if(em_block)
-		QDEL_NULL(em_block)
-
-	QDEL_NULL(light)
-	QDEL_NULL(static_light)
+	QDEL_NULL(bound_overlay)
+	QDEL_NULL(em_block)
 	airflow_dest = null
 	loc = null
+	buckled_to?.buckled = null
+	buckled_to = null
 	return ..()
 
 /atom/movable/proc/moveToNullspace()

code/modules/clothing/clothing.dm

@@ -528,6 +528,14 @@
 	pickup_sound = 'sound/items/pickup/gloves.ogg'
 	equip_sound = 'sound/items/equip/gloves.ogg'
 
+/obj/item/clothing/gloves/Destroy()
+	QDEL_NULL(cell)
+	if (ring && wearer)
+		wearer.equip_to_slot_if_possible(ring, slot_gloves)
+	ring = null
+	wearer = null
+	return ..()
+
 /obj/item/clothing/gloves/update_clothing_icon()
 	if (ismob(src.loc))
 		var/mob/M = src.loc
@@ -948,6 +956,10 @@
 
 	var/footstep_sound_override
 
+/obj/item/clothing/shoes/Destroy()
+	QDEL_NULL(holding)
+	return ..()
+
 /obj/item/clothing/shoes/proc/draw_knife()
 	set name = "Draw Boot Knife"
 	set desc = "Pull out your boot knife."

code/modules/clothing/spacesuits/rig/modules/modules.dm

@@ -166,9 +166,8 @@
 
 
 /obj/item/rig_module/Destroy()
-	for(var/sm in stat_modules)
-		qdel(sm)
-	stat_modules.Cut()
+	QDEL_LIST(stat_modules)
+	holder?.installed_modules -= src
 	holder = null
 	return ..()
 

code/modules/clothing/spacesuits/rig/rig.dm

@@ -224,13 +224,22 @@
 	piece.icon_supported_species_tags = icon_supported_species_tags
 
 /obj/item/rig/Destroy()
-	for(var/obj/item/piece in list(gloves,boots,helmet,chest))
-		qdel(piece)
 	STOP_PROCESSING(SSmobs, src)
-	qdel(wires)
-	wires = null
-	qdel(spark_system)
-	spark_system = null
+	QDEL_NULL(air_supply)
+	QDEL_NULL(boots)
+	QDEL_NULL(chest)
+	QDEL_NULL(helmet)
+	QDEL_NULL(gloves)
+	QDEL_NULL(cell)
+	selected_module = null
+	visor = null
+	speech = null
+	wearer?.wearing_rig = null
+	wearer = null
+	mob_icon = null
+	QDEL_LIST(installed_modules)
+	QDEL_NULL(wires)
+	QDEL_NULL(spark_system)
 	return ..()
 
 /obj/item/rig/proc/set_vision(var/active)

code/modules/lighting/lighting_static/static_lighting_atom.dm

@@ -1,10 +1,3 @@
-
-/atom
-	///The light source, datum. Dont fuck with this directly
-	var/tmp/datum/static_light_source/static_light
-	///Static light sources currently attached to this atom, this includes ones owned by atoms inside this atom
-	var/tmp/list/static_light_sources
-
 ///Pretty simple, just updates static lights on this atom
 /atom/proc/static_update_light()
 	set waitfor = FALSE

code/modules/mob/mob.dm

@@ -54,6 +54,8 @@
 
 	QDEL_NULL(ability_master)
 	QDEL_NULL(zone_sel)
+	if (length(machine?.climbers))
+		machine.climbers -= src
 	machine = null
 
 	client_colors = null
@@ -61,6 +63,7 @@
 	pinned?.Cut()
 	QDEL_LIST(embedded)
 	languages?.Cut()
+	holo?.clear_holo(src)
 	holo = null
 	l_hand = null
 	r_hand = null
@@ -104,11 +107,23 @@
 	QDEL_LIST(click_handlers)
 	vr_mob = null
 	old_mob = null
+	if (lastattacker && lastattacker.lastattacked == src)
+		lastattacker.lastattacked = null
+	if (lastattacked && lastattacked.lastattacker == src)
+		lastattacked.lastattacker = null
 	lastattacker = null
 	lastattacked = null
 	QDEL_NULL(pointing_effect)
-	QDEL_LIST(open_nanouis)
-	QDEL_LIST(tgui_open_uis)
+
+	// Cleanup for all UIs belonging to the client.
+	// For performance reasons we check the length of the lists first since the lists can potentially either be null or empty.
+	// But it's also plausible that they might contain null values, and while their aggressive checking means any valid entry in the list
+	// is guaranteed to be of type /datum/tgui, null entries in the list can potentially exist and will pass the as anything typecast.
+	if (length(tgui_open_uis))
+		for (var/datum/tgui/ui as anything in tgui_open_uis)
+			ui?.close()
+		tgui_open_uis.Cut()
+
 	QDEL_NULL(narsimage)
 	QDEL_NULL(narglow)
 	QDEL_NULL(riftimage)

code/modules/multiz/movement.dm

@@ -1,7 +1,3 @@
-/atom/movable
-	/** Used to check wether or not an atom is being handled by SSfalling. */
-	var/tmp/multiz_falling = 0
-
 /**
  * Verb for the mob to move up a z-level if possible.
  */

code/modules/multiz/zmimic/mimic_movable.dm

@@ -1,9 +1,3 @@
-/atom/movable
-	/// The mimic (if any) that's *directly* copying us.
-	var/tmp/atom/movable/openspace/mimic/bound_overlay
-	/// If TRUE, this atom is ignored by Z-Mimic.
-	var/z_flags
-
 /atom/movable/set_dir(ndir)
 	. = ..()
 	if (. && bound_overlay)

code/modules/organs/internal/brain.dm

@@ -124,8 +124,7 @@
 		addtimer(CALLBACK(src, PROC_REF(clear_screen)), 5)
 
 /obj/item/organ/internal/brain/Destroy()
-	if(brainmob)
-		QDEL_NULL(brainmob)
+	QDEL_NULL(brainmob)
 	return ..()
 
 /obj/item/organ/internal/brain/removed(var/mob/living/user)

html/changelogs/hellfirejag-even-more-mob-harddels.yml

@@ -0,0 +1,11 @@
+author: Hellfirejag
+delete-after: True
+changes:
+  - bugfix: "Fixed a hard del caused by mobs not properly cleaning up UI windows."
+  - bugfix: "Fixed hardsuits having several harddels"
+  - bugfix: "Fixed screen objects not always being cleaned up correctly and hard deleting"
+  - bugfix: "Fixed several niche circular references on mobs which caused hard deletes."
+  - bugfix: "Fixed gloves hard deleting if they were worn over a ring or were upgraded to shock gloves"
+  - bugfix: "Fixed static lights not being cleared properly if they were attached to a non-movable atom."
+  - bugfix: "Fixed boots having a hard delete if a knife was stored in them."
+  - bugfix: "Fixed a circular reference between mobs that have attacked each other that was causing hard deletes"