Optimize cardboard cutouts saving 1.5s+ on init times (#73404)

New regression in init times. Closes https://github.com/tgstation/dev-cycles-initiative/issues/32. CC @Fikou - Instead of creating a human and icon for *every* cardboard cutout when initialized, only creates the one we're actually using. When you're about to use a crayon, creates all of them. - Instead of using getFlatIcon, uses appearances directly.
2026-08-29 08:08:49 +01:00 · 2023-02-18 22:58:35 -07:00
parent 1191bce9b0
commit 09cfba5feb
23 changed files with 124 additions and 74 deletions
@@ -0,0 +1,13 @@
+#undef ASSERT
+
+/// Override BYOND's native ASSERT to optionally specify a message
+#define ASSERT(condition, message...) \
+	if (!(condition)) { \
+		CRASH(assertion_message(__FILE__, __LINE__, #condition, ##message)) \
+	}
+
+/proc/assertion_message(file, line, condition, message)
+	if (!isnull(message))
+		message = " - [message]"
+
+	return "[file]:[line]:Assertion failed: [condition][message]"
@@ -1,13 +1,13 @@
 ///Global list of all dynamically generated icons, for caching, so we don't have to generate multiple times.
-GLOBAL_LIST_EMPTY(dynamic_human_icons)
+GLOBAL_LIST_EMPTY(dynamic_human_appearances)

-///This proc takes an amount of arguments and creates a human dummy using the paths, for visuals you want!
-/proc/get_dynamic_human_icon(outfit_path, species_path = /datum/species/human, mob_spawn_path, r_hand, l_hand, bloody_slots = NONE, list/generated_dirs = GLOB.cardinals, animated = TRUE)
+/// Creates a human with the given parameters and returns an appearance of it
+/proc/get_dynamic_human_appearance(outfit_path, species_path = /datum/species/human, mob_spawn_path, r_hand, l_hand, bloody_slots = NONE, animated = TRUE)
 	if(!species_path)
 		return FALSE
-	var/arg_string = "[outfit_path]_[species_path]_[mob_spawn_path]_[l_hand]_[r_hand]_[bloody_slots]_[generated_dirs.Join(",")]"
-	if(GLOB.dynamic_human_icons[arg_string]) //if already exists in our cache, just return that
-		return GLOB.dynamic_human_icons[arg_string]
+	var/arg_string = "[outfit_path]_[species_path]_[mob_spawn_path]_[l_hand]_[r_hand]_[bloody_slots]"
+	if(GLOB.dynamic_human_appearances[arg_string]) //if already exists in our cache, just return that
+		return GLOB.dynamic_human_appearances[arg_string]
 	var/mob/living/carbon/human/dummy/consistent/dummy = new()
 	dummy.set_species(species_path)
 	dummy.stat = DEAD //this is to avoid side effects of mob spawners
@@ -41,20 +41,17 @@ GLOBAL_LIST_EMPTY(dynamic_human_icons)
 		if(bloody_slots & carried_item.slot_flags)
 			carried_item.add_mob_blood(dummy)
 	dummy.update_held_items()
-	var/icon/output = icon('icons/blanks/32x32.dmi', "nothing")
-	for(var/direction in generated_dirs)
-		var/icon/partial = getFlatIcon(dummy, defdir = direction, no_anim = !animated)
-		output.Insert(partial, dir = direction)
-	GLOB.dynamic_human_icons[arg_string] = output
+	var/mutable_appearance/output = dummy.appearance
+	GLOB.dynamic_human_appearances[arg_string] = output
 	qdel(dummy)
 	return output

 ///This exists to apply the icons async, as that cannot be done in Initialize because of possible sleeps.
-/proc/apply_dynamic_human_icon(atom/target, outfit_path, species_path = /datum/species/human, mob_spawn_path, r_hand, l_hand, bloody_slots = NONE, generated_dirs = GLOB.cardinals)
-	INVOKE_ASYNC(GLOBAL_PROC, GLOBAL_PROC_REF(set_dynamic_human_icon), args)
+/proc/apply_dynamic_human_appearance(atom/target, outfit_path, species_path = /datum/species/human, mob_spawn_path, r_hand, l_hand, bloody_slots = NONE)
+	INVOKE_ASYNC(GLOBAL_PROC, GLOBAL_PROC_REF(set_dynamic_human_appearance), args)

 ///This proc gets an argument of a target and runs
-/proc/set_dynamic_human_icon(list/arguments)
+/proc/set_dynamic_human_appearance(list/arguments)
 	var/atom/target = arguments[1] //1st argument is the target
-	var/icon/dynamic_icon = get_dynamic_human_icon(arglist(arguments.Copy(2))) //the rest of the arguments starting from 2 matter to the proc
-	target.icon = dynamic_icon
+	var/dynamic_appearance = get_dynamic_human_appearance(arglist(arguments.Copy(2))) //the rest of the arguments starting from 2 matter to the proc
+	target.appearance = dynamic_appearance
@@ -1,6 +1,3 @@
-///Global list of all cardboard cutout datums, cached by name to instance.
-GLOBAL_LIST(cardboard_cutouts)
-
 //Cardboard cutouts! They're man-shaped and can be colored with a crayon to look like a human in a certain outfit, although it's limited, discolored, and obvious to more than a cursory glance.
 /obj/item/cardboard_cutout
 	name = "cardboard cutout"
@@ -19,15 +16,21 @@ GLOBAL_LIST(cardboard_cutouts)

 /obj/item/cardboard_cutout/Initialize(mapload)
 	. = ..()
-	if(!GLOB.cardboard_cutouts)
-		INVOKE_ASYNC(GLOBAL_PROC, GLOBAL_PROC_REF(generate_cardboard_cutouts))
 	if(starting_cutout)
 		return INITIALIZE_HINT_LATELOAD

 /obj/item/cardboard_cutout/LateInitialize()
-	var/datum/cardboard_cutout/cutout = GLOB.cardboard_cutouts[starting_cutout]
-	if(!cutout)
-		CRASH("Given invalid starting_cutout! [starting_cutout]")
+	ASSERT(!isnull(starting_cutout))
+
+	var/datum/cardboard_cutout/cutout
+	for (var/datum/cardboard_cutout/cutout_subtype as anything in subtypesof(/datum/cardboard_cutout))
+		if (initial(cutout_subtype.name) != starting_cutout)
+			continue
+
+		cutout = get_cardboard_cutout_instance(cutout_subtype)
+
+	ASSERT(!isnull(cutout), "No cutout found with name [starting_cutout]")
+
 	cutout.apply(src)

 //ATTACK HAND IGNORING PARENT RETURN VALUE
@@ -39,12 +42,10 @@ GLOBAL_LIST(cardboard_cutouts)
 	push_over()

 /obj/item/cardboard_cutout/proc/push_over()
-	name = initial(name)
+	appearance = initial(appearance)
 	desc = "[initial(desc)] It's been pushed over."
-	icon = initial(icon)
 	icon_state = "cutout_pushed_over"
 	remove_atom_colour(FIXED_COLOUR_PRIORITY)
-	alpha = initial(alpha)
 	pushed_over = TRUE

 /obj/item/cardboard_cutout/attack_self(mob/living/user)
@@ -86,6 +87,16 @@ GLOBAL_LIST(cardboard_cutouts)
 		push_over()
 	return BULLET_ACT_HIT

+/proc/get_cardboard_cutout_instance(datum/cardboard_cutout/cardboard_cutout)
+	ASSERT(ispath(cardboard_cutout), "[cardboard_cutout] is not a path of /datum/cardboard_cutout")
+
+	var/static/list/cardboard_cutouts = list()
+
+	if(isnull(cardboard_cutouts[cardboard_cutout]))
+		cardboard_cutouts[cardboard_cutout] = new cardboard_cutout
+
+	return cardboard_cutouts[cardboard_cutout]
+
 /**
 * change_appearance: Changes a skin of the cardboard cutout based on a user's choice
 *
@@ -94,10 +105,14 @@ GLOBAL_LIST(cardboard_cutouts)
 * * user The mob choosing a skin of the cardboard cutout
 */
 /obj/item/cardboard_cutout/proc/change_appearance(obj/item/toy/crayon/crayon, mob/living/user)
+	var/list/appearances_by_name = list()
 	var/list/possible_appearances = list()
-	for(var/cutout_name in GLOB.cardboard_cutouts)
-		var/datum/cardboard_cutout/cutout = GLOB.cardboard_cutouts[cutout_name]
-		possible_appearances[cutout_name] = image(icon = cutout.applied_icon)
+
+	for (var/datum/cardboard_cutout/cutout_subtype as anything in subtypesof(/datum/cardboard_cutout))
+		var/datum/cardboard_cutout/cutout = get_cardboard_cutout_instance(cutout_subtype)
+		appearances_by_name[cutout.name] = cutout
+		possible_appearances[cutout.name] = image(icon = cutout.applied_appearance)
+
 	var/new_appearance = show_radial_menu(user, src, possible_appearances, custom_check = CALLBACK(src, PROC_REF(check_menu), user, crayon), radius = 36, require_near = TRUE)
 	if(!new_appearance)
 		return FALSE
@@ -112,7 +127,7 @@ GLOBAL_LIST(cardboard_cutouts)
 	icon = initial(icon)
 	if(!deceptive)
 		add_atom_colour("#FFD7A7", FIXED_COLOUR_PRIORITY)
-	var/datum/cardboard_cutout/cutout = GLOB.cardboard_cutouts[new_appearance]
+	var/datum/cardboard_cutout/cutout = appearances_by_name[new_appearance]
 	cutout.apply(src)
 	return TRUE

@@ -140,22 +155,19 @@ GLOBAL_LIST(cardboard_cutouts)
 		return FALSE
 	return TRUE

+// Cutouts always face forward
+/obj/item/cardboard_cutout/setDir(newdir)
+	SHOULD_CALL_PARENT(FALSE)
+	return
+
 /obj/item/cardboard_cutout/adaptive //Purchased by Syndicate agents, these cutouts are indistinguishable from normal cutouts but aren't discolored when their appearance is changed
 	deceptive = TRUE

-///Proc that returns a list of all the cardboard cutouts (we create all subtypes) to be used in the global list.
-/proc/generate_cardboard_cutouts()
-	var/list/cutouts = list()
-	for(var/datum/cardboard_cutout/cutout as anything in subtypesof(/datum/cardboard_cutout))
-		cutout = new cutout()
-		cutouts[cutout.name] = cutout
-	GLOB.cardboard_cutouts = cutouts
-
 /datum/cardboard_cutout
 	/// Name of the cutout, used for radial selection and the global list.
 	var/name = "Boardjak"
-	/// The icon we apply to the cardboard cutout.
-	var/applied_icon = null
+	/// The appearance we apply to the cardboard cutout.
+	var/mutable_appearance/applied_appearance = null
 	/// The base name we actually give to to the cardboard cutout. Can be overridden in get_name().
 	var/applied_name = "boardjak"
 	/// The desc we give to the cardboard cutout.
@@ -178,9 +190,9 @@ GLOBAL_LIST(cardboard_cutouts)
 /datum/cardboard_cutout/New()
 	. = ..()
 	if(direct_icon)
-		applied_icon = icon(direct_icon, direct_icon_state, dir = SOUTH, frame = 1)
+		applied_appearance = mutable_appearance(direct_icon, direct_icon_state)
 	else
-		applied_icon = get_dynamic_human_icon(outfit, species, mob_spawner, l_hand, r_hand, generated_dirs = list(SOUTH), animated = FALSE)
+		applied_appearance = get_dynamic_human_appearance(outfit, species, mob_spawner, l_hand, r_hand, animated = FALSE)

 /// This proc returns the name that the cardboard cutout item will use.
 /datum/cardboard_cutout/proc/get_name()
@@ -188,7 +200,7 @@ GLOBAL_LIST(cardboard_cutouts)

 /// This proc sets the cardboard cutout item's vars.
 /datum/cardboard_cutout/proc/apply(obj/item/cardboard_cutout/cutouts)
-	cutouts.icon = applied_icon
+	cutouts.appearance = applied_appearance
 	cutouts.name = get_name()
 	cutouts.desc = applied_desc

@@ -204,18 +204,19 @@
 	affects_us = FALSE

 /datum/hallucination/delusion/preset/syndies/make_delusion_image(mob/over_who)
-	delusion_icon_file = get_dynamic_human_icon( \
-		mob_spawn_path = pick( \
-			/obj/effect/mob_spawn/corpse/human/syndicatesoldier, \
-			/obj/effect/mob_spawn/corpse/human/syndicatecommando, \
-			/obj/effect/mob_spawn/corpse/human/syndicatestormtrooper, \
-		), \
-		r_hand = pick( \
-			/obj/item/knife/combat/survival, \
-			/obj/item/melee/energy/sword/saber, \
-			/obj/item/gun/ballistic/automatic/pistol, \
-			/obj/item/gun/ballistic/automatic/c20r, \
-			/obj/item/gun/ballistic/shotgun/bulldog, \
-		), \
-	)
+	delusion_icon_file = getFlatIcon(get_dynamic_human_appearance(
+		mob_spawn_path = pick(
+			/obj/effect/mob_spawn/corpse/human/syndicatesoldier,
+			/obj/effect/mob_spawn/corpse/human/syndicatecommando,
+			/obj/effect/mob_spawn/corpse/human/syndicatestormtrooper,
+		),
+		r_hand = pick(
+			/obj/item/knife/combat/survival,
+			/obj/item/melee/energy/sword/saber,
+			/obj/item/gun/ballistic/automatic/pistol,
+			/obj/item/gun/ballistic/automatic/c20r,
+			/obj/item/gun/ballistic/shotgun/bulldog,
+		),
+	))
+
 	return ..()
@@ -387,8 +387,6 @@ INITIALIZE_IMMEDIATE(/obj/effect/mapping_helpers/no_lava)
 	var/l_hand = NO_REPLACE
 	/// Which slots on the mob should be bloody?
 	var/bloody_slots = NONE
-	/// Directions we generate for the mob.
-	var/generated_dirs = list(NORTH, SOUTH, EAST, WEST)
 	/// Do we draw more than one frame for the mob?
 	var/animated = TRUE

@@ -406,7 +404,7 @@ INITIALIZE_IMMEDIATE(/obj/effect/mapping_helpers/no_lava)
 	return TRUE

 /obj/effect/mapping_helpers/atom_injector/human_icon_injector/inject(atom/target)
-	apply_dynamic_human_icon(target, outfit_path, species_path, mob_spawn_path, r_hand, l_hand, bloody_slots, generated_dirs, animated)
+	apply_dynamic_human_appearance(target, outfit_path, species_path, mob_spawn_path, r_hand, l_hand, bloody_slots, animated)

 /obj/effect/mapping_helpers/atom_injector/human_icon_injector/generate_stack_trace()
 	. = ..()
@@ -32,7 +32,7 @@

 /mob/living/basic/syndicate/Initialize(mapload)
 	. = ..()
-	apply_dynamic_human_icon(src, mob_spawn_path = mob_spawner, r_hand = r_hand, l_hand = l_hand)
+	apply_dynamic_human_appearance(src, mob_spawn_path = mob_spawner, r_hand = r_hand, l_hand = l_hand)
 	if(LAZYLEN(loot))
 		AddElement(/datum/element/death_drops, loot)
 	AddElement(/datum/element/footstep, footstep_type = FOOTSTEP_MOB_SHOE)
@@ -26,7 +26,7 @@

 /mob/living/simple_animal/hostile/boss/paper_wizard/Initialize(mapload)
 	. = ..()
-	apply_dynamic_human_icon(src, mob_spawn_path = /obj/effect/mob_spawn/corpse/human/wizard/paper)
+	apply_dynamic_human_appearance(src, mob_spawn_path = /obj/effect/mob_spawn/corpse/human/wizard/paper)

 /mob/living/simple_animal/hostile/boss/paper_wizard/Destroy()
 	QDEL_LIST(copies)
@@ -32,7 +32,7 @@

 /mob/living/simple_animal/hostile/cat_butcherer/Initialize(mapload)
 	. = ..()
-	apply_dynamic_human_icon(src, mob_spawn_path = /obj/effect/mob_spawn/corpse/human/cat_butcher, l_hand = /obj/item/circular_saw, bloody_slots = ITEM_SLOT_GLOVES|ITEM_SLOT_OCLOTHING)
+	apply_dynamic_human_appearance(src, mob_spawn_path = /obj/effect/mob_spawn/corpse/human/cat_butcher, l_hand = /obj/item/circular_saw, bloody_slots = ITEM_SLOT_GLOVES|ITEM_SLOT_OCLOTHING)

 /mob/living/simple_animal/hostile/cat_butcherer/AttackingTarget()
 	. = ..()
@@ -35,7 +35,7 @@

 /mob/living/simple_animal/hostile/dark_wizard/Initialize(mapload)
 	. = ..()
-	apply_dynamic_human_icon(src, mob_spawn_path = /obj/effect/mob_spawn/corpse/human/wizard/dark, r_hand = /obj/item/staff)
+	apply_dynamic_human_appearance(src, mob_spawn_path = /obj/effect/mob_spawn/corpse/human/wizard/dark, r_hand = /obj/item/staff)

 /obj/projectile/temp/earth_bolt
 	name = "earth bolt"
@@ -34,7 +34,7 @@

 /mob/living/simple_animal/hostile/nanotrasen/Initialize(mapload)
 	. = ..()
-	apply_dynamic_human_icon(src, mob_spawn_path = mob_spawner, r_hand = held_item)
+	apply_dynamic_human_appearance(src, mob_spawn_path = mob_spawner, r_hand = held_item)

 /mob/living/simple_animal/hostile/nanotrasen/screaming/Aggro()
 	..()
@@ -91,7 +91,7 @@

 /mob/living/simple_animal/hostile/retaliate/nanotrasenpeace/Initialize(mapload)
 	. = ..()
-	apply_dynamic_human_icon(src, mob_spawn_path = /obj/effect/mob_spawn/corpse/human/nanotrasensoldier, r_hand = held_item)
+	apply_dynamic_human_appearance(src, mob_spawn_path = /obj/effect/mob_spawn/corpse/human/nanotrasensoldier, r_hand = held_item)

 /mob/living/simple_animal/hostile/retaliate/nanotrasenpeace/Aggro()
 	..()
@@ -31,7 +31,7 @@

 /mob/living/simple_animal/hostile/pirate/Initialize(mapload)
 	. = ..()
-	apply_dynamic_human_icon(src, mob_spawn_path = mob_spawner, r_hand = held_item)
+	apply_dynamic_human_appearance(src, mob_spawn_path = mob_spawner, r_hand = held_item)

 /mob/living/simple_animal/hostile/pirate/melee
 	name = "Pirate Swashbuckler"
@@ -35,7 +35,7 @@

 /mob/living/simple_animal/hostile/russian/Initialize(mapload)
 	. = ..()
-	apply_dynamic_human_icon(src, mob_spawn_path = mob_spawner, r_hand = held_item)
+	apply_dynamic_human_appearance(src, mob_spawn_path = mob_spawner, r_hand = held_item)

 /mob/living/simple_animal/hostile/russian/ranged
 	icon_state = "russianranged"
@@ -43,7 +43,7 @@

 /mob/living/simple_animal/hostile/skeleton/Initialize(mapload)
 	. = ..()
-	apply_dynamic_human_icon(src, outfit, species, r_hand = held_item)
+	apply_dynamic_human_appearance(src, outfit, species, r_hand = held_item)

 /mob/living/simple_animal/hostile/skeleton/eskimo
 	name = "undead eskimo"
@@ -40,7 +40,7 @@

 /mob/living/simple_animal/hostile/wizard/Initialize(mapload)
 	. = ..()
-	apply_dynamic_human_icon(src, mob_spawn_path = /obj/effect/mob_spawn/corpse/human/wizard, r_hand = /obj/item/staff)
+	apply_dynamic_human_appearance(src, mob_spawn_path = /obj/effect/mob_spawn/corpse/human/wizard, r_hand = /obj/item/staff)
 	var/obj/item/implant/exile/exiled = new /obj/item/implant/exile(src)
 	exiled.implant(src)

@@ -29,7 +29,7 @@

 /mob/living/simple_animal/hostile/zombie/Initialize(mapload)
 	. = ..()
-	apply_dynamic_human_icon(src, outfit, /datum/species/zombie, bloody_slots = ITEM_SLOT_OCLOTHING)
+	apply_dynamic_human_appearance(src, outfit, /datum/species/zombie, bloody_slots = ITEM_SLOT_OCLOTHING)

 /mob/living/simple_animal/hostile/zombie/AttackingTarget()
 	. = ..()
@@ -91,6 +91,7 @@
 #include "breath.dm"
 #include "cable_powernets.dm"
 #include "card_mismatch.dm"
+#include "cardboard_cutouts.dm"
 #include "chain_pull_through_space.dm"
 #include "chat_filter.dm"
 #include "circuit_component_category.dm"
@@ -0,0 +1,27 @@
+/// Validates that cardboard cutouts have the proper icons
+/datum/unit_test/cardboard_cutouts
+
+/datum/unit_test/cardboard_cutouts/Run()
+	var/obj/item/cardboard_cutout/normal_cutout = new
+	test_screenshot("normal_cutout", getFlatIcon(normal_cutout))
+
+	var/obj/item/cardboard_cutout/nuclear_operative/nukie_cutout = new
+	test_screenshot("nukie_cutout", getFlatIcon(nukie_cutout))
+
+	nukie_cutout.push_over()
+	test_screenshot("nukie_cutout_pushed", getFlatIcon(nukie_cutout))
+
+#if DM_VERSION >= 515
+	// This is the only reason we're testing xenomorphs.
+	// Making a custom subtype with direct_icon is hacky.
+	ASSERT(!isnull(/datum/cardboard_cutout/xenomorph_maid::direct_icon))
+#endif
+
+	var/obj/item/cardboard_cutout/xenomorph/xenomorph_cutout = new
+	test_screenshot("xenomorph_cutout", getFlatIcon(xenomorph_cutout))
+
+/obj/item/cardboard_cutout/nuclear_operative
+	starting_cutout = "Nuclear Operative"
+
+/obj/item/cardboard_cutout/xenomorph
+	starting_cutout = "Xenomorph"
@@ -3,5 +3,5 @@

 /datum/unit_test/screenshot_dynamic_human_icons/Run()
 	// Complicated MODsuit setup
-	var/icon/icon = get_dynamic_human_icon(/datum/outfit/syndicatecommandocorpse)
-	test_screenshot("syndicate_commando", icon)
+	var/appearance = get_dynamic_human_appearance(/datum/outfit/syndicatecommandocorpse)
+	test_screenshot("syndicate_commando", get_flat_icon_for_all_directions(appearance))
@@ -44,6 +44,7 @@
 #include "code\__DEFINES\aquarium.dm"
 #include "code\__DEFINES\art.dm"
 #include "code\__DEFINES\assemblies.dm"
+#include "code\__DEFINES\assert.dm"
 #include "code\__DEFINES\atom_hud.dm"
 #include "code\__DEFINES\basic_mobs.dm"
 #include "code\__DEFINES\blackmarket.dm"