From e57b1c8335e2879009bb63a66be0a4c9cd31eeae Mon Sep 17 00:00:00 2001
From: Ghom <42542238+Ghommie@users.noreply.github.com>
Date: Thu, 2 Sep 2021 10:00:06 +0200
Subject: [PATCH] Using a soulstone on a construct shell no longer destroys it
 and the shade if no option is taken. Plus code improvement (#60982)

I have split the multi-purpose transfer_soul proc into three smaller procs, each with their own purpose, renamed some variables accordingly to guidelines and added some checks such as CanReach where opportune.
---
 code/modules/antagonists/cult/cult_items.dm   |  28 ++--
 code/modules/antagonists/cult/runes.dm        |   4 +-
 .../antagonists/wizard/equipment/soulstone.dm | 145 ++++++++----------
 .../modules/mob/living/simple_animal/shade.dm |   8 +-
 .../projectiles/guns/ballistic/revolver.dm    |   8 +-
 5 files changed, 89 insertions(+), 104 deletions(-)

diff --git a/code/modules/antagonists/cult/cult_items.dm b/code/modules/antagonists/cult/cult_items.dm
index e5959de8571..29ffeb9df1b 100644
--- a/code/modules/antagonists/cult/cult_items.dm
+++ b/code/modules/antagonists/cult/cult_items.dm
@@ -212,19 +212,21 @@
 	if(dash_toggled && !proximity)
 		jaunt.Teleport(user, target)
 		return
-	if(proximity)
-		if(ishuman(target))
-			var/mob/living/carbon/human/H = target
-			if(H.stat != CONSCIOUS)
-				var/obj/item/soulstone/SS = new /obj/item/soulstone(src)
-				SS.attack(H, user)
-				if(!LAZYLEN(SS.contents))
-					qdel(SS)
-		if(istype(target, /obj/structure/constructshell) && contents.len)
-			var/obj/item/soulstone/SS = contents[1]
-			if(istype(SS))
-				SS.transfer_soul("CONSTRUCT",target,user)
-				qdel(SS)
+	if(!proximity)
+		return
+	if(ishuman(target))
+		var/mob/living/carbon/human/human_target = target
+		if(human_target.stat != CONSCIOUS)
+			var/obj/item/soulstone/stone = new /obj/item/soulstone(src)
+			stone.attack(human_target, user)
+			if(!LAZYLEN(stone.contents))
+				qdel(stone)
+	if(istype(target, /obj/structure/constructshell) && contents.len)
+		var/obj/item/soulstone/stone = contents[1]
+		if(!istype(stone))
+			stone.forceMove(drop_location())
+		else if(!stone.transfer_to_construct(target, user) && !(locate(/mob/living/simple_animal/shade) in stone))
+			qdel(stone)
 
 /datum/action/innate/dash/cult
 	name = "Rend the Veil"
diff --git a/code/modules/antagonists/cult/runes.dm b/code/modules/antagonists/cult/runes.dm
index c88e5bbe341..6ee93bd0cbb 100644
--- a/code/modules/antagonists/cult/runes.dm
+++ b/code/modules/antagonists/cult/runes.dm
@@ -318,7 +318,7 @@ structure_check() searches for nearby cultist structures required for the invoca
 
 	if(iscyborg(sacrificial))
 		var/construct_class = show_radial_menu(first_invoker, sacrificial, GLOB.construct_radial_images, require_near = TRUE, tooltips = TRUE)
-		if(QDELETED(sacrificial))
+		if(QDELETED(sacrificial) || !construct_class)
 			return FALSE
 		sacrificial.grab_ghost()
 		make_new_construct_from_class(construct_class, THEME_CULT, sacrificial, first_invoker, TRUE, get_turf(src))
@@ -329,7 +329,7 @@ structure_check() searches for nearby cultist structures required for the invoca
 	var/obj/item/soulstone/stone = new /obj/item/soulstone(get_turf(src))
 	if(sacrificial.mind && !sacrificial.suiciding)
 		stone.invisibility = INVISIBILITY_MAXIMUM //so it's not picked up during transfer_soul()
-		stone.transfer_soul("FORCE", sacrificial, usr)
+		stone.capture_soul(sacrificial, first_invoker, TRUE)
 		stone.invisibility = 0
 
 	if(sacrificial)
diff --git a/code/modules/antagonists/wizard/equipment/soulstone.dm b/code/modules/antagonists/wizard/equipment/soulstone.dm
index d4d65a33d67..e3485445bb0 100644
--- a/code/modules/antagonists/wizard/equipment/soulstone.dm
+++ b/code/modules/antagonists/wizard/equipment/soulstone.dm
@@ -139,7 +139,7 @@
 		hot_potato(user)
 		return
 	log_combat(user, M, "captured [M.name]'s soul", src)
-	transfer_soul("VICTIM", M, user)
+	capture_soul(M, user)
 
 ///////////////////Options for using captured souls///////////////////////////////////////
 
@@ -234,88 +234,72 @@
 		if(SS.theme == THEME_HOLY && IS_CULTIST(user))
 			SS.hot_potato(user)
 			return
-		SS.transfer_soul("CONSTRUCT",src,user)
-		SS.was_used()
+		SS.transfer_to_construct(src, user)
 	else
 		return ..()
 
-////////////////////////////Proc for moving soul in and out off stone//////////////////////////////////////
+/// Procs for moving soul in and out off stone
 
+/// transfer the mind of a carbon mob (which is then dusted) into a shade mob inside src. If forced, sacrifical and stat checks are skipped.
+/obj/item/soulstone/proc/capture_soul(mob/living/carbon/victim, mob/user, forced = FALSE)
+	if(!iscarbon(victim)) //TODO: Add sacrifice stoning for non-organics, just because you have no body doesnt mean you dont have a soul
+		return FALSE
+	if(!forced)
+		var/datum/antagonist/cult/C = user?.mind?.has_antag_datum(/datum/antagonist/cult,TRUE)
+		if(C?.cult_team.is_sacrifice_target(victim.mind))
+			to_chat(user, span_cult("<b>\"This soul is mine.</b></span> <span class='cultlarge'>SACRIFICE THEM!\""))
+			return FALSE
+	if(contents.len)
+		return FALSE
+	if(!forced && (grab_sleeping ? victim.stat == CONSCIOUS : victim.stat != DEAD))
+		to_chat(user, "[span_userdanger("Capture failed!")]: Kill or maim the victim first!")
+		return FALSE
+	if(victim.client)
+		victim.unequip_everything()
+		init_shade(victim, user)
+		return TRUE
+	else
+		to_chat(user, "[span_userdanger("Capture failed!")]: The soul has already fled its mortal frame. You attempt to bring it back...")
+		return getCultGhost(victim,user)
 
-/obj/item/soulstone/proc/transfer_soul(choice as text, target, mob/user)
-	switch(choice)
-		if("FORCE")
-			if(!iscarbon(target)) //TODO: Add sacrifice stoning for non-organics, just because you have no body doesnt mean you dont have a soul
-				return FALSE
-			if(contents.len)
-				return FALSE
-			var/mob/living/carbon/T = target
-			if(T.client != null)
-				for(var/obj/item/W in T)
-					T.dropItemToGround(W)
-				init_shade(T, user)
-				return TRUE
-			else
-				to_chat(user, "[span_userdanger("Capture failed!")]: The soul has already fled its mortal frame. You attempt to bring it back...")
-				return getCultGhost(T,user)
+///captures a shade that was previously released from a soulstone.
+/obj/item/soulstone/proc/capture_shade(mob/living/simple_animal/shade/shade, mob/user)
+	if(contents.len)
+		to_chat(user, "[span_userdanger("Capture failed!")]: [src] is full! Free an existing soul to make room.")
+		return FALSE
+	shade.AddComponent(/datum/component/soulstoned, src)
+	if(theme == THEME_HOLY)
+		icon_state = "purified_soulstone2"
+		shade.mind?.remove_antag_datum(/datum/antagonist/cult)
+	if(theme == THEME_WIZARD)
+		icon_state = "mystic_soulstone2"
+	if(theme == THEME_CULT)
+		icon_state = "soulstone2"
+	name = "soulstone: Shade of [shade.real_name]"
+	to_chat(shade, span_notice("Your soul has been captured by [src]. Its arcane energies are reknitting your ethereal form."))
+	if(user != shade)
+		to_chat(user, "[span_info("<b>Capture successful!</b>:")] [shade.real_name]'s soul has been captured and stored within [src].")
+	return TRUE
 
-		if("VICTIM")
-			var/mob/living/carbon/human/T = target
-			var/datum/antagonist/cult/C = user.mind.has_antag_datum(/datum/antagonist/cult,TRUE)
-			if(C?.cult_team.is_sacrifice_target(T.mind))
-				to_chat(user, span_cult("<b>\"This soul is mine.</b></span> <span class='cultlarge'>SACRIFICE THEM!\""))
-				return FALSE
-			if(contents.len)
-				to_chat(user, "[span_userdanger("Capture failed!")]: [src] is full! Free an existing soul to make room.")
-			else
-				if((grab_sleeping && T.stat != CONSCIOUS) || (!grab_sleeping && T.stat == DEAD))
-					if(T.client == null)
-						to_chat(user, "[span_userdanger("Capture failed!")]: The soul has already fled its mortal frame. You attempt to bring it back...")
-						getCultGhost(T,user)
-					else
-						for(var/obj/item/W in T)
-							T.dropItemToGround(W)
-						init_shade(T, user, message_user = 1)
-						qdel(T)
-				else
-					to_chat(user, "[span_userdanger("Capture failed!")]: Kill or maim the victim first!")
+///transfer the mind of the shade to a construct mob selected by the user, then deletes both the shade and src.
+/obj/item/soulstone/proc/transfer_to_construct(obj/structure/constructshell/shell, mob/user)
+	var/mob/living/simple_animal/shade/shade = locate() in src
+	if(!shade)
+		to_chat(user, "[span_userdanger("Creation failed!")]: [src] is empty! Go kill someone!")
+		return FALSE
+	var/construct_class = show_radial_menu(user, src, GLOB.construct_radial_images, custom_check = CALLBACK(src, .proc/check_menu, user, shell), require_near = TRUE, tooltips = TRUE)
+	if(!shell || !construct_class)
+		return FALSE
+	make_new_construct_from_class(construct_class, theme, shade, user, FALSE, shell.loc)
+	shade.mind?.remove_antag_datum(/datum/antagonist/cult)
+	qdel(shell)
+	qdel(src)
+	return TRUE
 
-		if("SHADE")
-			var/mob/living/simple_animal/shade/T = target
-			if(contents.len)
-				to_chat(user, "[span_userdanger("Capture failed!")]: [src] is full! Free an existing soul to make room.")
-			else
-				T.AddComponent(/datum/component/soulstoned, src)
-				if(theme == THEME_HOLY)
-					icon_state = "purified_soulstone2"
-					T.mind?.remove_antag_datum(/datum/antagonist/cult)
-				if(theme == THEME_WIZARD)
-					icon_state = "mystic_soulstone2"
-				if(theme == THEME_CULT)
-					icon_state = "soulstone2"
-				name = "soulstone: Shade of [T.real_name]"
-				to_chat(T, span_notice("Your soul has been captured by [src]. Its arcane energies are reknitting your ethereal form."))
-				if(user != T)
-					to_chat(user, "[span_info("<b>Capture successful!</b>:")] [T.real_name]'s soul has been captured and stored within [src].")
-
-		if("CONSTRUCT")
-			var/obj/structure/constructshell/T = target
-			var/mob/living/simple_animal/shade/A = locate() in src
-			if(A)
-				var/construct_class = show_radial_menu(user, src, GLOB.construct_radial_images, custom_check = CALLBACK(src, .proc/check_menu, user), require_near = TRUE, tooltips = TRUE)
-				if(!T || !T.loc)
-					return
-				make_new_construct_from_class(construct_class, theme, A, user, FALSE, T.loc)
-				A.mind?.remove_antag_datum(/datum/antagonist/cult)
-				qdel(T)
-				qdel(src)
-			else
-				to_chat(user, "[span_userdanger("Creation failed!")]: [src] is empty! Go kill someone!")
-
-/obj/item/soulstone/proc/check_menu(mob/user)
+/obj/item/soulstone/proc/check_menu(mob/user, obj/structure/constructshell/shell)
 	if(!istype(user))
 		return FALSE
-	if(user.incapacitated() || !user.Adjacent(src))
+	if(user.incapacitated() || !user.is_holding(src) || !user.CanReach(shell, src))
 		return FALSE
 	return TRUE
 
@@ -422,24 +406,23 @@
 			to_chat(user, "[span_info("<b>Capture successful!</b>:")] [dusted_victim.real_name]'s soul has been ripped from [dusted_victim.p_their()] body and stored within [src].")
 
 
-/obj/item/soulstone/proc/getCultGhost(mob/living/carbon/human/T, mob/user)
+/obj/item/soulstone/proc/getCultGhost(mob/living/carbon/victim, mob/user)
 	var/mob/dead/observer/chosen_ghost
 
-	chosen_ghost = T.get_ghost(TRUE,TRUE) //Try to grab original owner's ghost first
+	chosen_ghost = victim.get_ghost(TRUE,TRUE) //Try to grab original owner's ghost first
 
 	if(!chosen_ghost || !chosen_ghost.client) //Failing that, we grab a ghosts
 		var/list/consenting_candidates = pollGhostCandidates("Would you like to play as a Shade?", "Cultist", ROLE_CULTIST, 50, POLL_IGNORE_SHADE)
 		if(consenting_candidates.len)
 			chosen_ghost = pick(consenting_candidates)
-	if(!T)
+	if(!victim || user.incapacitated() || !user.is_holding(src) || !user.CanReach(victim, src))
 		return FALSE
 	if(!chosen_ghost || !chosen_ghost.client)
 		to_chat(user, span_danger("There were no spirits willing to become a shade."))
 		return FALSE
 	if(contents.len) //If they used the soulstone on someone else in the meantime
 		return FALSE
-	for(var/obj/item/W in T)
-		T.dropItemToGround(W)
-	init_shade(T, user , shade_controller = chosen_ghost)
-	qdel(T)
+	victim.unequip_everything()
+	init_shade(victim, user, shade_controller = chosen_ghost)
+	qdel(victim)
 	return TRUE
diff --git a/code/modules/mob/living/simple_animal/shade.dm b/code/modules/mob/living/simple_animal/shade.dm
index 8c9a1abecdf..90a162799cd 100644
--- a/code/modules/mob/living/simple_animal/shade.dm
+++ b/code/modules/mob/living/simple_animal/shade.dm
@@ -65,9 +65,9 @@
 	else if(src != user)
 		return ..()
 
-/mob/living/simple_animal/shade/attackby(obj/item/O, mob/user, params)  //Marker -Agouri
-	if(istype(O, /obj/item/soulstone))
-		var/obj/item/soulstone/SS = O
-		SS.transfer_soul("SHADE", src, user)
+/mob/living/simple_animal/shade/attackby(obj/item/item, mob/user, params)  //Marker -Agouri
+	if(istype(item, /obj/item/soulstone))
+		var/obj/item/soulstone/stone = item
+		stone.capture_shade(src, user)
 	else
 		. = ..()
diff --git a/code/modules/projectiles/guns/ballistic/revolver.dm b/code/modules/projectiles/guns/ballistic/revolver.dm
index 48be4771801..77a63f165f8 100644
--- a/code/modules/projectiles/guns/ballistic/revolver.dm
+++ b/code/modules/projectiles/guns/ballistic/revolver.dm
@@ -217,10 +217,10 @@
 	desc = "To play with this revolver requires wagering your very soul."
 
 /obj/item/gun/ballistic/revolver/russian/soul/shoot_self(mob/living/user)
-	..()
-	var/obj/item/soulstone/anybody/revolver/SS = new /obj/item/soulstone/anybody/revolver(get_turf(src))
-	if(!SS.transfer_soul("FORCE", user)) //Something went wrong
-		qdel(SS)
+	. = ..()
+	var/obj/item/soulstone/anybody/revolver/stone = new /obj/item/soulstone/anybody/revolver(get_turf(src))
+	if(!stone.capture_soul(user, forced = TRUE)) //Something went wrong
+		qdel(stone)
 		return
 	user.visible_message(span_danger("[user.name]'s soul is captured by \the [src]!"), span_userdanger("You've lost the gamble! Your soul is forfeit!"))