From ddf9fffdd46e8ed9a4914dbf801ccc0d2bb1ed5b Mon Sep 17 00:00:00 2001 From: MrMelbert <51863163+MrMelbert@users.noreply.github.com> Date: Thu, 25 Jan 2024 02:38:50 -0600 Subject: [PATCH] Fixes a hard delete with AI laws, adds a stack trace for law datums associating with mobs when already associated (#81065) ## About The Pull Request - Adds a stack trace for law datums associating with mobs when already associated. - From what I can tell, any situation in which a law datum is attempting to associate with another mob when it already has an owner assigned is an error. So this might help track down similar bugs in the future. - Fixes a hard delete involving AI laws. - When a malf AI occupies an APC, it creates an APC copy of the AI. This is an entirely separate mob. - In creating this copy, it uses the malf AI's current laws for the new copy's laws. Exactly. The same datum. - So the copy attempts to associate with the datum, and nothing happens, but it still assigns it to the copy's laws var. - When the AI attempts to return to their core from the APC, it deletes the copy. Which deletes the law datum, which our main Ai still has a reference too, so hard delete. Womp womp. - I fixed this by implementing a new proc, `copy_lawset`. The APC copy now gets a copy of the laws when the shunt happens. - This is imperfect, as they are two separate law datums now, it means any law changes to the first will not directly affect the second... I think. The whole "parent" AI system should cover this in theory? But I don't know if it actually does? It's quite confusing. - We should really codify methods of linking lawsets together rather than the hackiness that cyborgs do currently. ## Changelog :cl: Melbert fix: Fixed AIs who shunt to APCs causing their laws to be deleted. /:cl: --- code/datums/ai_laws/ai_laws.dm | 20 +++++++++++++++++--- code/modules/power/apc/apc_malf.dm | 2 +- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/code/datums/ai_laws/ai_laws.dm b/code/datums/ai_laws/ai_laws.dm index 9b9f9c62841..971dc118f3a 100644 --- a/code/datums/ai_laws/ai_laws.dm +++ b/code/datums/ai_laws/ai_laws.dm @@ -139,7 +139,7 @@ GLOBAL_VAR(round_default_lawset) /// These laws will go away when an AI is reset var/list/hacked = list() -/datum/ai_laws/Destroy(force = FALSE, ...) +/datum/ai_laws/Destroy(force = FALSE) if(!QDELETED(owner)) //Stopgap to help with laws randomly being lost. This stack_trace will hopefully help find the real issues. if(force) //Unless we're forced... stack_trace("AI law datum for [owner] has been forcefully destroyed incorrectly; the owner variable should be cleared first!") @@ -149,6 +149,18 @@ GLOBAL_VAR(round_default_lawset) owner = null return ..() +/// Makes a copy of the lawset and returns a new law datum. +/datum/ai_laws/proc/copy_lawset() + var/datum/ai_laws/new_lawset = new type() + new_lawset.protected_zeroth = protected_zeroth + new_lawset.zeroth = zeroth + new_lawset.zeroth_borg = zeroth_borg + new_lawset.inherent = inherent.Copy() + new_lawset.supplied = supplied.Copy() + new_lawset.ion = ion.Copy() + new_lawset.hacked = hacked.Copy() + return new_lawset + /datum/ai_laws/pai name = "pAI Directives" zeroth = ("Serve your master.") @@ -429,8 +441,10 @@ GLOBAL_VAR(round_default_lawset) to_chat(to_who, examine_block(jointext(printable_laws, "\n"))) /datum/ai_laws/proc/associate(mob/living/silicon/M) - if(!owner) - owner = M + if(owner) + CRASH("AI law datum linked to [owner] attempted to associate with another mob [M]") + + owner = M /** * Generates a list of all laws on this datum, including rendered HTML tags if required diff --git a/code/modules/power/apc/apc_malf.dm b/code/modules/power/apc/apc_malf.dm index 3b070368804..62134de146e 100644 --- a/code/modules/power/apc/apc_malf.dm +++ b/code/modules/power/apc/apc_malf.dm @@ -37,7 +37,7 @@ if(!is_station_level(z)) return malf.ShutOffDoomsdayDevice() - occupier = new /mob/living/silicon/ai(src, malf.laws, malf) //DEAR GOD WHY? //IKR???? + occupier = new /mob/living/silicon/ai(src, malf.laws.copy_lawset(), malf) //DEAR GOD WHY? //IKR???? occupier.adjustOxyLoss(malf.getOxyLoss()) if(!findtext(occupier.name, "APC Copy")) occupier.name = "[malf.name] APC Copy"