From 47f88be07f25cf33ce8f592ef2e4ed887d120dd8 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Mon, 30 Oct 2017 04:52:18 -0400 Subject: [PATCH 1/2] Fixes TakeComponent (#32156) * Fixes TakeComponent * Add the note about OnTransfer and COMPONENT_INCOMPATIBLE * Fix behaviour * Missed datum.dm --- code/datums/components/README.md | 16 ++++--- code/datums/components/_component.dm | 66 ++++++++++++++++------------ code/datums/datum.dm | 4 +- 3 files changed, 50 insertions(+), 36 deletions(-) diff --git a/code/datums/components/README.md b/code/datums/components/README.md index 8d37e15de3..eeb6615a63 100644 --- a/code/datums/components/README.md +++ b/code/datums/components/README.md @@ -26,7 +26,7 @@ Stands have a lot of procs which mimic mob procs. Rather than inserting hooks fo ### Defines -1. `COMPONENT_INCOMPATIBLE` Return this from `/datum/component/Initialize` to have the component be deleted if it's applied to an incorrect type. `parent` must not be modified if this is to be returned. +1. `COMPONENT_INCOMPATIBLE` Return this from `/datum/component/Initialize` or `datum/component/OnTransfer` to have the component be deleted if it's applied to an incorrect type. `parent` must not be modified if this is to be returned. ### Vars @@ -42,12 +42,13 @@ Stands have a lot of procs which mimic mob procs. Rather than inserting hooks fo * `COMPONENT_DUPE_UNIQUE`: New component will be deleted, old component will first have `/datum/component/proc/InheritComponent(datum/component/new, TRUE)` on it 1. `/datum/component/var/dupe_type` (protected, type) * Definition of a duplicate component type - * `null` means exact match on `type` + * `null` means exact match on `type` (default) * Any other type means that and all subtypes 1. `/datum/component/var/list/signal_procs` (private) * Associated lazy list of signals -> `/datum/callback`s that will be run when the parent datum recieves that signal 1. `/datum/component/var/datum/parent` (protected, read-only) * The datum this component belongs to + * Never `null` in child procs ### Procs @@ -72,7 +73,7 @@ Stands have a lot of procs which mimic mob procs. Rather than inserting hooks fo * Will only be called if a component's callback returns `TRUE` 1. `/datum/proc/TakeComponent(datum/component/C)` (public, final) * Properly transfers ownership of a component from one datum to another - * Singals `COMSIG_COMPONENT_REMOVING` on the parent + * Signals `COMSIG_COMPONENT_REMOVING` on the parent * Called on the datum you want to own the component with another datum's component 1. `/datum/proc/SendSignal(signal, ...)` (public, final) * Call to send a signal to the components of the target datum @@ -96,10 +97,13 @@ Stands have a lot of procs which mimic mob procs. Rather than inserting hooks fo 1. `/datum/component/proc/AfterComponentActivated()` (abstract, async) * Called on a component that was activated after it's `parent`'s `ComponentActivated()` is called 1. `/datum/component/proc/OnTransfer(datum/new_parent)` (abstract, no-sleep) - * Called before the new `parent` is assigned in `TakeComponent()`, after the remove signal, before the added signal + * Called before `new_parent` is assigned to `parent` in `TakeComponent()` * Allows the component to react to ownership transfers -1. `/datum/component/proc/_RemoveNoSignal()` (private, final) - * Internal, clears the parent var and removes the component from the parents component list +1. `/datum/component/proc/_RemoveFromParent()` (private, final) + * Clears `parent` and removes the component from it's component list +1. `/datum/component/proc/_CheckDupesAndJoinParent` (private, final) + * Tries to add the component to it's `parent`s `datum_components` list + * Properly handles duplicate situations based on the `dupe_mode` var 1. `/datum/component/proc/RegisterSignal(signal(string/list of strings), proc_ref(type), override(boolean))` (protected, final) (Consider removing for performance gainz) * If signal is a list it will be as if RegisterSignal was called for each of the entries with the same following arguments * Makes a component listen for the specified `signal` on it's `parent` datum. diff --git a/code/datums/components/_component.dm b/code/datums/components/_component.dm index 9978b01955..4e187a6a08 100644 --- a/code/datums/components/_component.dm +++ b/code/datums/components/_component.dm @@ -13,28 +13,37 @@ parent = null qdel(src) return + + _CheckDupesAndJoinParent(P) +/datum/component/proc/_CheckDupesAndJoinParent() + var/datum/P = parent var/dm = dupe_mode + + var/datum/component/old if(dm != COMPONENT_DUPE_ALLOWED) var/dt = dupe_type - var/datum/component/old if(!dt) old = P.GetExactComponent(type) else old = P.GetComponent(dt) if(old) + //One or the other has to die switch(dm) if(COMPONENT_DUPE_UNIQUE) old.InheritComponent(src, TRUE) - parent = null //prevent COMPONENT_REMOVING signal + parent = null //prevent COMPONENT_REMOVING signal, no _RemoveFromParent because we aren't in their list yet qdel(src) return if(COMPONENT_DUPE_HIGHLANDER) InheritComponent(old, FALSE) + old._RemoveFromParent() qdel(old) - - //let the others know - P.SendSignal(COMSIG_COMPONENT_ADDED, src) + + //provided we didn't eat someone + if(!old) + //let the others know + P.SendSignal(COMSIG_COMPONENT_ADDED, src) //lazy init the parent's dc list var/list/dc = P.datum_components @@ -74,29 +83,28 @@ enabled = FALSE var/datum/P = parent if(P) - _RemoveNoSignal() + _RemoveFromParent() P.SendSignal(COMSIG_COMPONENT_REMOVING, src) LAZYCLEARLIST(signal_procs) return ..() -/datum/component/proc/_RemoveNoSignal() +/datum/component/proc/_RemoveFromParent() var/datum/P = parent - if(P) - var/list/dc = P.datum_components - var/our_type = type - for(var/I in _GetInverseTypeList(our_type)) - var/list/components_of_type = dc[I] - if(islist(components_of_type)) // - var/list/subtracted = components_of_type - src - if(subtracted.len == 1) //only 1 guy left - dc[I] = subtracted[1] //make him special - else - dc[I] = subtracted - else //just us - dc -= I - if(!dc.len) - P.datum_components = null - parent = null + var/list/dc = P.datum_components + var/our_type = type + for(var/I in _GetInverseTypeList(our_type)) + var/list/components_of_type = dc[I] + if(islist(components_of_type)) // + var/list/subtracted = components_of_type - src + if(subtracted.len == 1) //only 1 guy left + dc[I] = subtracted[1] //make him special + else + dc[I] = subtracted + else //just us + dc -= I + if(!dc.len) + P.datum_components = null + parent = null /datum/component/proc/RegisterSignal(sig_type_or_types, proc_on_self, override = FALSE) if(QDELETED(src)) @@ -201,7 +209,7 @@ var/nt = new_type args[1] = src var/datum/component/C = new nt(arglist(args)) - return QDELING(C) ? GetComponent(new_type) : C + return QDELING(C) ? GetExactComponent(new_type) : C /datum/proc/LoadComponent(component_type, ...) . = GetComponent(component_type) @@ -213,13 +221,15 @@ return var/datum/helicopter = C.parent if(helicopter == src) - //wat + //if we're taking to the same thing no need for anything return - C._RemoveNoSignal() + if(C.OnTransfer(src) == COMPONENT_INCOMPATIBLE) + qdel(C) + return + C._RemoveFromParent() helicopter.SendSignal(COMSIG_COMPONENT_REMOVING, C) - C.OnTransfer(src) C.parent = src - SendSignal(COMSIG_COMPONENT_ADDED, C) + C._CheckDupesAndJoinParent() /datum/proc/TransferComponents(datum/target) var/list/dc = datum_components diff --git a/code/datums/datum.dm b/code/datums/datum.dm index cd589fd8c3..f7b15035d1 100644 --- a/code/datums/datum.dm +++ b/code/datums/datum.dm @@ -27,11 +27,11 @@ if(islist(all_components)) for(var/I in all_components) var/datum/component/C = I - C._RemoveNoSignal() + C._RemoveFromParent() qdel(C) else var/datum/component/C = all_components - C._RemoveNoSignal() + C._RemoveFromParent() qdel(C) dc.Cut() return QDEL_HINT_QUEUE