From a42b91ea992f9d5dc72a9dc6bd0beb868cdd2061 Mon Sep 17 00:00:00 2001 From: warriorstar-orion Date: Fri, 27 Sep 2024 13:05:53 -0400 Subject: [PATCH] blackbox: Add accumulating feedback types for numeric and text values. (#26645) * blackbox: Add accumulating feedback types for numeric and text values. * correct introduction * i swear to god i can count to 7 * bloody VSC formatting on save * formatting changes and doc additions --- SQL/paradise_schema.sql | 2 +- SQL/updates/59-60.sql | 23 +++ code/__DEFINES/misc_defines.dm | 2 +- code/controllers/subsystem/SSblackbox.dm | 50 +++-- config/example/config.toml | 2 +- docs/references/feedback_data.md | 240 +++++++++++++++++------ 6 files changed, 243 insertions(+), 76 deletions(-) create mode 100644 SQL/updates/59-60.sql diff --git a/SQL/paradise_schema.sql b/SQL/paradise_schema.sql index 898e8d0cb8a..523d8eb7f5e 100644 --- a/SQL/paradise_schema.sql +++ b/SQL/paradise_schema.sql @@ -250,7 +250,7 @@ CREATE TABLE `feedback` ( `datetime` datetime NOT NULL, `round_id` int(8) NOT NULL, `key_name` varchar(32) NOT NULL, - `key_type` enum('text', 'amount', 'tally', 'nested tally', 'associative') NOT NULL, + `key_type` ENUM('text', 'amount', 'tally', 'nested tally', 'associative', 'ledger', 'nested ledger') NOT NULL, `version` tinyint(3) UNSIGNED NOT NULL, `json` LONGTEXT NOT NULL COLLATE 'utf8mb4_general_ci', PRIMARY KEY (`id`) diff --git a/SQL/updates/59-60.sql b/SQL/updates/59-60.sql new file mode 100644 index 00000000000..e826c7a0bff --- /dev/null +++ b/SQL/updates/59-60.sql @@ -0,0 +1,23 @@ +-- Migration: 59-60 +-- Author: warriorstar +-- Introduced: PR# 26645 + +-- This migration adds the 'ledger' and 'nested ledger' enum values to the +-- `feedback` table in conjunction with making those feedback types available +-- through SSblackbox. +-- No data migration is required. + +ALTER TABLE + `feedback` +MODIFY + COLUMN `key_type` ENUM( + 'text', + 'amount', + 'tally', + 'nested tally', + 'associative', + 'ledger', + 'nested ledger' + ) NOT NULL +AFTER + `key_name`; diff --git a/code/__DEFINES/misc_defines.dm b/code/__DEFINES/misc_defines.dm index 1e3392948ba..7d5b8ecdb25 100644 --- a/code/__DEFINES/misc_defines.dm +++ b/code/__DEFINES/misc_defines.dm @@ -423,7 +423,7 @@ #define INVESTIGATE_HOTMIC "hotmic" // The SQL version required by this version of the code -#define SQL_VERSION 59 +#define SQL_VERSION 60 // Vending machine stuff #define CAT_NORMAL (1<<0) diff --git a/code/controllers/subsystem/SSblackbox.dm b/code/controllers/subsystem/SSblackbox.dm index cdd10077d26..1141f715152 100644 --- a/code/controllers/subsystem/SSblackbox.dm +++ b/code/controllers/subsystem/SSblackbox.dm @@ -195,13 +195,16 @@ SUBSYSTEM_DEF(blackbox) * Arguments: * * key_type - Type of key. Either "text", "amount", "tally", "nested tally", "associative" * * key - Key of the data to be used (EG: "admin_verb") - * * increment - If using "amount", how much to increment why + * * stat - Either a number accumulated via "amount", "tally", or "nested tally"; or a number/string collected by "ledger" or "nested ledger". * * data - The actual data to logged * * overwrite - Do we want to overwrite the existing key * * ignore_seal - Does the feedback go in regardless of blackbox sealed status? (EG: map vote results) */ -/datum/controller/subsystem/blackbox/proc/record_feedback(key_type, key, increment, data, overwrite, ignore_seal) - if((sealed && !ignore_seal) || !key_type || !istext(key) || !isnum(increment || !data)) +/datum/controller/subsystem/blackbox/proc/record_feedback(key_type, key, stat, data, overwrite, ignore_seal) + var/is_invalid_value = !isnum(stat || !data) + if(key_type == "ledger" || key_type == "nested ledger") + is_invalid_value &&= !istext(stat) + if((sealed && !ignore_seal) || !key_type || !istext(key) || is_invalid_value) return var/datum/feedback_variable/FV = find_feedback_datum(key, key_type) switch(key_type) @@ -215,17 +218,17 @@ SUBSYSTEM_DEF(blackbox) else FV.json["data"] |= data if("amount") - FV.json["data"] += increment + FV.json["data"] += stat if("tally") if(!islist(FV.json["data"])) FV.json["data"] = list() - FV.json["data"]["[data]"] += increment + FV.json["data"]["[data]"] += stat if("nested tally") if(!islist(data)) return if(!islist(FV.json["data"])) FV.json["data"] = list() - FV.json["data"] = record_feedback_recurse_list(FV.json["data"], data, increment) + FV.json["data"] = record_feedback_recurse_list(FV.json["data"], data, stat) if("associative") if(!islist(data)) return @@ -235,6 +238,18 @@ SUBSYSTEM_DEF(blackbox) FV.json["data"]["[pos]"] = list() for(var/i in data) FV.json["data"]["[pos]"]["[i]"] = "[data[i]]" + if("ledger") + if(!islist(FV.json["data"])) + FV.json["data"] = list() + if(!islist(FV.json["data"]["[data]"])) + FV.json["data"]["[data]"] = list() + FV.json["data"]["[data]"] += list(stat) + if("nested ledger") + if(!islist(data)) + return + if(!islist(FV.json["data"])) + FV.json["data"] = list() + FV.json["data"] = record_feedback_recurse_list(FV.json["data"], data, stat, accumulate = FALSE) /** * Recursive list recorder @@ -244,21 +259,28 @@ SUBSYSTEM_DEF(blackbox) * Arguments: * * L - List to use * * key_list - List of keys to add - * * increment - How much to increase by + * * value - How much to increase by or append to * * depth - Depth to use + * * accumulate - TRUE if we are adding `value` to a tally, FALSE if we are appending it to a record */ -/datum/controller/subsystem/blackbox/proc/record_feedback_recurse_list(list/L, list/key_list, increment, depth = 1) +/datum/controller/subsystem/blackbox/proc/record_feedback_recurse_list(list/L, list/key_list, value, depth = 1, accumulate = TRUE) + var/key_depth = key_list[depth] if(depth == length(key_list)) - if(L.Find(key_list[depth])) - L["[key_list[depth]]"] += increment + if(L.Find(key_depth)) + if(accumulate) + L["[key_depth]"] += value + else + if(!islist(L["[key_depth]"])) + L["[key_depth]"] = list() + L["[key_depth]"] += list(value) else - var/list/list_found_index = list(key_list[depth] = increment) + var/list/list_found_index = accumulate ? list("[key_depth]" = value) : list("[key_depth]" = list(value)) L += list_found_index else - if(!L.Find(key_list[depth])) - var/list/list_go_down = list(key_list[depth] = list()) + if(!L.Find(key_depth)) + var/list/list_go_down = list("[key_depth]" = list()) L += list_go_down - L["[key_list[depth-1]]"] = .(L["[key_list[depth]]"], key_list, increment, ++depth) + L["[key_list[depth-1]]"] = .(L["[key_depth]"], key_list, value, ++depth, accumulate = accumulate) return L /** diff --git a/config/example/config.toml b/config/example/config.toml index 3c17b281b30..dbbd2398b41 100644 --- a/config/example/config.toml +++ b/config/example/config.toml @@ -181,7 +181,7 @@ ipc_screens = [ # Enable/disable the database on a whole sql_enabled = false # SQL version. If this is a mismatch, round start will be delayed -sql_version = 59 +sql_version = 60 # SQL server address. Can be an IP or DNS name sql_address = "127.0.0.1" # SQL server port diff --git a/docs/references/feedback_data.md b/docs/references/feedback_data.md index 4c94ce91537..c129a939765 100644 --- a/docs/references/feedback_data.md +++ b/docs/references/feedback_data.md @@ -25,12 +25,12 @@ Examples of bad things to record: base game design around" Also note that feedback data **must** be anonymous. The only exception here is -for data *anyone* on the server can see, such as round end antagonist reports. +for data _anyone_ on the server can see, such as round end antagonist reports. ## Feedback Data Recording -Feedback data can be recorded in 5 formats. `amount`, `associative`, `nested -tally`, `tally` and `text`. +Feedback data can be recorded in 7 formats. `amount`, `associative`, +`nested tally`, `tally` `text`, `ledger`, and `nested ledger`. ### Amount @@ -76,18 +76,18 @@ Will produce the following JSON: ```json { - "data": { - "1": { - "text": "example", - "path": "/obj/item", - "number": "4" - }, - "2": { - "number": "7", - "text": "example", - "other text": "sample" - } - } + "data": { + "1": { + "text": "example", + "path": "/obj/item", + "number": "4" + }, + "2": { + "number": "7", + "text": "example", + "other text": "sample" + } + } } ``` @@ -122,29 +122,29 @@ Will produce the following JSON: ```json { - "data":{ - "fruit":{ - "orange":{ - "apricot":4, - "orange":2 - }, - "red":{ - "apple":10 - } - }, - "vegetable":{ - "orange":{ - "carrot":1 - } - } - } + "data": { + "fruit": { + "orange": { + "apricot": 4, + "orange": 2 + }, + "red": { + "apple": 10 + } + }, + "vegetable": { + "orange": { + "carrot": 1 + } + } + } } ``` -#### NOTE +!!! note -Tracking values associated with a number can't merge with a nesting value, -trying to do so will append the list + Tracking values associated with a number can't merge with a nesting value, + trying to do so will append to the list. ```dm SSblackbox.record_feedback("nested tally", "example", 3, list("fruit", "orange")) @@ -154,23 +154,23 @@ Will produce the following JSON: ```json { - "data":{ - "fruit":{ - "orange":{ - "apricot":4, - "orange":2 - }, - "red":{ - "apple":10 - }, - "orange":3 - }, - "vegetable":{ - "orange":{ - "carrot":1 - } - } - } + "data": { + "fruit": { + "orange": { + "apricot": 4, + "orange": 2 + }, + "red": { + "apple": 10 + }, + "orange": 3 + }, + "vegetable": { + "orange": { + "carrot": 1 + } + } + } } ``` @@ -194,10 +194,10 @@ Will produce the following JSON: ```json { - "data":{ - "sample data":5, - "other data":2 - } + "data": { + "sample data": 5, + "other data": 2 + } } ``` @@ -218,10 +218,7 @@ Will produce the following JSON: ```json { - "data":[ - "sample text", - "other text" - ] + "data": ["sample text", "other text"] } ``` @@ -230,6 +227,131 @@ instead of a list with duplicates. Also take note how the `increment` parameter is not used here. It does nothing to the data, and `1` is used just as the value for consistency. +### Ledger + +!!! warning + + The `ledger` and `nested ledger` feedback types should only be used as a + last resort. The primary intent of the blackbox system is to track the + number of times something has happened. It is extremely rare that one should + require the granularity provided by these feedback types accumulating + multiple discrete statistics on a single row. They are provided as an escape + hatch for unusual situations, and to avoid unnecessary repetition of text in + the JSON. + +`ledger` is used for appending entries to a record. It is effectively the same +as `tally`, except instead of adding the value to the existing one, it stores +the value of each call in a list. This is useful for situations where you have a +specific key for which you'd like to store a unique value for each time the key +is used in feedback. For example, if the clown puts on multiple comedy shows and +you want to record the attendance for each one: + +```dm +SSblackbox.record_feedback("ledger", "tickets_sold_per_show", 15, "general_admission") +SSblackbox.record_feedback("ledger", "tickets_sold_per_show", 5, "front_row") +SSblackbox.record_feedback("ledger", "tickets_sold_per_show", 20, "general_admission") +SSblackbox.record_feedback("ledger", "tickets_sold_per_show", 2, "front_row") +``` + +Will produce the following JSON: + +```json +{ + "data": { + "tickets_sold_per_show": { + "general_admission": [15, 20], + "front_row": [5, 2] + } + } +} +``` + +Note that items here may be text. Unlike the `text` feedback type, items are added in order and duplicates are permitted. + +```dm +SSblackbox.record_feedback("ledger", "menu_items", "fried eggs", "breakfast") +SSblackbox.record_feedback("ledger", "menu_items", "coffee", "breakfast") +SSblackbox.record_feedback("ledger", "menu_items", "hamburgers", "lunch") +SSblackbox.record_feedback("ledger", "menu_items", "french fries", "lunch") +``` + +Will produce the following JSON: + +```json +{ + "data": { + "breakfast": ["fried eggs", "coffee"], + "lunch": ["hamburgers", "french fries"] + } +} +``` + +### Nested Ledger + +`nested ledger` uses the logic of `nested tally`, except instead of adding the value to the last element, it uses the last element as the tracking key for a list of items, appending each one. For example, if you wanted to store individual crew ratings for meals made by different chefs: + +```dm +SSblackbox.record_feedback("nested ledger", "meal_ratings", 5, list("Chef Marceau", "hamburger")) +SSblackbox.record_feedback("nested ledger", "meal_ratings", 2, list("Chef Marceau", "hamburger")) +SSblackbox.record_feedback("nested ledger", "meal_ratings", 1, list("Chef Poincare", "hamburger")) +SSblackbox.record_feedback("nested ledger", "meal_ratings", 3, list("Chef Poincare", "hamburger")) +``` + +Will produce the following JSON: + +```json +{ + "data": { + "Chef Marceau": { "hamburger": [5, 2] }, + "Chef Poincare": { "hamburger": [1, 3] } + } +} +``` + +As with `ledger`, text values may be accumulated in this manner. + +### Appropriate use of `ledger` + +If one were tracking the individual center coordinates of each ruin placed, and +the same ruin may be placed more thanonce, use of the `associative` feedback +type may result in this implementation: + +```dm +var/coord_string = "[central_turf.x],[central_turf.y],[central_turf.z]" +SSblackbox.record_feedback("associative", "ruin_placement", 1, list( + "map" = map_filename, + "coords" = coord_string +)) +``` + +returning the following JSON: + +```json +{ + "data": { + "1": { "map": "listeningpost.dmm", "coords": "127,169,5" }, + "2": { "map": "listeningpost.dmm", "coords": "64,134,4" } + } +} +``` + +This creates unnecessary repetition of the map name, as well as the `"map"` and `"coords"` keys. As well, the numeric keys associated with each row are meaningless. Using `ledger` may transform this into: + +```dm +var/coord_string = "[central_turf.x],[central_turf.y],[central_turf.z]" +SSblackbox.record_feedback("ledger", "ruin_placement", coord_string, map_filename) +``` + +returning the following JSON: + +```json +{ + "data": { + "listeningpost.dmm": ["127,169,5", "64,134,4"] + } +} +``` + ## Feedback Versioning If the logging content (i.e.: What data is logged) for a variable is ever