From 2dfbf0b81ad5a5deedc5bf3d6eece8e204d535af Mon Sep 17 00:00:00 2001 From: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com> Date: Mon, 11 May 2026 08:20:24 -0700 Subject: [PATCH] Adds a unit test that makes sure subsystem flags make sense (#96022) ## About The Pull Request There's a few of these that conflict, mostly relating to timing. This conflicting won't actually break anything but it does muddy "what is this doing" somewhat and it's good to be clear so here we go ## Why It's Good For The Game Makes the MC very slightly harder to confuse yourself with --- code/_globalvars/bitfields.dm | 12 ++++++++++++ code/controllers/subsystem/throwing.dm | 2 +- .../admin/view_variables/debug_variables.dm | 18 ++++++------------ code/modules/unit_tests/_unit_tests.dm | 1 + code/modules/unit_tests/subsystem_flags.dm | 12 ++++++++++++ 5 files changed, 32 insertions(+), 13 deletions(-) create mode 100644 code/modules/unit_tests/subsystem_flags.dm diff --git a/code/_globalvars/bitfields.dm b/code/_globalvars/bitfields.dm index f2f2ceb823f..8ac837d72fa 100644 --- a/code/_globalvars/bitfields.dm +++ b/code/_globalvars/bitfields.dm @@ -27,6 +27,18 @@ GLOBAL_LIST_INIT(bitfields, generate_bitfields()) var/name = pick(flags) return flags[name] +/// Returns null if no such field exists, a list of all matching flags by name otherwise +/proc/get_matching_bitflags(var_name, value) + var/list/valid_bitflags = get_valid_bitflags(var_name) + if(!length(valid_bitflags)) + return null + + var/list/flags = list() + for (var/bit_name in valid_bitflags) + if (value & valid_bitflags[bit_name]) + flags += bit_name + return flags + DEFINE_BITFIELD(admin_flags, list( "ADMIN" = R_ADMIN, "AUTOLOGIN" = R_AUTOADMIN, diff --git a/code/controllers/subsystem/throwing.dm b/code/controllers/subsystem/throwing.dm index 45dc79f6ac9..dd879a91028 100644 --- a/code/controllers/subsystem/throwing.dm +++ b/code/controllers/subsystem/throwing.dm @@ -5,7 +5,7 @@ SUBSYSTEM_DEF(throwing) name = "Throwing" priority = FIRE_PRIORITY_THROWING wait = 1 - ss_flags = SS_NO_INIT|SS_KEEP_TIMING|SS_TICKER + ss_flags = SS_NO_INIT | SS_TICKER runlevels = RUNLEVEL_GAME | RUNLEVEL_POSTGAME var/list/currentrun diff --git a/code/modules/admin/view_variables/debug_variables.dm b/code/modules/admin/view_variables/debug_variables.dm index ded5890cba8..bdd45073f19 100644 --- a/code/modules/admin/view_variables/debug_variables.dm +++ b/code/modules/admin/view_variables/debug_variables.dm @@ -103,20 +103,14 @@ return "/list ([list_value.len])" // if it's a number, is it a bitflag? - var/list/valid_bitflags - if(!isnum(name)) - valid_bitflags = get_valid_bitflags(name) + var/list/matching_bitflags = get_matching_bitflags(name, value) - if(!length(valid_bitflags)) - return "[VV_HTML_ENCODE(value)]" + if(!isnull(matching_bitflags)) + if(length(matching_bitflags)) + return "[VV_HTML_ENCODE(matching_bitflags.Join(", "))]" + return "NONE" - var/list/flags = list() - for (var/bit_name in valid_bitflags) - if (value & valid_bitflags[bit_name]) - flags += bit_name - if(length(flags)) - return "[VV_HTML_ENCODE(flags.Join(", "))]" - return "NONE" + return "[VV_HTML_ENCODE(value)]" /datum/proc/debug_variable_value(name, level, datum/owner, sanitize, display_flags) if("[src]" != "[type]") // If we have a name var, let's use it. diff --git a/code/modules/unit_tests/_unit_tests.dm b/code/modules/unit_tests/_unit_tests.dm index 5800b131c58..23153ef32f6 100644 --- a/code/modules/unit_tests/_unit_tests.dm +++ b/code/modules/unit_tests/_unit_tests.dm @@ -327,6 +327,7 @@ #include "strippable.dm" #include "stuns.dm" #include "style_hotswapping.dm" +#include "subsystem_flags.dm" #include "subsystem_init.dm" #include "suit_sensor.dm" #include "suit_storage_icons.dm" diff --git a/code/modules/unit_tests/subsystem_flags.dm b/code/modules/unit_tests/subsystem_flags.dm new file mode 100644 index 00000000000..60ce20d4560 --- /dev/null +++ b/code/modules/unit_tests/subsystem_flags.dm @@ -0,0 +1,12 @@ +// Ensures subystem flags are set in a coherent way +/datum/unit_test/subsystem_flags + +/datum/unit_test/subsystem_flags/Run() + for(var/datum/controller/subsystem/sub_lad as anything in subtypesof(/datum/controller/subsystem)) + if((sub_lad::ss_flags & (SS_TICKER | SS_KEEP_TIMING)) == (SS_TICKER | SS_KEEP_TIMING)) + var/list/matching = get_matching_bitflags("ss_flags", sub_lad::ss_flags) + TEST_FAIL("[sub_lad] {[matching.Join(" | ")]} had both SS_TICKER and SEE_KEEP_TIMING set, this is redundant! You should likely remove SS_KEEP_TIMING.") + if((sub_lad::ss_flags & (SS_POST_FIRE_TIMING | SS_KEEP_TIMING)) == (SS_POST_FIRE_TIMING | SS_KEEP_TIMING)) + var/list/matching = get_matching_bitflags("ss_flags", sub_lad::ss_flags) + TEST_FAIL("[sub_lad] {[matching.Join(" | ")]} had both SS_POST_FIRE_TIMING and SEE_KEEP_TIMING set, this is redundant! You should likely remove SS_KEEP_TIMING as SS_POST_FIRE_TIMING overrides it.") +