Additional GAGS configuration json verification (tgstation#59524)

This standardizes how values are read from the json for greyscale layers so that error handling can check for some additional things:
No extra keys in the json that are unknown
No missing keys that a layer needs to work
Values are the expected type for that key

A variety of error messages have been added for various ways the json can be malformed and should hopefully provide good feedback for anyone working with greyscale configurations.

code/datums/greyscale/_greyscale_config.dm

@@ -93,7 +93,7 @@
 		var/layer_type = SSgreyscale.layer_types[data["type"]]
 		if(!layer_type)
 			CRASH("An unknown layer type was specified in the json of greyscale configuration [DebugName()]: [data["layer_type"]]")
-		return new layer_type(icon_file, data)
+		return new layer_type(icon_file, data.Copy()) // We don't want anything in there touching our version of the data
 	var/list/output = list()
 	for(var/list/group as anything in data)
 		output += ReadLayerGroup(group)

code/datums/greyscale/json_reader.dm

@@ -0,0 +1,54 @@
+/// Takes a json list and extracts a single value.
+/// Subtypes represent different conversions of that value.
+/datum/json_reader
+
+/// Takes a value read directly from json and verifies/converts as needed to a result
+/datum/json_reader/proc/ReadJson(value)
+	return
+
+/datum/json_reader/text/ReadJson(value)
+	if(!istext(value))
+		CRASH("Text value expected but got '[value]'")
+	return value
+
+/datum/json_reader/number/ReadJson(value)
+	var/newvalue = text2num(value)
+	if(!isnum(newvalue))
+		CRASH("Number expected but got [newvalue]")
+	return newvalue
+
+/datum/json_reader/number_color_list/ReadJson(list/value)
+	if(!istype(value))
+		CRASH("Expected a list but got [value]")
+	var/list/new_values = list()
+	for(var/number_string in value)
+		var/new_value = text2num(number_string)
+		if(!isnum(new_value))
+			if(!istext(number_string) || number_string[1] != "#")
+				stack_trace("Expected list to only contain numbers or colors but got '[number_string]'")
+				continue
+			new_value = number_string
+		new_values += new_value
+	return new_values
+
+/datum/json_reader/blend_mode
+	var/static/list/blend_modes = list(
+		"add" = ICON_ADD,
+		"subtract" = ICON_SUBTRACT,
+		"multiply" = ICON_MULTIPLY,
+		"or" = ICON_OR,
+		"overlay" = ICON_OVERLAY,
+		"underlay" = ICON_UNDERLAY,
+	)
+
+/datum/json_reader/blend_mode/ReadJson(value)
+	var/new_value = blend_modes[lowertext(value)]
+	if(isnull(new_value))
+		CRASH("Blend mode expected but got '[value]'")
+	return new_value
+
+/datum/json_reader/greyscale_config/ReadJson(value)
+	var/newvalue = SSgreyscale.configurations[value]
+	if(!newvalue)
+		CRASH("Greyscale configuration type expected but got '[value]'")
+	return newvalue

code/datums/greyscale/layer.dm

@@ -3,26 +3,58 @@
 	var/list/color_ids
 	var/blend_mode
 
-	var/static/list/blend_modes = list(
-		"add" = ICON_ADD,
-		"subtract" = ICON_SUBTRACT,
-		"multiply" = ICON_MULTIPLY,
-		"or" = ICON_OR,
-		"overlay" = ICON_OVERLAY,
-		"underlay" = ICON_UNDERLAY,
-	)
+	var/static/list/json_readers
 
 /datum/greyscale_layer/New(icon_file, list/json_data)
-	color_ids = json_data["color_ids"]
-	for(var/i in color_ids)
-		if(isnum(i))
+	if(!json_readers)
+		json_readers = list()
+		for(var/path in subtypesof(/datum/json_reader))
+			json_readers[path] = new path
+
+	json_data -= "type" // This is used to look us up and doesn't need to be verified like the rest of the data
+
+	ReadJsonData(json_data)
+	Initialize(icon_file)
+
+/// Override this to do initial set up
+/datum/greyscale_layer/proc/Initialize(icon_file)
+	return
+
+/// Handles the processing of the json data and conversion to correct value types.
+/// Will error on incorrect, missing, or unexpected values.
+/datum/greyscale_layer/proc/ReadJsonData(list/json_data)
+	var/list/required_values = list()
+	var/list/optional_values = list()
+	GetExpectedValues(required_values, optional_values)
+	for(var/keyname in json_data)
+		if(required_values[keyname] && optional_values[keyname])
+			stack_trace("Key '[keyname]' found in both required and optional lists. Make sure keys are only in one or the other.")
+			continue
+		if(!required_values[keyname] && !optional_values[keyname])
+			stack_trace("Unknown key found in json for [src]: '[keyname]'")
 			continue
-		if(istext(i) && i[1] == "#")
+		if(!(keyname in vars))
+			stack_trace("[src] expects a value from '[keyname]' but has no var to hold the output.")
 			continue
-		CRASH("Color ids must be a color string or positive integer starting from 1, '[i]' is not valid.")
-	blend_mode = blend_modes[lowertext(json_data["blend_mode"])]
-	if(isnull(blend_mode))
-		CRASH("Greyscale config for [icon_file] is missing a blend mode on a layer.")
+		var/datum/json_reader/reader = required_values[keyname] || optional_values[keyname]
+		reader = json_readers[reader]
+		if(!reader)
+			stack_trace("[src] has an invalid json reader type '[required_values[keyname]]' for key '[keyname]'.")
+			continue
+		vars[keyname] = reader.ReadJson(json_data[keyname])
+
+	// Final check to make sure we got everything we needed
+	for(var/keyname in required_values)
+		if(isnull(json_data[keyname]))
+			stack_trace("[src] is missing required json data key '[keyname]'.")
+
+/// Gathers information from the layer about what variables are expected in the json.
+/// Override and add to the two argument lists if you want extra information in your layer.
+/// The lists are formatted like keyname:keytype_define.
+/// The key name is assigned to the var named the same on the layer type.
+/datum/greyscale_layer/proc/GetExpectedValues(list/required_values, list/optional_values)
+	optional_values[NAMEOF(src, color_ids)] = /datum/json_reader/number_color_list
+	required_values[NAMEOF(src, blend_mode)] = /datum/json_reader/blend_mode
 
 /// Used to actualy create the layer using the given colors
 /// Do not override, use InternalGenerate instead
@@ -45,20 +77,24 @@
 /// The most basic greyscale layer; a layer which is created from a single icon_state in the given icon file
 /datum/greyscale_layer/icon_state
 	layer_type = "icon_state"
+	var/icon_state
 	var/icon/icon
 	var/color_id
 
-/datum/greyscale_layer/icon_state/New(icon_file, list/json_data)
+/datum/greyscale_layer/icon_state/Initialize(icon_file)
 	. = ..()
-	var/icon_state = json_data["icon_state"]
 	var/list/icon_states = icon_states(icon_file)
 	if(!(icon_state in icon_states))
 		CRASH("Configured icon state \[[icon_state]\] was not found in [icon_file]. Double check your json configuration.")
-	icon = new(icon_file, json_data["icon_state"])
+	icon = new(icon_file, icon_state)
 
 	if(length(color_ids) > 1)
 		CRASH("Icon state layers can not have more than one color id")
 
+/datum/greyscale_layer/icon_state/GetExpectedValues(list/required_values, list/optional_values)
+	. = ..()
+	required_values[NAMEOF(src, icon_state)] = /datum/json_reader/text
+
 /datum/greyscale_layer/icon_state/InternalGenerate(list/colors, list/render_steps)
 	. = ..()
 	var/icon/new_icon = icon(icon)
@@ -70,17 +106,15 @@
 /datum/greyscale_layer/reference
 	layer_type = "reference"
 	var/icon_state
-	var/datum/greyscale_config/reference_config
+	var/datum/greyscale_config/reference_type
 
-/datum/greyscale_layer/reference/New(icon_file, list/json_data)
+/datum/greyscale_layer/reference/GetExpectedValues(list/required_values, list/optional_values)
 	. = ..()
-	icon_state = json_data["icon_state"] || ""
-	reference_config = SSgreyscale.configurations[json_data["reference_type"]]
-	if(!reference_config)
-		CRASH("An unknown greyscale configuration was given to a reference layer: [json_data["reference_type"]]")
+	optional_values[NAMEOF(src, icon_state)] = /datum/json_reader/text
+	required_values[NAMEOF(src, reference_type)] = /datum/json_reader/greyscale_config
 
 /datum/greyscale_layer/reference/InternalGenerate(list/colors, list/render_steps)
 	if(render_steps)
-		return reference_config.GenerateBundle(colors, render_steps)
+		return reference_type.GenerateBundle(colors, render_steps)
 	else
-		return reference_config.Generate(colors.Join())
+		return reference_type.Generate(colors.Join())

tgstation.dme

@@ -735,6 +735,7 @@
 #include "code\datums\elements\food\processable.dm"
 #include "code\datums\elements\food\venue_price.dm"
 #include "code\datums\greyscale\_greyscale_config.dm"
+#include "code\datums\greyscale\json_reader.dm"
 #include "code\datums\greyscale\layer.dm"
 #include "code\datums\greyscale\config_types\greyscale_configs.dm"
 #include "code\datums\greyscale\config_types\material_effects.dm"