From a57a8b8c40c286091593568b08fb91d9311ea72e Mon Sep 17 00:00:00 2001 From: Contrabang <91113370+Contrabang@users.noreply.github.com> Date: Sun, 27 Jul 2025 15:26:13 -0700 Subject: [PATCH] Adds CI to error on unnecessary variable modification (#29861) * begin the burndown? (y/n) * okay update requirements? * change the error name * there we go * tweak * first bit of the burndown * more * burndown complete * return the exit code! * review * Revert "first bit of the burndown" This reverts commit 34155bd991ea0c4cf2962601efc3af3387ad7e93. * fix * fixes * last fix * remove deprecated avulto stuff --- .github/workflows/ci.yml | 1 + .../subsystem/processing/SSprocessing.dm | 2 +- code/game/gamemodes/dynamic/antag_rulesets.dm | 1 - code/game/mecha/equipment/weapons/weapons.dm | 4 +- .../items/stacks/sheets/sheet_types.dm | 2 +- .../mining/lavaland/loot/tendril_loot.dm | 1 - code/modules/mob/living/carbon/human/npcs.dm | 1 - .../mob/living/carbon/human/species/golem.dm | 1 - .../mob/living/silicon/robot/robot_modules.dm | 1 - .../living/simple_animal/hostile/hivebot.dm | 3 +- .../particle_accelerator/particle_control.dm | 1 - .../projectiles/guns/projectile/launchers.dm | 5 +- tools/ci/check_legacy_attack_chain.py | 2 +- tools/ci/identical_variables.py | 65 +++++++++++++++++++ tools/requirements.txt | 2 +- 15 files changed, 75 insertions(+), 17 deletions(-) create mode 100644 tools/ci/identical_variables.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 405e52774bb..2f419221e2a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,6 +48,7 @@ jobs: python tools/ci/unticked_files.py ${GITHUB_WORKSPACE} python tools/ci/illegal_dme_files.py ${GITHUB_WORKSPACE} python tools/ci/define_sanity.py + python tools/ci/identical_variables.py python tools/ci/restrict_file_types.py python tools/ci/verify_sql_version.py python tools/ci/no_duplicate_definitions.py diff --git a/code/controllers/subsystem/processing/SSprocessing.dm b/code/controllers/subsystem/processing/SSprocessing.dm index 1abc4b89116..99007d3213a 100644 --- a/code/controllers/subsystem/processing/SSprocessing.dm +++ b/code/controllers/subsystem/processing/SSprocessing.dm @@ -4,7 +4,7 @@ SUBSYSTEM_DEF(processing) name = "Processing" priority = FIRE_PRIORITY_PROCESS flags = SS_BACKGROUND|SS_POST_FIRE_TIMING|SS_NO_INIT - wait = 10 + wait = 1 SECONDS var/stat_tag = "P" //Used for logging var/list/processing = list() diff --git a/code/game/gamemodes/dynamic/antag_rulesets.dm b/code/game/gamemodes/dynamic/antag_rulesets.dm index b800d184042..0e6945fb8c1 100644 --- a/code/game/gamemodes/dynamic/antag_rulesets.dm +++ b/code/game/gamemodes/dynamic/antag_rulesets.dm @@ -307,7 +307,6 @@ /datum/ruleset/team/cult name = "Cultist" - ruleset_cost = 1 ruleset_weight = 3 // antag_weight doesnt matter, since we've already allocated our budget for 4 cultists only antag_cost = 30 diff --git a/code/game/mecha/equipment/weapons/weapons.dm b/code/game/mecha/equipment/weapons/weapons.dm index e96ea753f33..ce3eaf51217 100644 --- a/code/game/mecha/equipment/weapons/weapons.dm +++ b/code/game/mecha/equipment/weapons/weapons.dm @@ -377,7 +377,8 @@ F.prime() do_after_cooldown() -/obj/item/mecha_parts/mecha_equipment/weapon/ballistic/missile_rack/flashbang/clusterbang//Because I am a heartless bastard -Sieve +//Because I am a heartless bastard -Sieve +/obj/item/mecha_parts/mecha_equipment/weapon/ballistic/missile_rack/flashbang/clusterbang equip_cooldown = 9 SECONDS name = "\improper SOB-3 Clusterbang Launcher" desc = "A weapon for combat exosuits. Launches primed clusterbangs. You monster." @@ -385,7 +386,6 @@ projectiles = 3 projectile = /obj/item/grenade/clusterbuster projectile_energy_cost = 1600 //getting off cheap seeing as this is 3 times the flashbangs held in the grenade launcher. - size=1 /obj/item/mecha_parts/mecha_equipment/weapon/ballistic/missile_rack/flashbang/clusterbang/limited/get_equip_info()//Limited version of the clusterbang launcher that can't reload return "* [chassis.selected==src?"":""][name][chassis.selected==src?"":""]\[[projectiles]\]" diff --git a/code/game/objects/items/stacks/sheets/sheet_types.dm b/code/game/objects/items/stacks/sheets/sheet_types.dm index e2af3ddf222..e5d1be2e49b 100644 --- a/code/game/objects/items/stacks/sheets/sheet_types.dm +++ b/code/game/objects/items/stacks/sheets/sheet_types.dm @@ -463,7 +463,7 @@ GLOBAL_LIST_INIT(durathread_recipes, list ( name = "raw durathread bundle" desc = "A bundle of raw durathread ready to be spun on the loom." singular_name = "raw durathread ball" - icon = 'icons/obj/stacks/organic.dmi' + icon_state = "sheet-durathreadraw" merge_type = /obj/item/stack/sheet/cotton/durathread pull_effort = 70 loom_result = /obj/item/stack/sheet/durathread diff --git a/code/modules/mining/lavaland/loot/tendril_loot.dm b/code/modules/mining/lavaland/loot/tendril_loot.dm index 09f151db1bf..2e852224248 100644 --- a/code/modules/mining/lavaland/loot/tendril_loot.dm +++ b/code/modules/mining/lavaland/loot/tendril_loot.dm @@ -435,7 +435,6 @@ to_chat(user, "[src] is still recharging.") /obj/effect/immortality_talisman - icon = 'icons/effects/effects.dmi' var/can_destroy = FALSE /obj/effect/immortality_talisman/Initialize(mapload) diff --git a/code/modules/mob/living/carbon/human/npcs.dm b/code/modules/mob/living/carbon/human/npcs.dm index 26b7cf4976d..616f2dd1736 100644 --- a/code/modules/mob/living/carbon/human/npcs.dm +++ b/code/modules/mob/living/carbon/human/npcs.dm @@ -5,7 +5,6 @@ species_restricted = list("Monkey") species_exception = list(/datum/species/monkey) - icon = 'icons/obj/clothing/under/misc.dmi' sprite_sheets = list("Monkey" = 'icons/mob/clothing/under/misc.dmi') /mob/living/carbon/human/monkey/punpun/Initialize(mapload) diff --git a/code/modules/mob/living/carbon/human/species/golem.dm b/code/modules/mob/living/carbon/human/species/golem.dm index f5b7bdcb92b..3eb64444c5e 100644 --- a/code/modules/mob/living/carbon/human/species/golem.dm +++ b/code/modules/mob/living/carbon/human/species/golem.dm @@ -504,7 +504,6 @@ /datum/action/innate/unstable_teleport name = "Unstable Teleport" check_flags = AB_CHECK_CONSCIOUS - button_overlay_icon = 'icons/mob/actions/actions.dmi' var/activated = FALSE // To prevent spamming var/cooldown = 150 var/last_teleport = 0 diff --git a/code/modules/mob/living/silicon/robot/robot_modules.dm b/code/modules/mob/living/silicon/robot/robot_modules.dm index 3a85d68884b..ed1c4eabaa1 100644 --- a/code/modules/mob/living/silicon/robot/robot_modules.dm +++ b/code/modules/mob/living/silicon/robot/robot_modules.dm @@ -394,7 +394,6 @@ name = "plasma syringe cannon" desc = "A syringe gun integrated into a medical cyborg's chassis. Fires heavy-duty plasma syringes tipped in poison." icon_state = "rapidsyringegun" - throw_speed = 3 throw_range = 7 force = 4 fire_sound = 'sound/items/syringeproj.ogg' diff --git a/code/modules/mob/living/simple_animal/hostile/hivebot.dm b/code/modules/mob/living/simple_animal/hostile/hivebot.dm index d8510a6579f..def1d8d926d 100644 --- a/code/modules/mob/living/simple_animal/hostile/hivebot.dm +++ b/code/modules/mob/living/simple_animal/hostile/hivebot.dm @@ -54,10 +54,9 @@ return FALSE do_sparks(3, 1, src) -/mob/living/simple_animal/hostile/hivebot/tele//this still needs work +/mob/living/simple_animal/hostile/hivebot/tele name = "Beacon" desc = "Some odd beacon thing." - icon = 'icons/mob/hivebot.dmi' icon_state = "def_radar-off" icon_living = "def_radar-off" health = 200 diff --git a/code/modules/power/engines/singularity/particle_accelerator/particle_control.dm b/code/modules/power/engines/singularity/particle_accelerator/particle_control.dm index a8101dd8ec4..401e7b169df 100644 --- a/code/modules/power/engines/singularity/particle_accelerator/particle_control.dm +++ b/code/modules/power/engines/singularity/particle_accelerator/particle_control.dm @@ -13,7 +13,6 @@ icon_state = "control_box" reference = "control_box" idle_power_consumption = 500 - active = FALSE dir = 1 var/strength_upper_limit = 2 var/interface_control = 1 diff --git a/code/modules/projectiles/guns/projectile/launchers.dm b/code/modules/projectiles/guns/projectile/launchers.dm index 6ff760fc0b9..1cb81b9d7ca 100644 --- a/code/modules/projectiles/guns/projectile/launchers.dm +++ b/code/modules/projectiles/guns/projectile/launchers.dm @@ -1,14 +1,13 @@ //KEEP IN MIND: These are different from gun/grenadelauncher. These are designed to shoot premade rocket and grenade projectiles, not flashbangs or chemistry casings etc. //Put handheld rocket launchers here if someone ever decides to make something so hilarious ~Paprika -/obj/item/gun/projectile/revolver/grenadelauncher//this is only used for underbarrel grenade launchers at the moment, but admins can still spawn it if they feel like being assholes +//this is only used for underbarrel grenade launchers at the moment, but admins can still spawn it if they feel like being assholes +/obj/item/gun/projectile/revolver/grenadelauncher desc = "A break-action grenade launcher." name = "grenade launcher" icon_state = "dbshotgun_sawn" - item_state = "gun" mag_type = /obj/item/ammo_box/magazine/internal/grenadelauncher fire_sound = 'sound/weapons/grenadelaunch.ogg' - w_class = WEIGHT_CLASS_NORMAL can_holster = FALSE // Not your normal revolver /obj/item/gun/projectile/revolver/grenadelauncher/attackby__legacy__attackchain(obj/item/A, mob/user, params) diff --git a/tools/ci/check_legacy_attack_chain.py b/tools/ci/check_legacy_attack_chain.py index 8e0d437dc42..eb3019bf3b7 100644 --- a/tools/ci/check_legacy_attack_chain.py +++ b/tools/ci/check_legacy_attack_chain.py @@ -4,7 +4,7 @@ import time from dataclasses import dataclass from collections import namedtuple, defaultdict -from avulto import DMM, DME, DMI, Dir, Path as p, paths, ProcDecl, TypeDecl +from avulto import DME, Path as p, ProcDecl, TypeDecl from avulto.ast import NodeKind from avulto import exceptions diff --git a/tools/ci/identical_variables.py b/tools/ci/identical_variables.py new file mode 100644 index 00000000000..a4de408c4af --- /dev/null +++ b/tools/ci/identical_variables.py @@ -0,0 +1,65 @@ +from avulto import DME +from collections import namedtuple +import os +import sys +import time + +Failure = namedtuple("Failure", ["filename", "lineno", "message"]) + +RED = "\033[0;31m" +GREEN = "\033[0;32m" +BLUE = "\033[0;34m" +NC = "\033[0m" # No Color + +def print_error(message: str, filename: str, line_number: int): + if os.getenv("GITHUB_ACTIONS") == "true": # We're on github, output in a special format. + print(f"::error file={filename},line={line_number},title=Identical Variables::{filename}:{line_number}: {message}") + else: + print(f"{filename}:{line_number}: {message}") + +def main(): + print("identical_variables started") + exit_code = 0 + start = time.time() + + dme = DME.from_file("paradise.dme") + all_failures = [] + + for path in dme.subtypesof("/"): + typepath = dme.type_decl(path) + + for variable_name in typepath.var_names(modified=True): + modded = typepath.var_decl(variable_name, False) + if(not modded): + all_failures.append(Failure(typepath.source_loc.file_path, typepath.source_loc.line, f"{RED}Avulto failed to read {path.rel}::{variable_name}.{NC} This is probably not your fault.")) + continue + if path.parent.is_root: + continue + parent_typepath = dme.type_decl(path.parent) + original = parent_typepath.var_decl(variable_name, True) + if(modded.const_val == original.const_val): + if(modded.const_val == None): # Both proc calls (like sound() or icon()) and nulls are treated as "None", this sucks. + continue + # Make an exception for directional helpers, where you cant guarantee. + if(modded.name in ["dir", "pixel_x", "pixel_y"] and path.parent.stem in ["directional", "offset"]): + continue + # Make an exception for subsystems, as they are much less OOP dependent. + if(path.child_of("/datum/controller/subsystem")): + continue + # And make an exception for this fucked up edge case. wtf. + if(path.rel == "/obj" and variable_name == "layer"): + continue + all_failures.append(Failure(typepath.source_loc.file_path, typepath.source_loc.line, f"{RED}{path.rel}{NC} has a identical variable to its parents: {RED}{variable_name}{NC} = {modded.const_val}")) + + if all_failures: + exit_code = 1 + for failure in all_failures: + print_error(failure.message, failure.filename, failure.lineno) + + end = time.time() + print(f"identical_variables tests completed in {end - start:.2f}s\n") + + sys.exit(exit_code) + +if __name__ == "__main__": + main() diff --git a/tools/requirements.txt b/tools/requirements.txt index c185a3a2ad4..f70ccfa61a8 100644 --- a/tools/requirements.txt +++ b/tools/requirements.txt @@ -8,4 +8,4 @@ PyYaml>=6.0.1 # maplint also beautifulsoup4>=4.12.2 # various CI checks -avulto==0.1.13 +avulto==0.1.16