[Port] unit test fixes (#1982)

* Fixes Ticked File Enforcement and Missing Unit Test (and makes said Unit Test Compile) (and genericizes the C&D list to the base unit test datum) (#77632)

Closes #77631

Hey there,

Ticked File Enforcement simply wasn't catching files that were missed.
That's a bit stupid, so I decided to look into what the issue might be,
and whoopsie daisies I did double periods back in #76592
(020ac24).

![image](https://github.com/tgstation/tgstation/assets/34697715/6023afe8-313d-4550-9a60-58a8bc211b4f)

I also added some debug info and some more checks to prevent such a
break from happening again on runtime of this script. I thought it was a
weird string concatenation issue (and not the simple break I thought it
was), so I rewrote how it adds `glob`s. I think it's cleaner so I'll
keep it anyhow

This PR also corrects the oversight of the missing unit test (introduced
in #77218 (6982760)) by reticking it in
the `_unit_tests.dm` file, and also makes it compile because it didn't
do that.

I also then had to do some more work to get the unit test to work.
* Genericizes the Create-and-Destroy "ignore" list to be a static list
on `/datum/unit_test` to allow it to be shared between these types of
tests that we need to test.
* Adds that list to C&D and the broken unit test regarding fantasy
bonuses
* Fixes some actually broken that the unit test was made to catch (beam
rifles, butterdogs and other slippery items, random ingredient boxes).
* Adds cases for things that the unit test and overall framework really
shouldn't be altering anyways (mythril), and was likely causing
inappropriate stack traces on master

Unit Tests WORK. Tools WORK.

![image](https://github.com/tgstation/tgstation/assets/34697715/9a59c0db-7a33-4546-918b-c73372a5b867)

:cl:
fix: Beam rifles will no longer inappropriately retain any bonuses they
may gain from wizardry.
fix: Inappropriate stack traces over bonuses being applied to components
that gain bonuses innately (like Mythril stacks) should cease.
/:cl:

* fix unticked unit test

* Dumb fix

---------

Co-authored-by: san7890 <[email protected]>

code/__DEFINES/traits.dm

@@ -293,6 +293,8 @@ Remember to update _globalvars/traits.dm if you're adding/removing/renaming trai
 #define TRAIT_ANTIMAGIC_NO_SELFBLOCK "anti_magic_no_selfblock"
 /// The user can do things like use magic staffs without penalty
 #define TRAIT_MAGICALLY_GIFTED "magically_gifted"
+/// This object innately spawns with fantasy variables already applied (the magical component is given to it on initialize), and thus we never want to give it the component again.
+#define TRAIT_INNATELY_FANTASTICAL_ITEM "innately_fantastical_item"
 #define TRAIT_DEPRESSION "depression"
 #define TRAIT_BLOOD_DEFICIENCY "blood_deficiency"
 #define TRAIT_JOLLY "jolly"

code/__HELPERS/~monkestation-helpers/atoms.dm

@@ -0,0 +1,6 @@
+/atom/proc/effective_contents(list/typecache = null)
+	var/static/list/default_typecache
+	if(!typecache)
+		default_typecache ||= typecacheof(list(/obj/effect, /atom/movable/screen))
+		typecache = default_typecache
+	return typecache_filter_list_reverse(src.contents, typecache)

code/datums/components/fantasy/_fantasy.dm

@@ -14,7 +14,7 @@
 
 ///affixes expects an initialized list
 /datum/component/fantasy/Initialize(quality, list/affixes = list(), canFail=FALSE, announce=FALSE)
-	if(!isitem(parent))
+	if(!isitem(parent) || HAS_TRAIT(parent, TRAIT_INNATELY_FANTASTICAL_ITEM))
 		return COMPONENT_INCOMPATIBLE
 
 	src.quality = quality

code/datums/components/slippery.dm

@@ -62,6 +62,7 @@
 	knockdown_time = source.reset_fantasy_variable("knockdown_time", knockdown_time)
 	paralyze_time = source.reset_fantasy_variable("paralyze_time", paralyze_time)
 	var/previous_lube_flags = LAZYACCESS(source.fantasy_modifications, "lube_flags")
+	LAZYREMOVE(source.fantasy_modifications, "lube_flags")
 	if(!isnull(previous_lube_flags))
 		lube_flags = previous_lube_flags
 

code/datums/materials/basemats.dm

@@ -325,10 +325,12 @@ Unless you know what you're doing, only use the first three numbers. They're in
 	. = ..()
 	if(isitem(source))
 		source.AddComponent(/datum/component/fantasy)
+		ADD_TRAIT(source, TRAIT_INNATELY_FANTASTICAL_ITEM, REF(src)) // DO THIS LAST OR WE WILL NEVER GET OUR BONUSES!!!
 
 /datum/material/mythril/on_removed_obj(atom/source, amount, material_flags)
 	. = ..()
 	if(isitem(source))
+		REMOVE_TRAIT(source, TRAIT_INNATELY_FANTASTICAL_ITEM, REF(src)) // DO THIS FIRST OR WE WILL NEVER GET OUR BONUSES DELETED!!!
 		qdel(source.GetComponent(/datum/component/fantasy))
 
 /datum/material/mythril/on_accidental_mat_consumption(mob/living/carbon/victim, obj/item/source_item)

code/game/objects/items.dm

@@ -231,7 +231,7 @@
 	var/throw_verb
 
 	/// A lazylist used for applying fantasy values, contains the actual modification applied to a variable.
-	var/list/fantasy_modifications
+	var/list/fantasy_modifications = null
 
 /obj/item/Initialize(mapload)
 	if(attack_verb_continuous)
@@ -1617,8 +1617,13 @@
 
 /// Modifies the fantasy variable
 /obj/item/proc/modify_fantasy_variable(variable_key, value, bonus, minimum = 0)
-	if(LAZYACCESS(fantasy_modifications, variable_key) != null)
+	var/result = LAZYACCESS(fantasy_modifications, variable_key)
+	if(!isnull(result))
+		if(HAS_TRAIT(src, TRAIT_INNATELY_FANTASTICAL_ITEM))
+			return result // we are immune to your foul magicks you inferior wizard, we keep our bonuses
+
 		stack_trace("modify_fantasy_variable was called twice for the same key '[variable_key]' on type '[type]' before reset_fantasy_variable could be called!")
+
 	var/intended_target = value + bonus
 	value = max(minimum, intended_target)
 
@@ -1630,9 +1635,14 @@
 /// Returns the original fantasy variable value
 /obj/item/proc/reset_fantasy_variable(variable_key, current_value)
 	var/modification = LAZYACCESS(fantasy_modifications, variable_key)
+
+	if(isnum(modification) && HAS_TRAIT(src, TRAIT_INNATELY_FANTASTICAL_ITEM))
+		return modification // we are immune to your foul magicks you inferior wizard, we keep our bonuses the way they are
+
 	LAZYREMOVE(fantasy_modifications, variable_key)
-	if(!modification)
+	if(isnull(modification))
 		return current_value
+
 	return current_value - modification
 
 /obj/item/proc/apply_fantasy_bonuses(bonus)

code/game/objects/items/storage/storage.dm

@@ -12,6 +12,9 @@
 
 /obj/item/storage/apply_fantasy_bonuses(bonus)
 	. = ..()
+	if(isnull(atom_storage)) // some abstract types of storage (yes i know) don't get a datum
+		return
+
 	atom_storage.max_slots = modify_fantasy_variable("max_slots", atom_storage.max_slots, round(bonus/2))
 	atom_storage.max_total_storage = modify_fantasy_variable("max_total_storage", atom_storage.max_total_storage, round(bonus/2))
 	LAZYSET(fantasy_modifications, "max_specific_storage", atom_storage.max_specific_storage)
@@ -25,9 +28,13 @@
 		atom_storage.max_specific_storage = WEIGHT_CLASS_TINY
 
 /obj/item/storage/remove_fantasy_bonuses(bonus)
+	if(isnull(atom_storage)) // some abstract types of storage (yes i know) don't get a datum
+		return ..()
+
 	atom_storage.max_slots = reset_fantasy_variable("max_slots", atom_storage.max_slots)
 	atom_storage.max_total_storage = reset_fantasy_variable("max_total_storage", atom_storage.max_total_storage)
 	var/previous_max_storage = LAZYACCESS(fantasy_modifications, "max_specific_storage")
+	LAZYREMOVE(fantasy_modifications, "max_specific_storage")
 	if(previous_max_storage)
 		atom_storage.max_specific_storage = previous_max_storage
 	return ..()

code/modules/projectiles/guns/energy/beam_rifle.dm

@@ -78,7 +78,7 @@
 	. = ..()
 	delay = modify_fantasy_variable("delay", delay, -bonus * 2)
 	aiming_time = modify_fantasy_variable("aiming_time", aiming_time, -bonus * 2)
-	recoil = modify_fantasy_variable("aiming_time", aiming_time, round(-bonus / 2))
+	recoil = modify_fantasy_variable("recoil", recoil, round(-bonus / 2))
 
 /obj/item/gun/energy/beam_rifle/remove_fantasy_bonuses(bonus)
 	delay = reset_fantasy_variable("delay", delay)

code/modules/unit_tests/_unit_tests.dm

@@ -128,6 +128,7 @@
 #include "heretic_rituals.dm"
 #include "high_five.dm"
 #include "holidays.dm"
+#include "human_through_recycler.dm"
 #include "hunger_curse.dm"
 #include "hydroponics_extractor_storage.dm"
 #include "hydroponics_validate_genes.dm"
@@ -151,6 +152,7 @@
 #include "mob_chains.dm"
 #include "mob_faction.dm"
 #include "mob_spawn.dm"
+#include "modify_fantasy_variable.dm"
 #include "modsuit.dm"
 #include "modular_map_loader.dm"
 #include "mouse_bite_cable.dm"

code/modules/unit_tests/human_through_recycler.dm

@@ -18,7 +18,7 @@
 	TEST_ASSERT(chewer.bloody, "The emagged recycler did not become bloody after crushing the assistant!")
 
 	// Now, let's test to see if all of their clothing got properly deleted.
-	TEST_ASSERT_EQUAL(length(assistant.contents), 0, "Assistant still has items in its contents after being put through an emagged recycler!")
+	TEST_ASSERT_EQUAL(length(assistant.effective_contents()), 0, "Assistant still has items in its contents after being put through an emagged recycler!")
 	// Consistent Assistants will always have the following: ID, PDA, backpack, a uniform, a headset, and a pair of shoes. If any of these are still present, then the recycler did not properly delete the assistant's clothing.
 	// However, let's check for EVERYTHING just in case, because we don't want to miss anything.
 	// This is just what we expect to be deleted.

code/modules/unit_tests/modify_fantasy_variable.dm

@@ -1,21 +1,46 @@
 // Unit test to make sure that there are no duplicate keys when modify_fantasy_variable is called when applying fantasy bonuses.
 // Also to make sure the fantasy_modifications list is null when fantasy bonuses are removed.
+/datum/unit_test/modify_fantasy_variable
+	priority = TEST_LONGER
+
 /datum/unit_test/modify_fantasy_variable/Run()
+	var/list/applicable_types = subtypesof(/obj/item) - uncreatables
 
-	for(var/obj/item/path as anything in subtypesof(/obj/item))
+	for(var/obj/item/path as anything in applicable_types)
 		var/obj/item/object = allocate(path)
+		// objects will have fantasy bonuses inherent to their type (like butterdogs and the slippery component), so we need to take this into account
+		var/number_of_extant_bonuses = LAZYLEN(object.fantasy_modifications)
+
+#define TEST_SUCCESS LAZYLEN(object.fantasy_modifications) == number_of_extant_bonuses
+
 		// Try positive
 		object.apply_fantasy_bonuses(bonus = 5)
 		object.remove_fantasy_bonuses(bonus = 5)
-		TEST_ASSERT_NULL(object.fantasy_modifications)
+		TEST_ASSERT(TEST_SUCCESS, generate_failure_message(object))
+
 		// Then negative
 		object.apply_fantasy_bonuses(bonus = -5)
 		object.remove_fantasy_bonuses(bonus = -5)
-		TEST_ASSERT_NULL(object.fantasy_modifications)
+		TEST_ASSERT(TEST_SUCCESS, generate_failure_message(object))
+
 		// Now try the extremes of each
 		object.apply_fantasy_bonuses(bonus = 500)
 		object.remove_fantasy_bonuses(bonus = 500)
-		TEST_ASSERT_NULL(object.fantasy_modifications)
+		TEST_ASSERT(TEST_SUCCESS, generate_failure_message(object))
+
 		object.apply_fantasy_bonuses(bonus = -500)
 		object.remove_fantasy_bonuses(bonus = -500)
-		TEST_ASSERT_NULL(object.fantasy_modifications)
+		TEST_ASSERT(TEST_SUCCESS, generate_failure_message(object))
+
+/// Returns a string that we use to describe the failure of the test.
+/datum/unit_test/modify_fantasy_variable/proc/generate_failure_message(obj/item/failed_object)
+	var/list/cached_modifications = failed_object.fantasy_modifications
+	var/length_of_modifications = LAZYLEN(cached_modifications)
+	var/list/failure_messages = list("Error found when adding+removing fantasy bonuses for [failed_object.type].")
+	failure_messages += "The length of the fantasy_modifications list was [length_of_modifications]."
+	if(length_of_modifications)
+		failure_messages += "The fantasy_modifications list was [cached_modifications.Join(", ")]."
+
+	return failure_messages.Join(" ")
+
+#undef TEST_SUCCESS

tgstation.dme

@@ -554,6 +554,7 @@
 #include "code\__HELPERS\sorts\TimSort.dm"
 #include "code\__HELPERS\~monkestation-helpers\announcements.dm"
 #include "code\__HELPERS\~monkestation-helpers\antags.dm"
+#include "code\__HELPERS\~monkestation-helpers\atoms.dm"
 #include "code\__HELPERS\~monkestation-helpers\icon_smoothing.dm"
 #include "code\__HELPERS\~monkestation-helpers\roundend.dm"
 #include "code\__HELPERS\~monkestation-helpers\time.dm"

tools/ticked_file_enforcement/ticked_file_enforcement.py

@@ -37,7 +37,7 @@ def post_error(string):
         post_error(f"Excluded file {full_file_path} does not exist, please remove it!")
         sys.exit(1)
 
-file_extensions = (".dm", ".dmf")
+file_extensions = ("dm", "dmf")
 
 reading = False
 lines = []
@@ -64,7 +64,12 @@ def post_error(string):
 
 scannable_files = []
 for file_extension in file_extensions:
-    scannable_files += glob.glob(scannable_directory + f"**/*.{file_extension}", recursive=True)
+    compiled_directory = f"{scannable_directory}/**/*.{file_extension}"
+    scannable_files += glob.glob(compiled_directory, recursive=True)
+
+if len(scannable_files) == 0:
+    post_error(f"No files were found in {scannable_directory}. Ticked File Enforcement has failed!")
+    sys.exit(1)
 
 for code_file in scannable_files:
     dm_path = ""
@@ -148,4 +153,4 @@ def compare_lines(a, b):
         post_error(f"The include at line {index + offset} is out of order ({line}, expected {sorted_lines[index]})")
         sys.exit(1)
 
-print(green(f"Ticked File Enforcement: [{file_reference}] All includes are in order!"))
+print(green(f"Ticked File Enforcement: [{file_reference}] All includes (for {len(scannable_files)} scanned files) are in order!"))