Skip to content

Commit

Permalink
Updates Mob_Spawn Unit Test To Use NAMEOF (tgstation#73035)
Browse files Browse the repository at this point in the history
## About The Pull Request

One of the key functions of this test is to iterate vars to see if it is
null or not, and then error if something is defined where it should not
be. Just to futureproof this test in case someone changes the name of a
variable on the mob spawner (e.g. haircolor to hair_color), we still
want this unit test to work!

So, let's switch it all over to NAMEOF to ensure that all of our stuff
still works the way we want it: safe-guarded against trivial changes
(it'll throw a compile error if one of these vars are changed, which is
good because it compels the coder to fix it).

I also re-arranged the unit test since we didn't need to have those
bulky lists in the middle of the actual checking code, and it should
work a-okay.
## Why It's Good For The Game

Someone in the future will sob a few tears if this unit test breaks and
remains broken throwing runtimes and bricking tgui or whatever because
it's very easy to just have this unit test not be useful in a very
"simple code improvement"
## Changelog
Nothing that concerns players.
  • Loading branch information
san7890 authored Jan 29, 2023
1 parent c317090 commit 66d7441
Showing 1 changed file with 25 additions and 21 deletions.
46 changes: 25 additions & 21 deletions code/modules/unit_tests/mob_spawn.dm
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
/datum/unit_test/mob_spawn

/datum/unit_test/mob_spawn/Run()
// The ghost role that we're going to iterate over. defined all the way up here for easy/cleaner static analysis
var/obj/effect/mob_spawn/ghost_role/ghost_role

//these are not expected to be filled out as they are base prototypes
var/list/prototypes = list(
Expand All @@ -12,39 +14,41 @@
/obj/effect/mob_spawn/ghost_role/human,
)

//ghost role checks
//vars that must not be set if the mob type isn't human
var/static/list/human_only_vars = list(
NAMEOF(ghost_role, facial_haircolor),
NAMEOF(ghost_role, facial_hairstyle),
NAMEOF(ghost_role, haircolor),
NAMEOF(ghost_role, hairstyle),
NAMEOF(ghost_role, mob_species),
NAMEOF(ghost_role, outfit),
NAMEOF(ghost_role, skin_tone),
)

//vars that must be set on all ghost roles.
var/static/list/required_vars = list(
//mob_type is not included because the errors on it are loud and some types choose their mob_type on selection
NAMEOF(ghost_role, prompt_name) = "Your ghost role has broken tgui without it.",
//these must be set even if show_flavor is false because the spawn menu still uses them and we simply must have higher quality roles
NAMEOF(ghost_role, flavour_text) = "Spawners menu uses it.",
NAMEOF(ghost_role, you_are_text) = "Spawners menu uses it.",
)

//ghost role checks - where the actual work is done
for(var/role_spawn_path in subtypesof(/obj/effect/mob_spawn/ghost_role) - prototypes)
var/obj/effect/mob_spawn/ghost_role/ghost_role = allocate(role_spawn_path)
ghost_role = allocate(role_spawn_path)

if(ghost_role.outfit_override)
TEST_FAIL("[ghost_role.type] has a defined \"outfit_override\" list, which is only for mapping. Do not set this!")

var/human_type = /mob/living/carbon/human // this exists to prevent being caught by checks
if(ghost_role.mob_type != human_type)
//vars that must not be set if the mob type isn't human
var/list/human_only_vars = list(
"mob_species",
"outfit",
"hairstyle",
"facial_hairstyle",
"haircolor",
"facial_haircolor",
"skin_tone",
)
for(var/human_only_var in human_only_vars)
if(ghost_role.vars[human_only_var])
TEST_FAIL("[ghost_role.type] has a defined \"[human_only_var]\" HUMAN ONLY var, but this type doesn't spawn humans.")

//vars that must be set on
var/list/required_vars = list(
//mob_type is not included because the errors on it are loud and some types choose their mob_type on selection
"prompt_name" = "Your ghost role has broken tgui without it.",
//these must be set even if show_flavor is false because the spawn menu still uses them and in 2021 we simply must have higher quality roles
"you_are_text" = "Spawners menu uses it.",
"flavour_text" = "Spawners menu uses it.",
)
for(var/required_var in required_vars)
if(required_var == "prompt_name" && !ghost_role.prompt_ghost)
if(required_var == NAMEOF(ghost_role, prompt_name) && !ghost_role.prompt_ghost)
continue //only case it makes sense why you shouldn't have a prompt_name
if(!ghost_role.vars[required_var])
TEST_FAIL("[ghost_role.type] must have \"[required_var]\" defined. Reason: [required_vars[required_var]]")
Expand Down

0 comments on commit 66d7441

Please sign in to comment.