From 065bb04dfc604ffcae4c90cbb346b533c6f236a4 Mon Sep 17 00:00:00 2001 From: Jeremiah <42397676+jlsnow301@users.noreply.github.com> Date: Fri, 10 Nov 2023 19:41:19 -0800 Subject: [PATCH] Code cleanup for qdel (#79568) ## About The Pull Request I want to put my hands in the engine. If you don't want me touching this I am not hurt if its closed Added 2 early returns and removes single letter vars ## Why It's Good For The Game Code improvement ## Changelog N/A nothing player facing --------- Co-authored-by: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com> --- code/controllers/subsystem/garbage.dm | 131 +++++++++++++------------- 1 file changed, 68 insertions(+), 63 deletions(-) diff --git a/code/controllers/subsystem/garbage.dm b/code/controllers/subsystem/garbage.dm index 88112f93226..d21e2da97d2 100644 --- a/code/controllers/subsystem/garbage.dm +++ b/code/controllers/subsystem/garbage.dm @@ -359,75 +359,80 @@ SUBSYSTEM_DEF(garbage) /datum/qdel_item/New(mytype) name = "[mytype]" - /// Should be treated as a replacement for the 'del' keyword. /// /// Datums passed to this will be given a chance to clean up references to allow the GC to collect them. -/proc/qdel(datum/D, force=FALSE, ...) - if(!istype(D)) - del(D) +/proc/qdel(datum/to_delete, force = FALSE, ...) + if(!istype(to_delete)) + del(to_delete) return - var/datum/qdel_item/I = SSgarbage.items[D.type] - if (!I) - I = SSgarbage.items[D.type] = new /datum/qdel_item(D.type) - I.qdels++ + var/datum/qdel_item/trash = SSgarbage.items[to_delete.type] + if (isnull(trash)) + trash = SSgarbage.items[to_delete.type] = new /datum/qdel_item(to_delete.type) + trash.qdels++ - if(isnull(D.gc_destroyed)) - if (SEND_SIGNAL(D, COMSIG_PREQDELETED, force)) // Give the components a chance to prevent their parent from being deleted + if(!isnull(to_delete.gc_destroyed)) + if(to_delete.gc_destroyed == GC_CURRENTLY_BEING_QDELETED) + CRASH("[to_delete.type] destroy proc was called multiple times, likely due to a qdel loop in the Destroy logic") + return + + if (SEND_SIGNAL(to_delete, COMSIG_PREQDELETED, force)) // Give the components a chance to prevent their parent from being deleted + return + + to_delete.gc_destroyed = GC_CURRENTLY_BEING_QDELETED + var/start_time = world.time + var/start_tick = world.tick_usage + SEND_SIGNAL(to_delete, COMSIG_QDELETING, force) // Let the (remaining) components know about the result of Destroy + var/hint = to_delete.Destroy(arglist(args.Copy(2))) // Let our friend know they're about to get fucked up. + + if(world.time != start_time) + trash.slept_destroy++ + else + trash.destroy_time += TICK_USAGE_TO_MS(start_tick) + + if(isnull(to_delete)) + return + + switch(hint) + if (QDEL_HINT_QUEUE) //qdel should queue the object for deletion. + SSgarbage.Queue(to_delete) + if (QDEL_HINT_IWILLGC) + to_delete.gc_destroyed = world.time return - D.gc_destroyed = GC_CURRENTLY_BEING_QDELETED - var/start_time = world.time - var/start_tick = world.tick_usage - SEND_SIGNAL(D, COMSIG_QDELETING, force) // Let the (remaining) components know about the result of Destroy - var/hint = D.Destroy(arglist(args.Copy(2))) // Let our friend know they're about to get fucked up. - if(world.time != start_time) - I.slept_destroy++ - else - I.destroy_time += TICK_USAGE_TO_MS(start_tick) - if(!D) - return - switch(hint) - if (QDEL_HINT_QUEUE) //qdel should queue the object for deletion. - SSgarbage.Queue(D) - if (QDEL_HINT_IWILLGC) - D.gc_destroyed = world.time + if (QDEL_HINT_LETMELIVE) //qdel should let the object live after calling destory. + if(!force) + to_delete.gc_destroyed = null //clear the gc variable (important!) return - if (QDEL_HINT_LETMELIVE) //qdel should let the object live after calling destory. - if(!force) - D.gc_destroyed = null //clear the gc variable (important!) - return - // Returning LETMELIVE after being told to force destroy - // indicates the objects Destroy() does not respect force - #ifdef TESTING - if(!I.no_respect_force) - testing("WARNING: [D.type] has been force deleted, but is \ - returning an immortal QDEL_HINT, indicating it does \ - not respect the force flag for qdel(). It has been \ - placed in the queue, further instances of this type \ - will also be queued.") - #endif - I.no_respect_force++ - - SSgarbage.Queue(D) - if (QDEL_HINT_HARDDEL) //qdel should assume this object won't gc, and queue a hard delete - SSgarbage.Queue(D, GC_QUEUE_HARDDELETE) - if (QDEL_HINT_HARDDEL_NOW) //qdel should assume this object won't gc, and hard del it post haste. - SSgarbage.HardDelete(D) - #ifdef REFERENCE_TRACKING - if (QDEL_HINT_FINDREFERENCE) //qdel will, if REFERENCE_TRACKING is enabled, display all references to this object, then queue the object for deletion. - SSgarbage.Queue(D) - INVOKE_ASYNC(D, TYPE_PROC_REF(/datum, find_references)) - if (QDEL_HINT_IFFAIL_FINDREFERENCE) //qdel will, if REFERENCE_TRACKING is enabled and the object fails to collect, display all references to this object. - SSgarbage.Queue(D) - SSgarbage.reference_find_on_fail[text_ref(D)] = TRUE + // Returning LETMELIVE after being told to force destroy + // indicates the objects Destroy() does not respect force + #ifdef TESTING + if(!trash.no_respect_force) + testing("WARNING: [to_delete.type] has been force deleted, but is \ + returning an immortal QDEL_HINT, indicating it does \ + not respect the force flag for qdel(). It has been \ + placed in the queue, further instances of this type \ + will also be queued.") #endif - else - #ifdef TESTING - if(!I.no_hint) - testing("WARNING: [D.type] is not returning a qdel hint. It is being placed in the queue. Further instances of this type will also be queued.") - #endif - I.no_hint++ - SSgarbage.Queue(D) - else if(D.gc_destroyed == GC_CURRENTLY_BEING_QDELETED) - CRASH("[D.type] destroy proc was called multiple times, likely due to a qdel loop in the Destroy logic") + trash.no_respect_force++ + + SSgarbage.Queue(to_delete) + if (QDEL_HINT_HARDDEL) //qdel should assume this object won't gc, and queue a hard delete + SSgarbage.Queue(to_delete, GC_QUEUE_HARDDELETE) + if (QDEL_HINT_HARDDEL_NOW) //qdel should assume this object won't gc, and hard del it post haste. + SSgarbage.HardDelete(to_delete) + #ifdef REFERENCE_TRACKING + if (QDEL_HINT_FINDREFERENCE) //qdel will, if REFERENCE_TRACKING is enabled, display all references to this object, then queue the object for deletion. + SSgarbage.Queue(to_delete) + INVOKE_ASYNC(to_delete, TYPE_PROC_REF(/datum, find_references)) + if (QDEL_HINT_IFFAIL_FINDREFERENCE) //qdel will, if REFERENCE_TRACKING is enabled and the object fails to collect, display all references to this object. + SSgarbage.Queue(to_delete) + SSgarbage.reference_find_on_fail[text_ref(to_delete)] = TRUE + #endif + else + #ifdef TESTING + if(!trash.no_hint) + testing("WARNING: [to_delete.type] is not returning a qdel hint. It is being placed in the queue. Further instances of this type will also be queued.") + #endif + trash.no_hint++ + SSgarbage.Queue(to_delete)