From 9efffccd71eed2d5f212f973e6d64e7ce2276021 Mon Sep 17 00:00:00 2001 From: Luc <89928798+lewcc@users.noreply.github.com> Date: Wed, 20 Mar 2024 16:17:45 -0400 Subject: [PATCH] Adds CI checks forbidding certain kinds of world loops (#24454) * Updates checkgrep to report unnecessary loops * improved message * global lists are better too * much better * Apply suggestions from code review Co-authored-by: GDN <96800819+GDNgit@users.noreply.github.com> * update to modern check_grep format --------- Co-authored-by: GDN <96800819+GDNgit@users.noreply.github.com> Co-authored-by: S34N <12197162+S34NW@users.noreply.github.com> --- code/modules/events/blob/blob_mobs.dm | 8 ++++++++ code/modules/events/blob/blob_powers.dm | 2 +- code/modules/events/blob/theblob.dm | 1 + code/modules/events/undead_event.dm | 2 +- code/modules/maptext_alerts/location_blurbs.dm | 2 +- code/modules/station_goals/bsa.dm | 2 +- tools/ci/check_grep2.py | 14 ++++++++++++++ 7 files changed, 27 insertions(+), 4 deletions(-) diff --git a/code/modules/events/blob/blob_mobs.dm b/code/modules/events/blob/blob_mobs.dm index afc6d5f6d81..84061c55058 100644 --- a/code/modules/events/blob/blob_mobs.dm +++ b/code/modules/events/blob/blob_mobs.dm @@ -20,6 +20,14 @@ fire_damage = 3 var/mob/camera/blob/overmind = null +/mob/living/simple_animal/hostile/blob/Initialize(mapload) + . = ..() + GLOB.blob_minions |= src + +/mob/living/simple_animal/hostile/blob/Destroy() + GLOB.blob_minions -= src + return ..() + /mob/living/simple_animal/hostile/blob/proc/adjustcolors(a_color) if(a_color) color = a_color diff --git a/code/modules/events/blob/blob_powers.dm b/code/modules/events/blob/blob_powers.dm index e52ea432f82..b5e8492a3a8 100644 --- a/code/modules/events/blob/blob_powers.dm +++ b/code/modules/events/blob/blob_powers.dm @@ -469,7 +469,7 @@ for(var/obj/structure/blob/BL in GLOB.blobs) BL.adjustcolors(blob_reagent_datum.color) - for(var/mob/living/simple_animal/hostile/blob/BLO) + for(var/mob/living/simple_animal/hostile/blob/BLO in GLOB.blob_minions) BLO.adjustcolors(blob_reagent_datum.complementary_color) to_chat(src, "Your reagent is now: [blob_reagent_datum.name] - [blob_reagent_datum.description]") diff --git a/code/modules/events/blob/theblob.dm b/code/modules/events/blob/theblob.dm index 9b616928aef..bc06566df54 100644 --- a/code/modules/events/blob/theblob.dm +++ b/code/modules/events/blob/theblob.dm @@ -2,6 +2,7 @@ GLOBAL_LIST_EMPTY(blobs) GLOBAL_LIST_EMPTY(blob_nodes) +GLOBAL_LIST_EMPTY(blob_minions) /obj/structure/blob name = "blob" diff --git a/code/modules/events/undead_event.dm b/code/modules/events/undead_event.dm index 9dab86beb5d..4685cefffce 100644 --- a/code/modules/events/undead_event.dm +++ b/code/modules/events/undead_event.dm @@ -8,7 +8,7 @@ RS.lightsoutAmount = pick(2,2,3) RS.start() RS.kill() - for(var/area/A) + for(var/area/A in GLOB.all_areas) if(!is_station_level(A.z)) continue //Spook on main station only. if(A.luminosity) continue // if(A.lighting_space) continue diff --git a/code/modules/maptext_alerts/location_blurbs.dm b/code/modules/maptext_alerts/location_blurbs.dm index 050ca0a42ea..519e3044c9b 100644 --- a/code/modules/maptext_alerts/location_blurbs.dm +++ b/code/modules/maptext_alerts/location_blurbs.dm @@ -118,7 +118,7 @@ var/atom/movable/screen/text/blurb/location_blurb = new() if(antag_check.antag_datums) - for(var/datum/antagonist/role) + for(var/datum/antagonist/role in antag_check.antag_datums) if(role.custom_blurb()) location_blurb.blurb_text = uppertext(role.custom_blurb()) location_blurb.text_color = role.blurb_text_color diff --git a/code/modules/station_goals/bsa.dm b/code/modules/station_goals/bsa.dm index e80a76b43b0..92923a597cc 100644 --- a/code/modules/station_goals/bsa.dm +++ b/code/modules/station_goals/bsa.dm @@ -22,7 +22,7 @@ /datum/station_goal/bluespace_cannon/check_completion() if(..()) return TRUE - for(var/obj/machinery/bsa/full/B) + for(var/obj/machinery/bsa/full/B in GLOB.machines) if(B && !B.stat && is_station_contact(B.z)) return TRUE return FALSE diff --git a/tools/ci/check_grep2.py b/tools/ci/check_grep2.py index c2145596ab9..561151a7564 100644 --- a/tools/ci/check_grep2.py +++ b/tools/ci/check_grep2.py @@ -133,6 +133,19 @@ def check_tgui_ui_new_argument(idx, line): if "\tui = new" in line and not TGUI_UI_NEW.search(line): return [(idx + 1, "Invalid argument within constructor, please make sure window sizing is in corresponding JavaScript file.")] +# checks for any (for var/type/x) loops that are not looping over bare datums: enforcing that the only case we will see this is if you genuinely want to loop over all datums in memory +FOR_ALL_DATUMS = re.compile(r"for\s*\(\s*var\/((\w+)(?:(?:\/\w+){2,})?)\)") +# double-checks that we don't have any attempts at looping like for(var/atom/a) +FOR_ALL_NOT_DATUMS = re.compile(r"for\s*\(\s*var\/((?:atom|area|turf|obj|mob)(?:\/\w+))\)") +def check_datum_loops(idx, line): + if FOR_ALL_DATUMS.search(line) or FOR_ALL_NOT_DATUMS.search(line): + return Failure( + idx + 1, + # yes this will concatenate the strings, don't look too hard + "Found a for loop without explicit contents. If you're trying to loop over everything in the world, first double check that you truly need to, and if so specify \'in world\'.\n" + "If you're trying to check bare datums, please ensure that your value is only cast to /datum, and please make sure you use \'as anything\', or use a global list instead." + ) + CODE_CHECKS = [ check_space_indentation, check_mixed_indentation, @@ -144,6 +157,7 @@ CODE_CHECKS = [ check_conditional_spacing, check_global_list_empty, check_tgui_ui_new_argument, + check_datum_loops, ]