Merge pull request #3688 from Citadel-Station-13/upstream-merge-32156

[MIRROR] Fixes TakeComponent

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.

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

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