Merge pull request #8753 from Spookerton/spkrtn/fix/assorted-221015

fixes for a number of logged errors

code/_helpers/logging.dm

@@ -253,29 +253,50 @@
 /proc/key_name_admin(var/whom, var/include_name = 1)
 	return key_name(whom, 1, include_name)
 
-// Helper procs for building detailed log lines
-/datum/proc/log_info_line()
-	return "[src] ([type])"
-
-/atom/log_info_line()
-	var/turf/t = get_turf(src)
-	if(istype(t))
-		return "([t]) ([t.x],[t.y],[t.z]) ([t.type])"
-	else if(loc)
-		return "([loc]) (0,0,0) ([loc.type])"
-	else
-		return "(NULL) (0,0,0) (NULL)"
-
-/mob/log_info_line()
-	return "[..()] ([ckey])"
-
-/proc/log_info_line(var/datum/d)
-	if(!istype(d))
-		return
-	return d.log_info_line()
 
 /mob/proc/simple_info_line()
 	return "[key_name(src)] ([x],[y],[z])"
 
+
 /client/proc/simple_info_line()
 	return "[key_name(src)] ([mob.x],[mob.y],[mob.z])"
+
+
+/proc/log_info_line(datum/thing)
+	if (isnull(thing))
+		return "*null*"
+	if (islist(thing))
+		var/list/result = list()
+		var/list/thing_list =  thing
+		for (var/key in thing_list)
+			var/value = isnum(key) ? null : thing[key]
+			result += "[log_info_line(key)][value ? " - [log_info_line(value)]" : ""]"
+		return "\[[jointext(result, ", ")]\]"
+	if (!istype(thing))
+		return json_encode(thing)
+	return thing.get_log_info_line()
+
+
+/datum/proc/get_log_info_line()
+	return "[src] ([type])"
+
+
+/weakref/get_log_info_line()
+	return "[ref_name] ([ref_type]) ([ref]) (WEAKREF)"
+
+
+/area/get_log_info_line()
+	return "[..()] ([isnum(z) ? "[x],[y],[z]" : "0,0,0"])"
+
+
+/turf/get_log_info_line()
+	return "[..()] ([x],[y],[z]) ([loc ? loc.type : "NULL"])"
+
+
+/atom/movable/get_log_info_line()
+	var/turf/turf = get_turf(src)
+	return "[..()] ([turf ? turf : "NULL"]) ([turf ? "[turf.x],[turf.y],[turf.z]" : "0,0,0"]) ([turf ? turf.type : "NULL"])"
+
+
+/mob/get_log_info_line()
+	return ckey ? "[..()] ([ckey])" : ..()

code/controllers/subsystems/antag.dm

@@ -27,11 +27,6 @@ SUBSYSTEM_DEF(antags)
 		var/datum/antagonist/A = new antag_type
 		antag_datums[A.id] = A
 
-/datum/controller/subsystem/antags/Shutdown()
-	for(var/thing in antag_datums)
-		qdel(thing)
-	. = ..()
-
 
 /datum/controller/subsystem/antags/proc/get_antag_id_from_name(var/role_text)
 	for(var/datum/antagonist/A as anything in antag_datums)

code/controllers/subsystems/garbage.dm

@@ -273,71 +273,73 @@ SUBSYSTEM_DEF(garbage)
 
 // Should be treated as a replacement for the 'del' keyword.
 // Datums passed to this will be given a chance to clean up references to allow the GC to collect them.
-/proc/qdel(datum/D, force=FALSE)
+/proc/qdel(datum/thing, force)
-	if(!istype(D))
+	if (!thing)
-		del(D)
 		return
-	var/datum/qdel_item/I = SSgarbage.items[D.type]
+	if (!istype(thing))
-	if (!I)
+		crash_with("qdel() can only handle /datum (sub)types, was passed: [log_info_line(thing)]")
-		I = SSgarbage.items[D.type] = new /datum/qdel_item(D.type)
+		del(thing)
-	I.qdels++
+		return
-
+	var/datum/qdel_item/qdel_item = SSgarbage.items[thing.type]
-
+	if (!qdel_item)
-	if(isnull(D.gc_destroyed))
+		qdel_item = new (thing.type)
-		if(SEND_SIGNAL(D, COMSIG_PARENT_PREQDELETED, force)) // Give the components a chance to prevent their parent from being deleted
+		SSgarbage.items[thing.type] = qdel_item
+	qdel_item.qdels++
+	if (isnull(thing.gc_destroyed))
+		if (SEND_SIGNAL(thing, COMSIG_PARENT_PREQDELETED, force)) // Give the components a chance to prevent their parent from being deleted
 			return
-		D.gc_destroyed = GC_CURRENTLY_BEING_QDELETED
+		thing.gc_destroyed = GC_CURRENTLY_BEING_QDELETED
 		var/start_time = world.time
 		var/start_tick = world.tick_usage
-		SEND_SIGNAL(D, COMSIG_PARENT_QDELETING, force) // Let the (remaining) components know about the result of Destroy
+		SEND_SIGNAL(thing, COMSIG_PARENT_QDELETING, force) // Let the (remaining) components know about the result of Destroy
-		var/hint = D.Destroy(force) // Let our friend know they're about to get fucked up.
+		var/hint = thing.Destroy(force) // Let our friend know they're about to get fucked up.
-		if(world.time != start_time)
+		if (world.time != start_time)
-			I.slept_destroy++
+			qdel_item.slept_destroy++
 		else
-			I.destroy_time += TICK_USAGE_TO_MS(start_tick)
+			qdel_item.destroy_time += TICK_USAGE_TO_MS(start_tick)
-		if(!D)
+		if (!thing)
 			return
-		switch(hint)
+		switch (hint)
 			if (QDEL_HINT_QUEUE)		//qdel should queue the object for deletion.
-				SSgarbage.PreQueue(D)
+				SSgarbage.PreQueue(thing)
 			if (QDEL_HINT_IWILLGC)
-				D.gc_destroyed = world.time
+				thing.gc_destroyed = world.time
 				return
 			if (QDEL_HINT_LETMELIVE)	//qdel should let the object live after calling destory.
 				if(!force)
-					D.gc_destroyed = null //clear the gc variable (important!)
+					thing.gc_destroyed = null //clear the gc variable (important!)
 					return
 				// Returning LETMELIVE after being told to force destroy
 				// indicates the objects Destroy() does not respect force
-				#ifdef TESTING
+#ifdef TESTING
-				if(!I.no_respect_force)
+				if(!qdel_item.no_respect_force)
-					crash_with("[D.type] has been force deleted, but is \
+					crash_with("[thing.type] has been force deleted, but is \
 						returning an immortal QDEL_HINT, indicating it does \
 						not respect the force flag for qdel(). It has been \
 						placed in the queue, further instances of this type \
 						will also be queued.")
-				#endif
+#endif
-				I.no_respect_force++
+				qdel_item.no_respect_force++
-
+				SSgarbage.PreQueue(thing)
-				SSgarbage.PreQueue(D)
 			if (QDEL_HINT_HARDDEL)		//qdel should assume this object won't gc, and queue a hard delete using a hard reference to save time from the locate()
-				SSgarbage.HardQueue(D)
+				SSgarbage.HardQueue(thing)
 			if (QDEL_HINT_HARDDEL_NOW)	//qdel should assume this object won't gc, and hard del it post haste.
-				SSgarbage.HardDelete(D)
+				SSgarbage.HardDelete(thing)
 			if (QDEL_HINT_FINDREFERENCE)//qdel will, if TESTING is enabled, display all references to this object, then queue the object for deletion.
-				SSgarbage.PreQueue(D)
+				SSgarbage.PreQueue(thing)
 				#ifdef TESTING
-				D.find_references()
+				thing.find_references()
 				#endif
 			else
 				#ifdef TESTING
-				if(!I.no_hint)
+				if (!qdel_item.no_hint)
-					crash_with("[D.type] is not returning a qdel hint. It is being placed in the queue. Further instances of this type will also be queued.")
+					crash_with("[thing.type] is not returning a qdel hint. It is being placed in the queue. Further instances of this type will also be queued.")
 				#endif
-				I.no_hint++
+				qdel_item.no_hint++
-				SSgarbage.PreQueue(D)
+				SSgarbage.PreQueue(thing)
-	else if(D.gc_destroyed == GC_CURRENTLY_BEING_QDELETED)
+	else if (thing.gc_destroyed == GC_CURRENTLY_BEING_QDELETED)
-		CRASH("[D.type] destroy proc was called multiple times, likely due to a qdel loop in the Destroy logic")
+		CRASH("[thing.type] destroy proc was called multiple times, likely due to a qdel loop in the Destroy logic")
+
 
 #ifdef TESTING
 

code/datums/weakref.dm

@@ -1,26 +1,30 @@
-//obtain a weak reference to a datum
-/proc/weakref(datum/D)
-	if(!istype(D))
-		return
-	if(QDELETED(D))
-		return
-	if(!D.weakref)
-		D.weakref = new/weakref(D)
-	return D.weakref
-
 /weakref
 	var/ref
+	var/ref_name
+	var/ref_type
 
-/weakref/New(datum/D)
-	ref = "\ref[D]"
 
 /weakref/Destroy()
-	// A weakref datum should not be manually destroyed as it is a shared resource,
+	SHOULD_CALL_PARENT(FALSE)
-	//  rather it should be automatically collected by the BYOND GC when all references are gone.
+	return QDEL_HINT_IWILLGC
-	return QDEL_HINT_LETMELIVE
+
+
+/weakref/New(datum/thing)
+	ref = "\ref[thing]"
+	ref_name = "[thing]"
+	ref_type = thing.type
+
 
 /weakref/proc/resolve()
-	var/datum/D = locate(ref)
+	var/datum/thing = locate(ref)
-	if(D && D.weakref == src)
+	if (thing && thing.weakref == src)
-		return D
+		return thing
 	return null
+
+
+/proc/weakref(datum/thing)
+	if (!istype(thing) || QDELING(thing))
+		return
+	if (!thing.weakref)
+		thing.weakref = new /weakref (thing)
+	return thing.weakref

code/game/objects/items/bodybag.dm

@@ -133,15 +133,19 @@
 	var/stasis_level = 3 //Every 'this' life ticks are applied to the mob (when life_ticks%stasis_level == 1)
 	var/obj/item/reagent_containers/syringe/syringe
 
-/obj/structure/closet/body_bag/cryobag/Initialize()
-	tank = new tank_type(null) //It's in nullspace to prevent ejection when the bag is opened.
-	..()
 
 /obj/structure/closet/body_bag/cryobag/Destroy()
 	QDEL_NULL(syringe)
 	QDEL_NULL(tank)
 	return ..()
 
+
+/obj/structure/closet/body_bag/cryobag/Initialize()
+	. = ..()
+	if (ispath(tank_type, /obj/item/tank))
+		tank = new tank_type // no loc to prevent tank being dropped when opened
+
+
 /obj/structure/closet/body_bag/cryobag/attack_hand(mob/living/user)
 	if(used)
 		var/confirm = alert(user, "Are you sure you want to open \the [src]? \

code/game/objects/items/contraband.dm

@@ -1,24 +1,22 @@
-//Let's get some REAL contraband stuff in here. Because come on, getting brigged for LIPSTICK is no fun.
-//
-// Includes drug powder. 
-//
-//Illicit drugs~
 /obj/item/storage/pill_bottle/happy
 	name = "bottle of Happy pills"
 	desc = "A recreational drug. When you want to see the rainbow. Probably not work-approved..."
 	wrapper_color = COLOR_PINK
 	starts_with = list(/obj/item/reagent_containers/pill/happy = 7)
 
+
 /obj/item/storage/pill_bottle/zoom
 	name = "bottle of Zoom pills"
 	desc = "Probably illegal. Trade brain for speed."
 	wrapper_color = COLOR_BLUE
 	starts_with = list(/obj/item/reagent_containers/pill/zoom = 7)
 
+
 /obj/item/reagent_containers/glass/beaker/vial/random
 	flags = 0
 	var/list/random_reagent_list = list(list("water" = 15) = 1, list("cleaner" = 15) = 1)
 
+
 /obj/item/reagent_containers/glass/beaker/vial/random/toxin
 	random_reagent_list = list(
 		list("mindbreaker" = 10, "bliss" = 20)	= 3,
@@ -26,26 +24,21 @@
 		list("impedrezene" = 15)						= 2,
 		list("zombiepowder" = 10)						= 1)
 
+
 /obj/item/reagent_containers/glass/beaker/vial/random/Initialize()
 	. = ..()
 	if(is_open_container())
 		flags ^= OPENCONTAINER
-
 	var/list/picked_reagents = pickweight(random_reagent_list)
 	for(var/reagent in picked_reagents)
 		reagents.add_reagent(reagent, picked_reagents[reagent])
-
 	var/list/names = new
 	for(var/datum/reagent/R in reagents.reagent_list)
 		names += R.name
-
 	desc = "Contains [english_list(names)]."
 	update_icon()
 
-/*/////////////////////////////////////
+
-// 			DRUG POWDER				//
-////////////////////////////////////// 
-*/
 /obj/item/reagent_containers/powder
 	name = "powder"
 	desc = "A powdered form of... something."
@@ -57,46 +50,47 @@
 	w_class = ITEMSIZE_TINY
 	volume = 50
 
+	/// The name of the reagent with the most volume in this powder.
+	var/main_reagent_name
+
+
+/obj/item/reagent_containers/powder/Initialize(mapload, datum/reagents/initial_reagents)
+	. = ..()
+	if (istype(initial_reagents))
+		initial_reagents.trans_to_holder(reagents, volume)
+	else if (islist(initial_reagents))
+		var/list/initial_reagents_list = initial_reagents
+		for (var/reagent_type in initial_reagents_list)
+			reagents.add_reagent(reagent_type, initial_reagents[reagent_type])
+	if (!reagents.total_volume)
+		return INITIALIZE_HINT_QDEL
+	var/datum/reagent/main_reagent = reagents.get_master_reagent()
+	main_reagent_name = lowertext(main_reagent.name)
+	color = reagents.get_color()
+
+
 /obj/item/reagent_containers/powder/examine(mob/user)
-	if(reagents)
+	. = ..()
-		var/datum/reagent/R = reagents.get_master_reagent()
+	if (isliving(user) && get_dist(user, src) > 2)
-		desc = "A powdered form of what appears to be [R.name]. There's about [reagents.total_volume] units here."
+		return
-	return ..()
+	. += "It seems to be about [reagents.total_volume] units of [main_reagent_name]."
 
-/obj/item/reagent_containers/powder/Initialize()
-	..()
-	get_appearance()
 
-/obj/item/reagent_containers/powder/proc/get_appearance()
+/obj/item/reagent_containers/powder/attackby(obj/item/item, mob/living/user)
-	/// Names and colors based on dominant reagent. 
+	if (!ishuman(user))
-	if (reagents.reagent_list.len > 0)
-		color = reagents.get_color()
-		var/datum/reagent/R = reagents.get_master_reagent()
-		var/new_name = lowertext(R)
-		name = "powdered [new_name]"
-
-/// Snorting. 
-
-/obj/item/reagent_containers/powder/attackby(var/obj/item/W, var/mob/living/user)
-
-	if(!ishuman(user)) /// You gotta be fleshy to snort the naughty drugs. 
 		return ..()
-
+	if (!istype(item, /obj/item/glass_extra/straw) && !istype(item, /obj/item/reagent_containers/rollingpaper))
-	if(!istype(W, /obj/item/glass_extra/straw) && !istype(W, /obj/item/reagent_containers/rollingpaper))
 		return ..()
-
+	reagents.trans_to_mob(user, amount_per_transfer_from_this, CHEM_BLOOD)
-	user.visible_message("<span class='warning'>[user] snorts [src] with [W]!</span>")
+	var/used_up = !reagents.total_volume
+	user.visible_message(
+		SPAN_ITALIC("\The [user] snorts some [name] with \a [item]."),
+		SPAN_ITALIC("You snort [used_up ? "the last" : "some"] of the [main_reagent_name] with \the [item].")
+	)
 	playsound(loc, 'sound/effects/snort.ogg', 50, 1)
-
+	if (used_up)
-	if(reagents)
-		reagents.trans_to_mob(user, amount_per_transfer_from_this, CHEM_BLOOD)
-
-	if(!reagents.total_volume) /// Did we use all of it? 
 		qdel(src)
 
-////// End powder. ///////////
-//////////////////////////////
-///// Drugs for loadout///////
 
 /obj/item/storage/pill_bottle/bliss
 	name = "unlabeled pill bottle"

code/modules/client/preference_setup/general/03_body.dm

@@ -730,6 +730,24 @@ var/global/list/valid_bloodtypes = list("A+", "A-", "B+", "B-", "AB+", "AB-", "O
 			pref.h_style = new_h_style
 			return TOPIC_REFRESH_UPDATE_PREVIEW
 
+	else if (href_list["hair_style_left"])
+		var/list/valid_hairstyles = pref.get_valid_hairstyles()
+		var/index = valid_hairstyles.Find(href_list["hair_style_left"])
+		if (!index || index == 1)
+			pref.h_style = valid_hairstyles[length(valid_hairstyles)]
+		else
+			pref.h_style = valid_hairstyles[index - 1]
+		return TOPIC_REFRESH_UPDATE_PREVIEW
+
+	else if (href_list["hair_style_right"])
+		var/list/valid_hairstyles = pref.get_valid_hairstyles()
+		var/index = valid_hairstyles.Find(href_list["hair_style_right"])
+		if (!index || index == length(valid_hairstyles))
+			pref.h_style = valid_hairstyles[1]
+		else
+			pref.h_style = valid_hairstyles[index + 1]
+		return TOPIC_REFRESH_UPDATE_PREVIEW
+
 	else if(href_list["grad_style"])
 		var/list/valid_gradients = GLOB.hair_gradients
 
@@ -738,26 +756,22 @@ var/global/list/valid_bloodtypes = list("A+", "A-", "B+", "B-", "AB+", "AB-", "O
 			pref.grad_style = new_grad_style
 			return TOPIC_REFRESH_UPDATE_PREVIEW
 
-	else if(href_list["hair_style_left"])
+	else if (href_list["grad_style_left"])
-		var/H = href_list["hair_style_left"]
+		var/list/valid_hair_gradients = GLOB.hair_gradients
-		var/list/valid_hairstyles = pref.get_valid_hairstyles()
+		var/index = valid_hair_gradients.Find(href_list["grad_style_left"])
-		var/start = valid_hairstyles.Find(H)
+		if (!index || index == 1)
-
+			pref.grad_style = valid_hair_gradients[length(valid_hair_gradients)]
-		if(start != 1) //If we're not the beginning of the list, become the previous element.
+		else
-			pref.h_style = valid_hairstyles[start-1]
+			pref.grad_style = valid_hair_gradients[index - 1]
-		else //But if we ARE, become the final element.
-			pref.h_style = valid_hairstyles[valid_hairstyles.len]
 		return TOPIC_REFRESH_UPDATE_PREVIEW
 
-	else if(href_list["hair_style_right"])
+	else if (href_list["grad_style_right"])
-		var/H = href_list["hair_style_right"]
+		var/list/valid_hair_gradients = GLOB.hair_gradients
-		var/list/valid_hairstyles = pref.get_valid_hairstyles()
+		var/index = valid_hair_gradients.Find(href_list["grad_style_right"])
-		var/start = valid_hairstyles.Find(H)
+		if (!index || index == length(valid_hair_gradients))
-
+			pref.grad_style = valid_hair_gradients[1]
-		if(start != valid_hairstyles.len) //If we're not the end of the list, become the next element.
+		else
-			pref.h_style = valid_hairstyles[start+1]
+			pref.grad_style = valid_hair_gradients[index + 1]
-		else //But if we ARE, become the first element.
-			pref.h_style = valid_hairstyles[1]
 		return TOPIC_REFRESH_UPDATE_PREVIEW
 
 	else if(href_list["facial_color"])
@@ -806,26 +820,22 @@ var/global/list/valid_bloodtypes = list("A+", "A-", "B+", "B-", "AB+", "AB-", "O
 			pref.f_style = new_f_style
 			return TOPIC_REFRESH_UPDATE_PREVIEW
 
-	else if(href_list["facial_style_left"])
+	else if (href_list["facial_style_left"])
-		var/F = href_list["facial_style_left"]
 		var/list/valid_facialhairstyles = pref.get_valid_facialhairstyles()
-		var/start = valid_facialhairstyles.Find(F)
+		var/index = valid_facialhairstyles.Find(href_list["facial_style_left"])
-
+		if (!index || index == 1)
-		if(start != 1) //If we're not the beginning of the list, become the previous element.
+			pref.f_style = valid_facialhairstyles[length(valid_facialhairstyles)]
-			pref.f_style = valid_facialhairstyles[start-1]
+		else
-		else //But if we ARE, become the final element.
+			pref.f_style = valid_facialhairstyles[index - 1]
-			pref.f_style = valid_facialhairstyles[valid_facialhairstyles.len]
 		return TOPIC_REFRESH_UPDATE_PREVIEW
 
-	else if(href_list["facial_style_right"])
+	else if (href_list["facial_style_right"])
-		var/F = href_list["facial_style_right"]
 		var/list/valid_facialhairstyles = pref.get_valid_facialhairstyles()
-		var/start = valid_facialhairstyles.Find(F)
+		var/index = valid_facialhairstyles.Find(href_list["facial_style_right"])
-
+		if (!index || index == length(valid_facialhairstyles))
-		if(start != valid_facialhairstyles.len) //If we're not the end of the list, become the next element.
-			pref.f_style = valid_facialhairstyles[start+1]
-		else //But if we ARE, become the first element.
 			pref.f_style = valid_facialhairstyles[1]
+		else
+			pref.f_style = valid_facialhairstyles[index + 1]
 		return TOPIC_REFRESH_UPDATE_PREVIEW
 
 	else if(href_list["marking_style"])

code/modules/reagents/reagent_containers/pill.dm

@@ -91,27 +91,16 @@
 
 	return
 
-/obj/item/reagent_containers/pill/attackby(obj/item/W as obj, mob/user as mob)
+/obj/item/reagent_containers/pill/attackby(obj/item/item, mob/living/user)
-	if(is_sharp(W))
+	if (is_sharp(item) || istype(item, /obj/item/card))
-		var/obj/item/reagent_containers/powder/J = new /obj/item/reagent_containers/powder(src.loc)
+		user.visible_message(
-		user.visible_message("<span class='warning'>[user] gently cuts up [src] with [W]!</span>")
+			SPAN_ITALIC("\The [user] cuts up \a [src] with \a [item]."),
-		playsound(src.loc, 'sound/effects/chop.ogg', 50, 1)
+			SPAN_ITALIC("You cut up \the [src] with \the [item].")
-
+		)
-		if(reagents)
+		playsound(loc, 'sound/effects/chop.ogg', 50, 1)
-			reagents.trans_to_obj(J, reagents.total_volume)
+		new /obj/item/reagent_containers/powder (loc, reagents)
-		J.get_appearance()
 		qdel(src)
-
+		return TRUE
-	if(istype(W, /obj/item/card/id))
-		var/obj/item/reagent_containers/powder/J = new /obj/item/reagent_containers/powder(src.loc)
-		user.visible_message("<span class='warning'>[user] clumsily chops up [src] with [W]!</span>")
-		playsound(src.loc, 'sound/effects/chop.ogg', 50, 1)
-
-		if(reagents)
-			reagents.trans_to_obj(J, reagents.total_volume)
-		J.get_appearance()
-		qdel(src)
-
 	return ..()
 
 ////////////////////////////////////////////////////////////////////////////////

code/unit_tests/subsystem_tests.dm

@@ -35,7 +35,7 @@
 	var/fail = FALSE
 	for(var/atom/atom in world)
 		if(!atom.initialized && !QDELETED(atom)) // Not ideal to skip over qdeleted atoms, but a lot of current code uses pre-init qdels
-			log_bad("Uninitialized atom: [atom.type] - [atom.log_info_line()]")
+			log_bad("Uninitialized atom: [atom.type] - [atom.get_log_info_line()]")
 			fail = TRUE
 	if(fail)
 		fail("There were uninitialized atoms.")