From 31e993bd4514a9d65be6c94249db9c3d5fb7ec18 Mon Sep 17 00:00:00 2001 From: SkyratBot <59378654+SkyratBot@users.noreply.github.com> Date: Thu, 7 Dec 2023 05:01:01 +0100 Subject: [PATCH] [MIRROR] Makes mutations clean up after themselves [MDB IGNORE] (#25481) * Makes mutations clean up after themselves (#80107) ## About The Pull Request They weren't doing this before. This was an issue because they hold refs to a `mob/living/carbon/owner` as well as a `datum/dna`, making it a potential source of hard dels. As far as I can tell, I do not see this causing any issues as `remove_mutation(mutation)` takes a type path as an arg rather than a reference, same thing with `get_mutation(mutation)`. I couldn't find any examples of a reference to a mutation being reused by anything after being removed. --- When I was investigating potential reasons for why it might have been like this I found more problem code. Timed dna injectors were setting `target` to the return value of `add_mutation()`/`remove_mutation()` which is a bool. This made no sense and would cause runtimes as well as mislead people into thinking that the return value of those procs was a `mob/living/carbon`. ## Why It's Good For The Game Fixes an oversight, and headache further down the line. ## Changelog :cl: fix: fixed mutations holding onto refs after removal fix: fixes timed dna injectors /:cl: * Makes mutations clean up after themselves --------- Co-authored-by: Bloop <13398309+vinylspiders@users.noreply.github.com> --- code/datums/dna.dm | 1 + code/datums/mutations/_mutations.dm | 12 +++++++----- code/datums/mutations/body.dm | 2 +- code/game/objects/items/dna_injector.dm | 4 ++-- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/code/datums/dna.dm b/code/datums/dna.dm index d27779387ed..9f16047fad6 100644 --- a/code/datums/dna.dm +++ b/code/datums/dna.dm @@ -114,6 +114,7 @@ GLOBAL_LIST_INIT(total_uf_len_by_block, populate_total_uf_len_by_block()) /datum/dna/Destroy() if(iscarbon(holder)) var/mob/living/carbon/cholder = holder + remove_all_mutations() // mutations hold a reference to the dna if(cholder.dna == src) cholder.dna = null holder = null diff --git a/code/datums/mutations/_mutations.dm b/code/datums/mutations/_mutations.dm index 176fb9d8388..bb9b37fd03e 100644 --- a/code/datums/mutations/_mutations.dm +++ b/code/datums/mutations/_mutations.dm @@ -86,6 +86,12 @@ copy_mutation(copymut) update_valid_chromosome_list() +/datum/mutation/human/Destroy() + power_path = null + dna = null + owner = null + return ..() + /datum/mutation/human/proc/on_acquiring(mob/living/carbon/human/acquirer) if(!acquirer || !istype(acquirer) || acquirer.stat == DEAD || (src in acquirer.dna.mutations)) return TRUE @@ -141,11 +147,7 @@ mut_overlay.Remove(get_visual_indicator()) owner.overlays_standing[layer_used] = mut_overlay owner.apply_overlay(layer_used) - if(power_path) - // Any powers we made are linked to our mutation datum, - // so deleting ourself will also delete it and remove it - // ...Why don't all mutations delete on loss? Not sure. - qdel(src) + qdel(src) /mob/living/carbon/proc/update_mutations_overlay() return diff --git a/code/datums/mutations/body.dm b/code/datums/mutations/body.dm index 24aa41321f3..0fdf2820d2f 100644 --- a/code/datums/mutations/body.dm +++ b/code/datums/mutations/body.dm @@ -220,7 +220,7 @@ . = owner.monkeyize() /datum/mutation/human/race/on_losing(mob/living/carbon/human/owner) - if(owner && owner.stat != DEAD && (owner.dna.mutations.Remove(src)) && ismonkey(owner)) + if(!QDELETED(owner) && owner.stat != DEAD && (owner.dna.mutations.Remove(src)) && ismonkey(owner)) owner.fully_replace_character_name(null, original_name) . = owner.humanize(original_species) diff --git a/code/game/objects/items/dna_injector.dm b/code/game/objects/items/dna_injector.dm index ca7af78ecba..0dc20c6dbb4 100644 --- a/code/game/objects/items/dna_injector.dm +++ b/code/game/objects/items/dna_injector.dm @@ -112,7 +112,7 @@ if(mutation == /datum/mutation/human/race) if(!ismonkey(target)) continue - target = target.dna.remove_mutation(mutation) + target.dna.remove_mutation(mutation) else target.dna.remove_mutation(mutation) for(var/mutation in add_mutations) @@ -120,7 +120,7 @@ continue //Skip permanent mutations we already have. if(mutation == /datum/mutation/human/race && !ismonkey(target)) message_admins("[ADMIN_LOOKUPFLW(user)] injected [key_name_admin(target)] with the [name] [span_danger("(MONKEY)")]") - target = target.dna.add_mutation(mutation, MUT_OTHER, endtime) + target.dna.add_mutation(mutation, MUT_OTHER, endtime) else target.dna.add_mutation(mutation, MUT_OTHER, endtime) if(fields)