From 0951d326d2d850f8bfa0bd5ef38a1107445f2bb4 Mon Sep 17 00:00:00 2001 From: "sentry[bot]" <39604003+sentry[bot]@users.noreply.github.com> Date: Wed, 27 May 2026 17:33:46 +0200 Subject: [PATCH] Fix storage close() null.vis_contents (#22521) * Please describe the intent of your changes in a clear fashion. This PR addresses a runtime error "Cannot modify null.vis_contents." occurring in `/obj/item/storage/proc/close`. **Problem:** The `close()` procedure for storage items was crashing when attempting to modify `storage_start.vis_contents` because `storage_start` was null. **Root Cause:** The issue stemmed from stale `s_active` references on mobs. When a storage item was destroyed, its `Destroy()` proc called `close_all()`. However, `close_all()` (via `can_see_contents()`) only cleared `s_active` for mobs with an active client. If a mob was clientless (e.g., due to a brief disconnect) during the storage's destruction, their `s_active` would retain a reference to the now-destroyed storage. Later, if `Move()` was called on this mob (e.g., due to inertial drift), it would attempt to call `s_active.close(src)` on the stale reference. Since the storage object was already destroyed and `storage_start` had been `QDEL_NULL`ed, accessing `storage_start.vis_contents` resulted in a null dereference. **Solution:** 1. **Clear all stale `s_active` references in `Destroy()`:** Modified `/obj/item/storage/Destroy()` to explicitly iterate through all mobs in `is_seeing` after `close_all()` and nullify their `s_active` if it points to the current storage object, regardless of their client status. This ensures no stale references persist after the storage is destroyed. 2. **Add null guard for `storage_start` in `close()`:** Implemented a defensive `if(storage_start)` check before accessing `storage_start.vis_contents` in `/obj/item/storage/proc/close`. This prevents a crash even if, under unforeseen circumstances, `storage_start` is null when `close()` is invoked. * Please make sure that, in the case of mapping changes, you include images of these changes in the PR's description. * Please make sure to mark your PR as wip or review required by making a comment with !wip or !review required * If you include sprites/sounds/... (assets) that you have not created yourself specify the license and original author below. * Ensure that you also credit them in the appropriate location / changelog as specified in the contributor guidelines ### Asset Licenses The following assets that **have not** been created by myself are included in this PR: | Path | Original Author | License | | --- | --- | --- | | icons/example.dmi | ExamplePerson (Example Station) | CC0 | Fixes SERVER-PROD-TV --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: VMSolidus --- code/game/objects/items/weapons/storage/storage.dm | 6 +++++- html/changelogs/hellfirejag-fix-storage-close-null.yml | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 html/changelogs/hellfirejag-fix-storage-close-null.yml diff --git a/code/game/objects/items/weapons/storage/storage.dm b/code/game/objects/items/weapons/storage/storage.dm index a37a33a9383..2c6602c6f55 100644 --- a/code/game/objects/items/weapons/storage/storage.dm +++ b/code/game/objects/items/weapons/storage/storage.dm @@ -93,6 +93,9 @@ /obj/item/storage/Destroy() close_all() + for(var/mob/M in is_seeing) + if(M.s_active == src) + M.s_active = null QDEL_NULL(boxes) QDEL_NULL(storage_start) QDEL_NULL(storage_continue) @@ -313,7 +316,8 @@ hide_from(user) user.s_active = null if(!length(can_see_contents())) - storage_start.vis_contents = list() + if(storage_start) + storage_start.vis_contents = list() QDEL_LIST(storage_screens) storage_screens = list() diff --git a/html/changelogs/hellfirejag-fix-storage-close-null.yml b/html/changelogs/hellfirejag-fix-storage-close-null.yml new file mode 100644 index 00000000000..c12ad9fb3fc --- /dev/null +++ b/html/changelogs/hellfirejag-fix-storage-close-null.yml @@ -0,0 +1,4 @@ +author: Hellfirejag +delete-after: True +changes: + - bugfix: "Fixed a hard del caused by storage items (like satchels)."