From 86287eea8894b29b72edb4ff78731071a4e809d0 Mon Sep 17 00:00:00 2001 From: Timberpoes Date: Wed, 23 Sep 2020 07:22:30 +0100 Subject: [PATCH] Fix antag datum runtimes when they deleted their own owners and make other silent errors now runtime. (#53721) heretic_monster antag datums nulled their own owner before calling code that relied on them having an owner to function. This has been fixed. Antag datum subtypes should not be modifying their own owners. Leave that to the parent code. This could lead to situations where a mind has null antag datums sitting around in its list of antag_datums. This has also probably been fixed. There are various checks dotted throughout the code to check if antag datums have owners. Most would either return early, continue in a for loop or silently skip over it. Antag datums should never be without an owner. These have all been removed. When an ownerless antag datum is detected, it should not die silently. It should die loudly and proudly, declaring that something, somewhere went wrong and that there is a willing and necessary martyr to the cause of addressing WHY something runtimes instead of just having it silently skipped over. --- .../antagonists/_common/antag_datum.dm | 20 ++++++---- .../eldritch_cult/eldritch_monster_antag.dm | 6 +-- code/modules/antagonists/fugitive/hunter.dm | 1 + code/modules/antagonists/monkey/monkey.dm | 1 + code/modules/antagonists/nukeop/nukeop.dm | 1 + code/modules/antagonists/pirate/pirate.dm | 1 + .../traitor/IAA/internal_affairs.dm | 38 +++++++++++++------ code/modules/antagonists/wizard/wizard.dm | 8 ++-- 8 files changed, 51 insertions(+), 25 deletions(-) diff --git a/code/modules/antagonists/_common/antag_datum.dm b/code/modules/antagonists/_common/antag_datum.dm index f8c054b9941..d1009dfe69c 100644 --- a/code/modules/antagonists/_common/antag_datum.dm +++ b/code/modules/antagonists/_common/antag_datum.dm @@ -30,7 +30,9 @@ GLOBAL_LIST_EMPTY(antagonists) /datum/antagonist/Destroy() GLOB.antagonists -= src - if(owner) + if(!owner) + stack_trace("Destroy()ing antagonist datum when it has no owner.") + else LAZYREMOVE(owner.antag_datums, src) owner = null return ..() @@ -133,14 +135,16 @@ GLOBAL_LIST_EMPTY(antagonists) ///Called by the remove_antag_datum() and remove_all_antag_datums() mind procs for the antag datum to handle its own removal and deletion. /datum/antagonist/proc/on_removal() SHOULD_CALL_PARENT(TRUE) + if(!owner) + CRASH("Antag datum with no owner.") + remove_innate_effects() clear_antag_moodies() - if(owner) - LAZYREMOVE(owner.antag_datums, src) - if(!LAZYLEN(owner.antag_datums)) - owner.current.remove_from_current_living_antags() - if(!silent && owner.current) - farewell() + LAZYREMOVE(owner.antag_datums, src) + if(!LAZYLEN(owner.antag_datums)) + owner.current.remove_from_current_living_antags() + if(!silent && owner.current) + farewell() var/datum/team/team = get_team() if(team) team.remove_member(owner) @@ -171,7 +175,7 @@ GLOBAL_LIST_EMPTY(antagonists) var/list/report = list() if(!owner) - CRASH("antagonist datum without owner") + CRASH("Antagonist datum without owner") report += printplayer(owner) diff --git a/code/modules/antagonists/eldritch_cult/eldritch_monster_antag.dm b/code/modules/antagonists/eldritch_cult/eldritch_monster_antag.dm index 529128fc0a4..0629c70f832 100644 --- a/code/modules/antagonists/eldritch_cult/eldritch_monster_antag.dm +++ b/code/modules/antagonists/eldritch_cult/eldritch_monster_antag.dm @@ -19,9 +19,9 @@ to_chat(owner, "You became an Eldritch Horror!") /datum/antagonist/heretic_monster/on_removal() - if(owner) - to_chat(owner, "Your master is no longer [master.owner.current.real_name]") - owner = null + if(master) + to_chat(master, "Your master is no longer [master.owner.current.real_name]") + master = null return ..() /datum/antagonist/heretic_monster/proc/set_owner(datum/antagonist/_master) diff --git a/code/modules/antagonists/fugitive/hunter.dm b/code/modules/antagonists/fugitive/hunter.dm index 090b243e531..23b53dacdce 100644 --- a/code/modules/antagonists/fugitive/hunter.dm +++ b/code/modules/antagonists/fugitive/hunter.dm @@ -77,6 +77,7 @@ var/list/fugitives_captured = list() for(var/datum/antagonist/fugitive/A in GLOB.antagonists) if(!A.owner) + stack_trace("Antagonist datum without owner in GLOB.antagonists: [A]") continue fugitives_counted += A if(A.owner.current.stat == DEAD) diff --git a/code/modules/antagonists/monkey/monkey.dm b/code/modules/antagonists/monkey/monkey.dm index 1a5cc266779..519fa00ac29 100644 --- a/code/modules/antagonists/monkey/monkey.dm +++ b/code/modules/antagonists/monkey/monkey.dm @@ -51,6 +51,7 @@ if(!new_team) for(var/datum/antagonist/monkey/H in GLOB.antagonists) if(!H.owner) + stack_trace("Antagonist datum without owner in GLOB.antagonists: [H]") continue if(H.monkey_team) monkey_team = H.monkey_team diff --git a/code/modules/antagonists/nukeop/nukeop.dm b/code/modules/antagonists/nukeop/nukeop.dm index c300677614e..84a2350347c 100644 --- a/code/modules/antagonists/nukeop/nukeop.dm +++ b/code/modules/antagonists/nukeop/nukeop.dm @@ -112,6 +112,7 @@ if(!always_new_team) for(var/datum/antagonist/nukeop/N in GLOB.antagonists) if(!N.owner) + stack_trace("Antagonist datum without owner in GLOB.antagonists: [N]") continue if(N.nuke_team) nuke_team = N.nuke_team diff --git a/code/modules/antagonists/pirate/pirate.dm b/code/modules/antagonists/pirate/pirate.dm index 91bc063869a..7bfbced12b0 100644 --- a/code/modules/antagonists/pirate/pirate.dm +++ b/code/modules/antagonists/pirate/pirate.dm @@ -18,6 +18,7 @@ if(!new_team) for(var/datum/antagonist/pirate/P in GLOB.antagonists) if(!P.owner) + stack_trace("Antagonist datum without owner in GLOB.antagonists: [P]") continue if(P.crew) crew = P.crew diff --git a/code/modules/antagonists/traitor/IAA/internal_affairs.dm b/code/modules/antagonists/traitor/IAA/internal_affairs.dm index ee29a31e690..48330e6de07 100644 --- a/code/modules/antagonists/traitor/IAA/internal_affairs.dm +++ b/code/modules/antagonists/traitor/IAA/internal_affairs.dm @@ -15,29 +15,39 @@ /datum/antagonist/traitor/internal_affairs/proc/give_pinpointer() - if(owner && owner.current) + if(!owner) + CRASH("Antag datum with no owner.") + + if(owner.current) owner.current.apply_status_effect(/datum/status_effect/agent_pinpointer) /datum/antagonist/traitor/internal_affairs/apply_innate_effects() - .=..() //in case the base is used in future - if(owner && owner.current) + . = ..() + + if(!owner) + CRASH("Antag datum with no owner.") + + if(owner.current) give_pinpointer(owner.current) /datum/antagonist/traitor/internal_affairs/remove_innate_effects() - .=..() - if(owner && owner.current) + . = ..() + + if(!owner) + CRASH("Antag datum with no owner.") + + if(owner.current) owner.current.remove_status_effect(/datum/status_effect/agent_pinpointer) /datum/antagonist/traitor/internal_affairs/on_gain() START_PROCESSING(SSprocessing, src) - .=..() + . = ..() /datum/antagonist/traitor/internal_affairs/on_removal() STOP_PROCESSING(SSprocessing,src) - .=..() + . = ..() /datum/antagonist/traitor/internal_affairs/process() iaa_process() - /datum/status_effect/agent_pinpointer id = "agent_pinpointer" duration = -1 @@ -101,7 +111,9 @@ return (istype(O, /datum/objective/assassinate/internal)||istype(O, /datum/objective/destroy/internal)) /datum/antagonist/traitor/proc/replace_escape_objective() - if(!owner || !objectives.len) + if(!owner) + CRASH("Antag datum with no owner.") + if(!objectives.len) return for (var/objective_ in objectives) if(!(istype(objective_, /datum/objective/escape)||istype(objective_, /datum/objective/survive))) @@ -113,7 +125,9 @@ add_objective(martyr_objective) /datum/antagonist/traitor/proc/reinstate_escape_objective() - if(!owner||!objectives.len) + if(!owner) + CRASH("Antag datum with no owner.") + if(!objectives.len) return for (var/objective_ in objectives) if(!istype(objective_, /datum/objective/martyr)) @@ -174,7 +188,9 @@ replace_escape_objective(owner) /datum/antagonist/traitor/internal_affairs/proc/iaa_process() - if(owner&&owner.current&&owner.current.stat!=DEAD) + if(!owner) + CRASH("Antag datum with no owner.") + if(owner.current && owner.current.stat != DEAD) for(var/objective_ in objectives) if(!is_internal_objective(objective_)) continue diff --git a/code/modules/antagonists/wizard/wizard.dm b/code/modules/antagonists/wizard/wizard.dm index 1773fab4723..ce92e0f799b 100644 --- a/code/modules/antagonists/wizard/wizard.dm +++ b/code/modules/antagonists/wizard/wizard.dm @@ -54,7 +54,9 @@ add_antag_hud(antag_hud_type, antag_hud_name, owner.current) /datum/antagonist/wizard/proc/send_to_lair() - if(!owner || !owner.current) + if(!owner) + CRASH("Antag datum with no owner.") + if(!owner.current) return if(!GLOB.wizardstart.len) SSjob.SendToLateJoin(owner.current) @@ -114,7 +116,7 @@ /datum/antagonist/wizard/proc/equip_wizard() if(!owner) - return + CRASH("Antag datum with no owner.") var/mob/living/carbon/human/H = owner.current if(!istype(H)) return @@ -191,7 +193,7 @@ /datum/antagonist/wizard/apprentice/equip_wizard() . = ..() if(!owner) - return + CRASH("Antag datum with no owner.") var/mob/living/carbon/human/H = owner.current if(!istype(H)) return