From 8a1cc5ccca992da7e48026c850334cbe188595c4 Mon Sep 17 00:00:00 2001
From: Paxilmaniac <82386923+Paxilmaniac@users.noreply.github.com>
Date: Fri, 10 Feb 2023 12:54:49 -0500
Subject: [PATCH] Fixes ghost roles trying to spawn people when there /should/
 be zero uses left (#73224)

## About The Pull Request

By means of making a particularly popular ghost role (do NOT put
catgirls on icebox bro you don't know the consequences) I have
discovered a strange bug where despite any of the checks that are in
place, it will still attempt to spawn more people than the role should
allow. I've found the likely caused to be that a lot of people are
clicking the role then clicking spawn at once, and the spawner simply
does not have time to tell everyone "wait no you can't do that" before
it deletes itself from running out of uses.

The solution? Even easier than the reservation thing: Subtract uses
**BEFORE** spawning the mob, as spawning the mob can take some time,
then if it fails to spawn somehow we can refund the uses.

## Why It's Good For The Game

Ghost roles spawning in the nullspace spawn room every now and then is a
bad thing I think.

## Changelog
:cl:
fix: Ghost role spawners will now no longer try and spawn people when
they /should/ have been out of uses but due to a bug were not
/:cl:
---
 code/modules/mob_spawn/mob_spawn.dm | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/code/modules/mob_spawn/mob_spawn.dm b/code/modules/mob_spawn/mob_spawn.dm
index 2476d96b4b5..6d4818bec1c 100644
--- a/code/modules/mob_spawn/mob_spawn.dm
+++ b/code/modules/mob_spawn/mob_spawn.dm
@@ -108,8 +108,10 @@
 	var/prompt_name = ""
 	///if false, you won't prompt for this role. best used for replacing the prompt system with something else like a radial, or something.
 	var/prompt_ghost = TRUE
-	///how many times this spawner can be used (it won't delete unless it's out of uses)
+	///how many times this spawner can be used (it won't delete unless it's out of uses and the var to delete itself is set)
 	var/uses = 1
+	/// Does the spawner delete itself when it runs out of uses?
+	var/deletes_on_zero_uses_left = TRUE
 
 	////descriptions
 
@@ -146,16 +148,19 @@
 /obj/effect/mob_spawn/ghost_role/attack_ghost(mob/user)
 	if(!SSticker.HasRoundStarted() || !loc)
 		return
+
 	if(prompt_ghost)
-		var/ghost_role = tgui_alert(usr, "Become [prompt_name]? (Warning, You can no longer be revived!)",, list("Yes", "No"))
+		var/ghost_role = tgui_alert(usr, "Become [prompt_name]? (Warning, You can no longer be revived!)", buttons = list("Yes", "No"), timeout = 10 SECONDS)
 		if(ghost_role != "Yes" || !loc || QDELETED(user))
 			return
+
 	if(!(GLOB.ghost_role_flags & GHOSTROLE_SPAWNER) && !(flags_1 & ADMIN_SPAWNED_1))
 		to_chat(user, span_warning("An admin has temporarily disabled non-admin ghost roles!"))
 		return
 	if(!uses) //just in case
 		to_chat(user, span_warning("This spawner is out of charges!"))
 		return
+
 	if(is_banned_from(user.key, role_ban))
 		to_chat(user, span_warning("You are banned from this role!"))
 		return
@@ -163,8 +168,15 @@
 		return
 	if(QDELETED(src) || QDELETED(user))
 		return
+
 	user.log_message("became a [prompt_name].", LOG_GAME)
-	create(user)
+	uses -= 1 // Remove a use before trying to spawn to prevent strangeness like the spawner trying to spawn more mobs than it should be able to
+
+	if(!(create(user)))
+		message_admins("[src] didn't return anything when creating a mob, this might be broken! The use of the spawner it would have taken has been refunded.")
+		uses += 1 // Oops! We messed up and somehow the mob didn't spawn, but that's alright we can refund the use.
+
+	check_uses() // Now we check if the spawner should delete itself or not
 
 /obj/effect/mob_spawn/ghost_role/special(mob/living/spawned_mob, mob/mob_possessor)
 	. = ..()
@@ -182,12 +194,9 @@
 		spawned_mob.mind.set_assigned_role(SSjob.GetJobType(spawner_job_path))
 		spawned_mind.name = spawned_mob.real_name
 
-//multiple use mob spawner functionality here- doesn't make sense on corpses
-/obj/effect/mob_spawn/ghost_role/create(mob/mob_possessor, newname)
-	. = ..()
-	if(uses > 0)
-		uses--
-	if(!uses)
+/// Checks if the spawner has zero uses left, if so, delete yourself... NOW!
+/obj/effect/mob_spawn/ghost_role/proc/check_uses()
+	if(!uses && deletes_on_zero_uses_left)
 		qdel(src)
 
 ///override this to add special spawn conditions to a ghost role