From deaee5c561e9fde63a8250beae23276d10f7e3bb Mon Sep 17 00:00:00 2001
From: A miscellaneous Fern <80640114+FernandoJ8@users.noreply.github.com>
Date: Sat, 19 Aug 2023 01:19:00 +0200
Subject: [PATCH] Removes some duplicated and redundant components from
 _datum_components() (#77615)

## About The Pull Request

As it stands, `_GetInverseTypeList()` (as used in `_JoinParent()` and
`_RemoveFromParent()`) has two issues:
1. As described in #76678, it adds duplicated components because even if
`current_type` is `/datum/component` AND we've already added it in `. =
list(our_type, current_type)`, the final line is outside the while loop
and adds it again.
2. Currently every `_datum_components` list has all of it's components
listed under the `/datum/component` key. It's used as an all-encompasing
list for when all the components an atom has need to be looped through,
but I think that's a questionable implementation. Making an entire new
list doesn't make much sense, given that accessing elements of an
associative list is very cheap, and it nearly doubles the work we do
when adding components to `_datum_components`.
Doing it with other parents at least removes the need for type checks to
get components of a given type, so I think those are fine to stay.
## Why It's Good For The Game

Closes #76678.
Avoids the questionably useful list of components with
`/datum/component` as its key from `_datum_components`.

I don't think there's anything player-facing here.
---
 code/datums/components/_component.dm | 23 ++++++++++++-----------
 code/datums/datum.dm                 | 15 ++++++++-------
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/code/datums/components/_component.dm b/code/datums/components/_component.dm
index c8bc176b5a9..fac24d6dd4d 100644
--- a/code/datums/components/_component.dm
+++ b/code/datums/components/_component.dm
@@ -237,12 +237,12 @@
  */
 /datum/component/proc/_GetInverseTypeList(our_type = type)
 	//we can do this one simple trick
+	. = list(our_type)
 	var/current_type = parent_type
-	. = list(our_type, current_type)
 	//and since most components are root level + 1, this won't even have to run
 	while (current_type != /datum/component)
 		current_type = type2parent(current_type)
-	. += current_type
+		. += current_type
 
 // The type arg is casted so initial works, you shouldn't be passing a real instance into this
 /**
@@ -481,15 +481,16 @@
 	var/list/dc = _datum_components
 	if(!dc)
 		return
-	var/comps = dc[/datum/component]
-	if(islist(comps))
-		for(var/datum/component/I in comps)
-			if(I.can_transfer)
-				target.TakeComponent(I)
-	else
-		var/datum/component/C = comps
-		if(C.can_transfer)
-			target.TakeComponent(comps)
+	for(var/component_key in dc)
+		var/component_or_list = dc[component_key]
+		if(islist(component_or_list))
+			for(var/datum/component/I in component_or_list)
+				if(I.can_transfer)
+					target.TakeComponent(I)
+		else
+			var/datum/component/C = component_or_list
+			if(C.can_transfer)
+				target.TakeComponent(C)
 
 /**
  * Return the object that is the host of any UI's that this component has
diff --git a/code/datums/datum.dm b/code/datums/datum.dm
index 67db3f62e28..3ad6a0fea11 100644
--- a/code/datums/datum.dm
+++ b/code/datums/datum.dm
@@ -128,13 +128,14 @@
 	//BEGIN: ECS SHIT
 	var/list/dc = _datum_components
 	if(dc)
-		var/all_components = dc[/datum/component]
-		if(length(all_components))
-			for(var/datum/component/component as anything in all_components)
-				qdel(component, FALSE, TRUE)
-		else
-			var/datum/component/C = all_components
-			qdel(C, FALSE, TRUE)
+		for(var/component_key in dc)
+			var/component_or_list = dc[component_key]
+			if(islist(component_or_list))
+				for(var/datum/component/component as anything in component_or_list)
+					qdel(component, FALSE, TRUE)
+			else
+				var/datum/component/C = component_or_list
+				qdel(C, FALSE, TRUE)
 		dc.Cut()
 
 	_clear_signal_refs()