diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 64778b8b6bc..4d58a67f013 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -688,6 +688,10 @@ This is good: Setting `is_red` in args is simple, and directly names the variable the argument sets. +### Don't create code that hangs references + +This is part of the larger issue of hard deletes, read this file for more info: [Guide to Harddels](HARDDEL_GUIDE.md)) + ### Other Notes * Code should be modular where possible; if you are working on a new addition, then strongly consider putting it in its own file unless it makes sense to put it with similar ones (i.e. a new tool would go in the "tools.dm" file) diff --git a/.github/HARDDEL_GUIDE.md b/.github/HARDDEL_GUIDE.md new file mode 100644 index 00000000000..4790150ea0d --- /dev/null +++ b/.github/HARDDEL_GUIDE.md @@ -0,0 +1,265 @@ +# Hard Deletes +1. [What is hard deletion](#What-is-hard-deletion) +2. [Causes of hard deletes](#causes-of-hard-deletes) +3. [Detecting hard deletes](#detecting-hard-deletes) +4. [Techniques for fixing hard deletes](#techniques-for-fixing-hard-deletes) +5. [Help my code is erroring how fix](#help-my-code-is-erroring-how-fix) + +## What is Hard Deletion +Hard deletion is a very expensive operation that basically clears all references to some "thing" from memory. Objects that undergo this process are referred to as hard deletes, or simply harddels + +What follows is a discussion of the theory behind this, why we would ever do it, and the what we do to avoid doing it as often as possible + +I'm gonna be using words like references and garbage collection, but don't worry, it's not complex, just a bit hard to pierce + +### Why do we need to Hard Delete? + +Ok so let's say you're some guy called Jerry, and you're writing a programming language + +You want your coders to be able to pass around objects without doing a full copy. So you'll store the pack of data somewhere in memory + +```dm +/someobject + var/id = 42 + var/name = "some shit" +``` + +Then you want them to be able to pass that object into say a proc, without doing a full copy. So you let them pass in the object's location in memory instead +This is called passing something by reference + +```dm +someshit(someobject) //This isn't making a copy of someobject, it's passing in a reference to it +``` + +This of course means they can store that location in memory in another object's vars, or in a list, or whatever + +```dm +/datum + var/reference + +/proc/someshit(mem_location) + var/datum/some_obj = new() + some_obj.reference = mem_location +``` + +But what happens when you get rid of the object we're passing around references to? If we just cleared it out from memory, everything that holds a reference to it would suddenly be pointing to nowhere, or worse, something totally different! + +So then, you've gotta do something to clean up these references when you want to delete an object + +We could hold a list of references to everything that references us, but god, that'd get really expensive wouldn't it + +Why not keep count of how many times we're referenced then? If an object's ref count is ever 0, nothing whatsoever cares about it, so we can freely get rid of it + +But if something's holding onto a reference to us, we're not gonna have any idea where or what it is + +So I guess you should scan all of memory for that reference? + +```dm +del(someobject) //We now need to scan memory until we find the thing holding a ref to us, and clear it +``` + +This pattern is about how BYOND handles this problem of hanging references, or Garbage Collection + +It's not a broken system, but as you can imagine scanning all of memory gets expensive fast + +What can we do to help that? + +### How we can avoid hard deletes + +If hard deletion is so slow, we're gonna need to clean up all our references ourselves + +In our codebase we do this with `/datum/proc/Destroy()`, a proc called by `qdel()`, whose purpose I will explain later + +This procs only job is cleaning up references to the object it's called on. Nothing more, nothing else. Don't let me catch you giving it side effects + +There's a long long list of things this does, since we use it a TON. So I can't really give you a short description. It will always move the object to nullspace though + +## Causes Of Hard Deletes + +Now that you know the theory, let's go over what can actually cause hard deletes. Some of this is obvious, some of it's much less so. + +The BYOND reference has a list [Here](https://secure.byond.com/docs/ref/#/DM/garbage), but it's not a complete one + +* Stored in a var +* An item in a list, or associated with a list item +* Has a tag +* Is on the map (always true for turfs) +* Inside another atom's contents +* Inside an atom's vis_contents +* A temporary value in a still-running proc +* Is a mob with a key +* Is an image object attached to an atom + +Let's briefly go over the more painful ones yeah? + +### Sleeping procs + +Any proc that calls `sleep()`, `spawn()`, or anything that creates a seperate "thread" (not technically a thread, but it's the same in these terms. Not gonna cause any race conditions tho) will hang references to any var inside it. This includes the usr it started from, the src it was called on, and any vars created as a part of processing + +### Static vars + +`/static` and `/global` vars count for this too, they'll hang references just as well as anything. Be wary of this, these suckers can be a pain to solve + +### Range() and View() like procs + +Some internal BYOND procs will hold references to objects passed into them for a time after the proc is finished doing work, because they cache the returned info to make some code faster. You should never run into this issue, since we wait for what should be long enough to avoid this issue as a part of garbage collection + +This is what `qdel()` does by the by, it literally just means queue deletion. A reference to the object gets put into a queue, and if it still exists after 5 minutes or so, we hard delete it + +### Walk() procs + +Calling `walk()` on something will put it in an internal queue, which it'll remain in until `walk(thing, 0)` is called on it, which removes it from the queue + +This sort is very cheap to harddel, since BYOND prioritizes checking this queue first when it's clearing refs, but it should be avoided since it causes false positives + +You can read more about how BYOND prioritizes these things [Here](https://www.patreon.com/posts/diving-for-35855766) + +## Detecting Hard Deletes + +For very simple hard deletes, simple inspection should be enough to find them. Look at what the object does during `Initialize()`, and see if it's doing anything it doesn't undo later. +If that fails, search the object's typepath, and look and see if anything is holding a reference to it without regard for the object deleting + +BYOND currently doesn't have the capability to give us information about where a hard delete is. Fortunately we can search for most all of then ourselves. +The procs to perform this search are hidden behind compile time defines, since they'd be way too risky to expose to admin button pressing + +If you're having issues solving a harddel and want to perform this check yourself, go to `_compile_options.dm` and uncomment `TESTING`, `REFERENCE_TRACKING`, and `GC_FAILURE_HARD_LOOKUP` + +You can read more about what each of these do in that file, but the long and short of it is if something would hard delete our code will search for the reference (This will look like your game crashing, just hold out) and print information about anything it finds to the runtime log, which you can find inside the round folder inside `/data/logs/year/month/day` + +It'll tell you what object is holding the ref if it's in an object, or what pattern of list transversal was required to find the ref if it's hiding in a list of some sort + +## Techniques For Fixing Hard Deletes + +Once you've found the issue, it becomes a matter of making sure the ref is cleared as a part of Destroy(). I'm gonna walk you through a few patterns and discuss how you might go about fixing them + +### Our Tools + +First and simplest we have `Destroy()`. Use this to clean up after yourself for simple cases + +```dm +/someobject/Initialize() + . = ..() + GLOB.somethings += src //We add ourselves to some global list + +/someobject/Destroy() + GLOB.somethings -= src //So when we Destroy() clean yourself from the list + return ..() +``` + +Next, and slightly more complex, pairs of objects that reference each other + +This is helpful when for cases where both objects "own" each other + +```dm +/someobject + var/someotherobject/buddy + +/someotherobject + var/someobject/friend + +/someobject/Initialize() + if(!buddy) + buddy = new() + buddy.friend = src + +/someotherobject/Initialize() + if(!friend) + friend = new() + friend.buddy = src + +/someobject/Destroy() + if(buddy) + buddy.friend = null //Make sure to clear their ref to you + buddy = null //We clear our ref to them to make sure nothing goes wrong + +/someotherobject/Destroy() + if(friend) + friend.buddy = null //Make sure to clear their ref to you + friend = null //We clear our ref to them to make sure nothing goes wrong +``` + +Something similar can be accomplished with `QDELETED()`, a define that checks to see if something has started being `Destroy()`'d yet, and `QDEL_NULL()`, a define that `qdel()`'s a var and then sets it to null + +Now let's discuss something a bit more complex, weakrefs + +You'll need a bit of context, so let's do that now + +BYOND has an internal bit of behavior that looks like this + +`var/string = "\ref[someobject]"` + +This essentially gets that object's position in memory directly. Unlike normal references, this doesn't count for hard deletes. You can retrieve the object in question by using `locate()` + +`var/someobject/someobj = locate(string)` + +This has some flaws however, since the bit of memory we're pointing to might change, which would cause issues. Fortunately we've developed a datum to handle worrying about this for you, `/datum/weakref` + +You can create one using the `WEAKREF()` proc, and use weakref.resolve() to retrieve the actual object + +This should be used for things that your object doesn't "own", but still cares about + +For instance, a paper bin would own the paper inside it, but the paper inside it would just hold a weakref to the bin + +There's no need to clean these up, just make sure you account for it being null, since it'll return that if the object doesn't exist or has been queued for deletion + +```dm +/someobject + var/datum/weakref/our_coin + +/someobject/proc/set_coin(/obj/item/coin/new_coin) + our_coin = WEAKREF(new_coin) + +/someobject/proc/get_value() + if(!our_coin) + return 0 + + var/obj/item/coin/potential_coin = our_coin.resolve() + if(!potential_coin) + our_coin = null //Remember to clear the weakref if we get nothing + return 0 + return potential_coin.value +``` + +Now, for the worst case scenario + +Let's say you've got a var that's used too often to be weakref'd without making the code too expensive + +You can't hold a paired reference to it because it's not like it would ever care about you outside of just clearing the ref + +So then, we want to temporarily remember to clear a reference when it's deleted + +This is where I might lose you, but we're gonna use signals + +`qdel()`, the proc that sets off this whole deletion business, sends a signal called `COMSIG_PARENT_QDELETING` + +We can listen for that signal, and if we hear it clear whatever reference we may have + +Here's an example + +```dm +/somemob + var/mob/target + +/somemob/proc/set_target(new_target) + if(target) + UnregisterSignal(target, COMSIG_PARENT_QDELETING) //We need to make sure any old signals are cleared + target = new_target + if(target) + RegisterSignal(target, COMSIG_PARENT_QDELETING, .proc/clear_target) //Call clear_target if target is ever qdel()'d + +/somemob/proc/clear_target(datum/source) + SIGNAL_HANDLER + set_target(null) +``` + +This really should be your last resort, since signals have some limitations. If some subtype of somemob also registered for parent_qdeleting on the same target you'd get a runtime, since signals don't support it + +But if you can't do anything else for reasons of conversion ease, or hot code, this will work + +## Help My Code Is Erroring How Fix + +First, do a quick check. + +Are you doing anything to the object in `Initialize()` that you don't undo in `Destroy()`? I don't mean like, setting its name, but are you adding it to any lists, stuff like that + +If this fails, you're just gonna have to read over this doc. You can skip the theory if you'd like, but it's all pretty important for having an understanding of this problem diff --git a/.github/gbp.toml b/.github/gbp.toml index ec09082c152..2a7a0cba695 100644 --- a/.github/gbp.toml +++ b/.github/gbp.toml @@ -9,6 +9,7 @@ reset_label = "GBP: Reset" "Feedback" = 2 "Fix" = 3 "Grammar and Formatting" = 1 +"Hard Deletes" = 12 "Logging" = 1 "Performance" = 12 "Priority: CRITICAL" = 20 diff --git a/code/__DEFINES/traits.dm b/code/__DEFINES/traits.dm index 035809f9bf4..a176ddf6fac 100644 --- a/code/__DEFINES/traits.dm +++ b/code/__DEFINES/traits.dm @@ -582,7 +582,7 @@ Remember to update _globalvars/traits.dm if you're adding/removing/renaming trai #define HIGHLANDER_TRAIT "highlander" ///generic atom traits -/// Trait from [/datum/component/rust]. Its rusty and should be applying a special overlay to denote this. +/// Trait from [/datum/element/rust]. Its rusty and should be applying a special overlay to denote this. #define TRAIT_RUSTY "rust_trait" #define DO_NOT_SPLASH "do_not_splash" diff --git a/code/_onclick/hud/credits.dm b/code/_onclick/hud/credits.dm index 7ff10f73541..a21293345f0 100644 --- a/code/_onclick/hud/credits.dm +++ b/code/_onclick/hud/credits.dm @@ -55,14 +55,15 @@ animate(src, alpha = 255, time = CREDIT_EASE_DURATION, flags = ANIMATION_PARALLEL) addtimer(CALLBACK(src, .proc/FadeOut), CREDIT_ROLL_SPEED - CREDIT_EASE_DURATION) QDEL_IN(src, CREDIT_ROLL_SPEED) - P.screen += src + if(parent) + parent.screen += src /atom/movable/screen/credit/Destroy() - var/client/P = parent - P.screen -= src icon = null - LAZYREMOVE(P.credits, src) - parent = null + if(parent) + parent.screen -= src + LAZYREMOVE(parent.credits, src) + parent = null return ..() /atom/movable/screen/credit/proc/FadeOut() diff --git a/code/_onclick/hud/plane_master_controller.dm b/code/_onclick/hud/plane_master_controller.dm index 39fdcc103b2..67fbb485475 100644 --- a/code/_onclick/hud/plane_master_controller.dm +++ b/code/_onclick/hud/plane_master_controller.dm @@ -6,8 +6,11 @@ var/datum/hud/owner_hud ///Ensures that all the planes are correctly in the controlled_planes list. -/atom/movable/plane_master_controller/New(hud) +/atom/movable/plane_master_controller/New(datum/hud/hud) . = ..() + if(!istype(hud)) + return + owner_hud = hud var/assoc_controlled_planes = list() for(var/i in controlled_planes) diff --git a/code/_onclick/hud/robot.dm b/code/_onclick/hud/robot.dm index dd6de6ea511..e3b981f3ced 100644 --- a/code/_onclick/hud/robot.dm +++ b/code/_onclick/hud/robot.dm @@ -287,8 +287,9 @@ return ..() /atom/movable/screen/robot/lamp/Destroy() - robot.lampButton = null - robot = null + if(robot) + robot.lampButton = null + robot = null return ..() /atom/movable/screen/robot/modPC @@ -303,8 +304,9 @@ robot.modularInterface?.interact(robot) /atom/movable/screen/robot/modPC/Destroy() - robot.interfaceButton = null - robot = null + if(robot) + robot.interfaceButton = null + robot = null return ..() /atom/movable/screen/robot/alerts diff --git a/code/_onclick/hud/screen_objects.dm b/code/_onclick/hud/screen_objects.dm index 4a8933167ad..46db4ce4cff 100644 --- a/code/_onclick/hud/screen_objects.dm +++ b/code/_onclick/hud/screen_objects.dm @@ -675,6 +675,8 @@ /atom/movable/screen/splash/New(client/C, visible, use_previous_title) //TODO: Make this use INITIALIZE_IMMEDIATE, except its not easy . = ..() + if(!istype(C)) + return holder = C diff --git a/code/controllers/subsystem/garbage.dm b/code/controllers/subsystem/garbage.dm index 432f3fb2bf0..f53b40bc96f 100644 --- a/code/controllers/subsystem/garbage.dm +++ b/code/controllers/subsystem/garbage.dm @@ -49,6 +49,10 @@ SUBSYSTEM_DEF(garbage) var/list/queues #ifdef REFERENCE_TRACKING var/list/reference_find_on_fail = list() + #ifdef REFERENCE_TRACKING_DEBUG + //Should we save found refs. Used for unit testing + var/should_save_refs = FALSE + #endif #endif diff --git a/code/datums/brain_damage/imaginary_friend.dm b/code/datums/brain_damage/imaginary_friend.dm index ae61315098e..2e244cecac2 100644 --- a/code/datums/brain_damage/imaginary_friend.dm +++ b/code/datums/brain_damage/imaginary_friend.dm @@ -89,6 +89,9 @@ to_chat(src, span_notice("You cannot directly influence the world around you, but you can see what [owner] cannot.")) /mob/camera/imaginary_friend/Initialize(mapload, _trauma) + if(!_trauma) + stack_trace("Imaginary friend created without trauma, wtf") + return INITIALIZE_HINT_QDEL . = ..() trauma = _trauma @@ -131,7 +134,7 @@ client.images |= current_image /mob/camera/imaginary_friend/Destroy() - if(owner.client) + if(owner?.client) owner.client.images.Remove(human_image) if(client) client.images.Remove(human_image) diff --git a/code/datums/components/areabound.dm b/code/datums/components/areabound.dm index b5c3655cbce..90b2e3007ee 100644 --- a/code/datums/components/areabound.dm +++ b/code/datums/components/areabound.dm @@ -3,6 +3,7 @@ var/area/bound_area var/turf/reset_turf var/datum/movement_detector/move_tracker + var/moving = FALSE //Used to prevent infinite recursion if your reset turf places you somewhere on enter or something /datum/component/areabound/Initialize() if(!ismovable(parent)) @@ -18,7 +19,12 @@ if(!reset_turf || reset_turf.loc != bound_area) stack_trace("Invalid areabound configuration") //qdel(src) return + if(moving) + stack_trace("Moved during a reset move, giving up to prevent infinite recursion. Turf: [reset_turf.type] at [reset_turf.x], [reset_turf.y], [reset_turf.z]") + return + moving = TRUE AM.forceMove(reset_turf) + moving = FALSE /datum/component/areabound/Destroy(force, silent) QDEL_NULL(move_tracker) diff --git a/code/datums/components/udder.dm b/code/datums/components/udder.dm index d265cf33288..83fd2a99426 100644 --- a/code/datums/components/udder.dm +++ b/code/datums/components/udder.dm @@ -127,6 +127,8 @@ name = "nutrient sac" /obj/item/udder/gutlunch/initial_conditions() + if(!udder_mob) + return if(udder_mob.gender == FEMALE) START_PROCESSING(SSobj, src) RegisterSignal(udder_mob, COMSIG_HOSTILE_PRE_ATTACKINGTARGET, .proc/on_mob_attacking) diff --git a/code/datums/datum.dm b/code/datums/datum.dm index 8e1fad41c2f..481a62b058f 100644 --- a/code/datums/datum.dm +++ b/code/datums/datum.dm @@ -103,8 +103,13 @@ continue qdel(timer) - //BEGIN: ECS SHIT + #ifdef REFERENCE_TRACKING + #ifdef REFERENCE_TRACKING_DEBUG + found_refs = null + #endif + #endif + //BEGIN: ECS SHIT var/list/dc = datum_components if(dc) var/all_components = dc[/datum/component] diff --git a/code/datums/elements/connect_loc.dm b/code/datums/elements/connect_loc.dm index 6837fd9a0b0..4de95339c6e 100644 --- a/code/datums/elements/connect_loc.dm +++ b/code/datums/elements/connect_loc.dm @@ -116,7 +116,20 @@ /datum/element/connect_loc_behalf/proc/on_moved(atom/movable/tracked, atom/old_loc) SIGNAL_HANDLER - var/datum/listener = targets[old_loc][tracked] + var/list/objects_in_old_loc = targets[old_loc] + //You may ask yourself, isn't this just silencing an error? + //The answer is yes, but there's no good cheap way to fix it + //What happens is the tracked object or hell the listener gets say, deleted, which makes targets[old_loc] return a null + //The null results in a bad index, because of course it does + //It's not a solvable problem though, since both actions, the destroy and the move, are sourced from the same signal send + //And sending a signal should be agnostic of the order of listeners + //So we need to either pick the order agnositic, or destroy safe + //And I picked destroy safe. Let's hope this is the right path! + if(!objects_in_old_loc) + return + var/datum/listener = objects_in_old_loc[tracked] + if(!listener) //See above + return unregister_signals(listener, tracked, old_loc) update_signals(listener, tracked) diff --git a/code/datums/mergers/_merger.dm b/code/datums/mergers/_merger.dm index d59470b9313..b7529ba9a24 100644 --- a/code/datums/mergers/_merger.dm +++ b/code/datums/mergers/_merger.dm @@ -84,10 +84,11 @@ /datum/merger/proc/Refresh() var/list/tips = list() - tips[origin] = NORTH|EAST|SOUTH|WEST var/list/checked_turfs = list() var/list/new_members = list() - new_members[origin] = NONE + if(origin) + tips[origin] = NORTH|EAST|SOUTH|WEST + new_members[origin] = NONE while(length(tips)) var/atom/focus = tips[length(tips)] var/dirs_to_check = tips[focus] diff --git a/code/game/machinery/computer/tram_controls.dm b/code/game/machinery/computer/tram_controls.dm index 535cda95cd1..d36ec789538 100644 --- a/code/game/machinery/computer/tram_controls.dm +++ b/code/game/machinery/computer/tram_controls.dm @@ -7,9 +7,11 @@ flags_1 = NODECONSTRUCT_1 resistance_flags = INDESTRUCTIBLE | LAVA_PROOF | FIRE_PROOF | UNACIDABLE | ACID_PROOF - var/obj/structure/industrial_lift/tram/tram_part light_color = LIGHT_COLOR_GREEN + ///Weakref to the tram piece we control + var/datum/weakref/tram_ref + /obj/machinery/computer/tram_controls/Initialize(mapload, obj/item/circuitboard/C) . = ..() AddComponent(/datum/component/usb_port, list(/obj/item/circuit_component/tram_controls)) @@ -26,12 +28,14 @@ * Locates tram parts in the lift global list after everything is done. */ /obj/machinery/computer/tram_controls/proc/find_tram() - tram_part = GLOB.central_tram //possibly setting to something null, that's fine + tram_ref = WEAKREF(GLOB.central_tram) /obj/machinery/computer/tram_controls/ui_state(mob/user) return GLOB.not_incapacitated_state /obj/machinery/computer/tram_controls/ui_status(mob/user,/datum/tgui/ui) + var/obj/structure/industrial_lift/tram/central/tram_part = tram_ref?.resolve() + if(tram_part?.travelling) return UI_CLOSE if(!in_range(user, src) && !isobserver(user)) @@ -45,6 +49,7 @@ ui.open() /obj/machinery/computer/tram_controls/ui_data(mob/user) + var/obj/structure/industrial_lift/tram/central/tram_part = tram_ref?.resolve() var/list/data = list() data["moving"] = tram_part?.travelling data["broken"] = tram_part ? FALSE : TRUE @@ -94,9 +99,12 @@ /// Attempts to sends the tram to the given destination /obj/machinery/computer/tram_controls/proc/try_send_tram(obj/effect/landmark/tram/to_where) + var/obj/structure/industrial_lift/tram/central/tram_part = tram_ref?.resolve() + if(!tram_part) + return FALSE if(tram_part.travelling) return FALSE - if(tram_part.controls_locked || tram_part.travelling) // someone else started + if(tram_part.controls_locked) // someone else started return FALSE tram_part.tram_travel(to_where) return TRUE @@ -131,12 +139,14 @@ . = ..() if (istype(parent, /obj/machinery/computer/tram_controls)) computer = parent - RegisterSignal(computer.tram_part, COMSIG_TRAM_SET_TRAVELLING, .proc/on_tram_set_travelling) - RegisterSignal(computer.tram_part, COMSIG_TRAM_TRAVEL, .proc/on_tram_travel) + var/obj/structure/industrial_lift/tram/central/tram_part = computer.tram_ref?.resolve() + RegisterSignal(tram_part, COMSIG_TRAM_SET_TRAVELLING, .proc/on_tram_set_travelling) + RegisterSignal(tram_part, COMSIG_TRAM_TRAVEL, .proc/on_tram_travel) /obj/item/circuit_component/tram_controls/unregister_usb_parent(atom/movable/parent) + var/obj/structure/industrial_lift/tram/central/tram_part = computer.tram_ref?.resolve() computer = null - UnregisterSignal(computer.tram_part, list(COMSIG_TRAM_SET_TRAVELLING, COMSIG_TRAM_TRAVEL)) + UnregisterSignal(tram_part, list(COMSIG_TRAM_SET_TRAVELLING, COMSIG_TRAM_TRAVEL)) return ..() /obj/item/circuit_component/tram_controls/input_received(datum/port/input/port) diff --git a/code/game/machinery/cryopod.dm b/code/game/machinery/cryopod.dm index bf34e6cf19d..88782964488 100644 --- a/code/game/machinery/cryopod.dm +++ b/code/game/machinery/cryopod.dm @@ -35,7 +35,7 @@ GLOBAL_LIST_EMPTY(cryopod_computers) /obj/machinery/computer/cryopod/Destroy() GLOB.cryopod_computers -= src - ..() + return ..() /obj/machinery/computer/cryopod/update_icon_state() if(machine_stat & (NOPOWER|BROKEN)) diff --git a/code/game/machinery/launch_pad.dm b/code/game/machinery/launch_pad.dm index 798681a46c7..f7d29f2fafb 100644 --- a/code/game/machinery/launch_pad.dm +++ b/code/game/machinery/launch_pad.dm @@ -229,7 +229,9 @@ briefcase = _briefcase /obj/machinery/launchpad/briefcase/Destroy() - QDEL_NULL(briefcase) + if(!QDELETED(briefcase)) + qdel(briefcase) + briefcase = null return ..() /obj/machinery/launchpad/briefcase/isAvailable() @@ -269,7 +271,8 @@ /obj/item/storage/briefcase/launchpad/Destroy() if(!QDELETED(pad)) - QDEL_NULL(pad) + qdel(pad) + pad = null return ..() /obj/item/storage/briefcase/launchpad/PopulateContents() diff --git a/code/game/machinery/navbeacon.dm b/code/game/machinery/navbeacon.dm index 60131dd1032..2e416b9515b 100644 --- a/code/game/machinery/navbeacon.dm +++ b/code/game/machinery/navbeacon.dm @@ -79,6 +79,8 @@ /obj/machinery/navbeacon/proc/glob_lists_register(init=FALSE) if(!init) glob_lists_deregister() + if(!codes) + return if(codes["patrol"]) if(!GLOB.navbeacons["[z]"]) GLOB.navbeacons["[z]"] = list() diff --git a/code/game/objects/effects/decals/crayon.dm b/code/game/objects/effects/decals/crayon.dm index da5366350cd..acb7e3daff5 100644 --- a/code/game/objects/effects/decals/crayon.dm +++ b/code/game/objects/effects/decals/crayon.dm @@ -46,4 +46,4 @@ GLOBAL_LIST(gang_tags) /obj/effect/decal/cleanable/crayon/gang/Destroy() LAZYREMOVE(GLOB.gang_tags, src) - ..() + return ..() diff --git a/code/game/objects/effects/effect_system/effect_shield.dm b/code/game/objects/effects/effect_system/effect_shield.dm index 4344fbb2607..00f943aa138 100644 --- a/code/game/objects/effects/effect_system/effect_shield.dm +++ b/code/game/objects/effects/effect_system/effect_shield.dm @@ -16,7 +16,7 @@ /obj/effect/shield/Destroy() var/turf/location = get_turf(src) location.heat_capacity=old_heat_capacity - ..() + return ..() /obj/effect/shield/singularity_act() return diff --git a/code/game/objects/items/devices/chameleonproj.dm b/code/game/objects/items/devices/chameleonproj.dm index a4752f251d4..4d0222abd7c 100644 --- a/code/game/objects/items/devices/chameleonproj.dm +++ b/code/game/objects/items/devices/chameleonproj.dm @@ -173,5 +173,7 @@ return /obj/effect/dummy/chameleon/Destroy() - master.disrupt(0) + if(master) + master.disrupt(0) + master = null return ..() diff --git a/code/game/objects/items/devices/forcefieldprojector.dm b/code/game/objects/items/devices/forcefieldprojector.dm index 3749898e26a..14fab5e0d5f 100644 --- a/code/game/objects/items/devices/forcefieldprojector.dm +++ b/code/game/objects/items/devices/forcefieldprojector.dm @@ -113,8 +113,9 @@ /obj/structure/projected_forcefield/Destroy() visible_message(span_warning("[src] flickers and disappears!")) playsound(src,'sound/weapons/resonator_blast.ogg',25,TRUE) - generator.current_fields -= src - generator = null + if(generator) + generator.current_fields -= src + generator = null return ..() /obj/structure/projected_forcefield/play_attack_sound(damage_amount, damage_type = BRUTE, damage_flag = 0) @@ -123,4 +124,5 @@ /obj/structure/projected_forcefield/take_damage(damage_amount, damage_type = BRUTE, damage_flag = 0, sound_effect = 1, attack_dir) if(sound_effect) play_attack_sound(damage_amount, damage_type, damage_flag) - generator.shield_integrity = max(generator.shield_integrity - damage_amount, 0) + if(generator) + generator.shield_integrity = max(generator.shield_integrity - damage_amount, 0) diff --git a/code/game/objects/items/food/bread.dm b/code/game/objects/items/food/bread.dm index e5d79bce8dc..6fb231e7ce2 100644 --- a/code/game/objects/items/food/bread.dm +++ b/code/game/objects/items/food/bread.dm @@ -263,6 +263,9 @@ /obj/item/food/deepfryholder/Initialize(mapload, obj/item/fried) + if(!fried) + stack_trace("A deepfried object was created with no fried target") + return INITIALIZE_HINT_QDEL . = ..() name = fried.name //We'll determine the other stuff when it's actually removed appearance = fried.appearance diff --git a/code/game/objects/items/grenades/clusterbuster.dm b/code/game/objects/items/grenades/clusterbuster.dm index 3b1b52cb0ca..1d84f885f01 100644 --- a/code/game/objects/items/grenades/clusterbuster.dm +++ b/code/game/objects/items/grenades/clusterbuster.dm @@ -70,7 +70,8 @@ ///////////////////////////////// /obj/effect/payload_spawner/Initialize(mapload, type, numspawned) ..() - spawn_payload(type, numspawned) + if(type && isnum(numspawned)) + spawn_payload(type, numspawned) return INITIALIZE_HINT_QDEL /obj/effect/payload_spawner/proc/spawn_payload(type, numspawned) diff --git a/code/game/objects/items/robot/robot_upgrades.dm b/code/game/objects/items/robot/robot_upgrades.dm index e9da2c71e21..3d16b62d14c 100644 --- a/code/game/objects/items/robot/robot_upgrades.dm +++ b/code/game/objects/items/robot/robot_upgrades.dm @@ -505,10 +505,11 @@ /obj/item/borg/upgrade/defib/backpack/proc/on_defib_instance_qdel_or_moved(obj/item/defibrillator/D) SIGNAL_HANDLER defib_instance = null - qdel(src) + if(!QDELETED(src)) + qdel(src) /obj/item/borg/upgrade/defib/backpack/Destroy() - if(defib_instance) + if(!QDELETED(defib_instance)) QDEL_NULL(defib_instance) return ..() diff --git a/code/game/objects/items/weaponry.dm b/code/game/objects/items/weaponry.dm index 8bb3d02e02c..41acb9be44a 100644 --- a/code/game/objects/items/weaponry.dm +++ b/code/game/objects/items/weaponry.dm @@ -237,9 +237,9 @@ for further reading, please see: https://github.com/tgstation/tgstation/pull/301 /obj/item/claymore/highlander/robot/Initialize() var/obj/item/robot_model/kiltkit = loc robot = kiltkit.loc + . = ..() if(!istype(robot)) - qdel(src) - return ..() + return INITIALIZE_HINT_QDEL /obj/item/claymore/highlander/robot/process() loc.layer = LARGE_MOB_LAYER diff --git a/code/game/objects/structures/construction_console/construction_console.dm b/code/game/objects/structures/construction_console/construction_console.dm index 10c3cd3f43e..a3bd5b03c4f 100644 --- a/code/game/objects/structures/construction_console/construction_console.dm +++ b/code/game/objects/structures/construction_console/construction_console.dm @@ -112,6 +112,9 @@ var/obj/machinery/computer/camera_advanced/base_construction/linked_console /mob/camera/ai_eye/remote/base_construction/Initialize(mapload, obj/machinery/computer/camera_advanced/console_link) + if(!linked_console) + stack_trace("A base consturuction drone was created with no linked console") + return INITIALIZE_HINT_QDEL linked_console = console_link return ..() diff --git a/code/game/objects/structures/crates_lockers/closets/secure/freezer.dm b/code/game/objects/structures/crates_lockers/closets/secure/freezer.dm index 586368d1b32..75ec9129877 100644 --- a/code/game/objects/structures/crates_lockers/closets/secure/freezer.dm +++ b/code/game/objects/structures/crates_lockers/closets/secure/freezer.dm @@ -5,7 +5,7 @@ /obj/structure/closet/secure_closet/freezer/Destroy() recursive_organ_check(src) - ..() + return ..() /obj/structure/closet/secure_closet/freezer/Initialize() . = ..() diff --git a/code/game/objects/structures/deployable_turret.dm b/code/game/objects/structures/deployable_turret.dm index 72a35b7e9e6..941a31503aa 100644 --- a/code/game/objects/structures/deployable_turret.dm +++ b/code/game/objects/structures/deployable_turret.dm @@ -40,7 +40,7 @@ /obj/machinery/deployable_turret/Destroy() target = null target_turf = null - ..() + return ..() /// Undeploying, for when you want to move your big dakka around /obj/machinery/deployable_turret/wrench_act(mob/living/user, obj/item/wrench/used_wrench) @@ -232,7 +232,7 @@ /obj/item/gun_control/Destroy() turret = null - ..() + return ..() /obj/item/gun_control/CanItemAutoclick() return TRUE diff --git a/code/game/objects/structures/industrial_lift.dm b/code/game/objects/structures/industrial_lift.dm index 56326212d02..9eab9ddab0b 100644 --- a/code/game/objects/structures/industrial_lift.dm +++ b/code/game/objects/structures/industrial_lift.dm @@ -458,6 +458,10 @@ GLOBAL_DATUM(central_tram, /obj/structure/industrial_lift/tram/central) SStramprocess.can_fire = TRUE GLOB.central_tram = src +/obj/structure/industrial_lift/tram/central/Destroy() + GLOB.central_tram = null + return ..() + /obj/structure/industrial_lift/tram/LateInitialize() . = ..() find_our_location() diff --git a/code/game/objects/structures/traps.dm b/code/game/objects/structures/traps.dm index 21678d18ad5..39a6d35ccac 100644 --- a/code/game/objects/structures/traps.dm +++ b/code/game/objects/structures/traps.dm @@ -130,6 +130,9 @@ /obj/structure/trap/stun/hunter/flare() ..() + var/turf/our_turf = get_turf(src) + if(!our_turf) + return stored_item.forceMove(get_turf(src)) forceMove(stored_item) if(caught) diff --git a/code/game/turfs/open/openspace.dm b/code/game/turfs/open/openspace.dm index 9c94438f6b2..8af72c7e4bc 100644 --- a/code/game/turfs/open/openspace.dm +++ b/code/game/turfs/open/openspace.dm @@ -161,6 +161,9 @@ GLOBAL_DATUM_INIT(openspace_backdrop_one_for_all, /atom/movable/openspace_backdr /turf/open/openspace/icemoon/Initialize() . = ..() var/turf/T = below() + //I wonder if I should error here + if(!T) + return if(T.turf_flags & NO_RUINS && protect_ruin) ChangeTurf(replacement_turf, null, CHANGETURF_IGNORE_AIR) return diff --git a/code/modules/admin/view_variables/reference_tracking.dm b/code/modules/admin/view_variables/reference_tracking.dm index f8ee92fb67e..40809719392 100644 --- a/code/modules/admin/view_variables/reference_tracking.dm +++ b/code/modules/admin/view_variables/reference_tracking.dm @@ -55,7 +55,7 @@ /datum/proc/DoSearchVar(potential_container, container_name, recursive_limit = 64, search_time = world.time) #ifdef REFERENCE_TRACKING_DEBUG - if(!found_refs) + if(!found_refs && SSgarbage.should_save_refs) found_refs = list() #endif @@ -89,7 +89,8 @@ if(variable == src) #ifdef REFERENCE_TRACKING_DEBUG - found_refs[varname] = TRUE + if(SSgarbage.should_save_refs) + found_refs[varname] = TRUE #endif log_reftracker("Found [type] \ref[src] in [datum_container.type]'s \ref[datum_container] [varname] var. [container_name]") continue @@ -107,7 +108,8 @@ //Check normal entrys if(element_in_list == src) #ifdef REFERENCE_TRACKING_DEBUG - found_refs[potential_cache] = TRUE + if(SSgarbage.should_save_refs) + found_refs[potential_cache] = TRUE #endif log_reftracker("Found [type] \ref[src] in list [container_name].") continue @@ -118,7 +120,8 @@ //Check assoc entrys if(assoc_val == src) #ifdef REFERENCE_TRACKING_DEBUG - found_refs[potential_cache] = TRUE + if(SSgarbage.should_save_refs) + found_refs[potential_cache] = TRUE #endif log_reftracker("Found [type] \ref[src] in list [container_name]\[[element_in_list]\]") continue diff --git a/code/modules/antagonists/blob/structures/_blob.dm b/code/modules/antagonists/blob/structures/_blob.dm index c1ff9a9ac13..6038aa50458 100644 --- a/code/modules/antagonists/blob/structures/_blob.dm +++ b/code/modules/antagonists/blob/structures/_blob.dm @@ -46,7 +46,8 @@ if(atmosblock) air_update_turf(TRUE, TRUE) ConsumeTile() - AddElement(/datum/element/swabable, CELL_LINE_TABLE_BLOB, CELL_VIRUS_TABLE_GENERIC, 2, 2) + if(!QDELETED(src)) //Consuming our tile can in rare cases cause us to del + AddElement(/datum/element/swabable, CELL_LINE_TABLE_BLOB, CELL_VIRUS_TABLE_GENERIC, 2, 2) /obj/structure/blob/proc/creation_action() //When it's created by the overmind, do this. return diff --git a/code/modules/antagonists/cult/blood_magic.dm b/code/modules/antagonists/cult/blood_magic.dm index cf261af1319..683103ba9bf 100644 --- a/code/modules/antagonists/cult/blood_magic.dm +++ b/code/modules/antagonists/cult/blood_magic.dm @@ -359,9 +359,10 @@ var/datum/action/innate/cult/blood_spell/source /obj/item/melee/blood_magic/New(loc, spell) - source = spell - uses = source.charges - health_cost = source.health_cost + if(spell) + source = spell + uses = source.charges + health_cost = source.health_cost ..() /obj/item/melee/blood_magic/Destroy() @@ -376,7 +377,7 @@ source.desc = source.base_desc source.desc += "
Has [uses] use\s remaining." source.UpdateButtonIcon() - ..() + return ..() /obj/item/melee/blood_magic/attack_self(mob/living/user) afterattack(user, user, TRUE) diff --git a/code/modules/antagonists/revenant/revenant.dm b/code/modules/antagonists/revenant/revenant.dm index c2afbe08cf6..fb56ec07c37 100644 --- a/code/modules/antagonists/revenant/revenant.dm +++ b/code/modules/antagonists/revenant/revenant.dm @@ -484,7 +484,7 @@ /obj/item/ectoplasm/revenant/Destroy() if(!QDELETED(revenant)) qdel(revenant) - ..() + return ..() //objectives /datum/objective/revenant diff --git a/code/modules/antagonists/traitor/equipment/Malf_Modules.dm b/code/modules/antagonists/traitor/equipment/Malf_Modules.dm index 5720aa45392..54b687bafbd 100644 --- a/code/modules/antagonists/traitor/equipment/Malf_Modules.dm +++ b/code/modules/antagonists/traitor/equipment/Malf_Modules.dm @@ -305,10 +305,10 @@ GLOBAL_LIST_INIT(malf_modules, subtypesof(/datum/ai_module)) /obj/machinery/doomsday_device/Initialize() . = ..() - owner = loc - if(!istype(owner)) + if(!isAI(loc)) stack_trace("Doomsday created outside an AI somehow, shit's fucking broke. Anyway, we're just gonna qdel now. Go make a github issue report.") - qdel(src) + return INITIALIZE_HINT_QDEL + owner = loc countdown = new(src) /obj/machinery/doomsday_device/Destroy() @@ -318,7 +318,7 @@ GLOBAL_LIST_INIT(malf_modules, subtypesof(/datum/ai_module)) SSshuttle.clearHostileEnvironment(src) SSmapping.remove_nuke_threat(src) set_security_level("red") - for(var/mob/living/silicon/robot/borg in owner.connected_robots) + for(var/mob/living/silicon/robot/borg in owner?.connected_robots) borg.lamp_doom = FALSE borg.toggle_headlamp(FALSE, TRUE) //forces borg lamp to update owner?.doomsday_device = null diff --git a/code/modules/atmospherics/machinery/bluespace_vendor.dm b/code/modules/atmospherics/machinery/bluespace_vendor.dm index cffb2fa1263..2183d8808c0 100644 --- a/code/modules/atmospherics/machinery/bluespace_vendor.dm +++ b/code/modules/atmospherics/machinery/bluespace_vendor.dm @@ -183,9 +183,10 @@ ///Unregister the connected_machine (either when qdel this or the sender) /obj/machinery/bluespace_vendor/proc/unregister_machine() SIGNAL_HANDLER - UnregisterSignal(connected_machine, COMSIG_PARENT_QDELETING) - LAZYREMOVE(connected_machine.vendors, src) - connected_machine = null + if(connected_machine) + UnregisterSignal(connected_machine, COMSIG_PARENT_QDELETING) + LAZYREMOVE(connected_machine.vendors, src) + connected_machine = null mode = BS_MODE_OFF update_appearance() diff --git a/code/modules/atmospherics/machinery/components/fusion/hfr_procs.dm b/code/modules/atmospherics/machinery/components/fusion/hfr_procs.dm index edb6a7cdecf..ebd65b6e8b3 100644 --- a/code/modules/atmospherics/machinery/components/fusion/hfr_procs.dm +++ b/code/modules/atmospherics/machinery/components/fusion/hfr_procs.dm @@ -117,10 +117,14 @@ */ /obj/machinery/atmospherics/components/unary/hypertorus/core/proc/unregister_signals(only_signals = FALSE) SIGNAL_HANDLER - UnregisterSignal(linked_interface, COMSIG_PARENT_QDELETING) - UnregisterSignal(linked_input, COMSIG_PARENT_QDELETING) - UnregisterSignal(linked_output, COMSIG_PARENT_QDELETING) - UnregisterSignal(linked_moderator, COMSIG_PARENT_QDELETING) + if(linked_interface) + UnregisterSignal(linked_interface, COMSIG_PARENT_QDELETING) + if(linked_input) + UnregisterSignal(linked_input, COMSIG_PARENT_QDELETING) + if(linked_output) + UnregisterSignal(linked_output, COMSIG_PARENT_QDELETING) + if(linked_moderator) + UnregisterSignal(linked_moderator, COMSIG_PARENT_QDELETING) for(var/obj/machinery/hypertorus/corner/corner in corners) UnregisterSignal(corner, COMSIG_PARENT_QDELETING) if(!only_signals) diff --git a/code/modules/atmospherics/machinery/components/tank.dm b/code/modules/atmospherics/machinery/components/tank.dm index 04b90f10e15..923d76b865c 100644 --- a/code/modules/atmospherics/machinery/components/tank.dm +++ b/code/modules/atmospherics/machinery/components/tank.dm @@ -89,7 +89,6 @@ if(gas_type) FillToPressure(gas_type) - setPipingLayer(piping_layer) QUEUE_SMOOTH(src) QUEUE_SMOOTH_NEIGHBORS(src) @@ -132,7 +131,6 @@ air_contents.assert_gas(gastype) air_contents.gases[gastype][MOLES] += moles_to_add air_contents.archive() - update_parents() /obj/machinery/atmospherics/components/tank/process_atmos() if(air_contents.react(src)) @@ -231,8 +229,6 @@ if(dir & initialize_directions & merger.members[src]) ToggleSidePort(dir) - update_parents() - /////////////////////////////////////////////////////////////////// // Appearance stuff @@ -546,5 +542,6 @@ var/list/new_custom_materials = list() new_custom_materials[material_end_product] = 20000 new_tank.set_custom_materials(new_custom_materials) + new_tank.on_construction(new_tank.pipe_color, new_tank.piping_layer) to_chat(user, "[new_tank] has been sealed and is ready to accept gases.") qdel(src) diff --git a/code/modules/buildmode/effects/line.dm b/code/modules/buildmode/effects/line.dm index f38e0c53871..394c0d205a5 100644 --- a/code/modules/buildmode/effects/line.dm +++ b/code/modules/buildmode/effects/line.dm @@ -3,6 +3,9 @@ var/client/cl /obj/effect/buildmode_line/New(client/C, atom/atom_a, atom/atom_b, linename) + if(!C || !atom_a || !atom_b) + stack_trace("Buildmode effect created with odd inputs") + return name = linename abstract_move(get_turf(atom_a)) I = image('icons/misc/mark.dmi', src, "line", 19.0) diff --git a/code/modules/capture_the_flag/ctf_game.dm b/code/modules/capture_the_flag/ctf_game.dm index 6dbe4a74f16..8be7c4e181b 100644 --- a/code/modules/capture_the_flag/ctf_game.dm +++ b/code/modules/capture_the_flag/ctf_game.dm @@ -51,6 +51,7 @@ /obj/item/ctf/proc/reset_flag(capture = FALSE) SIGNAL_HANDLER + STOP_PROCESSING(SSobj, src) var/turf/our_turf = get_turf(src.reset) if(!our_turf) @@ -61,7 +62,6 @@ if(istype(mob_area, game_area)) if(!capture) to_chat(M, span_userdanger("[src] has been returned to the base!")) - STOP_PROCESSING(SSobj, src) return TRUE //so if called by a signal, it doesn't delete //working with attack hand feels like taking my brain and putting it through an industrial pill press so i'm gonna be a bit liberal with the comments diff --git a/code/modules/cargo/gondolapod.dm b/code/modules/cargo/gondolapod.dm index 18f7b30212b..e507096b6b7 100644 --- a/code/modules/cargo/gondolapod.dm +++ b/code/modules/cargo/gondolapod.dm @@ -30,6 +30,9 @@ var/obj/structure/closet/supplypod/centcompod/linked_pod /mob/living/simple_animal/pet/gondola/gondolapod/Initialize(mapload, pod) + if(!pod) + stack_trace("Gondola pod created with no pod") + return INITIALIZE_HINT_QDEL linked_pod = pod name = linked_pod.name desc = linked_pod.desc @@ -73,6 +76,6 @@ update_appearance() /mob/living/simple_animal/pet/gondola/gondolapod/death() - qdel(linked_pod) //Will cause the open() proc for the linked supplypod to be called with the "broken" parameter set to true, meaning that it will dump its contents on death + QDEL_NULL(linked_pod) //Will cause the open() proc for the linked supplypod to be called with the "broken" parameter set to true, meaning that it will dump its contents on death qdel(src) ..() diff --git a/code/modules/cargo/supplypod.dm b/code/modules/cargo/supplypod.dm index 393b90c8fb2..cd4be6ab750 100644 --- a/code/modules/cargo/supplypod.dm +++ b/code/modules/cargo/supplypod.dm @@ -552,6 +552,9 @@ /obj/effect/pod_landingzone_effect/Initialize(mapload, obj/structure/closet/supplypod/pod) . = ..() + if(!pod) + stack_trace("Pod landingzone effect created with no pod") + return INITIALIZE_HINT_QDEL transform = matrix() * 1.5 animate(src, transform = matrix()*0.01, time = pod.delays[POD_TRANSIT]+pod.delays[POD_FALLING]) @@ -570,6 +573,9 @@ /obj/effect/pod_landingzone/Initialize(mapload, podParam, single_order = null, clientman) . = ..() + if(!podParam) + stack_trace("Pod landingzone created with no pod") + return INITIALIZE_HINT_QDEL if (ispath(podParam)) //We can pass either a path for a pod (as expressconsoles do), or a reference to an instantiated pod (as the centcom_podlauncher does) podParam = new podParam() //If its just a path, instantiate it pod = podParam diff --git a/code/modules/flufftext/Hallucination.dm b/code/modules/flufftext/Hallucination.dm index 69b22fca0f3..8748be37f25 100644 --- a/code/modules/flufftext/Hallucination.dm +++ b/code/modules/flufftext/Hallucination.dm @@ -113,6 +113,9 @@ GLOBAL_LIST_INIT(hallucination_list, list( /obj/effect/hallucination/simple/Initialize(mapload, mob/living/carbon/T) . = ..() + if(!T) + stack_trace("A hallucination was created with no target") + return INITIALIZE_HINT_QDEL target = T current_image = GetImage() if(target.client) diff --git a/code/modules/food_and_drinks/kitchen_machinery/food_cart.dm b/code/modules/food_and_drinks/kitchen_machinery/food_cart.dm index 8ba1f3a956f..d62c77ee971 100644 --- a/code/modules/food_and_drinks/kitchen_machinery/food_cart.dm +++ b/code/modules/food_and_drinks/kitchen_machinery/food_cart.dm @@ -122,13 +122,14 @@ /obj/machinery/food_cart/obj_break(damage_flag) . = ..() pack_up() - if(cart_griddle) + if(!QDELETED(cart_griddle)) QDEL_NULL(cart_griddle) - if(cart_smartfridge) + if(!QDELETED(cart_smartfridge)) QDEL_NULL(cart_smartfridge) - if(cart_table) + if(!QDELETED(cart_table)) QDEL_NULL(cart_table) - QDEL_NULL(cart_tent) + if(!QDELETED(cart_tent)) + QDEL_NULL(cart_tent) /obj/effect/food_cart_stand name = "food cart tent" diff --git a/code/modules/food_and_drinks/kitchen_machinery/oven.dm b/code/modules/food_and_drinks/kitchen_machinery/oven.dm index 2072b89106b..09f640923aa 100644 --- a/code/modules/food_and_drinks/kitchen_machinery/oven.dm +++ b/code/modules/food_and_drinks/kitchen_machinery/oven.dm @@ -142,6 +142,8 @@ return TRUE /obj/machinery/oven/proc/update_baking_audio() + if(!oven_loop) + return if(!open && used_tray?.contents.len) oven_loop.start() else diff --git a/code/modules/food_and_drinks/kitchen_machinery/processor.dm b/code/modules/food_and_drinks/kitchen_machinery/processor.dm index 53cdedda314..51d683a9d39 100644 --- a/code/modules/food_and_drinks/kitchen_machinery/processor.dm +++ b/code/modules/food_and_drinks/kitchen_machinery/processor.dm @@ -174,7 +174,7 @@ /obj/machinery/processor/dump_inventory_contents() . = ..() if(!LAZYLEN(processor_contents)) - processor_contents.Cut() + processor_contents = null /obj/machinery/processor/container_resist_act(mob/living/user) user.forceMove(drop_location()) diff --git a/code/modules/hydroponics/grown.dm b/code/modules/hydroponics/grown.dm index 413fdda4556..0a2135e29e7 100644 --- a/code/modules/hydroponics/grown.dm +++ b/code/modules/hydroponics/grown.dm @@ -51,6 +51,9 @@ // This is for adminspawn or map-placed growns. They get the default stats of their seed type. seed = new seed() seed.adjust_potency(50-seed.potency) + else if(!seed) + stack_trace("Grown object created without a seed. WTF") + return INITIALIZE_HINT_QDEL pixel_x = base_pixel_x + rand(-5, 5) pixel_y = base_pixel_y + rand(-5, 5) diff --git a/code/modules/mapping/mapping_helpers.dm b/code/modules/mapping/mapping_helpers.dm index 5786007f64a..3fe9e2ce7e0 100644 --- a/code/modules/mapping/mapping_helpers.dm +++ b/code/modules/mapping/mapping_helpers.dm @@ -283,11 +283,14 @@ INITIALIZE_IMMEDIATE(/obj/effect/mapping_helpers/no_lava) new /obj/item/toy/balloon/corgi(thing) else openturfs += thing + //cake + knife to cut it! - var/turf/food_turf = get_turf(pick(table)) - new /obj/item/kitchen/knife(food_turf) - var/obj/item/food/cake/birthday/iancake = new(food_turf) - iancake.desc = "Happy birthday, Ian!" + if(length(table)) + var/turf/food_turf = get_turf(pick(table)) + new /obj/item/kitchen/knife(food_turf) + var/obj/item/food/cake/birthday/iancake = new(food_turf) + iancake.desc = "Happy birthday, Ian!" + //some balloons! this picks an open turf and pops a few balloons in and around that turf, yay. for(var/i in 1 to balloon_clusters) var/turf/clusterspot = pick_n_take(openturfs) diff --git a/code/modules/mining/fulton.dm b/code/modules/mining/fulton.dm index 9b6659a8bb8..2546330fd89 100644 --- a/code/modules/mining/fulton.dm +++ b/code/modules/mining/fulton.dm @@ -170,7 +170,7 @@ GLOBAL_LIST_EMPTY(total_extraction_beacons) /obj/structure/extraction_point/Destroy() GLOB.total_extraction_beacons -= src - ..() + return ..() /obj/effect/extraction_holder name = "extraction holder" diff --git a/code/modules/mining/minebot.dm b/code/modules/mining/minebot.dm index 0efe4625634..308b25c2bad 100644 --- a/code/modules/mining/minebot.dm +++ b/code/modules/mining/minebot.dm @@ -44,7 +44,7 @@ /mob/living/simple_animal/hostile/mining_drone/Initialize() . = ..() - AddElement(/datum/element/footstep, FOOTSTEP_OBJ_ROBOT, 1, -6, vary = TRUE) + AddElement(/datum/element/footstep, FOOTSTEP_OBJ_ROBOT, 1, -6, sound_vary = TRUE) stored_gun = new(src) var/datum/action/innate/minedrone/toggle_light/toggle_light_action = new() diff --git a/code/modules/mob/living/blood.dm b/code/modules/mob/living/blood.dm index 33acf48fef2..406b8dd5269 100644 --- a/code/modules/mob/living/blood.dm +++ b/code/modules/mob/living/blood.dm @@ -329,6 +329,8 @@ var/obj/effect/decal/cleanable/blood/B = locate() in T if(!B) B = new /obj/effect/decal/cleanable/blood/splatter(T, get_static_viruses()) + if(QDELETED(B)) //Give it up + return B.bloodiness = min((B.bloodiness + BLOOD_AMOUNT_PER_DECAL), BLOOD_POOL_MAX) B.transfer_mob_blood_dna(src) //give blood info to the blood decal. if(temp_blood_DNA) diff --git a/code/modules/mob/living/carbon/human/species_types/jellypeople.dm b/code/modules/mob/living/carbon/human/species_types/jellypeople.dm index 2455a6c3dc4..279e45e2734 100644 --- a/code/modules/mob/living/carbon/human/species_types/jellypeople.dm +++ b/code/modules/mob/living/carbon/human/species_types/jellypeople.dm @@ -458,7 +458,7 @@ /datum/species/jelly/luminescent/proc/update_glow(mob/living/carbon/C, intensity) if(intensity) glow_intensity = intensity - glow.set_light(glow_intensity, glow_intensity, C.dna.features["mcolor"]) + glow.set_light_range_power_color(glow_intensity, glow_intensity, C.dna.features["mcolor"]) /obj/effect/dummy/luminescent_glow name = "luminescent glow" diff --git a/code/modules/mob/living/carbon/human/species_types/snail.dm b/code/modules/mob/living/carbon/human/species_types/snail.dm index bef62c0fe18..f03c38a53eb 100644 --- a/code/modules/mob/living/carbon/human/species_types/snail.dm +++ b/code/modules/mob/living/carbon/human/species_types/snail.dm @@ -62,7 +62,8 @@ /obj/item/storage/backpack/snail/dropped(mob/user, silent) . = ..() emptyStorage() - qdel(src) + if(!QDELETED(src)) + qdel(src) /obj/item/storage/backpack/snail/Initialize() . = ..() diff --git a/code/modules/mob/living/inhand_holder.dm b/code/modules/mob/living/inhand_holder.dm index dcf408cd99a..d2146c0c749 100644 --- a/code/modules/mob/living/inhand_holder.dm +++ b/code/modules/mob/living/inhand_holder.dm @@ -83,6 +83,12 @@ release(TRUE, FALSE) return +/obj/item/clothing/head/mob_holder/drone/Initialize(mapload, mob/living/M, worn_state, head_icon, lh_icon, rh_icon, worn_slot_flags = NONE) + //If we're not being put onto a drone, end it all + if(!isdrone(M)) + return INITIALIZE_HINT_QDEL + return ..() + /obj/item/clothing/head/mob_holder/drone/deposit(mob/living/L) . = ..() if(!isdrone(L)) diff --git a/code/modules/mob/living/silicon/ai/ai.dm b/code/modules/mob/living/silicon/ai/ai.dm index 19ef5fa9cdd..d68504ff090 100644 --- a/code/modules/mob/living/silicon/ai/ai.dm +++ b/code/modules/mob/living/silicon/ai/ai.dm @@ -1001,9 +1001,9 @@ return /mob/living/silicon/ai/spawned/Initialize(mapload, datum/ai_laws/L, mob/target_ai) - . = ..() if(!target_ai) target_ai = src //cheat! just give... ourselves as the spawned AI, because that's technically correct + . = ..() /mob/living/silicon/ai/proc/camera_visibility(mob/camera/ai_eye/moved_eye) GLOB.cameranet.visibility(moved_eye, client, all_eyes, TRUE) diff --git a/code/modules/mob/living/silicon/robot/robot.dm b/code/modules/mob/living/silicon/robot/robot.dm index 9b354c6c0c8..a5eee8671af 100644 --- a/code/modules/mob/living/silicon/robot/robot.dm +++ b/code/modules/mob/living/silicon/robot/robot.dm @@ -389,7 +389,7 @@ /mob/living/silicon/robot/proc/SetLockdown(state = TRUE) // They stay locked down if their wire is cut. - if(wires.is_cut(WIRE_LOCKDOWN)) + if(wires?.is_cut(WIRE_LOCKDOWN)) state = TRUE if(state) throw_alert("locked", /atom/movable/screen/alert/locked) diff --git a/code/modules/mob/living/simple_animal/friendly/robot_customer.dm b/code/modules/mob/living/simple_animal/friendly/robot_customer.dm index 4ac728daabc..c5ba70ce4a1 100644 --- a/code/modules/mob/living/simple_animal/friendly/robot_customer.dm +++ b/code/modules/mob/living/simple_animal/friendly/robot_customer.dm @@ -26,7 +26,7 @@ ADD_TRAIT(src, TRAIT_NOMOBSWAP, INNATE_TRAIT) //dont push me bitch ADD_TRAIT(src, TRAIT_NO_TELEPORT, INNATE_TRAIT) //dont teleport me bitch ADD_TRAIT(src, TRAIT_STRONG_GRABBER, INNATE_TRAIT) //strong arms bitch - AddElement(/datum/element/footstep, FOOTSTEP_OBJ_ROBOT, 1, -6, vary = TRUE) + AddElement(/datum/element/footstep, FOOTSTEP_OBJ_ROBOT, 1, -6, sound_vary = TRUE) var/datum/customer_data/customer_info = SSrestaurant.all_customers[customer_data] clothes_set = pick(customer_info.clothing_sets) ai_controller = customer_info.ai_controller_used diff --git a/code/modules/mob/living/simple_animal/guardian/types/support.dm b/code/modules/mob/living/simple_animal/guardian/types/support.dm index f2878bf1817..d7ce0d58d3e 100644 --- a/code/modules/mob/living/simple_animal/guardian/types/support.dm +++ b/code/modules/mob/living/simple_animal/guardian/types/support.dm @@ -100,7 +100,7 @@ /obj/structure/receiving_pad/New(loc, mob/living/simple_animal/hostile/guardian/healer/G) . = ..() - if(G.guardiancolor) + if(G?.guardiancolor) add_atom_colour(G.guardiancolor, FIXED_COLOUR_PRIORITY) /obj/structure/receiving_pad/proc/disappear() diff --git a/code/modules/mob/living/simple_animal/hostile/megafauna/wendigo.dm b/code/modules/mob/living/simple_animal/hostile/megafauna/wendigo.dm index 2c672c0b360..0377ded69c9 100644 --- a/code/modules/mob/living/simple_animal/hostile/megafauna/wendigo.dm +++ b/code/modules/mob/living/simple_animal/hostile/megafauna/wendigo.dm @@ -142,6 +142,8 @@ Difficulty: Hard /// Slams the ground around the source throwing back enemies caught nearby, delay is for the radius increase /proc/wendigo_slam(atom/source, range, delay, throw_range) var/turf/orgin = get_turf(source) + if(!orgin) + return var/list/all_turfs = RANGE_TURFS(range, orgin) for(var/i = 0 to range) playsound(orgin,'sound/effects/bamf.ogg', 600, TRUE, 10) diff --git a/code/modules/mob/living/simple_animal/hostile/mining_mobs/curse_blob.dm b/code/modules/mob/living/simple_animal/hostile/mining_mobs/curse_blob.dm index 6a7b5f0fb74..a63bd6485f5 100644 --- a/code/modules/mob/living/simple_animal/hostile/mining_mobs/curse_blob.dm +++ b/code/modules/mob/living/simple_animal/hostile/mining_mobs/curse_blob.dm @@ -57,7 +57,8 @@ /mob/living/simple_animal/hostile/asteroid/curseblob/proc/check_for_target() if(QDELETED(set_target) || set_target.stat != CONSCIOUS || z != set_target.z) - qdel(src) + if(!QDELETED(src)) + qdel(src) return TRUE /mob/living/simple_animal/hostile/asteroid/curseblob/GiveTarget(new_target) diff --git a/code/modules/modular_computers/computers/item/computer.dm b/code/modules/modular_computers/computers/item/computer.dm index eb1e67a25a8..be883add4f6 100644 --- a/code/modules/modular_computers/computers/item/computer.dm +++ b/code/modules/modular_computers/computers/item/computer.dm @@ -67,7 +67,6 @@ /obj/item/modular_computer/Destroy() kill_program(forced = TRUE) STOP_PROCESSING(SSobj, src) - QDEL_NULL(soundloop) for(var/H in all_components) var/obj/item/computer_hardware/CH = all_components[H] if(CH.holder == src) @@ -75,6 +74,8 @@ CH.holder = null all_components.Remove(CH.device_type) qdel(CH) + //Some components will actually try and interact with this, so let's do it later + QDEL_NULL(soundloop) physical = null return ..() diff --git a/code/modules/power/apc.dm b/code/modules/power/apc.dm index 33068dd4f53..f57cde9fb5e 100644 --- a/code/modules/power/apc.dm +++ b/code/modules/power/apc.dm @@ -256,17 +256,18 @@ if(malfai && operating) malfai.malf_picker.processing_time = clamp(malfai.malf_picker.processing_time - 10,0,1000) - area.power_light = FALSE - area.power_equip = FALSE - area.power_environ = FALSE - area.power_change() + if(area) + area.power_light = FALSE + area.power_equip = FALSE + area.power_environ = FALSE + area.power_change() QDEL_NULL(alarm_manager) if(occupier) malfvacate(1) - qdel(wires) - wires = null + if(wires) + QDEL_NULL(wires) if(cell) - qdel(cell) + QDEL_NULL(cell) if(terminal) disconnect_terminal() . = ..() diff --git a/code/modules/power/singularity/containment_field.dm b/code/modules/power/singularity/containment_field.dm index 0c02cf72ef8..8087b01ab61 100644 --- a/code/modules/power/singularity/containment_field.dm +++ b/code/modules/power/singularity/containment_field.dm @@ -29,8 +29,12 @@ AddElement(/datum/element/connect_loc, loc_connections) /obj/machinery/field/containment/Destroy() - field_gen_1.fields -= src - field_gen_2.fields -= src + if(field_gen_1) + field_gen_1.fields -= src + field_gen_1 = null + if(field_gen_2) + field_gen_2.fields -= src + field_gen_2 = null CanAtmosPass = ATMOS_PASS_YES air_update_turf(TRUE, FALSE) return ..() diff --git a/code/modules/projectiles/gun.dm b/code/modules/projectiles/gun.dm index 75d01ab57f5..d180d472fca 100644 --- a/code/modules/projectiles/gun.dm +++ b/code/modules/projectiles/gun.dm @@ -98,7 +98,7 @@ QDEL_NULL(chambered) if(azoom) QDEL_NULL(azoom) - if(isatom(suppressed)) + if(isatom(suppressed)) //SUPPRESSED IS USED AS BOTH A TRUE/FALSE AND AS A REF, WHAT THE FUCKKKKKKKKKKKKKKKKK QDEL_NULL(suppressed) return ..() diff --git a/code/modules/projectiles/guns/energy/dueling.dm b/code/modules/projectiles/guns/energy/dueling.dm index 4f941fd3618..f44a31c59db 100644 --- a/code/modules/projectiles/guns/energy/dueling.dm +++ b/code/modules/projectiles/guns/energy/dueling.dm @@ -183,11 +183,12 @@ /obj/item/gun/energy/dueling/Destroy() . = ..() - if(duel.gun_A == src) - duel.gun_A = null - if(duel.gun_B == src) - duel.gun_B = null - duel = null + if(duel) + if(duel.gun_A == src) + duel.gun_A = null + if(duel.gun_B == src) + duel.gun_B = null + duel = null /obj/item/gun/energy/dueling/can_trigger_gun(mob/living/user) . = ..() diff --git a/code/modules/projectiles/guns/magic.dm b/code/modules/projectiles/guns/magic.dm index 49c6bc0075b..77f8a121b31 100644 --- a/code/modules/projectiles/guns/magic.dm +++ b/code/modules/projectiles/guns/magic.dm @@ -53,7 +53,8 @@ /obj/item/gun/magic/Initialize() . = ..() charges = max_charges - chambered = new ammo_type(src) + if(ammo_type) + chambered = new ammo_type(src) if(can_charge) START_PROCESSING(SSobj, src) RegisterSignal(src, COMSIG_ITEM_RECHARGED, .proc/instant_recharge) diff --git a/code/modules/projectiles/projectile.dm b/code/modules/projectiles/projectile.dm index 1b50fb0a8e7..daf33915b71 100644 --- a/code/modules/projectiles/projectile.dm +++ b/code/modules/projectiles/projectile.dm @@ -168,16 +168,16 @@ var/wound_falloff_tile ///How much we want to drop the embed_chance value, if we can embed, per tile, for falloff purposes var/embed_falloff_tile + var/static/list/projectile_connections = list( + COMSIG_ATOM_ENTERED = .proc/on_entered, + ) /obj/projectile/Initialize() . = ..() decayedRange = range if(embedding) updateEmbedding() - var/static/list/loc_connections = list( - COMSIG_ATOM_ENTERED = .proc/on_entered, - ) - AddElement(/datum/element/connect_loc, loc_connections) + AddElement(/datum/element/connect_loc, projectile_connections) /obj/projectile/proc/Range() range-- diff --git a/code/modules/projectiles/projectile/energy/net_snare.dm b/code/modules/projectiles/projectile/energy/net_snare.dm index d4b0d284964..bddaf0b36ce 100644 --- a/code/modules/projectiles/projectile/energy/net_snare.dm +++ b/code/modules/projectiles/projectile/energy/net_snare.dm @@ -33,7 +33,7 @@ . = ..() var/obj/item/beacon/teletarget = null for(var/obj/machinery/computer/teleporter/com in GLOB.machines) - var/atom/target = com.target_ref.resolve() + var/atom/target = com.target_ref?.resolve() if(target) if(com.power_station && com.power_station.teleporter_hub && com.power_station.engaged) teletarget = target diff --git a/code/modules/projectiles/projectile/magic.dm b/code/modules/projectiles/projectile/magic.dm index 4b0fffe2ca1..52d1528f59c 100644 --- a/code/modules/projectiles/projectile/magic.dm +++ b/code/modules/projectiles/projectile/magic.dm @@ -280,6 +280,7 @@ /obj/projectile/magic/locker/Destroy() locker_suck = FALSE + RemoveElement(/datum/element/connect_loc, projectile_connections) //We do this manually so the forcemoves don't "hit" us. This behavior is kinda dumb, someone refactor this for(var/atom/movable/AM in contents) AM.forceMove(get_turf(src)) . = ..() diff --git a/code/modules/projectiles/projectile/special/gravity.dm b/code/modules/projectiles/projectile/special/gravity.dm index e19a067ce79..b3a844af02a 100644 --- a/code/modules/projectiles/projectile/special/gravity.dm +++ b/code/modules/projectiles/projectile/special/gravity.dm @@ -15,7 +15,7 @@ . = ..() var/obj/item/ammo_casing/energy/gravity/repulse/C = loc if(istype(C)) //Hard-coded maximum power so servers can't be crashed by trying to throw the entire Z level's items - power = min(C.gun.power, 15) + power = min(C.gun?.power, 15) /obj/projectile/gravityrepulse/on_hit() . = ..() @@ -50,7 +50,7 @@ . = ..() var/obj/item/ammo_casing/energy/gravity/attract/C = loc if(istype(C)) //Hard-coded maximum power so servers can't be crashed by trying to throw the entire Z level's items - power = min(C.gun.power, 15) + power = min(C.gun?.power, 15) /obj/projectile/gravityattract/on_hit() . = ..() @@ -84,7 +84,7 @@ . = ..() var/obj/item/ammo_casing/energy/gravity/chaos/C = loc if(istype(C)) //Hard-coded maximum power so servers can't be crashed by trying to throw the entire Z level's items - power = min(C.gun.power, 15) + power = min(C.gun?.power, 15) /obj/projectile/gravitychaos/on_hit() . = ..() diff --git a/code/modules/projectiles/projectile/special/hallucination.dm b/code/modules/projectiles/projectile/special/hallucination.dm index e55690d1585..35f0d8544d7 100644 --- a/code/modules/projectiles/projectile/special/hallucination.dm +++ b/code/modules/projectiles/projectile/special/hallucination.dm @@ -28,7 +28,7 @@ hal_target.client.images += fake_icon /obj/projectile/hallucination/Destroy() - if(hal_target.client) + if(hal_target?.client) hal_target.client.images -= fake_icon QDEL_NULL(fake_icon) return ..() diff --git a/code/modules/ruins/lavalandruin_code/puzzle.dm b/code/modules/ruins/lavalandruin_code/puzzle.dm index 4db6101e674..df3a7ba3ca2 100644 --- a/code/modules/ruins/lavalandruin_code/puzzle.dm +++ b/code/modules/ruins/lavalandruin_code/puzzle.dm @@ -242,7 +242,8 @@ /obj/structure/puzzle_element/Moved() . = ..() - source.validate() + if(source) + source.validate() //Admin abuse version so you can pick the icon before it sets up /obj/effect/sliding_puzzle/admin diff --git a/code/modules/shuttle/emergency.dm b/code/modules/shuttle/emergency.dm index 25f765b81d0..d584bcee4a2 100644 --- a/code/modules/shuttle/emergency.dm +++ b/code/modules/shuttle/emergency.dm @@ -605,7 +605,8 @@ /obj/machinery/computer/shuttle/pod/connect_to_shuttle(obj/docking_port/mobile/port, obj/docking_port/stationary/dock) . = ..() - possible_destinations += ";[port.id]_lavaland" + if(port) + possible_destinations += ";[port.id]_lavaland" /** * Signal handler for checking if we should lock or unlock escape pods accordingly to a newly set security level diff --git a/code/modules/shuttle/navigation_computer.dm b/code/modules/shuttle/navigation_computer.dm index 87af8171f93..14d86ebb826 100644 --- a/code/modules/shuttle/navigation_computer.dm +++ b/code/modules/shuttle/navigation_computer.dm @@ -33,8 +33,7 @@ if(!mapload) connect_to_shuttle(SSshuttle.get_containing_shuttle(src)) - for(var/port_id in SSshuttle.stationary) - var/obj/docking_port/stationary/S = SSshuttle.stationary[port_id] + for(var/obj/docking_port/stationary/S as anything in SSshuttle.stationary) if(S.id == shuttleId) jumpto_ports[S.id] = TRUE diff --git a/code/modules/spells/spell_types/lichdom.dm b/code/modules/spells/spell_types/lichdom.dm index 156b43db7e6..5d7b19ae890 100644 --- a/code/modules/spells/spell_types/lichdom.dm +++ b/code/modules/spells/spell_types/lichdom.dm @@ -89,6 +89,9 @@ /obj/item/phylactery/Initialize(mapload, datum/mind/newmind) . = ..() + if(!mind) + stack_trace("A phylactery was created with no target mind") + return INITIALIZE_HINT_QDEL mind = newmind name = "phylactery of [mind.name]" diff --git a/code/modules/spells/spell_types/shapeshift.dm b/code/modules/spells/spell_types/shapeshift.dm index cb5e0a74c4b..77e76e9661c 100644 --- a/code/modules/spells/spell_types/shapeshift.dm +++ b/code/modules/spells/spell_types/shapeshift.dm @@ -145,7 +145,8 @@ source = _source shape = loc if(!istype(shape)) - CRASH("shapeshift holder created outside mob/living") + stack_trace("shapeshift holder created outside mob/living") + return INITIALIZE_HINT_QDEL stored = caster if(stored.mind) stored.mind.transfer_to(shape) diff --git a/code/modules/spells/spell_types/touch_attacks.dm b/code/modules/spells/spell_types/touch_attacks.dm index c5b7df62ada..03eb5188b59 100644 --- a/code/modules/spells/spell_types/touch_attacks.dm +++ b/code/modules/spells/spell_types/touch_attacks.dm @@ -12,7 +12,7 @@ if(action?.owner) var/mob/guy_who_needs_to_know = action.owner to_chat(guy_who_needs_to_know, span_notice("The power of the spell dissipates from your hand.")) - ..() + return ..() /obj/effect/proc_holder/spell/targeted/touch/proc/remove_hand(recharge = FALSE) QDEL_NULL(attached_hand) diff --git a/code/modules/surgery/organs/external/_external_organs.dm b/code/modules/surgery/organs/external/_external_organs.dm index e8ad8e79651..506eec16a7e 100644 --- a/code/modules/surgery/organs/external/_external_organs.dm +++ b/code/modules/surgery/organs/external/_external_organs.dm @@ -34,12 +34,9 @@ */ /obj/item/organ/external/Initialize(mapload, mob_sprite) . = ..() - if(mob_sprite) set_sprite(mob_sprite) - cache_key = generate_icon_cache() - /obj/item/organ/external/Insert(mob/living/carbon/reciever, special, drop_if_replaced) var/obj/item/bodypart/limb = reciever.get_bodypart(zone) @@ -75,7 +72,7 @@ /obj/item/organ/external/proc/get_overlays(list/overlay_list, image_dir, image_layer, body_type, image_color) if(!sprite_datum) return - if(HAS_TRAIT(owner, TRAIT_INVISIBLE_MAN)) + if(owner && HAS_TRAIT(owner, TRAIT_INVISIBLE_MAN)) return var/gender = (body_type == FEMALE) ? "f" : "m" var/finished_icon_state = (sprite_datum.gender_specific ? gender : "m") + "_" + preference + "_" + sprite_datum.icon_state + mutant_bodyparts_layertext(image_layer) diff --git a/code/modules/surgery/organs/heart.dm b/code/modules/surgery/organs/heart.dm index 0090dcdaaac..339bb2a0249 100644 --- a/code/modules/surgery/organs/heart.dm +++ b/code/modules/surgery/organs/heart.dm @@ -438,6 +438,9 @@ /obj/structure/ethereal_crystal/Initialize(mapload, obj/item/organ/heart/ethereal/ethereal_heart) . = ..() + if(!ethereal_heart) + stack_trace("Our crystal has no related heart") + return INITIALIZE_HINT_QDEL src.ethereal_heart = ethereal_heart ethereal_heart.owner.visible_message(span_notice("The crystals fully encase [ethereal_heart.owner]!")) to_chat(ethereal_heart.owner, span_notice("You are encased in a huge crystal!")) diff --git a/code/modules/unit_tests/_unit_tests.dm b/code/modules/unit_tests/_unit_tests.dm index 3226ee2daa1..7fbdb853921 100644 --- a/code/modules/unit_tests/_unit_tests.dm +++ b/code/modules/unit_tests/_unit_tests.dm @@ -37,6 +37,9 @@ #define UNIT_TEST_FAILED 1 #define UNIT_TEST_SKIPPED 2 +#define TEST_DEFAULT 1 +#define TEST_DEL_WORLD INFINITY + /// A trait source when adding traits through unit tests #define TRAIT_SOURCE_UNIT_TESTS "unit_tests" @@ -52,7 +55,7 @@ #include "connect_loc.dm" #include "confusion.dm" #include "crayons.dm" -#include "deletions.dm" +#include "create_and_destroy.dm" #include "designs.dm" #include "dynamic_ruleset_sanity.dm" #include "egg_glands.dm" @@ -62,7 +65,6 @@ #include "heretic_knowledge.dm" #include "holidays.dm" #include "hydroponics_harvest.dm" -#include "initialize_sanity.dm" #include "keybinding_init.dm" #include "machine_disassembly.dm" #include "medical_wounds.dm" diff --git a/code/modules/unit_tests/create_and_destroy.dm b/code/modules/unit_tests/create_and_destroy.dm new file mode 100644 index 00000000000..88438f20ce2 --- /dev/null +++ b/code/modules/unit_tests/create_and_destroy.dm @@ -0,0 +1,189 @@ +///Delete one of every type, sleep a while, then check to see if anything has gone fucky +/datum/unit_test/create_and_destroy + //You absolutely must run last + priority = TEST_DEL_WORLD + +/datum/unit_test/create_and_destroy/Run() + //We'll spawn everything here + var/turf/spawn_at = run_loc_floor_bottom_left + var/list/ignore = list( + //Never meant to be created, errors out the ass for mobcode reasons + /mob/living/carbon, + //Nother template type, doesn't like being created with no seed + /obj/item/food/grown, + //And another + /obj/item/slimecross/recurring, + //This should be obvious + /obj/machinery/doomsday_device, + //Yet more templates + /obj/machinery/restaurant_portal, + //Template type + /obj/effect/mob_spawn, + //Template type + /obj/structure/holosign/robot_seat, + //Say it with me now, type template + /obj/effect/mapping_helpers/component_injector, + //template type + /obj/effect/mapping_helpers/trait_injector, + //Singleton + /mob/dview, + //Template, + /obj/effect/mapping_helpers/custom_icon, + ) + //This turf existing is an error in and of itself + ignore += typesof(/turf/baseturf_skipover) + ignore += typesof(/turf/baseturf_bottom) + //This demands a borg, so we'll let if off easy + ignore += typesof(/obj/item/modular_computer/tablet/integrated) + //This one demands a computer, ditto + ignore += typesof(/obj/item/modular_computer/processor) + //Needs special input, let's be nice + ignore += typesof(/obj/effect/abstract/proximity_checker) + //Very finiky, blacklisting to make things easier + ignore += typesof(/obj/item/poster/wanted) + //We can't pass a mind into this + ignore += typesof(/obj/item/phylactery) + //This expects a seed, we can't pass it + ignore += typesof(/obj/item/food/grown) + //Nothing to hallucinate if there's nothing to hallicinate + ignore += typesof(/obj/effect/hallucination) + //These want fried food to take on the shape of, we can't pass that in + ignore += typesof(/obj/item/food/deepfryholder) + //Can't pass in a thing to glow + ignore += typesof(/obj/effect/abstract/eye_lighting) + //It wants a lot more context then we have + ignore += typesof(/obj/effect/buildmode_line) + //We don't have a pod + ignore += typesof(/obj/effect/pod_landingzone_effect) + ignore += typesof(/obj/effect/pod_landingzone) + //We don't have a disease to pass in + ignore += typesof(/obj/effect/mapping_helpers/component_injector/infective) + //It's a trapdoor to nowhere + ignore += typesof(/obj/effect/mapping_helpers/trapdoor_placer) + //There's no shapeshift to hold + ignore += typesof(/obj/shapeshift_holder) + //No tauma to pass in + ignore += typesof(/mob/camera/imaginary_friend) + //No pod to gondola + ignore += typesof(/mob/living/simple_animal/pet/gondola/gondolapod) + //No heart to give + ignore += typesof(/obj/structure/ethereal_crystal) + //No linked console + ignore += typesof(/mob/camera/ai_eye/remote/base_construction) + //See above + ignore += typesof(/mob/camera/ai_eye/remote/shuttle_docker) + //Hangs a ref post invoke async, which we don't support. Could put a qdeleted check but it feels hacky + ignore += typesof(/obj/effect/anomaly/grav/high) + //See above + ignore += typesof(/obj/effect/timestop) + //Invoke async in init, skippppp + ignore += typesof(/mob/living/silicon/robot/model) + //This lad also sleeps + ignore += typesof(/obj/item/hilbertshotel) + //this boi spawns turf changing stuff, and it stacks and causes pain. Let's just not + ignore += typesof(/obj/effect/sliding_puzzle) + //Stacks baseturfs, can't be tested here + ignore += typesof(/obj/effect/temp_visual/lava_warning) + //Stacks baseturfs, can't be tested here + ignore += typesof(/obj/effect/landmark/ctf) + //Our system doesn't support it without warning spam from unregister calls on things that never registered + ignore += typesof(/obj/docking_port) + //Asks for a shuttle that may not exist, let's leave it alone + ignore += typesof(/obj/item/pinpointer/shuttle) + //This spawns beams as a part of init, which can sleep past an async proc. This hangs a ref, and fucks us. It's only a problem here because the beam sleeps with CHECK_TICK + ignore += typesof(/obj/structure/alien/resin/flower_bud) + //Needs a linked mecha + ignore += typesof(/obj/effect/skyfall_landingzone) + //Leads to errors as a consequence of the logic behind moving back to a tile that's moving you somewhere else + ignore += typesof(/obj/effect/mapping_helpers/component_injector/areabound) + //Expects a mob to holderize, we have nothing to give + ignore += typesof(/obj/item/clothing/head/mob_holder) + + var/list/cached_contents = spawn_at.contents.Copy() + var/baseturf_count = length(spawn_at.baseturfs) + + for(var/type_path in typesof(/atom/movable, /turf) - ignore) //No areas please + if(ispath(type_path, /turf)) + spawn_at.ChangeTurf(type_path, /turf/baseturf_skipover) + //We change it back to prevent pain, please don't ask + spawn_at.ChangeTurf(/turf/open/floor/wood, /turf/baseturf_skipover) + if(baseturf_count != length(spawn_at.baseturfs)) + Fail("[type_path] changed the amount of baseturfs we have [baseturf_count] -> [length(spawn_at.baseturfs)]") + baseturf_count = length(spawn_at.baseturfs) + else + var/atom/creation = new type_path(spawn_at) + if(QDELETED(creation)) + continue + //Go all in + qdel(creation, force = TRUE) + //This will hold a ref to the last thing we process unless we set it to null + //Yes byond is fucking sinful + creation = null + + //There's a lot of stuff that either spawns stuff in on create, or removes stuff on destroy. Let's cut it all out so things are easier to deal with + var/list/to_del = spawn_at.contents - cached_contents + if(length(to_del)) + for(var/atom/to_kill in to_del) + qdel(to_kill) + + //Hell code, we're bound to have ended the round somehow so let's stop if from ending while we work + SSticker.delay_end = TRUE + //Prevent the garbage subsystem from harddeling anything, if only to save time + SSgarbage.collection_timeout[GC_QUEUE_HARDDELETE] = 10000 HOURS + //Clear it, just in case + cached_contents.Cut() + + //Now that we've qdel'd everything, let's sleep until the gc has processed all the shit we care about + var/time_needed = SSgarbage.collection_timeout[GC_QUEUE_CHECK] + var/start_time = world.time + var/garbage_queue_processed = FALSE + + sleep(time_needed) + while(!garbage_queue_processed) + var/list/queue_to_check = SSgarbage.queues[GC_QUEUE_CHECK] + //How the hell did you manage to empty this? Good job! + if(!length(queue_to_check)) + garbage_queue_processed = TRUE + break + + var/list/oldest_packet = queue_to_check[1] + //Pull out the time we deld at + var/qdeld_at = oldest_packet[1] + //If we've found a packet that got del'd later then we finished, then all our shit has been processed + if(qdeld_at > start_time) + garbage_queue_processed = TRUE + break + + if(world.time > start_time + time_needed + 8 MINUTES) + Fail("Something has gone horribly wrong, the garbage queue has been processing for well over 10 minutes. What the hell did you do") + break + + //Immediately fire the gc right after + SSgarbage.next_fire = 1 + //Unless you've seriously fucked up, queue processing shouldn't take "that" long. Let her run for a bit, see if anything's changed + sleep(20 SECONDS) + + //Alright, time to see if anything messed up + var/list/cache_for_sonic_speed = SSgarbage.items + for(var/path in cache_for_sonic_speed) + var/datum/qdel_item/item = cache_for_sonic_speed[path] + if(item.failures) + Fail("[item.name] hard deleted [item.failures] times out of a total del count of [item.qdels]") + if(item.no_respect_force) + Fail("[item.name] failed to respect force deletion [item.no_respect_force] times out of a total del count of [item.qdels]") + if(item.no_hint) + Fail("[item.name] failed to return a qdel hint [item.no_hint] times out of a total del count of [item.qdels]") + + cache_for_sonic_speed = SSatoms.BadInitializeCalls + for(var/path in cache_for_sonic_speed) + var/fails = cache_for_sonic_speed[path] + if(fails & BAD_INIT_NO_HINT) + Fail("[path] didn't return an Initialize hint") + if(fails & BAD_INIT_QDEL_BEFORE) + Fail("[path] qdel'd in New()") + if(fails & BAD_INIT_SLEPT) + Fail("[path] slept during Initialize()") + + SSticker.delay_end = FALSE + //This shouldn't be needed, but let's be polite + SSgarbage.collection_timeout[GC_QUEUE_HARDDELETE] = 10 SECONDS diff --git a/code/modules/unit_tests/deletions.dm b/code/modules/unit_tests/deletions.dm deleted file mode 100644 index fe0fdf658b5..00000000000 --- a/code/modules/unit_tests/deletions.dm +++ /dev/null @@ -1,7 +0,0 @@ -/// This is for regression tests of deletions that used to runtime. -/// This would ideally be replaced by Del The World, unit testing every single deletion. -/datum/unit_test/deletion_regressions - -/datum/unit_test/deletion_regressions/Run() - qdel(new /obj/item/gun/energy/kinetic_accelerator/crossbow) - qdel(new /obj/item/gun/syringe/syndicate) diff --git a/code/modules/unit_tests/find_reference_sanity.dm b/code/modules/unit_tests/find_reference_sanity.dm index 53cccea9e7b..f41714f0659 100644 --- a/code/modules/unit_tests/find_reference_sanity.dm +++ b/code/modules/unit_tests/find_reference_sanity.dm @@ -6,20 +6,34 @@ var/list/test_list = list() var/list/test_assoc_list = list() +/atom/movable/ref_holder/Destroy() + test = null + test_list.Cut() + test_assoc_list.Cut() + return ..() + /atom/movable/ref_test var/atom/movable/ref_test/self_ref +/atom/movable/ref_test/Destroy(force) + self_ref = null + return ..() + /datum/unit_test/find_reference_sanity/Run() var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) + SSgarbage.should_save_refs = TRUE //Sanity check victim.DoSearchVar(testbed, "Sanity Check", search_time = 1) //We increment search time to get around an optimization TEST_ASSERT(!victim.found_refs.len, "The ref-tracking tool found a ref where none existed") + SSgarbage.should_save_refs = FALSE /datum/unit_test/find_reference_baseline/Run() var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) + SSgarbage.should_save_refs = TRUE + //Set up for the first round of tests testbed.test = victim testbed.test_list += victim @@ -30,10 +44,12 @@ TEST_ASSERT(victim.found_refs["test"], "The ref-tracking tool failed to find a regular value") TEST_ASSERT(victim.found_refs[testbed.test_list], "The ref-tracking tool failed to find a list entry") TEST_ASSERT(victim.found_refs[testbed.test_assoc_list], "The ref-tracking tool failed to find an assoc list value") + SSgarbage.should_save_refs = FALSE /datum/unit_test/find_reference_exotic/Run() var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) + SSgarbage.should_save_refs = TRUE //Second round, bit harder this time testbed.overlays += victim @@ -46,10 +62,12 @@ TEST_ASSERT(!victim.found_refs[testbed.overlays], "The ref-tracking tool found an overlays entry? That shouldn't be possible") TEST_ASSERT(victim.found_refs[testbed.vis_contents], "The ref-tracking tool failed to find a vis_contents entry") TEST_ASSERT(victim.found_refs[testbed.test_assoc_list], "The ref-tracking tool failed to find an assoc list key") + SSgarbage.should_save_refs = FALSE /datum/unit_test/find_reference_esoteric/Run() var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) + SSgarbage.should_save_refs = TRUE //Let's get a bit esoteric victim.self_ref = victim @@ -63,10 +81,12 @@ TEST_ASSERT(victim.found_refs["self_ref"], "The ref-tracking tool failed to find a self reference") TEST_ASSERT(victim.found_refs[to_find], "The ref-tracking tool failed to find a nested list entry") TEST_ASSERT(victim.found_refs[to_find_assoc], "The ref-tracking tool failed to find a nested assoc list entry") + SSgarbage.should_save_refs = FALSE /datum/unit_test/find_reference_null_key_entry/Run() var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) + SSgarbage.should_save_refs = TRUE //Calm before the storm testbed.test_assoc_list = list(null = victim) @@ -77,6 +97,8 @@ /datum/unit_test/find_reference_assoc_investigation/Run() var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) + SSgarbage.should_save_refs = TRUE + //Let's do some more complex assoc list investigation var/list/to_find_in_key = list(victim) testbed.test_assoc_list[to_find_in_key] = list("memes") @@ -86,3 +108,4 @@ victim.DoSearchVar(testbed, "Fifth Run", search_time = 6) TEST_ASSERT(victim.found_refs[to_find_in_key], "The ref-tracking tool failed to find a nested assoc list key") TEST_ASSERT(victim.found_refs[to_find_null_assoc_nested], "The ref-tracking tool failed to find a null key'd nested assoc list entry") + SSgarbage.should_save_refs = FALSE diff --git a/code/modules/unit_tests/initialize_sanity.dm b/code/modules/unit_tests/initialize_sanity.dm deleted file mode 100644 index d183f530c85..00000000000 --- a/code/modules/unit_tests/initialize_sanity.dm +++ /dev/null @@ -1,11 +0,0 @@ -/datum/unit_test/initialize_sanity/Run() - if(length(SSatoms.BadInitializeCalls)) - Fail("Bad Initialize() calls detected. Please read logs.") - var/list/init_failures_to_text = list( - "[BAD_INIT_QDEL_BEFORE]" = "Qdeleted Before Initialized", - "[BAD_INIT_DIDNT_INIT]" = "Did Not Initialize", - "[BAD_INIT_SLEPT]" = "Initialize() Slept", - "[BAD_INIT_NO_HINT]" = "No Initialize() Hint Returned", - ) - for(var/failure in SSatoms.BadInitializeCalls) - log_world("[failure]: [init_failures_to_text["[SSatoms.BadInitializeCalls[failure]]"]]") // You like stacked brackets? diff --git a/code/modules/unit_tests/unit_test.dm b/code/modules/unit_tests/unit_test.dm index 774578e32c6..4a5e3ed32c4 100644 --- a/code/modules/unit_tests/unit_test.dm +++ b/code/modules/unit_tests/unit_test.dm @@ -24,7 +24,8 @@ GLOBAL_VAR(test_log) /// The top right floor turf of the testing zone var/turf/run_loc_floor_top_right - + ///The priority of the test, the larger it is the later it fires + var/priority = TEST_DEFAULT //internal shit var/focus = FALSE var/succeeded = TRUE @@ -33,6 +34,9 @@ GLOBAL_VAR(test_log) var/static/datum/space_level/reservation +/proc/cmp_unit_test_priority(datum/unit_test/a, datum/unit_test/b) + return initial(a.priority) - initial(b.priority) + /datum/unit_test/New() if (isnull(reservation)) var/datum/map_template/unit_tests/template = new @@ -78,49 +82,55 @@ GLOBAL_VAR(test_log) allocated += instance return instance +/proc/RunUnitTest(test_path, list/test_results) + var/datum/unit_test/test = new test_path + + GLOB.current_test = test + var/duration = REALTIMEOFDAY + + test.Run() + + duration = REALTIMEOFDAY - duration + GLOB.current_test = null + GLOB.failed_any_test |= !test.succeeded + + var/list/log_entry = list("[test.succeeded ? "PASS" : "FAIL"]: [test_path] [duration / 10]s") + var/list/fail_reasons = test.fail_reasons + + for(var/J in 1 to LAZYLEN(fail_reasons)) + log_entry += "\tREASON #[J]: [fail_reasons[J]]" + var/message = log_entry.Join("\n") + log_test(message) + + test_results[test_path] = list("status" = test.succeeded ? UNIT_TEST_PASSED : UNIT_TEST_FAILED, "message" = message, "name" = test_path) + + qdel(test) + /proc/RunUnitTests() CHECK_TICK - var/tests_to_run = subtypesof(/datum/unit_test) + var/list/tests_to_run = subtypesof(/datum/unit_test) for (var/_test_to_run in tests_to_run) var/datum/unit_test/test_to_run = _test_to_run if (initial(test_to_run.focus)) tests_to_run = list(test_to_run) break + tests_to_run = sortTim(tests_to_run, /proc/cmp_unit_test_priority) + var/list/test_results = list() - for(var/I in tests_to_run) - var/datum/unit_test/test = new I - - GLOB.current_test = test - var/duration = REALTIMEOFDAY - - test.Run() - - duration = REALTIMEOFDAY - duration - GLOB.current_test = null - GLOB.failed_any_test |= !test.succeeded - - var/list/log_entry = list("[test.succeeded ? "PASS" : "FAIL"]: [I] [duration / 10]s") - var/list/fail_reasons = test.fail_reasons - - for(var/J in 1 to LAZYLEN(fail_reasons)) - log_entry += "\tREASON #[J]: [fail_reasons[J]]" - var/message = log_entry.Join("\n") - log_test(message) - - test_results[I] = list("status" = test.succeeded ? UNIT_TEST_PASSED : UNIT_TEST_FAILED, "message" = message, "name" = I) - - qdel(test) - - CHECK_TICK + for(var/unit_path in tests_to_run) + CHECK_TICK //We check tick first because the unit test we run last may be so expensive that checking tick will lock up this loop forever + RunUnitTest(unit_path, test_results) var/file_name = "data/unit_tests.json" fdel(file_name) file(file_name) << json_encode(test_results) SSticker.force_ending = TRUE + //We have to call this manually because del_text can preceed us, and SSticker doesn't fire in the post game + SSticker.standard_reboot() /datum/map_template/unit_tests name = "Unit Tests Zone" diff --git a/code/modules/vehicles/atv.dm b/code/modules/vehicles/atv.dm index 05429d3776e..2f3ea93335e 100644 --- a/code/modules/vehicles/atv.dm +++ b/code/modules/vehicles/atv.dm @@ -40,7 +40,10 @@ . = ..() if(!turret) return - turret.forceMove(get_turf(src)) + var/turf/our_turf = get_turf(src) + if(!our_turf) + return + turret.forceMove(our_turf) switch(dir) if(NORTH) turret.pixel_x = base_pixel_x diff --git a/code/modules/vehicles/mecha/combat/savannah_ivanov.dm b/code/modules/vehicles/mecha/combat/savannah_ivanov.dm index 712db29df03..41e9559b70b 100644 --- a/code/modules/vehicles/mecha/combat/savannah_ivanov.dm +++ b/code/modules/vehicles/mecha/combat/savannah_ivanov.dm @@ -372,6 +372,9 @@ /obj/effect/skyfall_landingzone/Initialize(mapload, obj/vehicle/sealed/mecha/combat/mecha) . = ..() + if(!mecha) + stack_trace("Skyfall landing zone created without mecha") + return INITIALIZE_HINT_QDEL src.mecha = mecha animate(src, alpha = 255, TOTAL_SKYFALL_LEAP_TIME/2, easing = CIRCULAR_EASING|EASE_OUT) RegisterSignal(mecha, COMSIG_MOVABLE_MOVED, .proc/follow) diff --git a/code/modules/wiremod/core/port.dm b/code/modules/wiremod/core/port.dm index 27f4f8cd9ae..a58016743fe 100644 --- a/code/modules/wiremod/core/port.dm +++ b/code/modules/wiremod/core/port.dm @@ -54,6 +54,8 @@ * Updates the value of the input and calls input_received on the connected component */ /datum/port/input/proc/set_input(value) + if(QDELETED(src)) //Pain + return set_value(value) if(trigger) TRIGGER_CIRCUIT_COMPONENT(connected_component, src)