From 53955051d08cca2ebd2d136ae71f7fc892a5f48d Mon Sep 17 00:00:00 2001 From: SkyratBot <59378654+SkyratBot@users.noreply.github.com> Date: Wed, 23 Aug 2023 07:18:53 +0200 Subject: [PATCH] [MIRROR] Adds a check grep for using `src` as a trait source rather that `REF(src)` [MDB IGNORE] (#23264) * Adds a check grep for using `src` as a trait source rather that `REF(src)` (#77836) ## About The Pull Request Using `src` as a trait source is an error and can often lead to hard-deletes If you wish to tie a source to a certain datum, it is common to use `REF(src)` instead. Ideally, we would lint or test for any use of a reference rather than a string in use in trait sources, but that's a bit harder to setup. Currently (from what I can see) the *only* erroneous use of references as sources are via `src`, so it being the most common error, I see it fine to lint for it. ## Changelog Nothing player facing. * Adds a check grep for using `src` as a trait source rather that `REF(src)` * Update style.dm --------- Co-authored-by: MrMelbert <51863163+MrMelbert@users.noreply.github.com> Co-authored-by: Giz <13398309+vinylspiders@users.noreply.github.com> --- code/datums/components/food/germ_sensitive.dm | 4 ++-- code/datums/components/label.dm | 4 ++-- code/datums/components/shuttle_cling.dm | 4 ++-- code/datums/components/style/style.dm | 4 ++-- .../modules/projectiles/projectile/energy/_energy.dm | 2 +- .../shuttle/shuttle_events/_shuttle_events.dm | 4 ++-- code/modules/shuttle/shuttle_events/meteors.dm | 4 ++-- tools/ci/check_grep.sh | 12 ++++++++++++ 8 files changed, 25 insertions(+), 13 deletions(-) diff --git a/code/datums/components/food/germ_sensitive.dm b/code/datums/components/food/germ_sensitive.dm index 7b362fb8182..22ba793c1ce 100644 --- a/code/datums/components/food/germ_sensitive.dm +++ b/code/datums/components/food/germ_sensitive.dm @@ -21,7 +21,7 @@ GLOBAL_LIST_INIT(floor_diseases, list( if(!isobj(parent)) return COMPONENT_INCOMPATIBLE - ADD_TRAIT(parent, TRAIT_GERM_SENSITIVE, src) + ADD_TRAIT(parent, TRAIT_GERM_SENSITIVE, REF(src)) RegisterSignal(parent, COMSIG_ATOM_EXAMINE, PROC_REF(examine)) RegisterSignal(parent, COMSIG_MOVABLE_MOVED, PROC_REF(handle_movement)) @@ -44,7 +44,7 @@ GLOBAL_LIST_INIT(floor_diseases, list( handle_movement() /datum/component/germ_sensitive/UnregisterFromParent() - REMOVE_TRAIT(parent, TRAIT_GERM_SENSITIVE, src) + REMOVE_TRAIT(parent, TRAIT_GERM_SENSITIVE, REF(src)) UnregisterSignal(parent, list( COMSIG_ATOM_EXAMINE, COMSIG_MOVABLE_MOVED, diff --git a/code/datums/components/label.dm b/code/datums/components/label.dm index 1b4257abc21..24b01ee52a3 100644 --- a/code/datums/components/label.dm +++ b/code/datums/components/label.dm @@ -83,7 +83,7 @@ /datum/component/label/proc/apply_label() var/atom/owner = parent owner.name += " ([label_name])" - ADD_TRAIT(owner, TRAIT_HAS_LABEL, src) + ADD_TRAIT(owner, TRAIT_HAS_LABEL, REF(src)) owner.update_appearance(UPDATE_ICON) /// Removes the label from the parent's name @@ -91,5 +91,5 @@ var/atom/owner = parent owner.name = replacetext(owner.name, "([label_name])", "") // Remove the label text from the parent's name, wherever it's located. owner.name = trim(owner.name) // Shave off any white space from the beginning or end of the parent's name. - REMOVE_TRAIT(owner, TRAIT_HAS_LABEL, src) + REMOVE_TRAIT(owner, TRAIT_HAS_LABEL, REF(src)) owner.update_appearance(UPDATE_ICON) diff --git a/code/datums/components/shuttle_cling.dm b/code/datums/components/shuttle_cling.dm index 17d07b2e051..bc6f022ab86 100644 --- a/code/datums/components/shuttle_cling.dm +++ b/code/datums/components/shuttle_cling.dm @@ -35,7 +35,7 @@ src.direction = direction - ADD_TRAIT(parent, TRAIT_HYPERSPACED, src) + ADD_TRAIT(parent, TRAIT_HYPERSPACED, REF(src)) RegisterSignals(parent, list(COMSIG_MOVABLE_MOVED, COMSIG_MOVABLE_UNBUCKLE, COMSIG_ATOM_NO_LONGER_PULLED), PROC_REF(update_state)) RegisterSignal(parent, SIGNAL_REMOVETRAIT(TRAIT_FREE_HYPERSPACE_MOVEMENT), PROC_REF(initialize_loop)) @@ -178,7 +178,7 @@ qdel(src) /datum/component/shuttle_cling/Destroy(force, silent) - REMOVE_TRAIT(parent, TRAIT_HYPERSPACED, src) + REMOVE_TRAIT(parent, TRAIT_HYPERSPACED, REF(src)) QDEL_NULL(hyperloop) return ..() diff --git a/code/datums/components/style/style.dm b/code/datums/components/style/style.dm index 79da4ff1b3f..42f5de156e0 100644 --- a/code/datums/components/style/style.dm +++ b/code/datums/components/style/style.dm @@ -98,7 +98,7 @@ RegisterSignal(src, COMSIG_ATOM_TOOL_ACT(TOOL_MULTITOOL), PROC_REF(on_parent_multitool)) - ADD_TRAIT(mob_parent, TRAIT_STYLISH, src) // SKYRAT EDIT ADD - allows style meter chads to do flips + ADD_TRAIT(mob_parent, TRAIT_STYLISH, REF(src)) // SKYRAT EDIT ADD - allows style meter chads to do flips /datum/component/style/RegisterWithParent() RegisterSignal(parent, COMSIG_MOB_ITEM_AFTERATTACK, PROC_REF(hotswap)) @@ -152,7 +152,7 @@ if(mob_parent.hud_used) mob_parent.hud_used.static_inventory -= meter mob_parent.hud_used.show_hud(mob_parent.hud_used.hud_version) - REMOVE_TRAIT(mob_parent, TRAIT_STYLISH, src) // SKYRAT EDIT ADD - allows style meter chads to do flips + REMOVE_TRAIT(mob_parent, TRAIT_STYLISH, REF(src)) // SKYRAT EDIT ADD - allows style meter chads to do flips return ..() diff --git a/code/modules/projectiles/projectile/energy/_energy.dm b/code/modules/projectiles/projectile/energy/_energy.dm index 6b715fdb74f..86ec80b3b20 100644 --- a/code/modules/projectiles/projectile/energy/_energy.dm +++ b/code/modules/projectiles/projectile/energy/_energy.dm @@ -11,4 +11,4 @@ . = ..() ADD_TRAIT(src, TRAIT_FREE_HYPERSPACE_MOVEMENT, INNATE_TRAIT) - ADD_TRAIT(src, TRAIT_FREE_HYPERSPACE_MOVEMENT, TRAIT_FREE_HYPERSPACE_SOFTCORDON_MOVEMENT) + ADD_TRAIT(src, TRAIT_FREE_HYPERSPACE_SOFTCORDON_MOVEMENT, INNATE_TRAIT) diff --git a/code/modules/shuttle/shuttle_events/_shuttle_events.dm b/code/modules/shuttle/shuttle_events/_shuttle_events.dm index 6736428c02a..35b00ac3e79 100644 --- a/code/modules/shuttle/shuttle_events/_shuttle_events.dm +++ b/code/modules/shuttle/shuttle_events/_shuttle_events.dm @@ -136,5 +136,5 @@ ///Do any post-spawn edits you need to do /datum/shuttle_event/simple_spawner/proc/post_spawn(atom/movable/spawnee) - ADD_TRAIT(spawnee, TRAIT_FREE_HYPERSPACE_SOFTCORDON_MOVEMENT, REF(src)) //Lets us spawn and move further away from the shuttle without being teleported into space - ADD_TRAIT(spawnee, TRAIT_DEL_ON_SPACE_DUMP, REF(src)) //if we hit the cordon, we get deleted. If the shuttle can make you, it can qdel you + ADD_TRAIT(spawnee, TRAIT_FREE_HYPERSPACE_SOFTCORDON_MOVEMENT, INNATE_TRAIT) //Lets us spawn and move further away from the shuttle without being teleported into space + ADD_TRAIT(spawnee, TRAIT_DEL_ON_SPACE_DUMP, INNATE_TRAIT) //if we hit the cordon, we get deleted. If the shuttle can make you, it can qdel you diff --git a/code/modules/shuttle/shuttle_events/meteors.dm b/code/modules/shuttle/shuttle_events/meteors.dm index d1ded58cbad..ef0b6002e57 100644 --- a/code/modules/shuttle/shuttle_events/meteors.dm +++ b/code/modules/shuttle/shuttle_events/meteors.dm @@ -2,8 +2,8 @@ spawning_list = list(/obj/effect/meteor) /datum/shuttle_event/simple_spawner/meteor/post_spawn(atom/movable/spawnee) - ADD_TRAIT(spawnee, TRAIT_FREE_HYPERSPACE_MOVEMENT, src) - ..() + . = ..() + ADD_TRAIT(spawnee, TRAIT_FREE_HYPERSPACE_MOVEMENT, INNATE_TRAIT) /datum/shuttle_event/simple_spawner/meteor/spawn_movable(spawn_type) var/turf/spawn_turf = get_spawn_turf() diff --git a/tools/ci/check_grep.sh b/tools/ci/check_grep.sh index 00dc3386d19..8763cca749e 100644 --- a/tools/ci/check_grep.sh +++ b/tools/ci/check_grep.sh @@ -140,6 +140,18 @@ if $grep 'can_perform_action\(\s*\)' $code_files; then st=1 fi; +part "src as a trait source" # ideally we'd lint / test for ANY datum reference as a trait source, but 'src' is the most common. +if $grep -i '(add_trait|remove_trait)\(.+,\s*.+,\s*src\)' $code_files; then + echo + echo -e "${RED}ERROR: Using 'src' as a trait source. Source must be a string key - dont't use references to datums as a source, perhaps use 'REF(src)'.${NC}" + st=1 +fi; +if $grep -i '(add_traits|remove_traits)\(.+,\s*src\)' $code_files; then + echo + echo -e "${RED}ERROR: Using 'src' as trait sources. Source must be a string key - dont't use references to datums as sources, perhaps use 'REF(src)'.${NC}" + st=1 +fi; + part "balloon_alert sanity" if $grep 'balloon_alert\(".*"\)' $code_files; then echo