From 7bb8ecb768e592f89d334567af3303dc849ed3cd Mon Sep 17 00:00:00 2001 From: SkyratBot <59378654+SkyratBot@users.noreply.github.com> Date: Fri, 25 Nov 2022 16:01:04 +0100 Subject: [PATCH] [MIRROR] Fixup list helpers, remove listoflist footgun from generic list procs, remove duplicated procs. [MDB IGNORE] (#17749) * 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. * Fixup list helpers, remove listoflist footgun from generic list procs, remove duplicated procs. Co-authored-by: Kyle Spier-Swenson --- 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 efbf5382f00..864227905a3 100644 --- a/code/modules/atmospherics/machinery/datum_pipeline.dm +++ b/code/modules/atmospherics/machinery/datum_pipeline.dm @@ -223,9 +223,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))