From 0cd30f127fe145f0dd66f7d5a9ece0f455fd5441 Mon Sep 17 00:00:00 2001 From: MichiRecRoom <1008889+LikeLakers2@users.noreply.github.com> Date: Mon, 3 Mar 2025 09:57:01 -0500 Subject: [PATCH] Nukes now delete themselves, rather than having SSticker do it through a callback (#89752) ## About The Pull Request The title pretty much says it all. I'm told that this was done because, previously, the station would be exploded by the nuclear device - but when that was removed, all that remained was `qdel(bomb)`. That said, there's some bizarre stuff behind what pushed me to make this PR. See below if you want to know.
Bizarre stuff For some reason, while trying to port over to Monkestation, I started getting hard-deletes for the nuclear bomb used in the `nuke_cinematic` unit test. ``` ## REF SEARCH Beginning search for references to a /obj/machinery/nuclearbomb/syndicate. ## REF SEARCH Refcount for /obj/machinery/nuclearbomb/syndicate: 7 ## REF SEARCH Finished searching globals ## REF SEARCH Finished searching native globals ## REF SEARCH Finished searching atoms ## REF SEARCH Found /obj/machinery/nuclearbomb/syndicate [0x201f850] in list Datums -> /datum/controller/subsystem/garbage [0x2100001c] -> queues (list) -> /list (list) -> /list (list). ## REF SEARCH Found /obj/machinery/nuclearbomb/syndicate [0x201f850] in list Datums -> /datum/callback [0x21057da8] (obj: Ticker proc: station_explosion_detonation args: ["the nuclear fission explosive"] user: null) -> arguments (list). ## REF SEARCH Finished searching datums ## REF SEARCH Finished searching clients ## REF SEARCH Completed search for references to a /obj/machinery/nuclearbomb/syndicate. ## TESTING: GC: -- [0x201f850] | /obj/machinery/nuclearbomb/syndicate was unable to be GC'd -- (ref count of 3) Error: /obj/machinery/nuclearbomb/syndicate hard deleted 1 times out of a total del count of 3 FAILURE #1: /obj/machinery/nuclearbomb/syndicate hard deleted 1 times out of a total del count of 3 at code/modules/unit_tests/create_and_destroy.dm:99 Error: FAIL /datum/unit_test/create_and_destroy 308.1s ``` This hard-del would happen on every integration test, no matter the map and no matter the byond version. I was even able to have it happen locally. Thing is, Monkestation only seems to have two differences in our code for actually exploding a nuke: 1. We mark some z-levels as safe from the nuclear bomb (so you can go to the lowest reaches of icebox and be safe from the bomb). This only modifies the list of z-levels to call the "gib everyone on z" code for. 2. We don't have tgstation/tgstation#75967 ported over - but making a new branch and cherry-picking both 77868 and 75967 didn't seem to fix the issue either. So something with how the callback is handled by the `play_cutscene` global proc is preventing this from being qdel'd properly - resulting in a hard-delete over five minutes after the nuke cinematic plays. I tried several things to understand why the callback was causing a hard-del, but only one gave me any tangible info - changing the reference to the bomb, into a weakref reference, which stopped the hard-del. However, I eventually realized the callback was entirely pointless - right now, all it does is qdel the bomb. So I just moved it from a callback to part of the `really_actually_explode()` proc.
## Why It's Good For The Game I'd say it fixes a hard-delete, but that hard-delete was only happening on Monkestation. So I guess it removes a bit of unnecessary indirection? ## Changelog :cl:MichiRecRoom fix: Nukes will now always be deleted upon exploding, no matter if they have a valid cutscene to play. /:cl: --- code/controllers/subsystem/ticker.dm | 4 ---- .../nukeop/equipment/nuclear_bomb/_nuclear_bomb.dm | 3 ++- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/code/controllers/subsystem/ticker.dm b/code/controllers/subsystem/ticker.dm index 1f05fdb2fca..1f0728dd846 100644 --- a/code/controllers/subsystem/ticker.dm +++ b/code/controllers/subsystem/ticker.dm @@ -373,10 +373,6 @@ SUBSYSTEM_DEF(ticker) else LAZYADD(round_end_events, cb) -/datum/controller/subsystem/ticker/proc/station_explosion_detonation(atom/bomb) - if(bomb) //BOOM - qdel(bomb) - /datum/controller/subsystem/ticker/proc/create_characters() for(var/i in GLOB.new_player_list) var/mob/dead/new_player/player = i diff --git a/code/modules/antagonists/nukeop/equipment/nuclear_bomb/_nuclear_bomb.dm b/code/modules/antagonists/nukeop/equipment/nuclear_bomb/_nuclear_bomb.dm index d7f78f6e7d7..91578e74737 100644 --- a/code/modules/antagonists/nukeop/equipment/nuclear_bomb/_nuclear_bomb.dm +++ b/code/modules/antagonists/nukeop/equipment/nuclear_bomb/_nuclear_bomb.dm @@ -598,7 +598,7 @@ GLOBAL_VAR(station_nuke_source) /obj/machinery/nuclearbomb/proc/really_actually_explode(detonation_status) var/cinematic = get_cinematic_type(detonation_status) if(!isnull(cinematic)) - play_cinematic(cinematic, world, CALLBACK(SSticker, TYPE_PROC_REF(/datum/controller/subsystem/ticker, station_explosion_detonation), src)) + play_cinematic(cinematic, world) var/drop_level = TRUE switch(detonation_status) @@ -629,6 +629,7 @@ GLOBAL_VAR(station_nuke_source) if(drop_level) SSsecurity_level.set_level(SEC_LEVEL_RED) + qdel(src) return TRUE /// Cause nuke effects to the passed z-levels.