From 1fbead50af8e89dbe7784710a523aecb30ca5338 Mon Sep 17 00:00:00 2001 From: VMSolidus Date: Thu, 11 Jun 2026 06:47:56 -0400 Subject: [PATCH] Mob Ref Cleanup Part 3 (#22599) Part 3 of the Mob Destroy Refactor, this time going through the entire list of every ref stored up to a mob/human, and (attempting) to verify and cleanup every possible circular ref. This PR also fixes some mistakes made with cleaning up UI elements, namely that tgui's really don't like it when you qdel them, and screen objects also weren't always clearing their own references if Qdel'ed directly. There were several niche situations too where circle refs might be retained by a mob. I also found an issue where static lights were only cleaned up on /atom/movable/ but actually existed farther up the chain on /atom. I don't know if any /atoms that aren't /atom/movable ever get static lights, but the fact that they can be needs to be correctly accounted for. I can't possibly have gotten all of them, but this is every single one I could find after 3 hours of work. --- code/ZAS/Atom.dm | 3 -- code/__HELPERS/icon_smoothing.dm | 5 --- code/_onclick/hud/screen_objects.dm | 9 +++++- .../subsystems/processing/airflow.dm | 7 ---- code/game/atom/_atom.dm | 5 +++ code/game/atoms_movable.dm | 32 ++++++++++++++----- code/modules/clothing/clothing.dm | 12 +++++++ .../spacesuits/rig/modules/modules.dm | 5 ++- code/modules/clothing/spacesuits/rig/rig.dm | 21 ++++++++---- .../lighting_static/static_lighting_atom.dm | 7 ---- code/modules/mob/mob.dm | 19 +++++++++-- code/modules/multiz/movement.dm | 4 --- code/modules/multiz/zmimic/mimic_movable.dm | 6 ---- code/modules/organs/internal/brain.dm | 3 +- .../hellfirejag-even-more-mob-harddels.yml | 11 +++++++ 15 files changed, 95 insertions(+), 54 deletions(-) create mode 100644 html/changelogs/hellfirejag-even-more-mob-harddels.yml diff --git a/code/ZAS/Atom.dm b/code/ZAS/Atom.dm index c6eff62e0a3..a6039eeb51e 100644 --- a/code/ZAS/Atom.dm +++ b/code/ZAS/Atom.dm @@ -74,6 +74,3 @@ result |= M.c_airblock(other) if(result == BLOCKED) return BLOCKED return result - -/atom/movable - var/atmos_canpass = CANPASS_ALWAYS diff --git a/code/__HELPERS/icon_smoothing.dm b/code/__HELPERS/icon_smoothing.dm index 9d001ba9f93..398014f31c3 100644 --- a/code/__HELPERS/icon_smoothing.dm +++ b/code/__HELPERS/icon_smoothing.dm @@ -52,11 +52,6 @@ /// Icon state of the overlay this object uses to attach to other objects. var/attach_overlay -/atom/movable - var/can_be_unanchored = 0 - var/obj/buckled_to - var/can_be_buckled = FALSE - /turf var/list/fixed_underlay /// Determines if we should attempt to generate turf underlays for this type. diff --git a/code/_onclick/hud/screen_objects.dm b/code/_onclick/hud/screen_objects.dm index 5e8fe70050f..6f77ed14c4c 100644 --- a/code/_onclick/hud/screen_objects.dm +++ b/code/_onclick/hud/screen_objects.dm @@ -28,7 +28,10 @@ screen_loc = null if(length(hud?.mymob?.client?.screen)) hud.mymob.client.screen -= src - hud = null + // All register signals MUST be mirrored with Unregister signals included in a Destroy proc, even if other procs also call Unregister Signal. + if (hud) + UnregisterSignal(hud, COMSIG_QDELETING) + hud = null return ..() /// Screen elements are always on top of the players screen and don't move so yes they are adjacent @@ -41,6 +44,10 @@ /atom/movable/screen/proc/handle_hud_destruction() SIGNAL_HANDLER + // If we were QDEL'ed directly (probably by our owner), then there's no need to Unregister Signal and qdel ourselves twice. + if (QDELING(src)) + return + UnregisterSignal(hud, COMSIG_QDELETING) qdel(src) diff --git a/code/controllers/subsystems/processing/airflow.dm b/code/controllers/subsystems/processing/airflow.dm index 9f8c1f82781..12b740a92ea 100644 --- a/code/controllers/subsystems/processing/airflow.dm +++ b/code/controllers/subsystems/processing/airflow.dm @@ -92,13 +92,6 @@ PROCESSING_SUBSYSTEM_DEF(airflow) #undef CLEAR_OBJECT -/atom/movable - var/tmp/airflow_xo - var/tmp/airflow_yo - var/tmp/airflow_od - var/tmp/airflow_process_delay - var/tmp/airflow_skip_speedcheck - /atom/movable/proc/prepare_airflow(n) if (!airflow_dest || airflow_speed < 0 || last_airflow > world.time - GLOB.vsc.airflow_delay) return FALSE diff --git a/code/game/atom/_atom.dm b/code/game/atom/_atom.dm index f859c948744..2ac365e804d 100644 --- a/code/game/atom/_atom.dm +++ b/code/game/atom/_atom.dm @@ -74,6 +74,10 @@ var/tmp/datum/dynamic_light_source/light ///Any light sources that are "inside" of us, for example, if src here was a mob that's carrying a flashlight, that flashlight's light source would be part of this list. var/tmp/list/hybrid_light_sources + ///The light source, datum. Dont fuck with this directly + var/tmp/datum/static_light_source/static_light + ///Static light sources currently attached to this atom, this includes ones owned by atoms inside this atom + var/tmp/list/static_light_sources //Values should avoid being close to -16, 16, -48, 48 etc. //Best keep them within 10 units of a multiple of 32, as when the light is closer to a wall, the probability @@ -152,6 +156,7 @@ overlays.Cut() QDEL_NULL(light) + QDEL_NULL(static_light) if(smoothing_flags & SMOOTH_QUEUED) SSicon_smooth.remove_from_queues(src) diff --git a/code/game/atoms_movable.dm b/code/game/atoms_movable.dm index ec9f6c8cd35..61a0c5975b7 100644 --- a/code/game/atoms_movable.dm +++ b/code/game/atoms_movable.dm @@ -92,6 +92,26 @@ var/tmp/airflow_time = 0 var/tmp/last_airflow = 0 + var/can_be_unanchored = 0 + var/obj/buckled_to + var/can_be_buckled = FALSE + + /** Used to check wether or not an atom is being handled by SSfalling. */ + var/tmp/multiz_falling = 0 + + var/tmp/airflow_xo + var/tmp/airflow_yo + var/tmp/airflow_od + var/tmp/airflow_process_delay + var/tmp/airflow_skip_speedcheck + + /// The mimic (if any) that's *directly* copying us. + var/tmp/atom/movable/openspace/mimic/bound_overlay + /// If TRUE, this atom is ignored by Z-Mimic. + var/z_flags + + var/atmos_canpass = CANPASS_ALWAYS + /atom/movable/Initialize(mapload, ...) . = ..() update_emissive_blocker() @@ -144,16 +164,12 @@ M.pulling = null pulledby = null - if (bound_overlay) - QDEL_NULL(bound_overlay) - - if(em_block) - QDEL_NULL(em_block) - - QDEL_NULL(light) - QDEL_NULL(static_light) + QDEL_NULL(bound_overlay) + QDEL_NULL(em_block) airflow_dest = null loc = null + buckled_to?.buckled = null + buckled_to = null return ..() /atom/movable/proc/moveToNullspace() diff --git a/code/modules/clothing/clothing.dm b/code/modules/clothing/clothing.dm index e55047a1a7b..e66e905753b 100644 --- a/code/modules/clothing/clothing.dm +++ b/code/modules/clothing/clothing.dm @@ -528,6 +528,14 @@ pickup_sound = 'sound/items/pickup/gloves.ogg' equip_sound = 'sound/items/equip/gloves.ogg' +/obj/item/clothing/gloves/Destroy() + QDEL_NULL(cell) + if (ring && wearer) + wearer.equip_to_slot_if_possible(ring, slot_gloves) + ring = null + wearer = null + return ..() + /obj/item/clothing/gloves/update_clothing_icon() if (ismob(src.loc)) var/mob/M = src.loc @@ -948,6 +956,10 @@ var/footstep_sound_override +/obj/item/clothing/shoes/Destroy() + QDEL_NULL(holding) + return ..() + /obj/item/clothing/shoes/proc/draw_knife() set name = "Draw Boot Knife" set desc = "Pull out your boot knife." diff --git a/code/modules/clothing/spacesuits/rig/modules/modules.dm b/code/modules/clothing/spacesuits/rig/modules/modules.dm index f635371fc80..460319ab452 100644 --- a/code/modules/clothing/spacesuits/rig/modules/modules.dm +++ b/code/modules/clothing/spacesuits/rig/modules/modules.dm @@ -166,9 +166,8 @@ /obj/item/rig_module/Destroy() - for(var/sm in stat_modules) - qdel(sm) - stat_modules.Cut() + QDEL_LIST(stat_modules) + holder?.installed_modules -= src holder = null return ..() diff --git a/code/modules/clothing/spacesuits/rig/rig.dm b/code/modules/clothing/spacesuits/rig/rig.dm index 4a70736f1ab..818d4cec3ed 100644 --- a/code/modules/clothing/spacesuits/rig/rig.dm +++ b/code/modules/clothing/spacesuits/rig/rig.dm @@ -224,13 +224,22 @@ piece.icon_supported_species_tags = icon_supported_species_tags /obj/item/rig/Destroy() - for(var/obj/item/piece in list(gloves,boots,helmet,chest)) - qdel(piece) STOP_PROCESSING(SSmobs, src) - qdel(wires) - wires = null - qdel(spark_system) - spark_system = null + QDEL_NULL(air_supply) + QDEL_NULL(boots) + QDEL_NULL(chest) + QDEL_NULL(helmet) + QDEL_NULL(gloves) + QDEL_NULL(cell) + selected_module = null + visor = null + speech = null + wearer?.wearing_rig = null + wearer = null + mob_icon = null + QDEL_LIST(installed_modules) + QDEL_NULL(wires) + QDEL_NULL(spark_system) return ..() /obj/item/rig/proc/set_vision(var/active) diff --git a/code/modules/lighting/lighting_static/static_lighting_atom.dm b/code/modules/lighting/lighting_static/static_lighting_atom.dm index 0f7be52b351..777adee179a 100644 --- a/code/modules/lighting/lighting_static/static_lighting_atom.dm +++ b/code/modules/lighting/lighting_static/static_lighting_atom.dm @@ -1,10 +1,3 @@ - -/atom - ///The light source, datum. Dont fuck with this directly - var/tmp/datum/static_light_source/static_light - ///Static light sources currently attached to this atom, this includes ones owned by atoms inside this atom - var/tmp/list/static_light_sources - ///Pretty simple, just updates static lights on this atom /atom/proc/static_update_light() set waitfor = FALSE diff --git a/code/modules/mob/mob.dm b/code/modules/mob/mob.dm index 452ccef59c9..0109952c96e 100644 --- a/code/modules/mob/mob.dm +++ b/code/modules/mob/mob.dm @@ -54,6 +54,8 @@ QDEL_NULL(ability_master) QDEL_NULL(zone_sel) + if (length(machine?.climbers)) + machine.climbers -= src machine = null client_colors = null @@ -61,6 +63,7 @@ pinned?.Cut() QDEL_LIST(embedded) languages?.Cut() + holo?.clear_holo(src) holo = null l_hand = null r_hand = null @@ -104,11 +107,23 @@ QDEL_LIST(click_handlers) vr_mob = null old_mob = null + if (lastattacker && lastattacker.lastattacked == src) + lastattacker.lastattacked = null + if (lastattacked && lastattacked.lastattacker == src) + lastattacked.lastattacker = null lastattacker = null lastattacked = null QDEL_NULL(pointing_effect) - QDEL_LIST(open_nanouis) - QDEL_LIST(tgui_open_uis) + + // Cleanup for all UIs belonging to the client. + // For performance reasons we check the length of the lists first since the lists can potentially either be null or empty. + // But it's also plausible that they might contain null values, and while their aggressive checking means any valid entry in the list + // is guaranteed to be of type /datum/tgui, null entries in the list can potentially exist and will pass the as anything typecast. + if (length(tgui_open_uis)) + for (var/datum/tgui/ui as anything in tgui_open_uis) + ui?.close() + tgui_open_uis.Cut() + QDEL_NULL(narsimage) QDEL_NULL(narglow) QDEL_NULL(riftimage) diff --git a/code/modules/multiz/movement.dm b/code/modules/multiz/movement.dm index d548bbc22e3..332ae7a5e49 100644 --- a/code/modules/multiz/movement.dm +++ b/code/modules/multiz/movement.dm @@ -1,7 +1,3 @@ -/atom/movable - /** Used to check wether or not an atom is being handled by SSfalling. */ - var/tmp/multiz_falling = 0 - /** * Verb for the mob to move up a z-level if possible. */ diff --git a/code/modules/multiz/zmimic/mimic_movable.dm b/code/modules/multiz/zmimic/mimic_movable.dm index 384e1277159..ab4a387a0dd 100644 --- a/code/modules/multiz/zmimic/mimic_movable.dm +++ b/code/modules/multiz/zmimic/mimic_movable.dm @@ -1,9 +1,3 @@ -/atom/movable - /// The mimic (if any) that's *directly* copying us. - var/tmp/atom/movable/openspace/mimic/bound_overlay - /// If TRUE, this atom is ignored by Z-Mimic. - var/z_flags - /atom/movable/set_dir(ndir) . = ..() if (. && bound_overlay) diff --git a/code/modules/organs/internal/brain.dm b/code/modules/organs/internal/brain.dm index 1b206239f17..d37772ea361 100644 --- a/code/modules/organs/internal/brain.dm +++ b/code/modules/organs/internal/brain.dm @@ -124,8 +124,7 @@ addtimer(CALLBACK(src, PROC_REF(clear_screen)), 5) /obj/item/organ/internal/brain/Destroy() - if(brainmob) - QDEL_NULL(brainmob) + QDEL_NULL(brainmob) return ..() /obj/item/organ/internal/brain/removed(var/mob/living/user) diff --git a/html/changelogs/hellfirejag-even-more-mob-harddels.yml b/html/changelogs/hellfirejag-even-more-mob-harddels.yml new file mode 100644 index 00000000000..424f62a94f3 --- /dev/null +++ b/html/changelogs/hellfirejag-even-more-mob-harddels.yml @@ -0,0 +1,11 @@ +author: Hellfirejag +delete-after: True +changes: + - bugfix: "Fixed a hard del caused by mobs not properly cleaning up UI windows." + - bugfix: "Fixed hardsuits having several harddels" + - bugfix: "Fixed screen objects not always being cleaned up correctly and hard deleting" + - bugfix: "Fixed several niche circular references on mobs which caused hard deletes." + - bugfix: "Fixed gloves hard deleting if they were worn over a ring or were upgraded to shock gloves" + - bugfix: "Fixed static lights not being cleared properly if they were attached to a non-movable atom." + - bugfix: "Fixed boots having a hard delete if a knife was stored in them." + - bugfix: "Fixed a circular reference between mobs that have attacked each other that was causing hard deletes"