[MIRROR] Adds a unit test to stop elements from using identical lists for their arguments. [MDB IGNORE] (#22732)

* Adds a unit test to stop elements from using identical lists for their arguments. (#76322)

## About The Pull Request
Ok, so a few days ago I made an issue report about multiple instances of
identical elements being generated because of uncached lists.
ninjanomnom (the mind being the element datums) cleared it up and said
an implementation of GetIdFromArguments() that also checks the list
contents wouldn't be worth the performance cost, while adding that a
unit test should be written to check that it doesn't happen at least
during init, which should catch a good chunk of cases.

Also, i'm stopping RemoveElement() from initializing new elements
whenever a cached element is not found. Ideally, there should be a focus
only unit test for that too, but that's something we should tackle on a
different PR.

Some of the code comments may be a tad inaccurate, as much as I'd like
to blame drowsiness for it. Regardless, the unit test takes less than
0.2 seconds to complete on my potato so it's fairly lite.

## Why It's Good For The Game
This will close #76279.

## Changelog
No player-facing change to be logged.

* Adds a unit test to stop elements from using identical lists for their arguments.

* Fixes unit test

* seeing double

---------

Co-authored-by: Ghom <42542238+Ghommie@users.noreply.github.com>
Co-authored-by: Giz <13398309+vinylspiders@users.noreply.github.com>

_maps/RandomZLevels/SnowCabin.dmm

@@ -4907,19 +4907,7 @@
 	desc = "It looks like fancy glitter to me.";
 	name = "icy wind"
 	},
-/mob/living/basic/statue{
-	desc = "Just a snowman. Just a snowman. Oh god, it's just a snowman.";
-	faction = list("statue","mining");
-	health = 5000;
-	icon_dead = "snowman";
-	icon_living = "snowman";
-	icon_state = "snowman";
-	loot = list(/obj/item/dnainjector/geladikinesis);
-	maxHealth = 5000;
-	melee_damage_lower = 65;
-	melee_damage_upper = 65;
-	name = "Frosty"
-	},
+/mob/living/basic/statue/frosty,
 /turf/open/misc/asteroid/snow/snow_cabin,
 /area/awaymission/cabin/caves)
 "RL" = (

code/__DEFINES/dcs/flags.dm

@@ -22,6 +22,13 @@
 /// Causes all detach arguments to be passed to detach instead of only being used to identify the element
 /// When this is used your Detach proc should have the same signature as your Attach proc
 #define ELEMENT_COMPLEX_DETACH (1 << 2)
+/**
+ * Stops lists used as arguments for the element from being sorted by the dcs_check_list_arguments unit test.
+ * For when changing the position of the keys is undesiderable, like for color matrices.
+ */
+#define ELEMENT_DONT_SORT_LIST_ARGS (1<<3)
+/// Elements with this flag will be ignored by the test (I would rather put some faith than have contributors stringify connect loc lists).
+#define ELEMENT_NO_LIST_UNIT_TEST (1<<4)
 
 // How multiple components of the exact same type are handled in the same datum
 /// old component is deleted (default)

code/__HELPERS/cmp.dm

@@ -10,6 +10,20 @@
 /proc/cmp_text_dsc(a,b)
 	return sorttext(a,b)
 
+/proc/cmp_embed_text_asc(a,b)
+	if(isdatum(a))
+		a = REF(a)
+	if(isdatum(b))
+		b = REF(b)
+	return sorttext("[b]", "[a]")
+
+/proc/cmp_embed_text_dsc(a,b)
+	if(isdatum(a))
+		a = REF(a)
+	if(isdatum(b))
+		b = REF(b)
+	return sorttext("[a]", "[b]")
+
 /proc/cmp_name_asc(atom/a, atom/b)
 	return sorttext(b.name, a.name)
 

code/__HELPERS/string_assoc_lists.dm

@@ -12,6 +12,6 @@ GLOBAL_LIST_EMPTY(string_assoc_lists)
 	. = GLOB.string_assoc_lists[string_id]
 
 	if(.)
-		return
+		return .
 
 	return GLOB.string_assoc_lists[string_id] = values

code/__HELPERS/string_assoc_nested_lists.dm

@@ -0,0 +1,18 @@
+GLOBAL_LIST_EMPTY(string_assoc_nested_lists)
+
+/**
+ * Caches associative nested lists with non-numeric stringify-able index keys and stringify-able values (text/typepath -> text/path/number).
+ */
+/datum/proc/string_assoc_nested_list(list/list)
+	var/list/string_id = list()
+	for(var/key in list)
+		var/assoc = list[key]
+		string_id += "[key]_[islist(assoc) ? "ASSLIST([string_assoc_nested_list(assoc)])" : assoc]"
+	string_id = string_id.Join("-")
+
+	. = GLOB.string_assoc_lists[string_id]
+
+	if(.)
+		return .
+
+	return GLOB.string_assoc_lists[string_id] = list

code/__HELPERS/string_lists.dm

@@ -9,7 +9,7 @@ GLOBAL_LIST_EMPTY(string_lists)
 	. = GLOB.string_lists[string_id]
 
 	if(.)
-		return
+		return .
 
 	return GLOB.string_lists[string_id] = values
 

code/__HELPERS/string_numbers_lists.dm

@@ -0,0 +1,19 @@
+GLOBAL_LIST_EMPTY(string_numbers_lists)
+
+/**
+ * Caches lists of numeric values.
+ */
+/datum/proc/string_numbers_list(list/values)
+	//Just to to be extra-safe. If you try to shove in text or paths, you deserve the runtime errors.
+	var/list/sum = 0
+	for(var/number in values)
+		sum += number
+
+	var/string_id = values.Join("-")
+
+	. = GLOB.string_numbers_lists[string_id]
+
+	if(.)
+		return .
+
+	return GLOB.string_numbers_lists[string_id] = values

code/controllers/subsystem/dcs.dm

@@ -5,10 +5,34 @@ PROCESSING_SUBSYSTEM_DEF(dcs)
 
 	var/list/elements_by_type = list()
 
+	/**
+	 * A nested assoc list of bespoke element types (keys) and superlists containing all lists used as arguments (values).
+	 * Inside the superlists, lists that've been sorted alphabetically are keys, while the original unsorted lists are values.
+	 *
+	 * e.g. list(
+	 *	/datum/element/first = list(list(A, B, C) = list(B, A, C), list(A, B) = list(A, B)),
+	 *	/datum/element/second = list(list(B, C) = list(C, B), list(D) = list(D)),
+	 * )
+	 *
+	 * Used by the dcs_check_list_arguments unit test.
+	 */
+	var/list/arguments_that_are_lists_by_element = list()
+	/**
+	 * An assoc list of list instances and their sorted counterparts.
+	 *
+	 * e.g. list(
+	 *	list(B, A, C) = list(A, B, C),
+	 *	list(C, B) = list(B, C),
+	 * )
+	 *
+	 * Used to make sure each list instance is sorted no more than once, or the unit test won't work.
+	 */
+	var/list/sorted_arguments_that_are_lists = list()
+
 /datum/controller/subsystem/processing/dcs/Recover()
 	_listen_lookup = SSdcs._listen_lookup
 
-/datum/controller/subsystem/processing/dcs/proc/GetElement(list/arguments)
+/datum/controller/subsystem/processing/dcs/proc/GetElement(list/arguments, init_element = TRUE)
 	var/datum/element/eletype = arguments[1]
 	var/element_id = eletype
 
@@ -19,7 +43,7 @@ PROCESSING_SUBSYSTEM_DEF(dcs)
 		element_id = GetIdFromArguments(arguments)
 
 	. = elements_by_type[element_id]
-	if(.)
+	if(. || !init_element)
 		return
 	. = elements_by_type[element_id] = new eletype
 
@@ -33,7 +57,6 @@ PROCESSING_SUBSYSTEM_DEF(dcs)
 	var/datum/element/eletype = arguments[1]
 	var/list/fullid = list("[eletype]")
 	var/list/named_arguments = list()
-
 	for(var/i in initial(eletype.argument_hash_start_idx) to length(arguments))
 		var/key = arguments[i]
 
@@ -43,14 +66,17 @@ PROCESSING_SUBSYSTEM_DEF(dcs)
 				fullid += key
 			else
 				if (!istext(value) && !isnum(value))
+					if(PERFORM_ALL_TESTS(dcs_check_list_arguments) && islist(value))
+						add_to_arguments_that_are_lists(value, eletype)
 					value = REF(value)
 				named_arguments[key] = value
-
 			continue
 
 		if (isnum(key))
 			fullid += "[key]"
 		else
+			if(PERFORM_ALL_TESTS(dcs_check_list_arguments) && islist(key))
+				add_to_arguments_that_are_lists(key, eletype)
 			fullid += REF(key)
 
 	if(length(named_arguments))
@@ -58,3 +84,22 @@ PROCESSING_SUBSYSTEM_DEF(dcs)
 		fullid += named_arguments
 
 	return list2params(fullid)
+
+/**
+ * Offloading the first half of the dcs_check_list_arguments here, which is populating the superlist
+ * with sublists that will be later compared with each other by the dcs_check_list_arguments unit test.
+ */
+/datum/controller/subsystem/processing/dcs/proc/add_to_arguments_that_are_lists(list/argument, datum/element/element_type)
+	if(initial(element_type.element_flags) & ELEMENT_NO_LIST_UNIT_TEST)
+		return
+	var/list/element_type_superlist = arguments_that_are_lists_by_element[element_type]
+	if(!element_type_superlist)
+		arguments_that_are_lists_by_element[element_type] = element_type_superlist = list()
+
+	var/list/sorted_argument = argument
+	if(!(initial(element_type.element_flags) & ELEMENT_DONT_SORT_LIST_ARGS))
+		sorted_argument = sorted_arguments_that_are_lists[argument]
+		if(!sorted_argument)
+			sorted_arguments_that_are_lists[argument] = sorted_argument = sortTim(argument.Copy(), GLOBAL_PROC_REF(cmp_embed_text_asc))
+
+	element_type_superlist[sorted_argument] = argument

code/datums/ai/monkey/monkey_controller.dm

@@ -30,9 +30,10 @@ have ways of interacting with a specific mob and control it.
 	idle_behavior = /datum/idle_behavior/idle_monkey
 
 /datum/ai_controller/monkey/New(atom/new_pawn)
-	AddElement(/datum/element/ai_control_examine, list(
+	var/static/list/control_examine = list(
 		ORGAN_SLOT_EYES = span_monkey("eyes have a primal look in them."),
-	))
+	)
+	AddElement(/datum/element/ai_control_examine, control_examine)
 	return ..()
 
 /datum/ai_controller/monkey/pun_pun

code/datums/elements/_element.dm

@@ -64,9 +64,9 @@
  * You only need additional arguments beyond the type if you're using [ELEMENT_BESPOKE]
  */
 /datum/proc/_RemoveElement(list/arguments)
-	var/datum/element/ele = SSdcs.GetElement(arguments)
-	if(!ele) // We couldn't fetch the element, likely because it was not an element.
-		return // the crash message has already been sent
+	var/datum/element/ele = SSdcs.GetElement(arguments, FALSE)
+	if(!ele) // We couldn't fetch the element, likely because it didn't exist.
+		return
 	if(ele.element_flags & ELEMENT_COMPLEX_DETACH)
 		arguments[1] = src
 		ele.Detach(arglist(arguments))

code/datums/elements/connect_loc.dm

@@ -1,7 +1,7 @@
 /// This element hooks a signal onto the loc the current object is on.
 /// When the object moves, it will unhook the signal and rehook it to the new object.
 /datum/element/connect_loc
-	element_flags = ELEMENT_BESPOKE
+	element_flags = ELEMENT_BESPOKE|ELEMENT_NO_LIST_UNIT_TEST
 	argument_hash_start_idx = 2
 
 	/// An assoc list of signal -> procpath to register to the loc this object is on.

code/datums/elements/decals/_decal.dm

@@ -1,5 +1,5 @@
 /datum/element/decal
-	element_flags = ELEMENT_BESPOKE|ELEMENT_DETACH_ON_HOST_DESTROY
+	element_flags = ELEMENT_BESPOKE|ELEMENT_DETACH_ON_HOST_DESTROY|ELEMENT_DONT_SORT_LIST_ARGS
 	argument_hash_start_idx = 2
 	/// Whether this decal can be cleaned.
 	var/cleanable

code/datums/elements/strippable.dm

@@ -14,7 +14,7 @@
 	/// An existing strip menus
 	var/list/strip_menus
 
-/datum/element/strippable/Attach(datum/target, list/items, should_strip_proc_path)
+/datum/element/strippable/Attach(datum/target, list/items = list(), should_strip_proc_path)
 	. = ..()
 	if (!isatom(target))
 		return ELEMENT_INCOMPATIBLE

code/game/machinery/firealarm.dm

@@ -74,13 +74,12 @@
 		), \
 	)
 
-	AddElement( \
-		/datum/element/contextual_screentip_mob_typechecks, \
-		list(/mob/living/silicon = list( \
-				SCREENTIP_CONTEXT_CTRL_LMB = "Toggle thermal sensors, which control auto-deploy" \
-			) \
-		) \
+	var/static/list/hovering_mob_typechecks = list(
+		/mob/living/silicon = list(
+			SCREENTIP_CONTEXT_CTRL_LMB = "Toggle thermal sensors, which control auto-deploy",
+		)
 	)
+	AddElement(/datum/element/contextual_screentip_mob_typechecks, hovering_mob_typechecks)
 
 	update_appearance()
 

code/game/machinery/hologram.dm

@@ -103,13 +103,12 @@ Possible to do for anyone motivated enough:
 	SET_PLANE_IMPLICIT(src, FLOOR_PLANE)
 	update_appearance()
 
-	AddElement( \
-		/datum/element/contextual_screentip_mob_typechecks, \
-		list(/mob/living/silicon = list( \
-				SCREENTIP_CONTEXT_ALT_LMB = "Disconnect all active calls" \
-			) \
-		) \
+	var/static/list/hovering_mob_typechecks = list(
+		/mob/living/silicon = list(
+			SCREENTIP_CONTEXT_ALT_LMB = "Disconnect all active calls",
+		)
 	)
+	AddElement(/datum/element/contextual_screentip_mob_typechecks, hovering_mob_typechecks)
 
 /obj/machinery/holopad/secure
 	name = "secure holopad"

code/game/objects/items/stacks/sheets/sheet_types.dm

@@ -164,16 +164,6 @@ GLOBAL_LIST_INIT(metal_recipes, list ( \
 	)
 	AddElement(/datum/element/contextual_screentip_tools, tool_behaviors)
 
-/obj/item/stack/sheet/iron/Initialize(mapload)
-	. = ..()
-	var/static/list/tool_behaviors = list(
-		TOOL_WELDER = list(
-			SCREENTIP_CONTEXT_LMB = "Craft iron rods",
-			SCREENTIP_CONTEXT_RMB = "Craft floor tiles",
-		),
-	)
-	AddElement(/datum/element/contextual_screentip_tools, tool_behaviors)
-
 /obj/item/stack/sheet/iron/examine(mob/user)
 	. = ..()
 	. += span_notice("You can build a wall girder (unanchored) by right clicking on an empty floor.")

code/game/objects/structures/gym/punching_bag.dm

@@ -24,15 +24,17 @@
 		lmb_text = "Punch", \
 	)
 
-	var/static/list/tool_behaviors = list(
-		TOOL_CROWBAR = list(
-			SCREENTIP_CONTEXT_RMB = "Deconstruct",
-		),
+	var/static/list/tool_behaviors
+	if(!tool_behaviors)
+		tool_behaviors = string_assoc_nested_list(list(
+			TOOL_CROWBAR = list(
+				SCREENTIP_CONTEXT_RMB = "Deconstruct",
+			),
 
-		TOOL_WRENCH = list(
-			SCREENTIP_CONTEXT_RMB = "Anchor",
-		),
-	)
+			TOOL_WRENCH = list(
+				SCREENTIP_CONTEXT_RMB = "Anchor",
+			),
+		))
 	AddElement(/datum/element/contextual_screentip_tools, tool_behaviors)
 
 /obj/structure/punching_bag/attack_hand(mob/living/user, list/modifiers)

code/game/objects/structures/gym/weight_machine.dm

@@ -44,15 +44,17 @@
 	weight_action = new(src)
 	weight_action.weightpress = src
 
-	var/static/list/tool_behaviors = list(
-		TOOL_CROWBAR = list(
-			SCREENTIP_CONTEXT_RMB = "Deconstruct",
-		),
+	var/static/list/tool_behaviors
+	if(!tool_behaviors)
+		tool_behaviors = string_assoc_nested_list(list(
+			TOOL_CROWBAR = list(
+				SCREENTIP_CONTEXT_RMB = "Deconstruct",
+			),
 
-		TOOL_WRENCH = list(
-			SCREENTIP_CONTEXT_RMB = "Anchor",
-		),
-	)
+			TOOL_WRENCH = list(
+				SCREENTIP_CONTEXT_RMB = "Anchor",
+			),
+		))
 	AddElement(/datum/element/contextual_screentip_tools, tool_behaviors)
 
 /obj/structure/weightmachine/Destroy()

code/game/turfs/open/floor/fancy_floor.dm

@@ -470,7 +470,7 @@
 /turf/open/floor/carpet/neon/Initialize(mapload)
 	. = ..()
 	AddElement(/datum/element/decal, neon_icon || icon, neon_icon_state || base_icon_state, dir, null, null, alpha, neon_color, smoothing_junction)
-	AddElement(/datum/element/decal, neon_icon || icon, neon_icon_state || base_icon_state, dir, EMISSIVE_PLANE, null, emissive_alpha, EMISSIVE_COLOR, smoothing_junction)
+	AddElement(/datum/element/decal, neon_icon || icon, neon_icon_state || base_icon_state, dir, EMISSIVE_PLANE, null, emissive_alpha, GLOB.emissive_color, smoothing_junction)
 
 /turf/open/floor/carpet/neon/simple
 	name = "simple neon carpet"

code/modules/atmospherics/machinery/components/unary_devices/outlet_injector.dm

@@ -22,11 +22,13 @@
 		id_tag = assign_random_name()
 	. = ..()
 
-	var/static/list/tool_screentips = list(
-		TOOL_MULTITOOL = list(
-			SCREENTIP_CONTEXT_LMB = "Log to link later with air sensor",
-		)
-	)
+	var/static/list/tool_screentips
+	if(!tool_screentips)
+		tool_screentips = string_assoc_nested_list(list(
+			TOOL_MULTITOOL = list(
+				SCREENTIP_CONTEXT_LMB = "Log to link later with air sensor",
+			)
+		))
 	AddElement(/datum/element/contextual_screentip_tools, tool_screentips)
 	register_context()
 

code/modules/atmospherics/machinery/components/unary_devices/vent_pump.dm

@@ -37,11 +37,13 @@
 /obj/machinery/atmospherics/components/unary/vent_pump/Initialize(mapload)
 	if(!id_tag)
 		id_tag = assign_random_name()
-		var/static/list/tool_screentips = list(
-			TOOL_MULTITOOL = list(
-				SCREENTIP_CONTEXT_LMB = "Log to link later with air sensor",
-			)
-		)
+		var/static/list/tool_screentips
+		if(!tool_screentips)
+			tool_screentips = string_assoc_nested_list(list(
+				TOOL_MULTITOOL = list(
+					SCREENTIP_CONTEXT_LMB = "Log to link later with air sensor",
+				)
+			))
 		AddElement(/datum/element/contextual_screentip_tools, tool_screentips)
 	. = ..()
 	assign_to_area()

code/modules/hydroponics/unique_plant_genes.dm

@@ -181,7 +181,10 @@
 	. = ..()
 	if(!.)
 		return
-
+	if(genes_to_check)
+		genes_to_check = string_list(genes_to_check)
+	if(traits_to_check)
+		traits_to_check = string_list(traits_to_check)
 	our_plant.AddElement(/datum/element/plant_backfire, cancel_action_on_backfire, traits_to_check, genes_to_check)
 	RegisterSignal(our_plant, COMSIG_PLANT_ON_BACKFIRE, PROC_REF(on_backfire))
 

code/modules/mob/living/basic/farm_animals/bee/_bee.dm

@@ -177,7 +177,9 @@
 /mob/living/basic/bee/proc/assign_reagent(datum/reagent/toxin)
 	if(!istype(toxin))
 		return
-	var/static/list/injection_range = list(1, 5)
+	var/static/list/injection_range
+	if(!injection_range)
+		injection_range = string_numbers_list(list(1, 5))
 	if(beegent) //clear the old since this one is going to have some new value
 		RemoveElement(/datum/element/venomous, beegent.type, injection_range)
 	beegent = toxin

code/modules/mob/living/basic/festivus_pole.dm

@@ -42,7 +42,8 @@
 
 /mob/living/basic/festivus/Initialize(mapload)
 	. = ..()
-	AddElement(/datum/element/death_drops, list(/obj/item/stack/rods))
+	var/static/list/death_loot = list(/obj/item/stack/rods)
+	AddElement(/datum/element/death_drops, death_loot)
 	AddComponent(/datum/component/aggro_emote, emote_list = string_list(list("growls")), emote_chance = 20)
 	var/datum/action/cooldown/mob_cooldown/charge_apc/charge_ability = new(src)
 	charge_ability.Grant(src)

code/modules/mob/living/basic/heretic/fire_shark.dm

@@ -31,7 +31,10 @@
 
 /mob/living/basic/fire_shark/Initialize(mapload)
 	. = ..()
-	AddElement(/datum/element/death_drops, list(/obj/effect/gibspawner/human))
+	var/static/list/death_loot
+	if(!death_loot)
+		death_loot = string_list(list(/obj/effect/gibspawner/human))
+	AddElement(/datum/element/death_drops, death_loot)
 	AddElement(/datum/element/death_gases, /datum/gas/plasma, 40)
 	AddElement(/datum/element/simple_flying)
 	AddElement(/datum/element/venomous, /datum/reagent/phlogiston, 2)

code/modules/mob/living/basic/heretic/star_gazer.dm

@@ -47,7 +47,8 @@
 
 /mob/living/basic/star_gazer/Initialize(mapload)
 	. = ..()
-	AddElement(/datum/element/death_drops, list(/obj/effect/temp_visual/cosmic_domain))
+	var/static/list/death_loot = list(/obj/effect/temp_visual/cosmic_domain)
+	AddElement(/datum/element/death_drops, death_loot)
 	AddElement(/datum/element/death_explosion, 3, 6, 12)
 	AddElement(/datum/element/footstep, FOOTSTEP_MOB_SHOE)
 	AddElement(/datum/element/wall_smasher, ENVIRONMENT_SMASH_RWALLS)

code/modules/mob/living/basic/lavaland/mining.dm

@@ -17,11 +17,14 @@
 	. = ..()
 	add_traits(list(TRAIT_LAVA_IMMUNE, TRAIT_ASHSTORM_IMMUNE), INNATE_TRAIT)
 	AddElement(/datum/element/mob_killed_tally, "mobs_killed_mining")
+	var/static/list/vulnerable_projectiles
+	if(!vulnerable_projectiles)
+		vulnerable_projectiles = string_list(MINING_MOB_PROJECTILE_VULNERABILITY)
 	AddElement(\
 		/datum/element/ranged_armour,\
 		minimum_projectile_force = 30,\
 		below_projectile_multiplier = 0.3,\
-		vulnerable_projectile_types = MINING_MOB_PROJECTILE_VULNERABILITY,\
+		vulnerable_projectile_types = vulnerable_projectiles,\
 		minimum_thrown_force = 20,\
 		throw_blocked_message = throw_blocked_message,\
 	)

code/modules/mob/living/basic/space_fauna/garden_gnome.dm

@@ -103,7 +103,8 @@
 	var/datum/callback/retaliate_callback = CALLBACK(src, PROC_REF(ai_retaliate_behaviour))
 	chosen_hat_colour = pick_weight(gnome_hat_colours)
 	apply_colour()
-	AddElement(/datum/element/death_drops, list(/obj/effect/gibspawner/generic))
+	var/static/list/death_loot = list(/obj/effect/gibspawner/generic)
+	AddElement(/datum/element/death_drops, death_loot)
 	AddElement(/datum/element/footstep, FOOTSTEP_MOB_SHOE)
 	AddComponent(/datum/component/ai_retaliate_advanced, retaliate_callback)
 	AddComponent(/datum/component/swarming)

code/modules/mob/living/basic/space_fauna/meteor_heart/meteor_heart.dm

@@ -35,7 +35,8 @@
 /mob/living/basic/meteor_heart/Initialize(mapload)
 	. = ..()
 	ADD_TRAIT(src, TRAIT_IMMOBILIZED, INNATE_TRAIT)
-	AddElement(/datum/element/death_drops, list(/obj/effect/temp_visual/meteor_heart_death))
+	var/static/list/death_loot = list(/obj/effect/temp_visual/meteor_heart_death)
+	AddElement(/datum/element/death_drops, death_loot)
 	AddElement(/datum/element/relay_attackers)
 
 	spikes = new(src)

code/modules/mob/living/basic/space_fauna/statue/statue.dm

@@ -50,15 +50,11 @@
 	pull_force = MOVE_FORCE_EXTREMELY_STRONG
 
 	ai_controller = /datum/ai_controller/basic_controller/statue
-	/// Loot this mob drops on death.
-	var/loot
 	/// Stores the creator in here if it has one.
 	var/mob/living/creator = null
 
 /mob/living/basic/statue/Initialize(mapload, mob/living/creator)
 	. = ..()
-	if(LAZYLEN(loot))
-		AddElement(/datum/element/death_drops, loot)
 	AddComponent(/datum/component/unobserved_actor, unobserved_flags = NO_OBSERVED_MOVEMENT | NO_OBSERVED_ATTACKS)
 	ADD_TRAIT(src, TRAIT_UNOBSERVANT, INNATE_TRAIT)
 
@@ -160,3 +156,20 @@
 
 /datum/ai_behavior/basic_melee_attack/statue
 	action_cooldown = 1 SECONDS
+
+/mob/living/basic/statue/frosty
+	name = "Frosty"
+	desc = "Just a snowman. Just a snowman. Oh god, it's just a snowman."
+	icon_dead = "snowman"
+	icon_living = "snowman"
+	icon_state = "snowman"
+	health = 5000
+	maxHealth = 5000
+	melee_damage_lower = 65
+	melee_damage_upper = 65
+	faction = list("statue","mining")
+
+/mob/living/basic/statue/frosty/Initialize(mapload)
+	. = ..()
+	var/static/list/death_loot = list(/obj/item/dnainjector/geladikinesis)
+	AddElement(/datum/element/death_drops, death_loot)

code/modules/mob/living/basic/space_fauna/wumborian_fugu/wumborian_fugu.dm

@@ -49,7 +49,8 @@
 
 /mob/living/basic/wumborian_fugu/Initialize(mapload)
 	. = ..()
-	AddElement(/datum/element/death_drops, loot = list(/obj/item/fugu_gland))
+	var/static/list/death_loot = list(/obj/item/fugu_gland)
+	AddElement(/datum/element/death_drops, death_loot)
 	add_traits(list(TRAIT_LAVA_IMMUNE, TRAIT_ASHSTORM_IMMUNE), ROUNDSTART_TRAIT)
 	expand = new(src)
 	expand.Grant(src)

code/modules/mob/living/basic/syndicate/syndicate.dm

@@ -34,6 +34,7 @@
 	. = ..()
 	apply_dynamic_human_appearance(src, mob_spawn_path = mob_spawner, r_hand = r_hand, l_hand = l_hand)
 	if(LAZYLEN(loot))
+		loot = string_list(loot)
 		AddElement(/datum/element/death_drops, loot)
 	AddElement(/datum/element/footstep, footstep_type = FOOTSTEP_MOB_SHOE)
 

code/modules/mob/living/basic/tree.dm

@@ -55,7 +55,8 @@
 /mob/living/basic/tree/Initialize(mapload)
 	. = ..()
 	AddElement(/datum/element/swabable, CELL_LINE_TABLE_PINE, CELL_VIRUS_TABLE_GENERIC_MOB, 1, 5)
-	AddElement(/datum/element/death_drops, list(/obj/item/stack/sheet/mineral/wood))
+	var/static/list/death_loot = list(/obj/item/stack/sheet/mineral/wood)
+	AddElement(/datum/element/death_drops, death_loot)
 	AddComponent(/datum/component/aggro_emote, emote_list = string_list(list("growls")), emote_chance = 20)
 
 /mob/living/basic/tree/Life(seconds_per_tick = SSMOBS_DT, times_fired)

code/modules/mob/living/simple_animal/hostile/mining_mobs/mining_mobs.dm

@@ -33,11 +33,14 @@
 	if(crusher_loot)
 		AddElement(/datum/element/crusher_loot, crusher_loot, crusher_drop_mod, del_on_death)
 	AddElement(/datum/element/mob_killed_tally, "mobs_killed_mining")
+	var/static/list/vulnerable_projectiles
+	if(!vulnerable_projectiles)
+		vulnerable_projectiles = string_list(MINING_MOB_PROJECTILE_VULNERABILITY)
 	AddElement(\
 		/datum/element/ranged_armour,\
 		minimum_projectile_force = 30,\
 		below_projectile_multiplier = 0.3,\
-		vulnerable_projectile_types = MINING_MOB_PROJECTILE_VULNERABILITY,\
+		vulnerable_projectile_types = vulnerable_projectiles,\
 		minimum_thrown_force = 20,\
 		throw_blocked_message = throw_message,\
 	)

code/modules/mob/living/simple_animal/hostile/retaliate/clown.dm

@@ -41,7 +41,9 @@
 /mob/living/simple_animal/hostile/retaliate/clown/Initialize(mapload)
 	. = ..()
 	if(attack_reagent)
-		var/static/list/injection_range = list(1, 5)
+		var/static/list/injection_range
+		if(!injection_range)
+			injection_range = string_numbers_list(list(1, 5))
 		AddElement(/datum/element/venomous, attack_reagent, injection_range)
 
 /mob/living/simple_animal/hostile/retaliate/clown/attack_hand(mob/living/carbon/human/user, list/modifiers)

code/modules/unit_tests/_unit_tests.dm

@@ -55,8 +55,13 @@
 #define TEST_DEFAULT 1
 /// After most test steps, used for tests that run long so shorter issues can be noticed faster
 #define TEST_LONGER 10
-/// This must be the last test to run due to the inherent nature of the test iterating every single tangible atom in the game and qdeleting all of them (while taking long sleeps to make sure the garbage collector fires properly) taking a large amount of time.
-#define TEST_CREATE_AND_DESTROY INFINITY
+/// This must be the one of last tests to run due to the inherent nature of the test iterating every single tangible atom in the game and qdeleting all of them (while taking long sleeps to make sure the garbage collector fires properly) taking a large amount of time.
+#define TEST_CREATE_AND_DESTROY 9001
+/**
+ * For tests that rely on create and destroy having iterated through every (tangible) atom so they don't have to do something similar.
+ * Keep in mind tho that create and destroy will absolutely break the test platform, anything that relies on its shape cannot come after it.
+ */
+#define TEST_AFTER_CREATE_AND_DESTROY INFINITY
 
 /// Change color to red on ANSI terminal output, if enabled with -DANSICOLORS.
 #ifdef ANSICOLORS
@@ -115,6 +120,7 @@
 #include "container_sanity.dm"
 #include "crayons.dm"
 #include "create_and_destroy.dm"
+#include "dcs_check_list_arguments.dm"
 #include "dcs_get_id_from_elements.dm"
 #include "designs.dm"
 #include "door_access.dm"

code/modules/unit_tests/create_and_destroy.dm

@@ -1,6 +1,6 @@
 ///Delete one of every type, sleep a while, then check to see if anything has gone fucky
 /datum/unit_test/create_and_destroy
-	//You absolutely must run last
+	//You absolutely must run after (almost) everything else
 	priority = TEST_CREATE_AND_DESTROY
 
 GLOBAL_VAR_INIT(running_create_and_destroy, FALSE)
@@ -156,8 +156,6 @@ GLOBAL_VAR_INIT(running_create_and_destroy, FALSE)
 				qdel(to_kill)
 
 	GLOB.running_create_and_destroy = FALSE
-	//Hell code, we're bound to have ended the round somehow so let's stop if from ending while we work
-	SSticker.delay_end = TRUE
 
 	// Drastically lower the amount of time it takes to GC, since we don't have clients that can hold it up.
 	SSgarbage.collection_timeout[GC_QUEUE_CHECK] = 10 SECONDS
@@ -229,7 +227,6 @@ GLOBAL_VAR_INIT(running_create_and_destroy, FALSE)
 		if(fails & BAD_INIT_SLEPT)
 			TEST_FAIL("[path] slept during Initialize()")
 
-	SSticker.delay_end = FALSE
 	//This shouldn't be needed, but let's be polite
 	SSgarbage.collection_timeout[GC_QUEUE_CHECK] = GC_CHECK_QUEUE
 	SSgarbage.collection_timeout[GC_QUEUE_HARDDELETE] = GC_DEL_QUEUE

code/modules/unit_tests/dcs_check_list_arguments.dm

@@ -0,0 +1,33 @@
+/**
+ * list arguments for bespoke elements are treated just like any other datum: as a text ref in the ID.
+ * Using un-cached lists in AddElement() and RemoveElement() calls will just create new elements over
+ * and over. That's what this unit test is for. It's not a catch-all, but it does a decent job at it.
+ */
+/datum/unit_test/dcs_check_list_arguments
+	/**
+	 * This unit test requires every (tangible) atom to have been created at least once
+	 * so its search is more accurate. That's why it's run after create_and_destroy.
+	 */
+	priority = TEST_AFTER_CREATE_AND_DESTROY
+
+/datum/unit_test/dcs_check_list_arguments/Run()
+	for(var/element_type in SSdcs.arguments_that_are_lists_by_element)
+		// Keeps tracks of the lists that shouldn't be compared with again.
+		var/list/to_ignore = list()
+		var/list/superlist = SSdcs.arguments_that_are_lists_by_element[element_type]
+		for(var/list/current as anything in superlist)
+			to_ignore[current] = TRUE
+			var/list/bad_lists
+			for(var/list/compare as anything in superlist)
+				if(to_ignore[compare])
+					continue
+				if(deep_compare_list(current, compare))
+					if(!bad_lists)
+						bad_lists = list(list(current))
+					bad_lists += list(compare)
+					to_ignore[compare] = TRUE
+			if(bad_lists)
+				//Include the original, unsorted list in the report. It should be easier to find by the contributor.
+				var/list/unsorted_list = superlist[current]
+				TEST_FAIL("found [length(bad_lists)] identical lists used as argument for element [element_type]. List: [json_encode(unsorted_list)].\n\
+					Make sure it's a cached list, or use one of the string_list proc. Also, use the ELEMENT_DONT_SORT_LIST_ARGS flag if the key position of your lists matters.")

code/modules/unit_tests/strippable.dm

@@ -1,7 +1,7 @@
 /datum/unit_test/strip_menu_ui_status/Run()
 	// We just need something that doesn't have strippable by default, so we can add it ourselves.
 	var/obj/target = allocate(/obj/item/pen, run_loc_floor_bottom_left)
-	var/datum/element/strippable/strippable = target.AddElement(/datum/element/strippable, list())
+	var/datum/element/strippable/strippable = target.AddElement(/datum/element/strippable)
 
 	var/mob/living/carbon/human/user = allocate(/mob/living/carbon/human/consistent, run_loc_floor_bottom_left)
 	ADD_TRAIT(user, TRAIT_PRESERVE_UI_WITHOUT_CLIENT, TRAIT_SOURCE_UNIT_TESTS)

code/modules/unit_tests/unit_test.dm

@@ -225,9 +225,12 @@ GLOBAL_VAR_INIT(focused_tests, focused_tests())
 
 	var/list/test_results = list()
 
+	//Hell code, we're bound to end the round somehow so let's stop if from ending while we work
+	SSticker.delay_end = TRUE
 	for(var/unit_path in tests_to_run)
 		CHECK_TICK //We check tick first because the unit test we run last may be so expensive that checking tick will lock up this loop forever
 		RunUnitTest(unit_path, test_results)
+	SSticker.delay_end = FALSE
 
 	var/file_name = "data/unit_tests.json"
 	fdel(file_name)

modular_skyrat/modules/awaymissions_skyrat/mothership_astrum/mob.dm

@@ -34,7 +34,8 @@
 
 /mob/living/basic/abductor/Initialize(mapload)
 	. = ..()
-	if(length(loot))
+	if(LAZYLEN(loot))
+		loot = string_list(loot)
 		AddElement(/datum/element/death_drops, loot)
 	AddElement(/datum/element/footstep, FOOTSTEP_MOB_SHOE)
 

tgstation.dme

@@ -502,7 +502,9 @@
 #include "code\__HELPERS\stat_tracking.dm"
 #include "code\__HELPERS\stoplag.dm"
 #include "code\__HELPERS\string_assoc_lists.dm"
+#include "code\__HELPERS\string_assoc_nested_lists.dm"
 #include "code\__HELPERS\string_lists.dm"
+#include "code\__HELPERS\string_numbers_lists.dm"
 #include "code\__HELPERS\text.dm"
 #include "code\__HELPERS\time.dm"
 #include "code\__HELPERS\traits.dm"