Fix itchy skillchip status effect "Curse of Mundanity", adds unit test for effects set to "Curse of Mundanity" (and missing IDs) (#88240)

## About The Pull Request

Fixes a lot of status effects having the default alert, adds a unit test
to check that status effects don't forget to change it

I chose a test rather than just making the default "no alert" because I
think people making status effects should think about whether it should
have an alert

Also I added a test for effects which did not set an ID because that's
kind of important, I applied the same mindset here to account for
abstract types but admittedly less sold on this one.

## Changelog

🆑 Melbert
fix: You should be afflicted by the "Curse of Mundanity" far, far less
/🆑

code/modules/unit_tests/_unit_tests.dm

@@ -288,7 +288,7 @@
 #include "spritesheets.dm"
 #include "stack_singular_name.dm"
 #include "station_trait_tests.dm"
-#include "status_effect_ticks.dm"
+#include "status_effect_validity.dm"
 #include "stomach.dm"
 #include "storage.dm"
 #include "strange_reagent.dm"

code/modules/unit_tests/status_effect_ticks.dm

@@ -1,23 +0,0 @@
-/// Validates status effect tick interval setup
-/datum/unit_test/status_effect_ticks
-
-/datum/unit_test/status_effect_ticks/Run()
-	for(var/datum/status_effect/checking as anything in subtypesof(/datum/status_effect))
-		var/tick_speed = initial(checking.tick_interval)
-		if(tick_speed == STATUS_EFFECT_NO_TICK)
-			continue
-		if(tick_speed == INFINITY)
-			TEST_FAIL("Status effect [checking] has tick_interval set to INFINITY, this is not how you prevent ticks - use tick_interval = STATUS_EFFECT_NO_TICK instead.")
-			continue
-		if(tick_speed == 0)
-			TEST_FAIL("Status effect [checking] has tick_interval set to 0, this is not how you prevent ticks - use tick_interval = STATUS_EFFECT_NO_TICK instead.")
-			continue
-		switch(initial(checking.processing_speed))
-			if(STATUS_EFFECT_FAST_PROCESS)
-				if(tick_speed < SSfastprocess.wait)
-					TEST_FAIL("Status effect [checking] has tick_interval set to [tick_speed], which is faster than SSfastprocess can tick ([SSfastprocess.wait]).")
-			if(STATUS_EFFECT_NORMAL_PROCESS)
-				if(tick_speed < SSprocessing.wait)
-					TEST_FAIL("Status effect [checking] has tick_interval set to [tick_speed], which is faster than SSprocessing can tick ([SSprocessing.wait]).")
-			else
-				TEST_FAIL("Invalid processing speed for status effect [checking] : [initial(checking.processing_speed)]")

code/modules/unit_tests/status_effect_validity.dm

@@ -0,0 +1,61 @@
+/// Validates status effect tick interval setup
+/datum/unit_test/status_effect_ticks
+
+/datum/unit_test/status_effect_ticks/Run()
+	for(var/datum/status_effect/checking as anything in subtypesof(/datum/status_effect))
+		if(initial(checking.id) == STATUS_EFFECT_ID_ABSTRACT)
+			continue
+		var/tick_speed = initial(checking.tick_interval)
+		if(tick_speed == STATUS_EFFECT_NO_TICK)
+			continue
+		if(tick_speed == INFINITY)
+			TEST_FAIL("Status effect [checking] has tick_interval set to INFINITY, this is not how you prevent ticks - use tick_interval = STATUS_EFFECT_NO_TICK instead.")
+			continue
+		if(tick_speed == 0)
+			TEST_FAIL("Status effect [checking] has tick_interval set to 0, this is not how you prevent ticks - use tick_interval = STATUS_EFFECT_NO_TICK instead.")
+			continue
+		switch(initial(checking.processing_speed))
+			if(STATUS_EFFECT_FAST_PROCESS)
+				if(tick_speed < SSfastprocess.wait)
+					TEST_FAIL("Status effect [checking] has tick_interval set to [tick_speed], which is faster than SSfastprocess can tick ([SSfastprocess.wait]).")
+			if(STATUS_EFFECT_NORMAL_PROCESS)
+				if(tick_speed < SSprocessing.wait)
+					TEST_FAIL("Status effect [checking] has tick_interval set to [tick_speed], which is faster than SSprocessing can tick ([SSprocessing.wait]).")
+			else
+				TEST_FAIL("Invalid processing speed for status effect [checking] : [initial(checking.processing_speed)]")
+
+/// Validates status effect alert type setup
+/datum/unit_test/status_effect_alert
+
+/datum/unit_test/status_effect_alert/Run()
+	// The base typepath is used to indicate "I didn't set an alert type"
+	var/bad_alert_type = /datum/status_effect::alert_type
+	TEST_ASSERT_NOTNULL(bad_alert_type, "No alert type defined in /datum/status_effect - This test may be redundant now.")
+
+	for(var/datum/status_effect/checking as anything in subtypesof(/datum/status_effect))
+		if(initial(checking.id) == STATUS_EFFECT_ID_ABSTRACT)
+			continue
+		if(initial(checking.alert_type) != bad_alert_type)
+			continue
+		TEST_FAIL("[checking] has not set alert_type. If you don't want an alert, set alert_type = null - \
+			Otherwise, give it an alert subtype.")
+
+/// Validates status effect id setup
+/datum/unit_test/status_effect_ids
+
+/datum/unit_test/status_effect_ids/Run()
+	// The base id is used to indicate "I didn't set an id"
+	var/bad_id = /datum/status_effect::id
+	TEST_ASSERT_NOTNULL(bad_id, "No id defined in /datum/status_effect - This test may be redundant now.")
+
+	for(var/datum/status_effect/checking as anything in subtypesof(/datum/status_effect))
+		if(initial(checking.id) == STATUS_EFFECT_ID_ABSTRACT)
+			// we are just assuming that a child of an abstract should not be abstract.
+			// of course in practice, this may not always be the case - but if you're
+			// structuring a status effect like this, you can just change the parent id to anything else
+			var/datum/status_effect/checking_parent = initial(checking.parent_type)
+			if(initial(checking_parent.id) != STATUS_EFFECT_ID_ABSTRACT)
+				continue
+		if(initial(checking.id) != bad_id)
+			continue
+		TEST_FAIL("[checking] has not set an id. This is required for status effects.")