From a08f698230ed6c5b5753f19c02f5c196d8007b37 Mon Sep 17 00:00:00 2001 From: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com> Date: Mon, 20 Sep 2021 09:17:09 -0700 Subject: [PATCH] Harddeletes: Accident edition (#61562) So I started with fixing some timing stuff in #61540, decided to look into foam harddels since I've known about them for a while, got bored, tried to figure out the plane master failures I've seen, and well uh, did. I'm sure there'll be more, but for now: Fixes foam darts sticking around post qdel due to dumb guncode, adds a stack trace to handle it. Makes map popups actually clear the screen they're effecting of their objects, preventing plane master harddels, most commonly sourced from admin pod memes. Not clearing from the screen WAS SOMEHOW AN INTENDED FEATURE!?!@ because "clients log out often anyway and that clears screen so it's fine" I am having a meltdown Changes examine timers from a ref + signal to a ref(), the timer's 1 second, it's not gonna cause any collisions for the love of christ --- code/_onclick/hud/map_popups.dm | 13 +++---- code/game/machinery/computer/camera.dm | 4 +-- code/game/objects/items/devices/spyglasses.dm | 6 ++-- code/modules/client/preferences.dm | 1 - code/modules/mob/mob.dm | 36 +++++++++---------- .../file_system/programs/secureye.dm | 4 +-- .../ammunition/caseless/_caseless.dm | 12 ++++--- code/modules/projectiles/guns/ballistic.dm | 15 ++++---- 8 files changed, 45 insertions(+), 46 deletions(-) diff --git a/code/_onclick/hud/map_popups.dm b/code/_onclick/hud/map_popups.dm index f711e4af1cc..991e252b146 100644 --- a/code/_onclick/hud/map_popups.dm +++ b/code/_onclick/hud/map_popups.dm @@ -53,23 +53,18 @@ screen_maps[screen_obj.assigned_map] = list() // NOTE: Possibly an expensive operation var/list/screen_map = screen_maps[screen_obj.assigned_map] - if(!screen_map.Find(screen_obj)) - screen_map += screen_obj - if(!screen.Find(screen_obj)) - screen += screen_obj + screen_map |= screen_obj + screen |= screen_obj /** * Clears the map of registered screen objects. - * - * Not really needed most of the time, as the client's screen list gets reset - * on relog. any of the buttons are going to get caught by garbage collection - * anyway. they're effectively qdel'd. */ /client/proc/clear_map(map_name) - if(!map_name || !(map_name in screen_maps)) + if(!map_name || !screen_maps[map_name]) return FALSE for(var/atom/movable/screen/screen_obj in screen_maps[map_name]) screen_maps[map_name] -= screen_obj + screen -= screen_obj if(screen_obj.del_on_map_removal) qdel(screen_obj) screen_maps -= map_name diff --git a/code/game/machinery/computer/camera.dm b/code/game/machinery/computer/camera.dm index f60cb7b2df7..614b6382538 100644 --- a/code/game/machinery/computer/camera.dm +++ b/code/game/machinery/computer/camera.dm @@ -51,9 +51,9 @@ cam_background.del_on_map_removal = FALSE /obj/machinery/computer/security/Destroy() - qdel(cam_screen) + QDEL_NULL(cam_screen) QDEL_LIST(cam_plane_masters) - qdel(cam_background) + QDEL_NULL(cam_background) return ..() /obj/machinery/computer/security/connect_to_shuttle(obj/docking_port/mobile/port, obj/docking_port/stationary/dock) diff --git a/code/game/objects/items/devices/spyglasses.dm b/code/game/objects/items/devices/spyglasses.dm index 6145f7a2091..a1e22d5ffef 100644 --- a/code/game/objects/items/devices/spyglasses.dm +++ b/code/game/objects/items/devices/spyglasses.dm @@ -11,7 +11,7 @@ return if(!linked_bug) user.audible_message(span_warning("[src] lets off a shrill beep!")) - if("spypopup_map" in user.client.screen_maps) //alright, the popup this object uses is already IN use, so the window is open. no point in doing any other work here, so we're good. + if(user.client.screen_maps["spypopup_map"]) //alright, the popup this object uses is already IN use, so the window is open. no point in doing any other work here, so we're good. return user.client.setup_popup("spypopup", 3, 3, 2) user.client.register_map_obj(linked_bug.cam_screen) @@ -78,9 +78,9 @@ /obj/item/clothing/accessory/spy_bug/Destroy() if(linked_glasses) linked_glasses.linked_bug = null - qdel(cam_screen) + QDEL_NULL(cam_screen) QDEL_LIST(cam_plane_masters) - qdel(tracker) + QDEL_NULL(tracker) . = ..() /obj/item/clothing/accessory/spy_bug/proc/update_view()//this doesn't do anything too crazy, just updates the vis_contents of its screen obj diff --git a/code/modules/client/preferences.dm b/code/modules/client/preferences.dm index 592c614e4c3..81f2bc8bfbe 100644 --- a/code/modules/client/preferences.dm +++ b/code/modules/client/preferences.dm @@ -376,7 +376,6 @@ INITIALIZE_IMMEDIATE(/atom/movable/screen/character_preview_view) qdel(plane_master) client?.clear_map(assigned_map) - client?.screen -= src preferences?.character_preview_view = null diff --git a/code/modules/mob/mob.dm b/code/modules/mob/mob.dm index 501c4735ee1..d15f912df9b 100644 --- a/code/modules/mob/mob.dm +++ b/code/modules/mob/mob.dm @@ -445,34 +445,36 @@ * [this byond forum post](https://secure.byond.com/forum/?post=1326139&page=2#comment8198716) * for why this isn't atom/verb/examine() */ -/mob/verb/examinate(atom/A as mob|obj|turf in view()) //It used to be oview(12), but I can't really say why +/mob/verb/examinate(atom/examinify as mob|obj|turf in view()) //It used to be oview(12), but I can't really say why set name = "Examine" set category = "IC" - if(isturf(A) && !(sight & SEE_TURFS) && !(A in view(client ? client.view : world.view, src))) + if(isturf(examinify) && !(sight & SEE_TURFS) && !(examinify in view(client ? client.view : world.view, src))) // shift-click catcher may issue examinate() calls for out-of-sight turfs return - if(is_blind() && !blind_examine_check(A)) //blind people see things differently (through touch) + if(is_blind() && !blind_examine_check(examinify)) //blind people see things differently (through touch) return - face_atom(A) + face_atom(examinify) var/list/result if(client) LAZYINITLIST(client.recent_examines) - if(isnull(client.recent_examines[A]) || client.recent_examines[A] < world.time) - result = A.examine(src) - client.recent_examines[A] = world.time + EXAMINE_MORE_TIME // set the value to when the examine cooldown ends - RegisterSignal(A, COMSIG_PARENT_QDELETING, .proc/clear_from_recent_examines, override=TRUE) // to flush the value if deleted early - addtimer(CALLBACK(src, .proc/clear_from_recent_examines, A), EXAMINE_MORE_TIME) - handle_eye_contact(A) + var/ref_to_atom = ref(examinify) + var/examine_time = client.recent_examines[ref_to_atom] + + if(isnull(examine_time) || examine_time < world.time) + result = examinify.examine(src) + client.recent_examines[ref_to_atom] = world.time + EXAMINE_MORE_TIME // set the value to when the examine cooldown ends + addtimer(CALLBACK(src, .proc/clear_from_recent_examines, ref_to_atom), EXAMINE_MORE_TIME) + handle_eye_contact(examinify) else - result = A.examine_more(src) + result = examinify.examine_more(src) else - result = A.examine(src) // if a tree is examined but no client is there to see it, did the tree ever really exist? + result = examinify.examine(src) // if a tree is examined but no client is there to see it, did the tree ever really exist? to_chat(src, "[result.Join("\n")]") - SEND_SIGNAL(src, COMSIG_MOB_EXAMINATE, A) + SEND_SIGNAL(src, COMSIG_MOB_EXAMINATE, examinify) /mob/proc/blind_examine_check(atom/examined_thing) @@ -508,7 +510,7 @@ /// how long it takes for the blind person to find the thing they're examining var/examine_delay_length = rand(1 SECONDS, 2 SECONDS) - if(client?.recent_examines && client?.recent_examines[examined_thing]) //easier to find things we just touched + if(client?.recent_examines && client?.recent_examines[ref(examined_thing)]) //easier to find things we just touched examine_delay_length = 0.33 SECONDS else if(isobj(examined_thing)) examine_delay_length *= 1.5 @@ -528,13 +530,11 @@ return TRUE -/mob/proc/clear_from_recent_examines(atom/A) +/mob/proc/clear_from_recent_examines(ref_to_clear) SIGNAL_HANDLER - if(!client) return - UnregisterSignal(A, COMSIG_PARENT_QDELETING) - LAZYREMOVE(client.recent_examines, A) + LAZYREMOVE(client.recent_examines, ref_to_clear) /** * handle_eye_contact() is called when we examine() something. If we examine an alive mob with a mind who has examined us in the last second within 5 tiles, we make eye contact! diff --git a/code/modules/modular_computers/file_system/programs/secureye.dm b/code/modules/modular_computers/file_system/programs/secureye.dm index d2d7590cb18..0ece187e5a4 100644 --- a/code/modules/modular_computers/file_system/programs/secureye.dm +++ b/code/modules/modular_computers/file_system/programs/secureye.dm @@ -54,9 +54,9 @@ cam_background.del_on_map_removal = FALSE /datum/computer_file/program/secureye/Destroy() - qdel(cam_screen) + QDEL_NULL(cam_screen) QDEL_LIST(cam_plane_masters) - qdel(cam_background) + QDEL_NULL(cam_background) return ..() /datum/computer_file/program/secureye/ui_interact(mob/user, datum/tgui/ui) diff --git a/code/modules/projectiles/ammunition/caseless/_caseless.dm b/code/modules/projectiles/ammunition/caseless/_caseless.dm index 6718e88a872..ef813060b13 100644 --- a/code/modules/projectiles/ammunition/caseless/_caseless.dm +++ b/code/modules/projectiles/ammunition/caseless/_caseless.dm @@ -4,12 +4,14 @@ heavy_metal = FALSE /obj/item/ammo_casing/caseless/fire_casing(atom/target, mob/living/user, params, distro, quiet, zone_override, spread, atom/fired_from) - if (..()) //successfully firing - moveToNullspace() - QDEL_NULL(src) - return TRUE - else + if (!..()) //failed firing return FALSE + if(istype(fired_from, /obj/item/gun)) + var/obj/item/gun/shot_from = fired_from + if(shot_from.chambered == src) + shot_from.chambered = null //Nuke it. Nuke it now. + qdel(src) + return TRUE /obj/item/ammo_casing/caseless/update_icon_state() . = ..() diff --git a/code/modules/projectiles/guns/ballistic.dm b/code/modules/projectiles/guns/ballistic.dm index a6fa9700178..fd5b969c31d 100644 --- a/code/modules/projectiles/guns/ballistic.dm +++ b/code/modules/projectiles/guns/ballistic.dm @@ -213,12 +213,15 @@ /obj/item/gun/ballistic/handle_chamber(empty_chamber = TRUE, from_firing = TRUE, chamber_next_round = TRUE) if(!semi_auto && from_firing) return - var/obj/item/ammo_casing/AC = chambered //Find chambered round - if(istype(AC)) //there's a chambered round - if(casing_ejector || !from_firing) - AC.forceMove(drop_location()) //Eject casing onto ground. - AC.bounce_away(TRUE) - SEND_SIGNAL(AC, COMSIG_CASING_EJECTED) + var/obj/item/ammo_casing/casing = chambered //Find chambered round + if(istype(casing)) //there's a chambered round + if(QDELING(casing)) + stack_trace("Trying to move a qdeleted casing of type [casing.type]!") + chambered = null + else if(casing_ejector || !from_firing) + casing.forceMove(drop_location()) //Eject casing onto ground. + casing.bounce_away(TRUE) + SEND_SIGNAL(casing, COMSIG_CASING_EJECTED) chambered = null else if(empty_chamber) chambered = null