From 8eed0b2a1cc72d06dd88e5189e4458a063415a80 Mon Sep 17 00:00:00 2001 From: Emmett Gaines Date: Sun, 13 Jun 2021 14:23:13 -0400 Subject: [PATCH] Additional GAGS configuration json verification (#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 | 2 +- code/datums/greyscale/json_reader.dm | 54 +++++++++++++ code/datums/greyscale/layer.dm | 88 +++++++++++++++------- tgstation.dme | 1 + 4 files changed, 117 insertions(+), 28 deletions(-) create mode 100644 code/datums/greyscale/json_reader.dm diff --git a/code/datums/greyscale/_greyscale_config.dm b/code/datums/greyscale/_greyscale_config.dm index 60ed15ca6a3ba..99c1d715fd670 100644 --- a/code/datums/greyscale/_greyscale_config.dm +++ b/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) diff --git a/code/datums/greyscale/json_reader.dm b/code/datums/greyscale/json_reader.dm new file mode 100644 index 0000000000000..883c3993541e5 --- /dev/null +++ b/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 diff --git a/code/datums/greyscale/layer.dm b/code/datums/greyscale/layer.dm index bfcb37ed47a7e..bdf2c4e7f776e 100644 --- a/code/datums/greyscale/layer.dm +++ b/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()) diff --git a/tgstation.dme b/tgstation.dme index e479343a17665..b59be1c5cc50c 100644 --- a/tgstation.dme +++ b/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"