From 64a6b6bb27cbc26d904c2c3e4bb9839afffc5b52 Mon Sep 17 00:00:00 2001 From: SkyratBot <59378654+SkyratBot@users.noreply.github.com> Date: Mon, 13 Feb 2023 07:18:03 +0100 Subject: [PATCH] [MIRROR] Refactors the detective scanner to fix potentially getting stuck in the 'scanning' state [MDB IGNORE] (#19309) * Refactors the detective scanner to fix potentially getting stuck in the 'scanning' state (#73058) ## About The Pull Request Detective scanners make use of a `scanning` boolean to determine whether or not you can perform another action with it. It gets set to `FALSE` in its procs, but if any of those procs runtime for whatever reason this could lead to that never happening. Which in turn leads to it getting stuck in the `scanning = TRUE` state indefinitely. --- This PR just moves the functions into wrapper functions so that should a runtime occur, the boolean will still get set to `FALSE`. Also changed the boolean's name from `scanning` to `scanner_busy` because it is also used for the print function. Fixes https://github.com/Skyrat-SS13/Skyrat-tg/issues/18241 ## Why It's Good For The Game Code resiliency ## Changelog :cl: fix: fixes a potential bug that can cause the detective scanner to get stuck in a scanning state indefinitely /:cl: * Refactors the detective scanner to fix potentially getting stuck in the 'scanning' state --------- Co-authored-by: Bloop --- code/modules/detectivework/scanner.dm | 259 ++++++++++++++------------ 1 file changed, 143 insertions(+), 116 deletions(-) diff --git a/code/modules/detectivework/scanner.dm b/code/modules/detectivework/scanner.dm index 9a873ffeec3..b455016dac8 100644 --- a/code/modules/detectivework/scanner.dm +++ b/code/modules/detectivework/scanner.dm @@ -15,7 +15,8 @@ flags_1 = CONDUCT_1 item_flags = NOBLUDGEON slot_flags = ITEM_SLOT_BELT - var/scanning = FALSE + /// if the scanner is currently busy processing + var/scanner_busy = FALSE var/list/log = list() var/range = 8 var/view_check = TRUE @@ -28,20 +29,29 @@ /datum/action/item_action/display_detective_scan_results/Trigger(trigger_flags) var/obj/item/detective_scanner/scanner = target if(istype(scanner)) - scanner.displayDetectiveScanResults(usr) + scanner.display_detective_scan_results(usr) /obj/item/detective_scanner/attack_self(mob/user) if(!LAZYLEN(log)) balloon_alert(user, "no logs!") return - if(scanning) + if(scanner_busy) balloon_alert(user, "scanner busy!") return - scanning = TRUE + scanner_busy = TRUE balloon_alert(user, "printing report...") - addtimer(CALLBACK(src, PROC_REF(PrintReport)), 10 SECONDS) + addtimer(CALLBACK(src, PROC_REF(safe_print_report)), 10 SECONDS) -/obj/item/detective_scanner/proc/PrintReport() +/** + * safe_print_repot - a wrapper proc for print_report + * + * Calls print_report(), and should a runtime occur within we can still reset the 'busy' state + */ +/obj/item/detective_scanner/proc/safe_print_report() + print_report() + scanner_busy = FALSE + +/obj/item/detective_scanner/proc/print_report() // Create our paper var/obj/item/paper/report_paper = new(get_turf(src)) @@ -63,124 +73,141 @@ // Clear the logs log = list() - scanning = FALSE /obj/item/detective_scanner/pre_attack_secondary(atom/A, mob/user, params) - scan(A, user) + safe_scan(user, atom_to_scan = A) return SECONDARY_ATTACK_CANCEL_ATTACK_CHAIN /obj/item/detective_scanner/afterattack(atom/A, mob/user, params) . = ..() - scan(A, user) + safe_scan(user, atom_to_scan = A) return . | AFTERATTACK_PROCESSED_ITEM -/obj/item/detective_scanner/proc/scan(atom/A, mob/user) +/** + * safe_scan - a wrapper proc for scan() + * + * calls scan(), and should a runtime occur within we can still reset the 'busy' state + */ +/obj/item/detective_scanner/proc/safe_scan(mob/user, atom/atom_to_scan) set waitfor = FALSE - if(!scanning) - // Can remotely scan objects and mobs. - if((get_dist(A, user) > range) || (!(A in view(range, user)) && view_check) || (loc != user)) - return - - scanning = TRUE - - user.visible_message( - span_notice("\The [user] points the [src.name] at \the [A] and performs a forensic scan."), - ignored_mobs = user - ) - to_chat(user, span_notice("You scan \the [A]. The scanner is now analysing the results...")) - - - // GATHER INFORMATION - - //Make our assoc list array - // The keys are the headers used for it, and the value is a list of each line printed - var/list/det_data = list() - var/list/blood = GET_ATOM_BLOOD_DNA(A) - det_data[DETSCAN_CATEGORY_FIBER] = GET_ATOM_FIBRES(A) - - var/target_name = A.name - - // Start gathering - - if(ishuman(A)) - - var/mob/living/carbon/human/H = A - if(!H.gloves) - LAZYADD(det_data[DETSCAN_CATEGORY_FINGERS], md5(H.dna?.unique_identity)) - - else if(!ismob(A)) - - det_data[DETSCAN_CATEGORY_FINGERS] = GET_ATOM_FINGERPRINTS(A) - - // Only get reagents from non-mobs. - for(var/datum/reagent/present_reagent as anything in A.reagents?.reagent_list) - LAZYADD(det_data[DETSCAN_CATEGORY_DRINK], \ - "Reagent: [present_reagent.name] Volume: [present_reagent.volume]") - - // Get blood data from the blood reagent. - if(!istype(present_reagent, /datum/reagent/blood)) - continue - - var/blood_DNA = present_reagent.data["blood_DNA"] - var/blood_type = present_reagent.data["blood_type"] - if(!blood_DNA || !blood_type) - continue - - LAZYSET(blood, blood_DNA, blood_type) - - if(istype(A, /obj/item/card/id)) - var/obj/item/card/id/user_id = A - for(var/region in DETSCAN_ACCESS_ORDER()) - var/access_in_region = SSid_access.accesses_by_region[region] & user_id.GetAccess() - if(!length(access_in_region)) - continue - LAZYADD(det_data[DETSCAN_CATEGORY_ACCESS], "[region]:") - var/list/access_names = list() - for(var/access_num in access_in_region) - access_names += SSid_access.get_access_desc(access_num) - LAZYADD(det_data[DETSCAN_CATEGORY_ACCESS], english_list(access_names)) - - - for(var/bloodtype in blood) - LAZYADD(det_data[DETSCAN_CATEGORY_BLOOD], \ - "Type: [blood[bloodtype]] DNA (UE): [bloodtype]") - - // sends it off to be modified by the items - SEND_SIGNAL(A, COMSIG_DETECTIVE_SCANNED, user, det_data) - - // We gathered everything. Create a fork and slowly display the results to the holder of the scanner. - - var/found_something = FALSE - add_log("[station_time_timestamp()][get_timestamp()] - [target_name]", 0) - - for(var/category in DETSCAN_DEFAULT_ORDER()) - if(!LAZYLEN(det_data[category])) - continue // no data found, move to next category - sleep(3 SECONDS) - add_log(span_info("[category]:")) - for(var/line in det_data[category]) - add_log(line) - found_something = TRUE - - // Get a new user - var/mob/holder = null - if(ismob(src.loc)) - holder = src.loc - - if(!found_something) - add_log("# No forensic traces found #", 0) // Don't display this to the holder user - if(holder) - to_chat(holder, span_warning("Unable to locate any fingerprints, materials, fibers, or blood on \the [target_name]!")) - else - if(holder) - to_chat(holder, span_notice("You finish scanning \the [target_name].")) - - add_log("---------------------------------------------------------", 0) - scanning = FALSE + if(scanner_busy) return + if(!scan(user, atom_to_scan)) // this should only return FALSE if a runtime occurs during the scan proc, so ideally never + balloon_alert(user, "scanner error!") // but in case it does, we 'error' instead of just bricking the scanner + scanner_busy = FALSE + +/** + * scan - scans an atom for forensic data and outputs it to the mob holding the scanner + * + * This should always return TRUE barring a runtime + */ +/obj/item/detective_scanner/proc/scan(mob/user, atom/scanned_atom) + // Can remotely scan objects and mobs. + if((get_dist(scanned_atom, user) > range) || (!(scanned_atom in view(range, user)) && view_check) || (loc != user)) + return TRUE + + scanner_busy = TRUE + + + user.visible_message( + span_notice("\The [user] points the [src.name] at \the [scanned_atom] and performs a forensic scan."), + ignored_mobs = user + ) + to_chat(user, span_notice("You scan \the [scanned_atom]. The scanner is now analysing the results...")) + + + // GATHER INFORMATION + + //Make our assoc list array + // The keys are the headers used for it, and the value is a list of each line printed + var/list/det_data = list() + var/list/blood = GET_ATOM_BLOOD_DNA(scanned_atom) + det_data[DETSCAN_CATEGORY_FIBER] = GET_ATOM_FIBRES(scanned_atom) + + var/target_name = scanned_atom.name + + // Start gathering + + if(ishuman(scanned_atom)) + + var/mob/living/carbon/human/scanned_human = scanned_atom + if(!scanned_human.gloves) + LAZYADD(det_data[DETSCAN_CATEGORY_FINGERS], md5(scanned_human.dna?.unique_identity)) + + else if(!ismob(scanned_atom)) + + det_data[DETSCAN_CATEGORY_FINGERS] = GET_ATOM_FINGERPRINTS(scanned_atom) + + // Only get reagents from non-mobs. + for(var/datum/reagent/present_reagent as anything in scanned_atom.reagents?.reagent_list) + LAZYADD(det_data[DETSCAN_CATEGORY_DRINK], \ + "Reagent: [present_reagent.name] Volume: [present_reagent.volume]") + + // Get blood data from the blood reagent. + if(!istype(present_reagent, /datum/reagent/blood)) + continue + + var/blood_DNA = present_reagent.data["blood_DNA"] + var/blood_type = present_reagent.data["blood_type"] + if(!blood_DNA || !blood_type) + continue + + LAZYSET(blood, blood_DNA, blood_type) + + if(istype(scanned_atom, /obj/item/card/id)) + var/obj/item/card/id/user_id = scanned_atom + for(var/region in DETSCAN_ACCESS_ORDER()) + var/access_in_region = SSid_access.accesses_by_region[region] & user_id.GetAccess() + if(!length(access_in_region)) + continue + LAZYADD(det_data[DETSCAN_CATEGORY_ACCESS], "[region]:") + var/list/access_names = list() + for(var/access_num in access_in_region) + access_names += SSid_access.get_access_desc(access_num) + LAZYADD(det_data[DETSCAN_CATEGORY_ACCESS], english_list(access_names)) + + + for(var/bloodtype in blood) + LAZYADD(det_data[DETSCAN_CATEGORY_BLOOD], \ + "Type: [blood[bloodtype]] DNA (UE): [bloodtype]") + + // sends it off to be modified by the items + SEND_SIGNAL(scanned_atom, COMSIG_DETECTIVE_SCANNED, user, det_data) + + // We gathered everything. Create a fork and slowly display the results to the holder of the scanner. + + var/found_something = FALSE + add_log("[station_time_timestamp()][get_timestamp()] - [target_name]", 0) + + for(var/category in DETSCAN_DEFAULT_ORDER()) + if(!LAZYLEN(det_data[category])) + continue // no data found, move to next category + sleep(3 SECONDS) + add_log(span_info("[category]:")) + for(var/line in det_data[category]) + add_log(line) + found_something = TRUE + + // Make sure the original user is still holding the scanner before sending them the results + var/mob/holder = null + if(src.loc == user) + holder = user + else if(ismob(src.loc)) + holder = src.loc + + if(!found_something) + add_log("# No forensic traces found #", 0) // Don't display this to the holder user + if(holder) + to_chat(holder, span_warning("Unable to locate any fingerprints, materials, fibers, or blood on \the [target_name]!")) + else + if(holder) + to_chat(holder, span_notice("You finish scanning \the [target_name].")) + + add_log("---------------------------------------------------------", 0) + return TRUE /obj/item/detective_scanner/proc/add_log(msg, broadcast = 1) - if(scanning) + if(scanner_busy) if(broadcast && ismob(loc)) var/mob/logger = loc to_chat(logger, msg) @@ -198,7 +225,7 @@ if(!LAZYLEN(log)) balloon_alert(user, "no logs!") return - if(scanning) + if(scanner_busy) balloon_alert(user, "scanner busy!") return balloon_alert(user, "deleting logs...") @@ -208,15 +235,15 @@ /obj/item/detective_scanner/examine(mob/user) . = ..() - if(LAZYLEN(log) && !scanning) + if(LAZYLEN(log) && !scanner_busy) . += span_notice("Alt-click to clear scanner logs.") -/obj/item/detective_scanner/proc/displayDetectiveScanResults(mob/living/user) +/obj/item/detective_scanner/proc/display_detective_scan_results(mob/living/user) // No need for can-use checks since the action button should do proper checks if(!LAZYLEN(log)) balloon_alert(user, "no logs!") return - if(scanning) + if(scanner_busy) balloon_alert(user, "scanner busy!") return to_chat(user, span_notice("Scanner Report"))