Turns out a bunch of components do not properly transfer because of (#42691)

one of three things.

    1. They don't use RegisterWithParent or UnregisterFromParent to unregister
       and register signals

    2. They use callbacks which refer to a source object, which is usually deleted
       on transfer, or lost in some manner, or simply makes no sense at all to be
       transferred

    3. the component was never designed to be transferred at all

TransferComponents gave no shits about any of this and just blindly transferred
all components, if they were actually capable of it or not.

I only noticed this because it was causing chairs to break as they would not register signals
and verbs correctly for rotation after being picked up and then placed down, and a player
reported that issue via ahelp.

Luckily we caught it before the rot got anywhere, only chairs and the shuttle subystem
tend to use this proc (Shuttle uses it on turfs), can you imagine if everything was using
this LMAO

Which is good because it's more dangerous than a loaded gun

I have added a can_transfer var, that is true when a component is valid to
actually transfer, which means the dev has actually thought about what happens when
you take the parent object away and swap it for another and all the crazy that is entailed
by this

I have done my best to audit what components are actually
transferable, but things are basically a hot mess (Thanks @Cyberboss )

The following components required edits:
Forensics:
did not register/deregister the clean_act signal properly, did not checkblood on new parent

Rotation:
did not use RegisterWithParent or UnregisterFromParent, turned out
to not be transferable anyway due to having callbacks that can be
passed in to the parent with unknown sources that we can't feasibly
reuse (i.e if you're transferred from a chair to a bed, your old rotation
call backs are no longer valid). Turns out the use case it was for (just chairs)
didn't need it anyway, so I just made it non valid for transfer.

Wet Floor:
Honestly this one is just a hot mess, it should be a subtype of the slippery
component with the extra wet turf handling.

As it is it basically manages a slippery component on top of it's own extra
functionality, so that's a major code smell.

I added registration/unregistration of the signals, and made it's pretransfer
remove the slippery component and the posttransfer add it back (via update_flags)

Components that seem transferable without issues
mirage_border
orbiter
decal
spill
storage (I hope to earth)

code/__DEFINES/components.dm

@@ -7,6 +7,7 @@
 #define GET_COMPONENT(varname, path) GET_COMPONENT_FROM(varname, path, src)
 
 #define COMPONENT_INCOMPATIBLE 1
+#define COMPONENT_NOTRANSFER 2
 
 // How multiple components of the exact same type are handled in the same datum
 

code/datums/components/_component.dm

@@ -2,6 +2,10 @@
 	var/dupe_mode = COMPONENT_DUPE_HIGHLANDER
 	var/dupe_type
 	var/datum/parent
+	//only set to true if you are able to properly transfer this component
+	//At a minimum RegisterWithParent and UnregisterFromParent should be used
+	//Make sure you also implement PostTransfer for any post transfer handling
+	var/can_transfer = FALSE
 
 /datum/component/New(datum/P, ...)
 	parent = P
@@ -154,7 +158,7 @@
 	return
 
 /datum/component/proc/PostTransfer()
-	return
+	return COMPONENT_INCOMPATIBLE //Do not support transfer by default as you must properly support it
 
 /datum/component/proc/_GetInverseTypeList(our_type = type)
 	//we can do this one simple trick
@@ -281,10 +285,13 @@
 	if(target.parent)
 		target.RemoveComponent()
 	target.parent = src
-	if(target.PostTransfer() == COMPONENT_INCOMPATIBLE)
+	var/result = target.PostTransfer()
-		var/c_type = target.type
+	switch(result)
-		qdel(target)
+		if(COMPONENT_INCOMPATIBLE)
-		CRASH("Incompatible [c_type] transfer attempt to a [type]!")
+			var/c_type = target.type
+			qdel(target)
+			CRASH("Incompatible [c_type] transfer attempt to a [type]!")
+
 	if(target == AddComponent(target))
 		target._JoinParent()
 
@@ -294,10 +301,13 @@
 		return
 	var/comps = dc[/datum/component]
 	if(islist(comps))
-		for(var/I in comps)
+		for(var/datum/component/I in comps)
-			target.TakeComponent(I)
+			if(I.can_transfer)
+				target.TakeComponent(I)
 	else
-		target.TakeComponent(comps)
+		var/datum/component/C = comps
+		if(C.can_transfer)
+			target.TakeComponent(comps)
 
 /datum/component/ui_host()
 	return parent

code/datums/components/decal.dm

@@ -1,6 +1,6 @@
 /datum/component/decal
 	dupe_mode = COMPONENT_DUPE_ALLOWED
-
+	can_transfer = TRUE
 	var/cleanable
 	var/description
 	var/mutable_appearance/pic
@@ -72,4 +72,4 @@
 		qdel(src)
 
 /datum/component/decal/proc/examine(datum/source, mob/user)
-	to_chat(user, description)
+	to_chat(user, description)

code/datums/components/forensics.dm

@@ -1,5 +1,6 @@
 /datum/component/forensics
 	dupe_mode = COMPONENT_DUPE_UNIQUE
+	can_transfer = TRUE
 	var/list/fingerprints		//assoc print = print
 	var/list/hiddenprints		//assoc ckey = realname/gloves/ckey
 	var/list/blood_DNA			//assoc dna = bloodtype
@@ -21,8 +22,18 @@
 	blood_DNA = new_blood_DNA
 	fibers = new_fibers
 	check_blood()
+
+/datum/component/forensics/RegisterWithParent()
+	check_blood()
 	RegisterSignal(parent, COMSIG_COMPONENT_CLEAN_ACT, .proc/clean_act)
 
+/datum/component/forensics/UnregisterFromParent()
+    UnregisterSignal(parent, list(COMSIG_COMPONENT_CLEAN_ACT))
+
+/datum/component/forensics/PostTransfer()
+	if(!isatom(parent))
+		return COMPONENT_INCOMPATIBLE
+
 /datum/component/forensics/proc/wipe_fingerprints()
 	fingerprints = null
 	return TRUE

code/datums/components/lockon_aiming.dm

@@ -237,6 +237,3 @@
 		LOCKON_RANGING_BREAK_CHECK
 		cd++
 		CHECK_TICK
-
-/datum/component/lockon_aiming/PostTransfer(datum/new_parent)
-	return COMPONENT_INCOMPATIBLE

code/datums/components/mirage_border.dm

@@ -1,4 +1,5 @@
 /datum/component/mirage_border
+	can_transfer = TRUE
 	var/obj/effect/abstract/mirage_holder/holder
 
 /datum/component/mirage_border/Initialize(turf/target, direction, range=world.view)

code/datums/components/orbiter.dm

@@ -1,4 +1,5 @@
 /datum/component/orbiter
+	can_transfer = TRUE
 	dupe_mode = COMPONENT_DUPE_UNIQUE_PASSARGS
 	var/list/orbiters
 	var/datum/callback/orbiter_spy
@@ -153,4 +154,4 @@
 /atom/proc/transfer_observers_to(atom/target)
 	if(!orbiters || !istype(target) || !get_turf(target) || target == src)
 		return
-	target.TakeComponent(orbiters)
+	target.TakeComponent(orbiters)

code/datums/components/rotation.dm

@@ -44,15 +44,17 @@
 	if(src.rotation_flags & ROTATION_CLOCKWISE)
 		default_rotation_direction = ROTATION_CLOCKWISE
 
-	if(src.rotation_flags & ROTATION_ALTCLICK)
+/datum/component/simple_rotation/proc/add_signals()
+	if(rotation_flags & ROTATION_ALTCLICK)
 		RegisterSignal(parent, COMSIG_CLICK_ALT, .proc/HandRot)
 		RegisterSignal(parent, COMSIG_PARENT_EXAMINE, .proc/ExamineMessage)
-	if(src.rotation_flags & ROTATION_WRENCH)
+	if(rotation_flags & ROTATION_WRENCH)
 		RegisterSignal(parent, COMSIG_PARENT_ATTACKBY, .proc/WrenchRot)
 
-	if(src.rotation_flags & ROTATION_VERBS)
+/datum/component/simple_rotation/proc/add_verbs()
+	if(rotation_flags & ROTATION_VERBS)
 		var/atom/movable/AM = parent
-		if(src.rotation_flags & ROTATION_FLIP)
+		if(rotation_flags & ROTATION_FLIP)
 			AM.verbs += /atom/movable/proc/simple_rotate_flip
 		if(src.rotation_flags & ROTATION_CLOCKWISE)
 			AM.verbs += /atom/movable/proc/simple_rotate_clockwise
@@ -66,11 +68,30 @@
 		AM.verbs -= /atom/movable/proc/simple_rotate_clockwise
 		AM.verbs -= /atom/movable/proc/simple_rotate_counterclockwise
 
-/datum/component/simple_rotation/Destroy()
+/datum/component/simple_rotation/proc/remove_signals()
+		UnregisterSignal(parent, list(COMSIG_CLICK_ALT, COMSIG_PARENT_EXAMINE, COMSIG_PARENT_ATTACKBY))
+
+/datum/component/simple_rotation/RegisterWithParent()
+	add_verbs()
+	add_signals()
+	. = ..()
+
+/datum/component/simple_rotation/PostTransfer()
+	//Because of the callbacks which we don't track cleanly we can't transfer this
+	//item cleanly, better to let the new of the new item create a new rotation datum
+	//instead (there's no real state worth transferring)
+	return COMPONENT_NOTRANSFER
+
+/datum/component/simple_rotation/UnregisterFromParent()
 	remove_verbs()
+	remove_signals()
+	. = ..()
+
+/datum/component/simple_rotation/Destroy()
 	QDEL_NULL(can_user_rotate)
 	QDEL_NULL(can_be_rotated)
 	QDEL_NULL(after_rotation)
+	//Signals + verbs removed via UnRegister
 	. = ..()
 
 /datum/component/simple_rotation/RemoveComponent()

code/datums/components/spill.dm

@@ -2,6 +2,7 @@
 // Yes this exists purely for the spaghetti meme
 
 /datum/component/spill
+	can_transfer = TRUE
 	var/preexisting_item_flags
 
 	var/list/droptext
@@ -53,4 +54,4 @@
 	if(droptext)
 		fool.visible_message(arglist(droptext))
 	if(dropsound)
-		playsound(master, pick(dropsound), 30)
+		playsound(master, pick(dropsound), 30)

code/datums/components/storage/concrete/_concrete.dm

@@ -4,6 +4,7 @@
 // /mob/living/Move() in /modules/mob/living/living.dm - hiding storage boxes on mob movement
 
 /datum/component/storage/concrete
+	can_transfer = TRUE
 	var/drop_all_on_deconstruct = TRUE
 	var/drop_all_on_destroy = FALSE
 	var/transfer_contents_on_component_transfer = FALSE

code/datums/components/wet_floor.dm

@@ -1,5 +1,6 @@
 /datum/component/wet_floor
 	dupe_mode = COMPONENT_DUPE_UNIQUE_PASSARGS
+	can_transfer = TRUE
 	var/highest_strength = TURF_DRY
 	var/lube_flags = NONE			//why do we have this?
 	var/list/time_left_list			//In deciseconds.
@@ -26,14 +27,19 @@
 	if(!isopenturf(parent))
 		return COMPONENT_INCOMPATIBLE
 	add_wet(strength, duration_minimum, duration_add, duration_maximum)
-	RegisterSignal(parent, COMSIG_TURF_IS_WET, .proc/is_wet)
-	RegisterSignal(parent, COMSIG_TURF_MAKE_DRY, .proc/dry)
 	permanent = _permanent
 	if(!permanent)
 		START_PROCESSING(SSwet_floors, src)
 	addtimer(CALLBACK(src, .proc/gc, TRUE), 1)		//GC after initialization.
 	last_process = world.time
 
+/datum/component/wet_floor/RegisterWithParent()
+	RegisterSignal(parent, COMSIG_TURF_IS_WET, .proc/is_wet)
+	RegisterSignal(parent, COMSIG_TURF_MAKE_DRY, .proc/dry)
+
+/datum/component/wet_floor/UnregisterFromParent()
+	UnregisterSignal(parent, list(COMSIG_TURF_IS_WET, COMSIG_TURF_MAKE_DRY))
+
 /datum/component/wet_floor/Destroy()
 	STOP_PROCESSING(SSwet_floors, src)
 	var/turf/T = parent
@@ -136,12 +142,19 @@
 /datum/component/wet_floor/PreTransfer()
 	var/turf/O = parent
 	O.cut_overlay(current_overlay)
+	//That turf is no longer slippery, we're out of here
+	//Slippery components don't transfer due to callbacks
+	qdel(O.GetComponent(/datum/component/slippery))
 
 /datum/component/wet_floor/PostTransfer()
 	if(!isopenturf(parent))
 		return COMPONENT_INCOMPATIBLE
 	var/turf/T = parent
 	T.add_overlay(current_overlay)
+	//Make sure to add/update any slippery component on the new turf (update_flags calls LoadComponent)
+	update_flags()
+
+	//NB it's possible we get deleted after this, due to inherit
 
 /datum/component/wet_floor/proc/add_wet(type, duration_minimum = 0, duration_add = 0, duration_maximum = MAXIMUM_WET_TIME, _permanent = FALSE)
 	var/static/list/allowed_types = list(TURF_WET_WATER, TURF_WET_LUBE, TURF_WET_ICE, TURF_WET_PERMAFROST)