Some minor fixes (#4150)

* Fix runtime in is_valid_z_level (by replacing it with another runtime to narrow things down)

* Skip nulls in `/proc/strip_appearance_underlays`

* clarify stack trace

* better

* okay this is safe to do then

* get rid of all uses of `throw EXCEPTION()`

* debug try/catch for PARSE_LIGHT_COLOR

* revert this so i can tm #4229

* resolves some opendream lints

includes port of tgstation/tgstation#84656 and partial port of tgstation/tgstation#84648

* Removes stupid listlike var access code (#84648)

[Removes all other listlike var
accesses](tgstation/tgstation@4c5996b)

Also fucking dumpsters an unused proc that allowed for arbitrary
variable modifcation. Bad juju

This is undefined behavior and errors in later 515 versions. also it's
stupid as hell

* some more linting fixes

* add more info to PARSE_LIGHT_COLOR debug

* fix runtime in `/datum/ai_planning_subtree/stare_at_thing/SelectBehaviors`

* Prevents persistence from trying to load more engravings than exists (#86065)

## About The Pull Request

During initializing persistent wall engravings, the game picked a number
between 15 and 25, and attempted to load that many. However, if there
were less engravings than that, the loop went on even after the list it
was calling `pick_n_take` on was empty, and multiple times it has logged
a runtime claiming that the engraving was in an incorrect format when it
tried to parse the returned nulls.

This PR ensures that the game will not attempt to load more engravings
than the amount that exists in the persistence files, ensuring less
incorrect error messages during initialization.

## Why It's Good For The Game

Less incorrect lines during initialization on maps that have not
received enough engravings.

## Changelog

Nothing player facing.

* no len, only length

* hrrrrnnggh colonel, I’m trying to fix runtimes but I'm *dummy thicc* and the clap from my asscheeks keeps alerting runtime.log

* revert this for now

* e

* werfijfrjweuirweufjnrfweuijneruwifjn

* qwedfwerfuijhfwerg8yuwerf8yguh

* remove enqueue logging, hopefully reducing lists

* clean up particle spewer code quite a bit

* Rebuild plane masters after client login if switching between 515 and 516 (#87889)

This makes `/datum/player_details` properly track BYOND version and
build separately, as opposed to just the full version string.

Whenever a client logs in, and their BYOND version is 516, while their
previous version was 515, or vice versa, it'll set a newly added client
var, `rebuild_plane_masters`, to TRUE.

During the `COMSIG_MOB_LOGIN` signal handler of a mob's HUD
(`/datum/hud/proc/client_refresh`), it will check to see if
`rebuild_plane_masters` is TRUE - if so, it will set the appropriate
`relay_loc` of (based on client BYOND version) of its plane master
groups, and rebuild their plane masters.

Makes testing stuff across 515 and 516 easier, as your screen won't
break when switching between the two.

516 is _still_ in private alpha, so no user-facing changes.

* Admin deleting a mob now ghostizes it beforehand, preventing a runtime (#87887)

* aeiou

* fix addtimer runtime with modsuit construction

* clean up some false positive runtimes

---------

Co-authored-by: LemonInTheDark <[email protected]>
Co-authored-by: Profakos <[email protected]>

Author: 3 people

code/__DEFINES/dcs/signals/signals_datum.dm

@@ -13,6 +13,8 @@
 #define COMSIG_PREQDELETED "parent_preqdeleted"
 /// just before a datum's Destroy() is called: (force), at this point none of the other components chose to interrupt qdel and Destroy will be called
 #define COMSIG_QDELETING "parent_qdeleting"
+/// Called whenever an admin manually deletes an object, via the "Delete" verb, before qdel() is called: (client/deleting_admin)
+#define COMSIG_ADMIN_DELETING "parent_admin_deleting"
 /// generic topic handler (usr, href_list)
 #define COMSIG_TOPIC "handle_topic"
 /// handler for vv_do_topic (usr, href_list)

code/__DEFINES/lighting.dm

@@ -96,10 +96,17 @@ GLOBAL_LIST_INIT(em_mask_matrix, EM_MASK_MATRIX)
 #define PARSE_LIGHT_COLOR(source) \
 do { \
 	if (source.light_color != COLOR_WHITE) { \
-		var/list/color_map = rgb2num(source.light_color); \
-		source.lum_r = color_map[1] / 255; \
-		source.lum_g = color_map[2] / 255; \
-		source.lum_b = color_map[3] / 255; \
+		try { \
+			var/list/color_map = rgb2num(source.light_color); \
+			source.lum_r = color_map[1] / 255; \
+			source.lum_g = color_map[2] / 255; \
+			source.lum_b = color_map[3] / 255; \
+		} catch() { \
+			stack_trace("[source.source_atom] ([source.source_atom?.type]) had invalid light color [source.light_color || "null"]"); \
+			source.lum_r = 1; \
+			source.lum_g = 1; \
+			source.lum_b = 1; \
+		}; \
 	} else { \
 		source.lum_r = 1; \
 		source.lum_g = 1; \

code/__DEFINES/~monkestation/logging.dm

@@ -1,3 +1,3 @@
 #define LOG_CATEGORY_GAME_LOOC "game-looc"
 #define LOG_CATEGORY_STORYTELLER "storyteller"
-#define LOG_CATEGORY_ENQUEUE "enqueue"
+/* #define LOG_CATEGORY_ENQUEUE "enqueue" */

code/__HELPERS/areas.dm

@@ -147,7 +147,7 @@ GLOBAL_LIST_INIT(typecache_powerfailure_safe_areas, typecacheof(list(
 	 * A list of all machinery tied to an area along with the area itself. key=area name,value=list(area,list of machinery)
 	 * we use this to keep track of what areas are affected by the blueprints & what machinery of these areas needs to be reconfigured accordingly
 	 */
-	var/area/affected_areas = list()
+	var/list/area/affected_areas = list()
 	for(var/turf/the_turf as anything in turfs)
 		var/area/old_area = the_turf.loc
 

code/__HELPERS/icons.dm

@@ -1507,6 +1507,8 @@ GLOBAL_LIST_EMPTY(icon_dimensions)
 /proc/strip_appearance_underlays(mutable_appearance/appearance)
 	var/base_plane = PLANE_TO_TRUE(appearance.plane)
 	for(var/mutable_appearance/underlay as anything in appearance.underlays)
+		if(isnull(underlay))
+			continue
 		if(PLANE_TO_TRUE(underlay.plane) != base_plane)
 			appearance.underlays -= underlay
 	return appearance

code/__HELPERS/levels.dm

@@ -11,6 +11,9 @@
  * returns TRUE if connection is valid, FALSE otherwise.
  */
 /proc/is_valid_z_level(turf/source_loc, turf/checking_loc)
+	// if either locs are null, then well, it's not valid, is it?
+	if(isnull(source_loc) || isnull(checking_loc))
+		return FALSE
 	// if we're both on "station", regardless of multi-z, we'll pass by.
 	if(is_station_level(source_loc.z) && is_station_level(checking_loc.z))
 		return TRUE

code/__HELPERS/logging/debug.dm

@@ -60,5 +60,7 @@
 #endif
 	SEND_TEXT(world.log, text)
 
+/*
 /proc/log_enqueue(text, list/data)
 	logger.Log(LOG_CATEGORY_ENQUEUE, text, data)
+*/

code/__HELPERS/paths/path.dm

@@ -382,7 +382,7 @@ GLOBAL_LIST_INIT(can_pass_info_vars, GLOBAL_PROC_REF(can_pass_check_vars))
 
 /datum/can_pass_info/proc/compare_against(datum/can_pass_info/check_against)
 	for(var/comparable_var in GLOB.can_pass_info_vars)
-		if(!(vars[comparable_var] ~= check_against[comparable_var]))
+		if(!(vars[comparable_var] ~= check_against.vars[comparable_var]))
 			return FALSE
 	if(!pulling_info != !check_against.pulling_info)
 		return FALSE

code/__HELPERS/records.dm

@@ -0,0 +1,6 @@
+// eww
+/proc/overwrite_field_if_available(datum/record/base, datum/record/other, field_name)
+	if(!istype(base) || !istype(other))
+		return
+	if(other.vars[field_name])
+		base.vars[field_name] = other.vars[field_name]

code/__HELPERS/~monkestation-helpers/records.dm

@@ -91,6 +91,10 @@
 
 	var/list/modifiers = params2list(params)
 
+	if(!client?.holder && (isobserver(A) || isaicamera(A)) && A.invisibility > see_invisible)
+		message_admins("[ADMIN_LOOKUPFLW(src)] clicked on [key_name_admin(A)] ([A?.type]) [ADMIN_FLW(A)], which they should not be able to see!")
+		log_admin_private("[key_name(src)] clicked on [key_name(A)] ([A?.type]), which they should not be able to see!")
+
 	if(client)
 		client.imode.update_istate(src, modifiers)
 

code/_onclick/click.dm

@@ -146,8 +146,16 @@ GLOBAL_LIST_INIT(available_ui_styles, list(
 
 /datum/hud/proc/client_refresh(datum/source)
 	SIGNAL_HANDLER
-	RegisterSignal(mymob.client, COMSIG_CLIENT_SET_EYE, PROC_REF(on_eye_change))
-	on_eye_change(null, null, mymob.client.eye)
+	var/client/client = mymob.client
+	if(client.rebuild_plane_masters)
+		var/new_relay_loc = (client.byond_version > 515) ? "1,1" : "CENTER"
+		for(var/group_key as anything in master_groups)
+			var/datum/plane_master_group/group = master_groups[group_key]
+			group.relay_loc = new_relay_loc
+			group.rebuild_plane_masters()
+		client.rebuild_plane_masters = FALSE
+	RegisterSignal(client, COMSIG_CLIENT_SET_EYE, PROC_REF(on_eye_change))
+	on_eye_change(null, null, client.eye)
 
 /datum/hud/proc/clear_client(datum/source)
 	SIGNAL_HANDLER

code/_onclick/hud/hud.dm

@@ -160,10 +160,11 @@
 	if(ready)
 		new_player.ready = PLAYER_READY_TO_PLAY
 		base_icon_state = "ready"
-		if(!new_player.client.readied_store)
-			new_player.client.readied_store = new(new_player)
-		else
-			new_player.client.readied_store.ui_interact(new_player)
+		var/client/new_client = new_player.client
+		if(new_client)
+			if(!new_client.readied_store)
+				new_client.readied_store = new(new_player)
+			new_client.readied_store.ui_interact(new_player)
 	else
 		new_player.ready = PLAYER_NOT_READY
 		base_icon_state = "not_ready"

code/_onclick/hud/new_player.dm

@@ -108,7 +108,7 @@
 
 	var/avg_iter_count = 0
 	var/avg_drift = 0
-	var/list/enqueue_log = list()
+	/* var/list/enqueue_log = list() */
 
 	//Do not blindly add vars here to the bottom, put it where it goes above
 	//If your var only has two values, put it in as a flag.
@@ -197,7 +197,7 @@
 
 	var/iter_count = 0
 
-	enqueue_log.Cut()
+	/* enqueue_log.Cut() */
 	for (queue_node = Master.queue_head; queue_node; queue_node = queue_node.queue_next)
 		iter_count++
 		if(iter_count >= ENQUEUE_SANITY)
@@ -207,25 +207,23 @@
 				examine_block(span_userdanger("ERROR: [msg]")),
 				type = MESSAGE_TYPE_DEBUG
 			)
-			log_enqueue(msg, list("enqueue_log" = enqueue_log.Copy()))
-#if defined(INIT_ORDER_PLEXORA) && !defined(UNIT_TESTS)
+			/* log_enqueue(msg, list("enqueue_log" = enqueue_log.Copy())) */
 			SSplexora.mc_alert("[src] has likely entered an infinite loop in enqueue(), we're restarting the MC immediately!")
-#endif
 			stack_trace("enqueue() entered an infinite loop, we're restarting the MC!")
-			enqueue_log.Cut()
+			/* enqueue_log.Cut() */
 			Recreate_MC()
 			return
 
 
 		queue_node_priority = queue_node.queued_priority
 		queue_node_flags = queue_node.flags
 
-		enqueue_log["[iter_count]"] = list(
+		/* enqueue_log["[iter_count]"] = list(
 			"node" = "[queue_node]",
 			"next" = "[queue_node.queue_next || "(none)"]",
 			"priority" = queue_node_priority,
 			"flags" = queue_node_flags,
-		)
+		) */
 
 		if (queue_node_flags & (SS_TICKER|SS_BACKGROUND) == SS_TICKER)
 			if ((SS_flags & (SS_TICKER|SS_BACKGROUND)) != SS_TICKER)

code/controllers/subsystem.dm

@@ -87,7 +87,7 @@ SUBSYSTEM_DEF(blackbox)
 
 	for(var/player_key in GLOB.player_details)
 		var/datum/player_details/PD = GLOB.player_details[player_key]
-		record_feedback("tally", "client_byond_version", 1, PD.byond_version)
+		record_feedback("tally", "client_byond_version", 1, PD.full_byond_version())
 
 /datum/controller/subsystem/blackbox/Shutdown()
 	sealed = FALSE

code/controllers/subsystem/blackbox.dm

@@ -27,7 +27,7 @@
 
 	var/successfully_loaded_engravings = 0
 
-	for(var/iteration in 1 to rand(MIN_PERSISTENT_ENGRAVINGS, MAX_PERSISTENT_ENGRAVINGS))
+	for(var/iteration in 1 to min(rand(MIN_PERSISTENT_ENGRAVINGS, MAX_PERSISTENT_ENGRAVINGS), length(saved_engravings)))
 		var/engraving = pick_n_take(saved_engravings)
 		if(!islist(engraving))
 			stack_trace("something's wrong with the engraving data! one of the saved engravings wasn't a list!")

code/controllers/subsystem/persistence/engravings.dm

@@ -5,7 +5,7 @@
 		if(!istype(ending_human) || !ending_human.mind?.original_character_slot_index || !ending_human.client?.prefs.read_preference(/datum/preference/toggle/persistent_scars))
 			continue
 
-		var/mob/living/carbon/human/original_human = ending_human.mind.original_character.resolve()
+		var/mob/living/carbon/human/original_human = ending_human.mind.original_character?.resolve()
 
 		if(!original_human)
 			continue

code/controllers/subsystem/persistence/scars.dm

@@ -234,7 +234,7 @@ SUBSYSTEM_DEF(spatial_grid)
 	. = list()
 
 	//technically THIS list only contains lists, but inside those lists are grid cell datums and we can go without a SINGLE var init if we do this
-	var/list/datum/spatial_grid_cell/grid_level = grids_by_z_level[center_turf.z]
+	var/list/list/datum/spatial_grid_cell/grid_level = grids_by_z_level[center_turf.z]
 
 	switch(type)
 		if(SPATIAL_GRID_CONTENTS_TYPE_CLIENTS)

code/controllers/subsystem/spatial_gridmap.dm

@@ -473,9 +473,8 @@ SUBSYSTEM_DEF(ticker)
 					continue
 				item.post_equip_item(new_player_mob.client?.prefs, new_player_living)
 
-		if(new_player_mob.client.readied_store)
-			if(new_player_mob.client.readied_store.bought_item)
-				new_player_mob.client.readied_store.finalize_purchase_spawn(new_player_mob, new_player_living)
+		if(new_player_mob.client?.readied_store?.bought_item)
+			new_player_mob.client.readied_store.finalize_purchase_spawn(new_player_mob, new_player_living)
 
 		CHECK_TICK
 

code/controllers/subsystem/ticker.dm

@@ -2,8 +2,7 @@
 /datum/ai_planning_subtree/stare_at_thing
 
 /datum/ai_planning_subtree/stare_at_thing/SelectBehaviors(datum/ai_controller/controller, seconds_per_tick)
-	var/datum/weakref/weak_target = controller.blackboard[BB_STATIONARY_CAUSE]
-	var/atom/target = weak_target?.resolve()
+	var/atom/target = controller.blackboard[BB_STATIONARY_CAUSE]
 
 	if(isnull(target)) // No target? Time to locate one using the list we set in this mob's blackboard.
 		var/list/potential_scares = controller.blackboard[BB_STATIONARY_TARGETS]

code/datums/ai/basic_mobs/basic_subtrees/stare_at_thing.dm

@@ -95,8 +95,6 @@
 	var/atom/hunted = controller.blackboard[hunting_target_key]
 
 	if(QDELETED(hunted))
-		//Target is gone for some reason. forget about this task!
-		controller[hunting_target_key] = null
 		finish_action(controller, FALSE, hunting_target_key)
 	else
 		target_caught(hunter, hunted)

code/datums/ai/hunting_behavior/hunting_behaviors.dm

@@ -155,7 +155,7 @@
 
 /// Unlocks a previously locked weakref
 /datum/cinematic/proc/unlock_mob(datum/weakref/mob_ref)
-	var/mob/locked_mob = mob_ref.resolve()
+	var/mob/locked_mob = mob_ref?.resolve()
 	if(isnull(locked_mob))
 		return
 	REMOVE_TRAIT(locked_mob, TRAIT_NO_TRANSFORM, CINEMATIC_SOURCE)

code/datums/cinematics/_cinematic.dm

@@ -274,17 +274,17 @@
  */
 /datum/proc/GetExactComponent(datum/component/c_type)
 	RETURN_TYPE(c_type)
-	if(initial(c_type.dupe_mode) == COMPONENT_DUPE_ALLOWED || initial(c_type.dupe_mode) == COMPONENT_DUPE_SELECTIVE)
+	var/initial_type_mode = c_type::dupe_mode
+	if(initial_type_mode == COMPONENT_DUPE_ALLOWED || initial_type_mode == COMPONENT_DUPE_SELECTIVE)
 		stack_trace("GetComponent was called to get a component of which multiple copies could be on an object. This can easily break and should be changed. Type: \[[c_type]\]")
-	var/list/dc = _datum_components
-	if(!dc)
+	var/list/all_components = _datum_components
+	if(!all_components)
 		return null
-	var/datum/component/C = dc[c_type]
-	if(C)
-		if(length(C))
-			C = C[1]
-		if(C.type == c_type)
-			return C
+	var/datum/component/potential_component
+	if(length(all_components))
+		potential_component = all_components[c_type]
+	if(potential_component?.type == c_type)
+		return potential_component
 	return null
 
 /**

code/datums/components/_component.dm

@@ -276,8 +276,9 @@
 	for(var/datum/greyscale_layer/layer as anything in group)
 		var/icon/layer_icon
 		if(islist(layer))
+			var/list/layer_list = layer
 			layer_icon = GenerateLayerGroup(colors, layer, render_steps, new_icon || last_external_icon)
-			layer = layer[1] // When there are multiple layers in a group like this we use the first one's blend mode
+			layer = layer_list[1] // When there are multiple layers in a group like this we use the first one's blend mode
 		else
 			layer_icon = layer.Generate(colors, render_steps, new_icon || last_external_icon)
 

code/datums/greyscale/_greyscale_config.dm

@@ -11,8 +11,10 @@
 	mind.set_current(src)
 	// There's nowhere else to set this up, mind code makes me depressed
 	mind.antag_hud = add_alt_appearance(/datum/atom_hud/alternate_appearance/basic/antagonist_hud, "combo_hud", mind)
+	RegisterSignal(src, COMSIG_ADMIN_DELETING, PROC_REF(ghost_before_admin_delete), override = TRUE)
 	SEND_SIGNAL(src, COMSIG_MOB_MIND_INITIALIZED, mind)
 
+
 /mob/living/carbon/mind_initialize()
 	..()
 	last_mind = mind
@@ -35,3 +37,8 @@
 	. = ..()
 	mind.set_assigned_role(SSjob.GetJobType(/datum/job/personal_ai))
 	mind.special_role = ""
+
+/// Signal proc for [COMSIG_ADMIN_DELETING], to ghostize a mob beforehand if an admin is manually deleting it.
+/mob/proc/ghost_before_admin_delete(datum/source)
+	SIGNAL_HANDLER
+	ghostize(can_reenter_corpse = FALSE)

code/datums/mind/initialization.dm

@@ -182,9 +182,8 @@
 					radios += independent_radio
 
 	for(var/obj/item/radio/called_radio as anything in radios)
-		if(QDELETED(called_radio))
-			return
-		called_radio.on_recieve_message()
+		if(!QDELETED(called_radio))
+			called_radio.on_recieve_message()
 
 	// From the list of radios, find all mobs who can hear those.
 	var/list/receive = get_hearers_in_radio_ranges(radios)
@@ -205,9 +204,11 @@
 	var/rendered = virt.compose_message(virt, language, message, frequency, spans)
 
 	for(var/atom/movable/hearer as anything in receive)
-		if(QDELETED(hearer))
+		if(isnull(hearer))
 			stack_trace("null found in the hearers list returned by the spatial grid. this is bad")
 			continue
+		else if(QDELING(hearer))
+			continue
 		spans -= blacklisted_spans
 		hearer.Hear(rendered, virt, language, message, frequency, spans, message_mods, message_range = INFINITY)
 

code/game/machinery/telecomms/broadcasting.dm

@@ -77,7 +77,7 @@ GLOBAL_VAR_INIT(chicks_from_eggs, 0)
 		pre_hatch()
 		GLOB.chicks_from_eggs++
 
-	reagents.expose(hit_atom, TOUCH)
+	reagents?.expose(hit_atom, TOUCH)
 
 /obj/item/food/egg/attackby(obj/item/item, mob/user, params)
 	if(istype(item, /obj/item/toy/crayon))

code/game/objects/items/food/egg.dm

@@ -1451,7 +1451,7 @@
 			if(response.body == "[]")
 				dat += "<center><b>0 bans detected for [ckey]</b></center>"
 			else
-				bans = json_decode(response["body"])
+				bans = json_decode(response.body)
 
 				//Ignore bans from non-whitelisted sources, if a whitelist exists
 				var/list/valid_sources

code/modules/admin/topic.dm

@@ -16,6 +16,7 @@
 		log_admin("[key_name(usr)] deleted [D] [coords]")
 		message_admins("[key_name_admin(usr)] deleted [D] [jmp_coords]")
 		SSblackbox.record_feedback("tally", "admin_verb", 1, "Delete") //If you are copy-pasting this, ensure the 2nd parameter is unique to the new proc!
+		SEND_SIGNAL(D, COMSIG_ADMIN_DELETING, src)
 		if(isturf(D))
 			var/turf/T = D
 			T.ScrapeAway()

code/modules/admin/view_variables/admin_delete.dm

@@ -5,10 +5,11 @@
 	if(D)
 		if(islist(D))
 			var/index = name
+			var/list/L = D
 			if (value)
-				name = D[name] //name is really the index until this line
+				name = L[name] //name is really the index until this line
 			else
-				value = D[name]
+				value = L[name]
 			header = "<li style='backgroundColor:white'>([VV_HREF_TARGET_1V(D, VV_HK_LIST_EDIT, "E", index)]) ([VV_HREF_TARGET_1V(D, VV_HK_LIST_CHANGE, "C", index)]) ([VV_HREF_TARGET_1V(D, VV_HK_LIST_REMOVE, "-", index)]) "
 		else
 			header = "<li style='backgroundColor:white'>([VV_HREF_TARGET_1V(D, VV_HK_BASIC_EDIT, "E", name)]) ([VV_HREF_TARGET_1V(D, VV_HK_BASIC_CHANGE, "C", name)]) ([VV_HREF_TARGET_1V(D, VV_HK_BASIC_MASSEDIT, "M", name)]) "