From 0d769e0ffaaa2b0f2be2edb9659c233860420ec1 Mon Sep 17 00:00:00 2001 From: Jacquerel Date: Fri, 21 Jul 2023 13:51:56 +0100 Subject: [PATCH] Removes two redundant components (#76866) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## About The Pull Request We're starting to get to have enough components that people don't realise that what they want already exists but doesn't have the name they expect 🙃 I recently added `track_hierarchical_movement` which is similar enough to `connect_containers` that it shouldn't independently exist, even if I like sending a new signal more than the ugly setup pattern for `connect_loc`. `trait_loc` is actually older than `give_turf_traits` but `give_turf_traits` covers more edge cases than `turf_loc` so seems like the better one to maintain. HOWEVER `give_turf_traits` held a list of references to atoms in it, which isn't great in an element. I couldn't think of a way to completely eliminate the list, but it isn't a list of references any more so it shouldn't cause any hard deletions. ## Why It's Good For The Game Having two components which do the same thing but marginally differently is confusing and going to cause us trouble down the line. ## Changelog Not player facing --- .../signals_atom/signals_atom_movable.dm | 3 -- .../components/track_hierarchical_movement.dm | 43 ------------------- code/datums/elements/give_turf_traits.dm | 38 ++++++++-------- code/datums/elements/trait_loc.dm | 34 --------------- code/game/objects/structures/holosign.dm | 3 +- .../machinery/components/fusion/hfr_core.dm | 3 +- code/modules/pai/card.dm | 13 +++++- code/modules/pai/pai.dm | 1 - tgstation.dme | 2 - 9 files changed, 34 insertions(+), 106 deletions(-) delete mode 100644 code/datums/components/track_hierarchical_movement.dm delete mode 100644 code/datums/elements/trait_loc.dm diff --git a/code/__DEFINES/dcs/signals/signals_atom/signals_atom_movable.dm b/code/__DEFINES/dcs/signals/signals_atom/signals_atom_movable.dm index ca93ee078a6..07954aab2f5 100644 --- a/code/__DEFINES/dcs/signals/signals_atom/signals_atom_movable.dm +++ b/code/__DEFINES/dcs/signals/signals_atom/signals_atom_movable.dm @@ -109,6 +109,3 @@ #define COMSIG_MOVABLE_MESSAGE_GET_NAME_PART "movable_message_get_name_part" ///The index of the name part #define NAME_PART_INDEX 1 - -///from /datum/component/track_hierarchical_movement/on_moved() if atom or any of its containers has moved: (atom/old_loc, dir, forced, list/old_locs) -#define COMSIG_MOVABLE_OR_CONTAINER_MOVED "movable_or_container_moved" diff --git a/code/datums/components/track_hierarchical_movement.dm b/code/datums/components/track_hierarchical_movement.dm deleted file mode 100644 index ed2b5e0dc64..00000000000 --- a/code/datums/components/track_hierarchical_movement.dm +++ /dev/null @@ -1,43 +0,0 @@ -/** - * Component which outputs a signal if the attached atom or any of its containers moves - */ -/datum/component/track_hierarchical_movement - /// List of things we're currently listening out for movement from - var/list/containers - -/datum/component/track_hierarchical_movement/Initialize() - . = ..() - if (!ismovable(parent)) - return COMPONENT_INCOMPATIBLE - -/datum/component/track_hierarchical_movement/Destroy(force, silent) - LAZYCLEARLIST(containers) - return ..() - -/datum/component/track_hierarchical_movement/RegisterWithParent() - . = ..() - RegisterSignal(parent, COMSIG_MOVABLE_MOVED, PROC_REF(on_moved)) - rebuild_hierarchy() - -/datum/component/track_hierarchical_movement/UnregisterFromParent() - . = ..() - UnregisterSignal(parent, COMSIG_MOVABLE_MOVED) - for (var/container in containers) - UnregisterSignal(container, COMSIG_MOVABLE_MOVED) - -/// Something in our hierarchy moved, send signal then rebuild hierarchy -/datum/component/track_hierarchical_movement/proc/on_moved(datum/source, old_loc, dir, forced) - SIGNAL_HANDLER - SEND_SIGNAL(parent, COMSIG_MOVABLE_OR_CONTAINER_MOVED, old_loc, dir, forced) - rebuild_hierarchy() - -/// Listen for the movement signal on every parent which isn't the floor or the void -/datum/component/track_hierarchical_movement/proc/rebuild_hierarchy() - for (var/container in containers) - UnregisterSignal(container, COMSIG_MOVABLE_MOVED) - LAZYCLEARLIST(containers) - var/atom/checked = parent - while (!isnull(checked.loc) && !isturf(checked.loc)) - RegisterSignal(checked.loc, COMSIG_MOVABLE_MOVED, PROC_REF(on_moved)) - checked = checked.loc - LAZYADD(containers, checked) diff --git a/code/datums/elements/give_turf_traits.dm b/code/datums/elements/give_turf_traits.dm index e374b16d63c..3c53d4a5e73 100644 --- a/code/datums/elements/give_turf_traits.dm +++ b/code/datums/elements/give_turf_traits.dm @@ -1,11 +1,11 @@ -///A bespoke element that adds a set of traits to the turf while occupied by at least one attached movabled. +/// A bespoke element that adds a set of traits to the turf while occupied by at least one attached movabled. /datum/element/give_turf_traits element_flags = ELEMENT_DETACH_ON_HOST_DESTROY|ELEMENT_BESPOKE argument_hash_start_idx = 2 ///A list of traits that are added to the turf while occupied. var/list/traits - ///The list of occupied turfs: Assoc value is a list of movables with this element that are occupying the turf. - var/list/occupied_turfs = list() + ///List of sources we are using to reapply traits when turf changes + var/list/trait_sources = list() /datum/element/give_turf_traits/Attach(atom/movable/target, list/traits) . = ..() @@ -24,7 +24,7 @@ remove_from_occupied_turfs(source.loc, source) return ..() -///Removes the trait from the old turf and adds it to the new one. +/// Removes the trait from the old turf and adds it to the new one. /datum/element/give_turf_traits/proc/on_moved(atom/movable/source, atom/old_loc) SIGNAL_HANDLER if(isturf(old_loc)) @@ -38,16 +38,14 @@ * Otherwise, it just adds the movable to the assoc value of lists occupying the turf. */ /datum/element/give_turf_traits/proc/add_to_occupied_turfs(turf/location, atom/movable/source) - if(occupied_turfs[location]) - occupied_turfs[location] += source - return - - occupied_turfs[location] = list(source) - RegisterSignal(location, COMSIG_TURF_CHANGE, PROC_REF(pre_change_turf)) + var/trait_source = REF(source) + if(isnull(trait_sources) || isnull(trait_sources[location])) + RegisterSignal(location, COMSIG_TURF_CHANGE, PROC_REF(pre_change_turf)) + LAZYADDASSOCLIST(trait_sources, location, trait_source) var/update_movespeeds = (TRAIT_TURF_IGNORE_SLOWDOWN in traits) && !HAS_TRAIT(location, TRAIT_TURF_IGNORE_SLOWDOWN) for(var/trait in traits) - ADD_TRAIT(location, trait, REF(src)) + ADD_TRAIT(location, trait, trait_source) if(update_movespeeds) for(var/mob/living/living in location) living.update_turf_movespeed() @@ -57,25 +55,25 @@ * Otherwise, it just removes the movable from the assoc value of lists occupying the turf. */ /datum/element/give_turf_traits/proc/remove_from_occupied_turfs(turf/location, atom/movable/source) - LAZYREMOVE(occupied_turfs[location], source) - if(occupied_turfs[location]) - return - - occupied_turfs -= location - UnregisterSignal(location, COMSIG_TURF_CHANGE) + var/trait_source = REF(source) + LAZYREMOVEASSOC(trait_sources, location, trait_source) + if(isnull(trait_sources) || isnull(trait_sources[location])) + UnregisterSignal(location, COMSIG_TURF_CHANGE) for(var/trait in traits) - REMOVE_TRAIT(location, trait, REF(src)) + REMOVE_TRAIT(location, trait, trait_source) if((TRAIT_TURF_IGNORE_SLOWDOWN in traits) && !HAS_TRAIT(location, TRAIT_TURF_IGNORE_SLOWDOWN)) for(var/mob/living/living in location) living.update_turf_movespeed() -///Signals and components are carried over when the turf is changed, so they've to be readded post-change. +/// Signals and components are carried over when the turf is changed, so they've to be readded post-change. /datum/element/give_turf_traits/proc/pre_change_turf(turf/changed, path, list/new_baseturfs, flags, list/post_change_callbacks) SIGNAL_HANDLER post_change_callbacks += CALLBACK(src, PROC_REF(reoccupy_turf)) +/// Reapply turf traits to the provided turf /datum/element/give_turf_traits/proc/reoccupy_turf(turf/changed) for(var/trait in traits) - ADD_TRAIT(changed, trait, REF(src)) + for(var/source in trait_sources[changed]) + ADD_TRAIT(changed, trait, source) diff --git a/code/datums/elements/trait_loc.dm b/code/datums/elements/trait_loc.dm deleted file mode 100644 index 512a1edb527..00000000000 --- a/code/datums/elements/trait_loc.dm +++ /dev/null @@ -1,34 +0,0 @@ -/** - * # Trait Loc Element - * - * Adds a trait to the movable's loc, and handles relocating the trait if the movable itself moves. - */ -/datum/element/trait_loc - element_flags = ELEMENT_BESPOKE|ELEMENT_DETACH_ON_HOST_DESTROY // handles if our movable is deleted - argument_hash_start_idx = 2 - /// What trait to apply to the movable's loc. - var/trait_to_give - -/datum/element/trait_loc/Attach(atom/movable/target, trait_to_give) - . = ..() - if(!ismovable(target)) - return ELEMENT_INCOMPATIBLE - - src.trait_to_give = trait_to_give - - RegisterSignal(target, COMSIG_MOVABLE_MOVED, PROC_REF(on_movable_relocated)) - if(target.loc) - ADD_TRAIT(target.loc, trait_to_give, REF(target)) - -/datum/element/trait_loc/Detach(atom/movable/source, ...) - . = ..() - UnregisterSignal(source, COMSIG_MOVABLE_MOVED) - if(source.loc) - REMOVE_TRAIT(source.loc, trait_to_give, REF(source)) - -/datum/element/trait_loc/proc/on_movable_relocated(atom/movable/source, atom/old_loc) - SIGNAL_HANDLER - - REMOVE_TRAIT(old_loc, trait_to_give, REF(source)) - if(source.loc) - ADD_TRAIT(source.loc, trait_to_give, REF(source)) diff --git a/code/game/objects/structures/holosign.dm b/code/game/objects/structures/holosign.dm index 11b9ddb7dc5..3e38b11337c 100644 --- a/code/game/objects/structures/holosign.dm +++ b/code/game/objects/structures/holosign.dm @@ -123,7 +123,8 @@ /obj/structure/holosign/barrier/atmos/Initialize(mapload) . = ..() air_update_turf(TRUE, TRUE) - AddElement(/datum/element/trait_loc, TRAIT_FIREDOOR_STOP) + var/static/list/turf_traits = list(TRAIT_FIREDOOR_STOP) + AddElement(/datum/element/give_turf_traits, turf_traits) /obj/structure/holosign/barrier/atmos/block_superconductivity() //Didn't used to do this, but it's "normal", and will help ease heat flow transitions with the players. return TRUE diff --git a/code/modules/atmospherics/machinery/components/fusion/hfr_core.dm b/code/modules/atmospherics/machinery/components/fusion/hfr_core.dm index 6bc3debb4b7..2d7db14ef07 100644 --- a/code/modules/atmospherics/machinery/components/fusion/hfr_core.dm +++ b/code/modules/atmospherics/machinery/components/fusion/hfr_core.dm @@ -176,7 +176,8 @@ investigate_log("has been created.", INVESTIGATE_HYPERTORUS) // Our center is unreachable, so prevent stuff from getting stuck in there - AddElement(/datum/element/trait_loc, TRAIT_SECLUDED_LOCATION) + var/static/list/turf_traits = list(TRAIT_SECLUDED_LOCATION) + AddElement(/datum/element/give_turf_traits, turf_traits) /obj/machinery/atmospherics/components/unary/hypertorus/core/Destroy() unregister_signals(TRUE) diff --git a/code/modules/pai/card.dm b/code/modules/pai/card.dm index 69e6a22beb8..ec810f9d924 100644 --- a/code/modules/pai/card.dm +++ b/code/modules/pai/card.dm @@ -66,10 +66,21 @@ /obj/item/pai_card/Initialize(mapload) . = ..() - AddComponent(/datum/component/track_hierarchical_movement) + + var/static/list/containers_connections = list(COMSIG_MOVABLE_MOVED = PROC_REF(card_moved)) + AddComponent(/datum/component/connect_containers, tracked = src, connections = containers_connections) update_appearance() SSpai.pai_card_list += src +/obj/item/pai_card/Moved(atom/old_loc, movement_dir, forced, list/old_locs, momentum_change) + . = ..() + card_moved() + +/// Called when we, our loc, or our loc's loc, or our loc's loc's loc, or etc has moved +/obj/item/pai_card/proc/card_moved() + SIGNAL_HANDLER + pai?.check_distance() + /obj/item/pai_card/suicide_act(mob/living/user) user.visible_message(span_suicide("[user] is staring sadly at [src]! [user.p_They()] can't keep living without real human intimacy!")) return OXYLOSS diff --git a/code/modules/pai/pai.dm b/code/modules/pai/pai.dm index 141c51ac03f..8ed1fd5bfd9 100644 --- a/code/modules/pai/pai.dm +++ b/code/modules/pai/pai.dm @@ -235,7 +235,6 @@ update_appearance(UPDATE_DESC) RegisterSignal(src, COMSIG_LIVING_CULT_SACRIFICED, PROC_REF(on_cult_sacrificed)) - RegisterSignal(card, COMSIG_MOVABLE_OR_CONTAINER_MOVED, PROC_REF(check_distance)) /mob/living/silicon/pai/Moved(atom/old_loc, movement_dir, forced, list/old_locs, momentum_change) . = ..() diff --git a/tgstation.dme b/tgstation.dme index 530c14992ea..8a92d746feb 100644 --- a/tgstation.dme +++ b/tgstation.dme @@ -1053,7 +1053,6 @@ #include "code\datums\components\tippable.dm" #include "code\datums\components\toggle_attached_clothing.dm" #include "code\datums\components\toggle_suit.dm" -#include "code\datums\components\track_hierarchical_movement.dm" #include "code\datums\components\transforming.dm" #include "code\datums\components\trapdoor.dm" #include "code\datums\components\twohanded.dm" @@ -1288,7 +1287,6 @@ #include "code\datums\elements\tenacious.dm" #include "code\datums\elements\tiny_mob_hunter.dm" #include "code\datums\elements\tool_flash.dm" -#include "code\datums\elements\trait_loc.dm" #include "code\datums\elements\turf_transparency.dm" #include "code\datums\elements\undertile.dm" #include "code\datums\elements\unfriend_attacker.dm"