[MIRROR] Allows vv investigate /appearance + better checking image (#…

…2128) (#3013)

* Allows vv investigate /appearance + better checking image (#82670)

* Allows vv investigate /appearance + better checking image

---------

Co-authored-by: NovaBot <[email protected]>
Co-authored-by: EvilDragonfiend <[email protected]>

code/__DEFINES/vv.dm

@@ -54,6 +54,8 @@
 
 //Helpers for vv_get_dropdown()
 #define VV_DROPDOWN_OPTION(href_key, name) . += "<option value='?_src_=vars;[HrefToken()];[href_key]=TRUE;target=[REF(src)]'>[name]</option>"
+//Same with VV_DROPDOWN_OPTION, but global proc doesn't have src
+#define VV_DROPDOWN_OPTION_APPEARANCE(thing, href_key, name) . += "<option value='?_src_=vars;[HrefToken()];[href_key]=TRUE;target=[REF(thing)]'>[name]</option>"
 
 // VV HREF KEYS
 #define VV_HK_TARGET "target"

code/modules/admin/view_variables/debug_variable_appearance.dm

@@ -0,0 +1,236 @@
+/* 	< OH MY GOD. Can't you just make "/image/proc/foo()" instead of making these? >
+ * 		/appearance is a hardcoded byond type, and it is very internal type.
+ *		Its type is actually /image, but it isn't truly /image. We defined it as "/appearance"
+ * 		new procs to /image will only work to actual /image references, but...
+ * 		/appearance references are not capable of executing procs, because these are not real /image
+ * 		This is why these global procs exist. Welcome to the curse.
+ */
+#define ADD_UNUSED_VAR(varlist, thing, varname) if(NAMEOF(##thing, ##varname)) ##varlist += #varname
+#define RESULT_VARIABLE_NOT_FOUND "_switch_result_variable_not_found"
+
+/// An alias datum that allows us to access and view the variables of an appearance by keeping certain known, yet undocumented, variables that we can access and read in a datum for debugging purposes.
+/// Kindly do not use this outside of a debugging context.
+/image/appearance
+	parent_type = /atom/movable // This is necessary to access the variables on compile-time.
+
+	// var/override // Sad point. We can't steal byond internal variable name
+#ifdef OPENDREAM
+	// opendream doens't support mouse_drop_zone yet. Remove this once OD supports it.
+	var/mouse_drop_zone
+#endif
+
+/image/appearance/New(loc, ...)
+	. = ..()
+	CRASH("something tried to use '/image/appearance', but this isn't actual type we use. Do not fucking do this.")
+
+/// Makes a var list of /appearance type actually uses. This will be only called once.
+/proc/build_virtual_appearance_vars()
+	var/list/used_variables = list("vis_flags") // manual listing.
+	. = used_variables
+	var/list/unused_var_names = list()
+
+	var/image/appearance/nameof_reference // We don't copy vars from this.
+	pass(nameof_reference) // compiler complains unused variable
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, appearance) // it only does self-reference
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, x) // xyz are always 0
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, y)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, z)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, weak_reference) // it's not a good idea to make a weak_ref on this, and this won't have it
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, vars) // inherited from /image, but /appearance hasn't this
+
+	// Even if these vars are essential for image, these only exists in an actual type
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, filter_data)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, realized_overlays)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, realized_underlays)
+
+	// we have no reason to show these, right?
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, _active_timers)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, _datum_components)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, _listen_lookup)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, _signal_procs)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, __auxtools_weakref_id)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, _status_traits)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, cooldowns)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, datum_flags)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, verbs)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, gc_destroyed)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, harddel_deets_dumped)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, open_uis)
+	ADD_UNUSED_VAR(unused_var_names, nameof_reference, tgui_shared_states)
+
+	var/image/dummy_image = image(null, null) // actual type we'll copy variable names
+	for(var/each in dummy_image.vars) // try to inherit var list from /image
+		if(each in unused_var_names)
+			continue
+		used_variables += each
+	del(dummy_image)
+	dummy_image = null
+
+	return used_variables
+
+/// debug_variable() proc but made for /appearance type specifically
+/proc/debug_variable_appearance(var_name, appearance)
+	var/value
+	try
+		value = locate_appearance_variable(var_name, appearance)
+	catch
+		return "<li style='backgroundColor:white'>(READ ONLY) <font color='blue'>[var_name] = (untrackable)</font></li>"
+	if(value == RESULT_VARIABLE_NOT_FOUND)
+		return "<li style='backgroundColor:white'>(READ ONLY) [var_name] <font color='blue'>(Undefined var name in switch)</font></li>"
+	return "<li style='backgroundColor:white'>(READ ONLY) [var_name] = [_debug_variable_value(var_name, value, 0, appearance, sanitize = TRUE, display_flags = NONE)]</li>"
+
+/// manually locate a variable through string value.
+/// appearance type needs a manual var referencing because it doesn't have "vars" variable internally.
+/// There's no way doing this in a fancier way.
+/proc/locate_appearance_variable(var_name, image/appearance/appearance) // WARN: /image/appearance is a mocking type, not real one
+	switch(var_name) // Welcome to this curse
+		// appearance doesn't have "vars" variable.
+		// This means you need to target a variable manually through this way.
+
+		// appearance vars in DM document
+		if(NAMEOF(appearance, alpha))
+			return appearance.alpha
+		if(NAMEOF(appearance, appearance_flags))
+			return appearance.appearance_flags
+		if(NAMEOF(appearance, blend_mode))
+			return appearance.blend_mode
+		if(NAMEOF(appearance, color))
+			return appearance.color
+		if(NAMEOF(appearance, desc))
+			return appearance.desc
+		if(NAMEOF(appearance, gender))
+			return appearance.gender
+		if(NAMEOF(appearance, icon))
+			return appearance.icon
+		if(NAMEOF(appearance, icon_state))
+			return appearance.icon_state
+		if(NAMEOF(appearance, invisibility))
+			return appearance.invisibility
+		if(NAMEOF(appearance, infra_luminosity))
+			return appearance.infra_luminosity
+		if(NAMEOF(appearance, filters))
+			return appearance.filters
+		if(NAMEOF(appearance, layer))
+			return appearance.layer
+		if(NAMEOF(appearance, luminosity))
+			return appearance.luminosity
+		if(NAMEOF(appearance, maptext))
+			return appearance.maptext
+		if(NAMEOF(appearance, maptext_width))
+			return appearance.maptext_width
+		if(NAMEOF(appearance, maptext_height))
+			return appearance.maptext_height
+		if(NAMEOF(appearance, maptext_x))
+			return appearance.maptext_x
+		if(NAMEOF(appearance, maptext_y))
+			return appearance.maptext_y
+		if(NAMEOF(appearance, mouse_over_pointer))
+			return appearance.mouse_over_pointer
+		if(NAMEOF(appearance, mouse_drag_pointer))
+			return appearance.mouse_drag_pointer
+		if(NAMEOF(appearance, mouse_drop_pointer))
+			return appearance.mouse_drop_pointer
+		if(NAMEOF(appearance, mouse_drop_zone))
+			return appearance:mouse_drop_zone
+		if(NAMEOF(appearance, mouse_opacity))
+			return appearance.mouse_opacity
+		if(NAMEOF(appearance, name))
+			return appearance.name
+		if(NAMEOF(appearance, opacity))
+			return appearance.opacity
+		if(NAMEOF(appearance, overlays))
+			return appearance.overlays
+		if("override") // only /image has this. mocking type can't steal byond internal var name
+			var/image/image_appearance = appearance
+			return image_appearance.override
+		if(NAMEOF(appearance, pixel_x))
+			return appearance.pixel_x
+		if(NAMEOF(appearance, pixel_y))
+			return appearance.pixel_y
+		if(NAMEOF(appearance, pixel_w))
+			return appearance.pixel_w
+		if(NAMEOF(appearance, pixel_z))
+			return appearance.pixel_z
+		if(NAMEOF(appearance, plane))
+			return appearance.plane
+		if(NAMEOF(appearance, render_source))
+			return appearance.render_source
+		if(NAMEOF(appearance, render_target))
+			return appearance.render_target
+		if(NAMEOF(appearance, suffix))
+			return appearance.suffix
+		if(NAMEOF(appearance, text))
+			return appearance.text
+		if(NAMEOF(appearance, transform))
+			return appearance.transform
+		if(NAMEOF(appearance, underlays))
+			return appearance.underlays
+
+		if(NAMEOF(appearance, parent_type))
+			return appearance.parent_type
+		if(NAMEOF(appearance, type))
+			return /image/appearance // don't fool people
+
+		// These are not documented ones but trackable values. Maybe we'd want these.
+		if(NAMEOF(appearance, animate_movement))
+			return appearance.animate_movement
+		if(NAMEOF(appearance, dir))
+			return appearance.dir
+		if(NAMEOF(appearance, glide_size))
+			return appearance.glide_size
+		if("pixel_step_size")
+			return "" //atom_appearance.pixel_step_size
+			// DM compiler complains this
+
+		// I am not sure if these will be ever detected, but I made a connection just in case.
+		if(NAMEOF(appearance, contents)) // It's not a thing, but I don't believe how DM will change /appearance in future.
+			return appearance.contents
+		if(NAMEOF(appearance, loc)) // same reason above
+			return appearance.loc
+		if(NAMEOF(appearance, vis_contents)) // same reason above
+			return appearance.vis_contents
+		if(NAMEOF(appearance, vis_flags)) // DM document says /appearance has this, but it throws error
+			return appearance.vis_flags
+
+		// we wouldn't need these, but let's these trackable anyway...
+		if(NAMEOF(appearance, density))
+			return appearance.density
+		if(NAMEOF(appearance, screen_loc))
+			return appearance.screen_loc
+		if(NAMEOF(appearance, verbs))
+			return appearance.verbs
+		if(NAMEOF(appearance, tag))
+			return appearance.tag
+	return RESULT_VARIABLE_NOT_FOUND
+
+/// Shows a header name on top when you investigate an appearance
+/proc/vv_get_header_appearance(image/thing)
+	. = list()
+	var/icon_name = "<b>[thing.icon || "null"]</b><br/>"
+	. += replacetext(icon_name, "icons/obj", "") // shortens the name. We know the path already.
+	if(thing.icon)
+		. += thing.icon_state ? "\"[thing.icon_state]\"" : "(icon_state = null)"
+
+/image/vv_get_header() // it should redirect to global proc version because /appearance can't call a proc, unless we want dupe code here
+	return vv_get_header_appearance(src)
+
+/// Makes a format name for shortened vv name.
+/proc/get_appearance_vv_summary_name(image/thing)
+	var/icon_file_name = thing.icon ? splittext("[thing.icon]", "/") : "null"
+	if(islist(icon_file_name))
+		icon_file_name = length(icon_file_name) ? icon_file_name[length(icon_file_name)] : "null"
+	if(thing.icon_state)
+		return "[icon_file_name]:[thing.icon_state]"
+	else
+		return "[icon_file_name]"
+
+/proc/vv_get_dropdown_appearance(image/thing)
+	. = list()
+	// Don't add any vv option carelessly unless you have a good reason to add one for /appearance.
+	// /appearance type shouldn't allow general options. Even "Mark Datum" is a questionable behaviour here.
+	VV_DROPDOWN_OPTION_APPEARANCE(thing, "", "---")
+	VV_DROPDOWN_OPTION_APPEARANCE(thing, VV_HK_EXPOSE, "Show VV To Player") // only legit option
+	return .
+
+#undef ADD_UNUSED_VAR
+#undef RESULT_VARIABLE_NOT_FOUND

code/modules/admin/view_variables/debug_variables.dm

@@ -49,9 +49,12 @@
 		return "/icon (<span class='value'>[value]</span>)"
 		#endif
 
-	if(isappearance(value))
-		var/image/actually_an_appearance = value
-		return "/appearance (<span class='value'>[actually_an_appearance.icon]</span>)"
+
+	if(isappearance(value)) // Reminder: Do not replace this into /image/debug_variable_value() proc. /appearance can't do that.
+		return "<a href='?_src_=vars;[HrefToken()];Vars=[REF(value)]'>/appearance (<span class='value'>[get_appearance_vv_summary_name(value)]</span>) [REF(value)]</a>"
+
+	if(isimage(value))
+		return "<a href='?_src_=vars;[HrefToken()];Vars=[REF(value)]'>[value:type] (<span class='value'>[get_appearance_vv_summary_name(value)]</span>) [REF(value)]</a>"
 
 	if(isfilter(value))
 		var/datum/filter_value = value

code/modules/admin/view_variables/view_variables.dm

@@ -1,7 +1,10 @@
+#define ICON_STATE_CHECKED 1 /// this dmi is checked. We don't check this one anymore.
+#define ICON_STATE_NULL 2 /// this dmi has null-named icon_state, allowing it to show a sprite on vv editor.
+
 ADMIN_VERB_AND_CONTEXT_MENU(debug_variables, R_NONE, "View Variables", "View the variables of a datum.", ADMIN_CATEGORY_DEBUG, datum/thing in world)
 	user.debug_variables(thing)
-
 // This is kept as a seperate proc because admins are able to show VV to non-admins
+
 /client/proc/debug_variables(datum/thing in world)
 	set category = "Debug"
 	set name = "View Variables"
@@ -18,26 +21,44 @@ ADMIN_VERB_AND_CONTEXT_MENU(debug_variables, R_NONE, "View Variables", "View the
 	var/datum/asset/asset_cache_datum = get_asset_datum(/datum/asset/simple/vv)
 	asset_cache_datum.send(usr)
 
+	var/isappearance = isappearance(thing)
 	var/islist = islist(thing) || (!isdatum(thing) && hascall(thing, "Cut")) // Some special lists dont count as lists, but can be detected by if they have list procs
-	if(!islist && !isdatum(thing))
+	if(!islist && !isdatum(thing) && !isappearance)
 		return
 
 	var/title = ""
 	var/refid = REF(thing)
 	var/icon/sprite
 	var/hash
 
-	var/type = islist? /list : thing.type
+	var/type = islist? /list : (isappearance ? "/appearance" : thing.type)
 	var/no_icon = FALSE
 
 	if(isatom(thing))
 		sprite = getFlatIcon(thing)
 		if(!sprite)
 			no_icon = TRUE
 
-	else if(isimage(thing))
+	else if(isimage(thing) || isappearance)
+		// icon_state=null shows first image even if dmi has no icon_state for null name.
+		// This list remembers which dmi has null icon_state, to determine if icon_state=null should display a sprite
+		// (NOTE: icon_state="" is correct, but saying null is obvious)
+		var/static/list/dmi_nullstate_checklist = list()
 		var/image/image_object = thing
-		sprite = icon(image_object.icon, image_object.icon_state)
+		var/icon_filename_text = "[image_object.icon]" // "icon(null)" type can exist. textifying filters it.
+		if(icon_filename_text)
+			if(image_object.icon_state)
+				sprite = icon(image_object.icon, image_object.icon_state)
+
+			else // it means: icon_state=""
+				if(!dmi_nullstate_checklist[icon_filename_text])
+					dmi_nullstate_checklist[icon_filename_text] = ICON_STATE_CHECKED
+					if("" in icon_states(image_object.icon))
+						// this dmi has nullstate. We'll allow "icon_state=null" to show image.
+						dmi_nullstate_checklist[icon_filename_text] = ICON_STATE_NULL
+
+				if(dmi_nullstate_checklist[icon_filename_text] == ICON_STATE_NULL)
+					sprite = icon(image_object.icon, image_object.icon_state)
 
 	var/sprite_text
 	if(sprite)
@@ -48,7 +69,7 @@ ADMIN_VERB_AND_CONTEXT_MENU(debug_variables, R_NONE, "View Variables", "View the
 	title = "[thing] ([REF(thing)]) = [type]"
 	var/formatted_type = replacetext("[type]", "/", "<wbr>/")
 
-	var/list/header = islist ? list("<b>/list</b>") : thing.vv_get_header()
+	var/list/header = islist ? list("<b>/list</b>") : (isappearance ? vv_get_header_appearance(thing) : thing.vv_get_header())
 
 	var/ref_line = "@[copytext(refid, 2, -1)]" // get rid of the brackets, add a @ prefix for copy pasting in asay
 
@@ -82,11 +103,16 @@ ADMIN_VERB_AND_CONTEXT_MENU(debug_variables, R_NONE, "View Variables", "View the
 			var/name = dropdownoptions[i]
 			var/link = dropdownoptions[name]
 			dropdownoptions[i] = "<option value[link? "='[link]'":""]>[name]</option>"
+	else if(isappearance)
+		dropdownoptions = vv_get_dropdown_appearance(thing)
 	else
 		dropdownoptions = thing.vv_get_dropdown()
 
 	var/list/names = list()
-	if(!islist)
+	if(isappearance)
+		var/static/list/virtual_appearance_vars = build_virtual_appearance_vars()
+		names = virtual_appearance_vars.Copy()
+	else if(!islist)
 		for(var/varname in thing.vars)
 			names += varname
 
@@ -101,6 +127,10 @@ ADMIN_VERB_AND_CONTEXT_MENU(debug_variables, R_NONE, "View Variables", "View the
 			if(IS_NORMAL_LIST(list_value) && IS_VALID_ASSOC_KEY(key))
 				value = list_value[key]
 			variable_html += debug_variable(i, value, 0, list_value)
+	else if(isappearance)
+		names = sort_list(names)
+		for(var/varname in names)
+			variable_html += debug_variable_appearance(varname, thing)
 	else
 		names = sort_list(names)
 		for(var/varname in names)
@@ -285,3 +315,6 @@ datumrefresh=[refid];[HrefToken()]'>Refresh</a>
 
 /client/proc/vv_update_display(datum/thing, span, content)
 	src << output("[span]:[content]", "variables[REF(thing)].browser:replace_span")
+
+#undef ICON_STATE_CHECKED
+#undef ICON_STATE_NULL

code/modules/unit_tests/unit_test.dm

@@ -222,6 +222,8 @@ GLOBAL_VAR_INIT(focused_tests, focused_tests())
 	var/list/returnable_list = list()
 	// The following are just generic, singular types.
 	returnable_list = list(
+		//this is somehow a subtype of /atom/movable, because of its purpose...
+		/image/appearance,
 		//Never meant to be created, errors out the ass for mobcode reasons
 		/mob/living/carbon,
 		//And another

tgstation.dme

@@ -3016,6 +3016,7 @@
 #include "code\modules\admin\verbs\SDQL2\SDQL_2_wrappers.dm"
 #include "code\modules\admin\view_variables\admin_delete.dm"
 #include "code\modules\admin\view_variables\color_matrix_editor.dm"
+#include "code\modules\admin\view_variables\debug_variable_appearance.dm"
 #include "code\modules\admin\view_variables\debug_variables.dm"
 #include "code\modules\admin\view_variables\filterrific.dm"
 #include "code\modules\admin\view_variables\get_variables.dm"