From eac6f2d07d7101c9d39f9b169f3c0bd0e1bb6d8c Mon Sep 17 00:00:00 2001 From: Kyle Spier-Swenson Date: Fri, 25 Nov 2022 00:51:03 -0800 Subject: [PATCH] Fixup list helpers, remove listoflist footgun from generic list procs, remove duplicated procs. (#71280) Add helper defines for handling list values in lists to remove the footgun where `+=` and `-=` with lists as the Right hand side argument causes the list contents to be added or removed, not the list itself. Use said helpers to remove the footgun from list helpers that could reasonably be expected to get called on list of lists. Remove duplicated clear nulls from list proc. this pr will fail to compile until i go move those over to the preexisting one, but the compile errors will tell me where all the consumers are. This likely fixes some bug(s) in the issue tracker, but we don't know what they are. --- code/__HELPERS/_lists.dm | 25 +++++++++++-------- code/modules/admin/verbs/hiddenprints.dm | 2 +- .../atmospherics/machinery/datum_pipeline.dm | 3 +-- code/modules/events/brand_intelligence.dm | 2 +- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/code/__HELPERS/_lists.dm b/code/__HELPERS/_lists.dm index f5c8c082f23..f58b89ca971 100644 --- a/code/__HELPERS/_lists.dm +++ b/code/__HELPERS/_lists.dm @@ -9,6 +9,14 @@ * Misc */ +// Generic listoflist safe add and removal macros: +///If value is a list, wrap it in a list so it can be used with list add/remove operations +#define LIST_VALUE_WRAP_LISTS(value) (islist(value) ? list(value) : value) +///Add an untyped item to a list, taking care to handle list items by wrapping them in a list to remove the footgun +#define UNTYPED_LIST_ADD(list, item) (list += LIST_VALUE_WRAP_LISTS(item)) +///Remove an untyped item to a list, taking care to handle list items by wrapping them in a list to remove the footgun +#define UNTYPED_LIST_REMOVE(list, item) (list -= LIST_VALUE_WRAP_LISTS(item)) + ///Initialize the lazylist #define LAZYINITLIST(L) if (!L) { L = list(); } ///If the provided list is empty, set it to null @@ -381,7 +389,7 @@ if(skiprep) for(var/e in first) if(!(e in result) && !(e in second)) - result += e + UNTYPED_LIST_ADD(result, e) else result = first - second return result @@ -482,7 +490,7 @@ if(!value) continue for(var/i in 1 to value / gcf) - output += item + UNTYPED_LIST_ADD(output, item) return output /// Takes a list of numbers as input, returns the highest value that is cleanly divides them all @@ -573,7 +581,7 @@ /proc/unique_list(list/inserted_list) . = list() for(var/i in inserted_list) - . |= i + . |= LIST_VALUE_WRAP_LISTS(i) ///same as unique_list, but returns nothing and acts on list in place (also handles associated values properly) /proc/unique_list_in_place(list/inserted_list) @@ -735,11 +743,6 @@ if(checked_datum.vars[varname] == value) return checked_datum -///remove all nulls from a list -/proc/remove_nulls_from_list(list/inserted_list) - while(inserted_list.Remove(null)) - continue - return inserted_list ///Copies a list, and all lists inside it recusively ///Does not copy any other reference type @@ -779,7 +782,7 @@ return null . = list() for(var/key in key_list) - . |= key_list[key] + . |= LIST_VALUE_WRAP_LISTS(key_list[key]) ///Make a normal list an associative one /proc/make_associative(list/flat_list) @@ -825,7 +828,7 @@ /proc/assoc_to_keys(list/input) var/list/keys = list() for(var/key in input) - keys += key + UNTYPED_LIST_ADD(keys, key) return keys ///compare two lists, returns TRUE if they are the same @@ -859,7 +862,7 @@ . = list() for(var/i in list_to_filter) if(condition.Invoke(i)) - . |= i + . |= LIST_VALUE_WRAP_LISTS(i) ///Returns a list with all weakrefs resolved /proc/recursive_list_resolve(list/list_to_resolve) diff --git a/code/modules/admin/verbs/hiddenprints.dm b/code/modules/admin/verbs/hiddenprints.dm index f5332d890ae..a2c5ad63f24 100644 --- a/code/modules/admin/verbs/hiddenprints.dm +++ b/code/modules/admin/verbs/hiddenprints.dm @@ -9,7 +9,7 @@ victim_hiddenprints = list() var/list/hiddenprints = flatten_list(victim_hiddenprints) - remove_nulls_from_list(hiddenprints) + list_clear_nulls(hiddenprints) if(!length(hiddenprints)) hiddenprints = list("Nobody has touched this yet!") diff --git a/code/modules/atmospherics/machinery/datum_pipeline.dm b/code/modules/atmospherics/machinery/datum_pipeline.dm index c80d543b6e9..48dd0cbdbca 100644 --- a/code/modules/atmospherics/machinery/datum_pipeline.dm +++ b/code/modules/atmospherics/machinery/datum_pipeline.dm @@ -208,9 +208,8 @@ /datum/pipeline/proc/return_air() . = other_airs + air - if(null in .) + if(list_clear_nulls(.)) stack_trace("[src] has one or more null gas mixtures, which may cause bugs. Null mixtures will not be considered in reconcile_air().") - return remove_nulls_from_list(.) /// Called when the pipenet needs to update and mix together all the air mixes /datum/pipeline/proc/reconcile_air() diff --git a/code/modules/events/brand_intelligence.dm b/code/modules/events/brand_intelligence.dm index a80e8bb902d..94cf816d3b5 100644 --- a/code/modules/events/brand_intelligence.dm +++ b/code/modules/events/brand_intelligence.dm @@ -79,7 +79,7 @@ originMachine.visible_message(span_notice("[originMachine] beeps and seems lifeless.")) kill() return - vendingMachines = remove_nulls_from_list(vendingMachines) + list_clear_nulls(vendingMachines) if(!vendingMachines.len) //if every machine is infected for(var/obj/machinery/vending/upriser in infectedMachines) if(!QDELETED(upriser))