From 3b695cf65ecc136a90fc0953d16cd8921b10d4ff Mon Sep 17 00:00:00 2001
From: SkyratBot <59378654+SkyratBot@users.noreply.github.com>
Date: Sun, 29 Oct 2023 02:06:14 +0200
Subject: [PATCH] [MIRROR] Ground sinking sanity checks and doc cleanup [MDB
 IGNORE] (#24632)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Ground sinking sanity checks and doc cleanup (#79214)

## About The Pull Request

The asynchronous sinking component of garden gnomes did not check if the
gnome still existed, when the sinking timer ran out, which caused null
exception runtimes. I have added sanity checks to this. In addition, the
sinking used do_after, but failing the do_after had no effect on
anything, and infact, made the mob sink even faster into the ground, so
I have added an early return, and a call for unsinking.

Also tweaked proc names and documentation.

Funny story: the only reason I have found out about this bug is because
sometimes garden gnomes spawn on a downstream map, and they cause a CI
failure when the nuke cinematic unit test explodes them. 😔

## Why It's Good For The Game
Runtimes bad. Also if an action has a do_after call, it should probably
fail if it the timer fails.

## Changelog

:cl:
fix: Gnomes no longer runtime if they explode while sinking into the
ground
/:cl:

* Ground sinking sanity checks and doc cleanup

---------

Co-authored-by: Profakos <profakos@gmail.com>
---
 code/datums/components/ground_sinking.dm | 27 +++++++++++++++++-------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/code/datums/components/ground_sinking.dm b/code/datums/components/ground_sinking.dm
index 0fb5fb9eac6..9b70cb0ab60 100644
--- a/code/datums/components/ground_sinking.dm
+++ b/code/datums/components/ground_sinking.dm
@@ -66,7 +66,7 @@
 	if(ground_sinking_start_timer)
 		deltimer(ground_sinking_start_timer)
 
-/// When you take damage, reset the cooldown and start processing
+/// When you move, reset the cooldown and start processing
 /datum/component/ground_sinking/proc/on_moved(mob/living/basic/living_target, atom/OldLoc, Dir, Forced)
 	SIGNAL_HANDLER
 	if(sinked || is_sinking)
@@ -79,18 +79,24 @@
 /datum/component/ground_sinking/proc/start_sinking(mob/living/basic/living_target)
 	if(!sinked || is_sinking)
 		is_sinking = TRUE
-		INVOKE_ASYNC(src, PROC_REF(sink_once), living_target)
+		INVOKE_ASYNC(src, PROC_REF(sinking_progress), living_target)
 
-/datum/component/ground_sinking/proc/sink_once(mob/living/basic/living_target)
+/// Makes the mob try to sink three times. Unsinks if interrupted.
+/datum/component/ground_sinking/proc/sinking_progress(mob/living/basic/living_target)
 	living_target.visible_message(span_notice("[living_target] starts sinking into the ground!"))
 	for(var/i in 1 to 3)
-		if(do_after(living_target, sink_speed, living_target))
-			sink_count += 1
-			living_target.icon_state = "[target_icon_state]_burried_[sink_count]"
+		if(QDELETED(living_target))
+			return
+		if(!do_after(living_target, sink_speed, living_target))
+			unsink()
+			return
+		sink_count += 1
+		living_target.icon_state = "[target_icon_state]_burried_[sink_count]"
 	sink_count = 0
-	is_sinked(living_target)
+	finish_sinking(living_target)
 
-/datum/component/ground_sinking/proc/is_sinked(mob/living/basic/living_target)
+/// The mob has fully sunk, updates its regeneration, damage resistance and density
+/datum/component/ground_sinking/proc/finish_sinking(mob/living/basic/living_target)
 	sinked = TRUE
 	is_sinking = FALSE
 	living_target.density = FALSE
@@ -98,8 +104,11 @@
 	if(heal_when_sinked)
 		start_regenerating()
 
+/// The mob pops out of the ground
 /datum/component/ground_sinking/proc/unsink()
 	var/mob/living/basic/living_target = parent
+	if(QDELETED(parent))
+		return
 	if(sinked && heal_when_sinked)
 		stop_regenerating()
 	living_target.icon_state = target_icon_state
@@ -107,6 +116,7 @@
 	living_target.density = TRUE
 	sinked = FALSE
 
+/// The mop starts regaining health
 /datum/component/ground_sinking/proc/start_regenerating()
 	var/mob/living/basic/living_parent = parent
 	if (living_parent.stat == DEAD)
@@ -122,6 +132,7 @@
 	animate(filter, alpha = 200, time = 0.5 SECONDS, loop = -1)
 	animate(alpha = 0, time = 0.5 SECONDS)
 
+/// Stops regaining health
 /datum/component/ground_sinking/proc/stop_regenerating()
 	STOP_PROCESSING(SSobj, src)
 	var/mob/living/basic/living_parent = parent