Files
Bubberstation/code/modules/unit_tests/metabolizing.dm
SkyratBot 3cbd4752fd [MIRROR] Stack traces when a cliented or ckeyed mob is Destroy()ed. [MDB IGNORE] (#19031)
Stack traces when a cliented or ckeyed mob is Destroy()ed. (#72797)

## About The Pull Request

One thing that repeatedly pops up in admin channels is investigating
causes of death when a player just vanishes from the game. These are
almost universally qdeletion issues.

9 years ago `/mob/Destroy()` was commented with:
`//This makes sure that mobs with clients/keys are not just deleted from
the game.`

Whatever code may have existed back then has long since been replaced.

**I consider that Destroy()ing a cliented or ckeyed mob is a runtime
error case.**

Code which may result in deleting a mob should handle removing and/or
reassigning any client or ckey - or call generic procs that do so -
prior to destruction. This should ideally result in a clear log trail
that allows admins to see what happened. Where this isn't the case, a
stack trace will now be available to help narrow down the cause of
qdeletion so an issue report can be made, and so admins have SOME info
to investigate on.

An example of where this would help catch bugs is #72782 - It was
clearly unintended behaviour to qdel the mob in the first place and this
stack trace would have immediately highlighted exactly where the death
came from.
```
[2023-01-18 12:44:40.415] runtime error: Mob with client has been deleted. (code/modules/mob/mob.dm:29)
 - proc name:  stack trace (/proc/_stack_trace)
 -   source file: stack_trace.dm,4
 -   usr: Julia Watson (/mob/living/carbon/human)
 -   src: null
 -   usr.loc: the floor (111,143,2) (/turf/open/floor/iron)
 -   call stack:
 -  stack trace("Mob with client has been delet...", "code/modules/mob/mob.dm", 29)
 - Julia Watson (/mob/living/carbon/human): Destroy(0)
 - Julia Watson (/mob/living/carbon/human): Destroy(0)
 - Julia Watson (/mob/living/carbon/human): Destroy(0)
 - Julia Watson (/mob/living/carbon/human): Destroy(0)
 - qdel(Julia Watson (/mob/living/carbon/human), 0)
 - the mouse (/mob/living/basic/mouse): try consume cheese(Julia Watson (/mob/living/carbon/human))
 - the mouse (/mob/living/basic/mouse): tamed(the mouse (/mob/living/basic/mouse), Julia Watson (/mob/living/carbon/human))
 - /datum/callback (/datum/callback): Invoke(the mouse (/mob/living/basic/mouse), Julia Watson (/mob/living/carbon/human))
 - /datum/component/tameable (/datum/component/tameable): on tame(the mouse (/mob/living/basic/mouse), Julia Watson (/mob/living/carbon/human))
 - the mouse (/mob/living/basic/mouse):  SendSignal("simplemob_sentiencepotion", /list (/list))
 - the intelligence potion (/obj/item/slimepotion/slime/sentience): attack(the mouse (/mob/living/basic/mouse), Julia Watson (/mob/living/carbon/human), "icon-x=16;icon-y=7;left=1;butt...")
 - the mouse (/mob/living/basic/mouse): attackby(the intelligence potion (/obj/item/slimepotion/slime/sentience), Julia Watson (/mob/living/carbon/human), "icon-x=16;icon-y=7;left=1;butt...")
 - the intelligence potion (/obj/item/slimepotion/slime/sentience): melee attack chain(Julia Watson (/mob/living/carbon/human), the mouse (/mob/living/basic/mouse), "icon-x=16;icon-y=7;left=1;butt...")
 - Julia Watson (/mob/living/carbon/human): ClickOn(the mouse (/mob/living/basic/mouse), "icon-x=16;icon-y=7;left=1;butt...")
 - the mouse (/mob/living/basic/mouse): Click(the floor (112,143,2) (/turf/open/floor/iron), "mapwindow.map", "icon-x=16;icon-y=7;left=1;butt...")
 - 
```
See also #67300.

An example of where this would help identify causes of death where
previously there were none - Scenarios that were fixed by #62949 and
#66104 caused administrative headaches figuring out causes of death when
players in exploding crates just got literally fucking qdeleted. Which
was pretty trivial for players to do to other players.

Issue reports can be made and bugs can be fixed as we go along.

There are examples of quote "false positives" unquote.

Right click -> Delete, which can be used on any atom.
```
[2023-01-18 11:40:54.597] runtime error: Mob without client but with associated ckey has been deleted. (code/modules/mob/mob.dm:32)
 - proc name:  stack trace (/proc/_stack_trace)
 -   source file: stack_trace.dm,4
 -   usr: Norah Rader (/mob/dead/observer)
 -   src: null
 -   usr.loc: the floor (109,143,2) (/turf/open/floor/iron)
 -   call stack:
 -  stack trace("Mob without client but with as...", "code/modules/mob/mob.dm", 32)
 - Fulton Enderly (/mob/dead/observer): Destroy(0)
 - Fulton Enderly (/mob/dead/observer): Destroy(0)
 - qdel(Fulton Enderly (/mob/dead/observer), 0)
 - Timberpoes (/client): admin delete(Fulton Enderly (/mob/dead/observer))
 - Timberpoes (/client): Delete(Fulton Enderly (/mob/dead/observer))
```
 
Admin gibself
```
[2023-01-18 11:41:17.635] runtime error: Mob with client has been deleted. (code/modules/mob/mob.dm:29)
 - proc name:  stack trace (/proc/_stack_trace)
 -   source file: stack_trace.dm,4
 -   usr: Norah Rader (/mob/living/carbon/human)
 -   src: null
 -   usr.loc: the floor (109,145,2) (/turf/open/floor/iron)
 -   call stack:
 -  stack trace("Mob with client has been delet...", "code/modules/mob/mob.dm", 29)
 - Norah Rader (/mob/living/carbon/human): Destroy(0)
 - Norah Rader (/mob/living/carbon/human): Destroy(0)
 - Norah Rader (/mob/living/carbon/human): Destroy(0)
 - Norah Rader (/mob/living/carbon/human): Destroy(0)
 - qdel(Norah Rader (/mob/living/carbon/human), 0)
 - Norah Rader (/mob/living/carbon/human): gib(1, 1, 1, 0)
 - Norah Rader (/mob/living/carbon/human): gib(1, 1, 1, 0)
 - Timberpoes (/client): Gibself()
```

Over time these stack traces can be checked for how well the true cause
of death is logged, and whether there is a better procedure for deleting
cliented/ckeyed mobs than just qdel(target).
## Why It's Good For The Game

qdeletion of cliented or ckeyed mobs is death and often lacks any
specific logging.

This obfuscates bugs and hinders admin investigation.

stack_tracing highlights where this happens, revealing the previously
obfuscated bugs more clearly. It also provides relevant information on a
cause of death that can allow admin investigation to take place despite
the absence of logging.
## Changelog
🆑
admin: When a player-owned mob is deleted from the game world, a stack
trace is now dropped in the runtime logs. This allows admins and coders
to investigate these issues easier.
/🆑

Co-authored-by: Timberpoes <silent_insomnia_pp@hotmail.co.uk>
2023-01-31 02:16:52 +00:00

122 lines
4.7 KiB
Plaintext

/datum/unit_test/metabolization/Run()
// Pause natural mob life so it can be handled entirely by the test
SSmobs.pause()
var/mob/living/carbon/human/human = allocate(/mob/living/carbon/human/consistent)
var/list/blacklisted_reagents = list(
/datum/reagent/eigenstate, //Creates clones after a delay which get into other tests
)
var/list/reagents_to_check = subtypesof(/datum/reagent) - blacklisted_reagents - GLOB.fake_reagent_blacklist
for (var/reagent_type in reagents_to_check)
test_reagent(human, reagent_type)
/datum/unit_test/metabolization/proc/test_reagent(mob/living/carbon/C, reagent_type)
C.reagents.add_reagent(reagent_type, 10)
C.reagents.metabolize(C, SSMOBS_DT, 0, can_overdose = TRUE)
C.reagents.clear_reagents()
/datum/unit_test/metabolization/Destroy()
SSmobs.ignite()
return ..()
/datum/unit_test/on_mob_end_metabolize/Run()
SSmobs.pause()
var/mob/living/carbon/human/user = allocate(/mob/living/carbon/human/consistent)
var/obj/item/reagent_containers/pill/pill = allocate(/obj/item/reagent_containers/pill)
var/datum/reagent/drug/methamphetamine/meth = /datum/reagent/drug/methamphetamine
// Give them enough meth to be consumed in 2 metabolizations
pill.reagents.add_reagent(meth, 1.9 * initial(meth.metabolization_rate) * SSMOBS_DT)
pill.attack(user, user)
user.Life(SSMOBS_DT)
TEST_ASSERT(user.reagents.has_reagent(meth), "User does not have meth in their system after consuming it")
TEST_ASSERT(user.has_movespeed_modifier(/datum/movespeed_modifier/reagent/methamphetamine), "User consumed meth, but did not gain movespeed modifier")
user.Life(SSMOBS_DT)
TEST_ASSERT(!user.reagents.has_reagent(meth), "User still has meth in their system when it should've finished metabolizing")
TEST_ASSERT(!user.has_movespeed_modifier(/datum/movespeed_modifier/reagent/methamphetamine), "User still has movespeed modifier despite not containing any more meth")
/datum/unit_test/on_mob_end_metabolize/Destroy()
SSmobs.ignite()
return ..()
/datum/unit_test/addictions/Run()
SSmobs.pause()
var/mob/living/carbon/human/pill_user = allocate(/mob/living/carbon/human/consistent)
var/mob/living/carbon/human/syringe_user = allocate(/mob/living/carbon/human/consistent)
var/mob/living/carbon/human/pill_syringe_user = allocate(/mob/living/carbon/human/consistent)
var/datum/mind/pill_mind = new /datum/mind(null)
pill_mind.active = TRUE
pill_mind.transfer_to(pill_user)
var/datum/mind/syringe_mind = new /datum/mind(null)
syringe_mind.active = TRUE
syringe_mind.transfer_to(syringe_user)
var/datum/mind/pill_syringe_mind = new /datum/mind(null)
pill_syringe_mind.active = TRUE
pill_syringe_mind.transfer_to(pill_syringe_user)
var/obj/item/reagent_containers/pill/pill = allocate(/obj/item/reagent_containers/pill)
var/obj/item/reagent_containers/pill/pill_two = allocate(/obj/item/reagent_containers/pill)
var/obj/item/reagent_containers/syringe/syringe = allocate(/obj/item/reagent_containers/syringe)
var/datum/reagent/drug/methamphetamine/meth = allocate(/datum/reagent/drug/methamphetamine)
var/addiction_type_to_check
for(var/key in meth.addiction_types)
addiction_type_to_check = key //idk how to do this otherwise
// Let's start with stomach metabolism
pill.reagents.add_reagent(meth.type, 5)
pill.attack(pill_user, pill_user)
// Set the metabolism efficiency to 1.0 so it transfers all reagents to the body in one go.
var/obj/item/organ/internal/stomach/pill_belly = pill_user.getorganslot(ORGAN_SLOT_STOMACH)
pill_belly.metabolism_efficiency = 1
pill_user.Life()
TEST_ASSERT(pill_user.mind.addiction_points[addiction_type_to_check], "User did not gain addiction points after metabolizing meth")
// Then injected metabolism
syringe.volume = 5
syringe.amount_per_transfer_from_this = 5
syringe.reagents.add_reagent(meth.type, 5)
syringe.melee_attack_chain(syringe_user, syringe_user)
syringe_user.Life()
TEST_ASSERT(syringe_user.mind.addiction_points[addiction_type_to_check], "User did not gain addiction points after metabolizing meth")
// One half syringe
syringe.reagents.remove_all()
syringe.volume = 5
syringe.amount_per_transfer_from_this = 5
syringe.reagents.add_reagent(meth.type, (5 * 0.5) + 1)
// One half pill
pill_two.reagents.add_reagent(meth.type, (5 * 0.5) + 1)
pill_two.attack(pill_syringe_user, pill_syringe_user)
syringe.melee_attack_chain(pill_syringe_user, pill_syringe_user)
// Set the metabolism efficiency to 1.0 so it transfers all reagents to the body in one go.
pill_belly = pill_syringe_user.getorganslot(ORGAN_SLOT_STOMACH)
pill_belly.metabolism_efficiency = 1
pill_syringe_user.Life()
TEST_ASSERT(pill_syringe_user.mind.addiction_points[addiction_type_to_check], "User did not gain addiction points after metabolizing meth")
/datum/unit_test/addictions/Destroy()
SSmobs.ignite()
return ..()