Unit Tests for AI Planning Subtrees not having required element/component (#77539)

## About The Pull Request

Hey there,

I've personally fallen for this stupid thing twice (in #77503 and #75627
(d3575161ca)), so I decided to spend a few
hours to crack out a unit test to ensure that I (and no one else) falls
for this stupid thing again.

Let me know if there's a smarter way to code something like this, but I
couldn't figure out a better way to accomodate the current framework and
be as agnostic to certain oddities as possible.
## Why It's Good For The Game
Catches stuff like this:

```txt
[2023-08-11 21:10:04.019]     FAILURE #1: The mob Garden Gnome does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #2: The mob the morph does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #3: The mob the guard spiderling (946) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate/to_flee) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #4: The mob the ambush spiderling (255) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate/to_flee) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #5: The mob the scout spiderling (375) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate/to_flee) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #6: The mob the flesh spiderling (337) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate/to_flee) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #7: The mob the hunter spiderling (869) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate/to_flee) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #8: The mob the nurse spiderling (629) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate/to_flee) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #9: The mob the tangle spiderling (19) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate/to_flee) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #10: The mob the broodmother spiderling (855) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate/to_flee) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #11: The mob the viper spiderling (519) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate/to_flee) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #12: The mob the tarantula spiderling (963) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate/to_flee) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
 -     FAILURE #13: The mob the spiderling (100) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_ELEMENT, but has a planning subtree (/datum/ai_planning_subtree/target_retaliate/to_flee) that requires it! at code/modules/unit_tests/ensure_subtree_element.dm:45
```

(ignore the part about gnomes and morphs, this was an earlier version of
the unit test. everything else was relevant and is fixed)
## Changelog
🆑
fix: Growing spiders will now retaliate against you like they were
always meant to.
/🆑
This commit is contained in:
san7890
2023-08-12 16:08:43 -06:00
committed by GitHub
parent 56f8b64461
commit 8e08a38cfb
11 changed files with 86 additions and 4 deletions
+3
View File
@@ -1005,6 +1005,7 @@ Remember to update _globalvars/traits.dm if you're adding/removing/renaming trai
*/
#define TRAIT_UNIQUE_IMMERSE "unique_immerse"
// unique trait sources, still defines
#define EMP_TRAIT "emp_trait"
#define STATUE_MUTE "statue"
@@ -1087,6 +1088,8 @@ Remember to update _globalvars/traits.dm if you're adding/removing/renaming trai
#define PAI_FOLDED "pai-folded"
/// Trait applied to brain mobs when they lack external aid for locomotion, such as being inside a mech.
#define BRAIN_UNAIDED "brain-unaided"
/// Trait applied to a mob when it gets a required "operational datum" (components/elements). Sends out the source as the type of the element.
#define TRAIT_SUBTREE_REQUIRED_OPERATIONAL_DATUM "element-required"
/// Trait applied by MODsuits.
#define MOD_TRAIT "mod"
/// Trait applied by element
+5 -2
View File
@@ -64,7 +64,8 @@ multiple modular subtrees with behaviors
if(idle_behavior)
idle_behavior = new idle_behavior()
PossessPawn(new_pawn)
if(!isnull(new_pawn)) // unit tests need the ai_controller to exist in isolation due to list schenanigans i hate it here
PossessPawn(new_pawn)
/datum/ai_controller/Destroy(force, ...)
set_ai_status(AI_STATUS_OFF)
@@ -148,6 +149,9 @@ multiple modular subtrees with behaviors
///Proc for deinitializing the pawn to the old controller
/datum/ai_controller/proc/UnpossessPawn(destroy)
if(isnull(pawn))
return // instantiated without an applicable pawn, fine
UnregisterSignal(pawn, list(COMSIG_MOB_LOGIN, COMSIG_MOB_LOGOUT, COMSIG_MOB_STATCHANGE))
if(ai_movement.moving_controllers[src])
ai_movement.stop_moving_towards(src)
@@ -155,7 +159,6 @@ multiple modular subtrees with behaviors
pawn = null
if(destroy)
qdel(src)
return
///Returns TRUE if the ai controller can actually run at the moment.
/datum/ai_controller/proc/able_to_run()
+4 -1
View File
@@ -1,6 +1,9 @@
///A subtree is attached to a controller and is occasionally called by /ai_controller/SelectBehaviors(), this mainly exists to act as a way to subtype and modify SelectBehaviors() without needing to subtype the ai controller itself
/datum/ai_planning_subtree
/// A list of typepaths of "operational datums" (elements/components) we absolutely NEED to run. Checked in unit tests, as well as be a nice reminder to developers that such a thing might be needed.
/// Note that in the Attach/Inititalize/New (or any future equivalent for these procs), you will need to add the trait TRAIT_SUBTREE_REQUIRED_OPERATIONAL_DATUM to the mob in question
/// in order for unit tests to succeed. This will break obviously enough if you don't do this and declare the required datum here however.
var/list/operational_datums = null
///Determines what behaviors should the controller try processing; if this returns SUBTREE_RETURN_FINISH_PLANNING then the controller won't go through the other subtrees should multiple exist in controller.planning_subtrees
/datum/ai_planning_subtree/proc/SelectBehaviors(datum/ai_controller/controller, seconds_per_tick)
@@ -12,8 +12,8 @@
/datum/ai_planning_subtree/basic_melee_attack_subtree/average_speed
melee_attack_behavior = /datum/ai_behavior/basic_melee_attack/average_speed
//If you give this to something without the element you are a dumbass.
/datum/ai_planning_subtree/basic_ranged_attack_subtree
operational_datums = list(/datum/element/ranged_attacks)
var/datum/ai_behavior/basic_ranged_attack/ranged_attack_behavior = /datum/ai_behavior/basic_ranged_attack
/datum/ai_planning_subtree/basic_ranged_attack_subtree/SelectBehaviors(datum/ai_controller/controller, seconds_per_tick)
@@ -1,5 +1,6 @@
/// Sets the BB target to a mob which you can see and who has recently attacked you
/datum/ai_planning_subtree/target_retaliate
operational_datums = list(/datum/element/ai_retaliate, /datum/component/ai_retaliate_advanced)
/// Blackboard key which tells us how to select valid targets
var/targetting_datum_key = BB_TARGETTING_DATUM
/// Blackboard key in which to store selected target
@@ -13,6 +13,8 @@
src.post_retaliate_callback = post_retaliate_callback
parent.AddElement(/datum/element/relay_attackers)
ADD_TRAIT(parent, TRAIT_SUBTREE_REQUIRED_OPERATIONAL_DATUM, type)
/datum/component/ai_retaliate_advanced/RegisterWithParent()
RegisterSignal(parent, COMSIG_ATOM_WAS_ATTACKED, PROC_REF(on_attacked))
+2
View File
@@ -12,6 +12,8 @@
target.AddElement(/datum/element/relay_attackers)
RegisterSignal(target, COMSIG_ATOM_WAS_ATTACKED, PROC_REF(on_attacked))
ADD_TRAIT(target, TRAIT_SUBTREE_REQUIRED_OPERATIONAL_DATUM, type)
/datum/element/ai_retaliate/Detach(datum/source, ...)
. = ..()
UnregisterSignal(source, COMSIG_ATOM_WAS_ATTACKED)
+2
View File
@@ -20,6 +20,8 @@
if(casingtype && projectiletype)
CRASH("Set both casing type and projectile type in [target]'s ranged attacks element! uhoh! stinky!")
ADD_TRAIT(target, TRAIT_SUBTREE_REQUIRED_OPERATIONAL_DATUM, type)
/datum/element/ranged_attacks/Detach(datum/target)
UnregisterSignal(target, COMSIG_MOB_ATTACK_RANGED)
return ..()
@@ -38,6 +38,7 @@
AddComponent(/datum/component/swarming)
AddElement(/datum/element/footstep, FOOTSTEP_MOB_CLAW, volume = 0.2) // they're small but you can hear 'em
AddElement(/datum/element/web_walker, /datum/movespeed_modifier/spiderling_web)
AddElement(/datum/element/ai_retaliate)
// keep in mind we have infinite range (the entire pipenet is our playground, it's just a matter of random choice as to where we end up) so lower and upper both have their gives and takes.
// but, also remember the more time we aren't in a vent, the more susceptible we are to dying to anything and everything.
+1
View File
@@ -130,6 +130,7 @@
#include "dynamic_ruleset_sanity.dm"
#include "egg_glands.dm"
#include "emoting.dm"
#include "ensure_subtree_operational_datum.dm"
#include "explosion_action.dm"
#include "fish_unit_tests.dm"
#include "focus_only_tests.dm"
@@ -0,0 +1,64 @@
/// The subtree that requires the operational datum.
#define REQUIRED_SUBTREE "required_subtree"
/// The list of typepaths of applicable operational datums that would satisfy the requirement.
#define REQUIRED_OPERATIONAL_DATUMS "required_operational_datums"
/// Unit Test that ensure that if we add a specific planning subtree to a basic mob's planning tree, that we also have the operational datum needed for it (component/element).
/// This can be extended to other "mandatory" operational datums for certain subtrees to work.
/datum/unit_test/ensure_subtree_operational_datum
/// Associated list of mobs that we need to test this on. Key is the typepath of the mob, value is a list of the planning subtree and the operational datums that are required for it.
var/list/testable_mobs = list()
/datum/unit_test/ensure_subtree_operational_datum/Run()
gather_testable_mobs()
test_applicable_mobs()
/// First, look for all mobs that have a planning subtree that requires an element, then add it to the list for stuff to test afterwards. Done like this to not have one mumbo proc that's hard to read.
/datum/unit_test/ensure_subtree_operational_datum/proc/gather_testable_mobs()
for(var/mob/living/basic/checkable_mob as anything in subtypesof(/mob/living/basic))
var/datum/ai_controller/testable_controller = initial(checkable_mob.ai_controller)
if(isnull(testable_controller))
continue
// we can't do inital() memes on lists so it's allocation time
testable_controller = allocate(testable_controller)
var/list/ai_planning_subtrees = testable_controller.planning_subtrees // list of instantiated datums. easy money
if(!length(ai_planning_subtrees))
continue
for(var/datum/ai_planning_subtree/testable_subtree as anything in ai_planning_subtrees)
var/list/necessary_datums = testable_subtree.operational_datums
if(isnull(necessary_datums))
continue
testable_mobs[checkable_mob] = list(
REQUIRED_OPERATIONAL_DATUMS = necessary_datums,
REQUIRED_SUBTREE = testable_subtree.type,
)
/// Then, test the mobs that we've found
/datum/unit_test/ensure_subtree_operational_datum/proc/test_applicable_mobs()
for(var/mob/living/basic/checkable_mob as anything in testable_mobs)
var/list/checkable_mob_data = testable_mobs[checkable_mob]
checkable_mob = allocate(checkable_mob)
var/datum/ai_planning_subtree/test_subtree = checkable_mob_data[REQUIRED_SUBTREE]
var/list/trait_sources = GET_TRAIT_SOURCES(checkable_mob, TRAIT_SUBTREE_REQUIRED_OPERATIONAL_DATUM)
if(!length(trait_sources)) // yes yes we could use `COUNT_TRAIT_SOURCES` but why invoke the same macro twice
TEST_FAIL("The mob [checkable_mob] ([checkable_mob.type]) does not have ANY instances of TRAIT_SUBTREE_REQUIRED_OPERATIONAL_DATUM, but has a planning subtree ([test_subtree]) that requires it!")
continue
var/has_element = FALSE
var/list/testable_operational_datums = checkable_mob_data[REQUIRED_OPERATIONAL_DATUMS]
for(var/iterable in trait_sources)
if(iterable in testable_operational_datums)
has_element = TRUE
break
if(!has_element)
var/list/message_list = list("The mob [checkable_mob] ([checkable_mob.type]) has a planning subtree ([test_subtree]) that requires a component/element, but does not have any!")
message_list += "Needs one of the following to satisfy the requirement: ([testable_operational_datums.Join(", ")])"
TEST_FAIL(message_list.Join(" "))
#undef REQUIRED_SUBTREE
#undef REQUIRED_OPERATIONAL_DATUMS