From bf7f4e6e6901d077df73edf15d5d2079fb3f2b6d Mon Sep 17 00:00:00 2001
From: oranges <email@oranges.net.nz>
Date: Mon, 9 Oct 2017 04:06:43 +1300
Subject: [PATCH 1/2] Minor refactor to storage items (#31324)

They now do not call the on exit storage hook when items are being
deleted, as well as do not bother to reset a bunch of values

we now cache the see contents list for a very small speed boost

the fancy storage specific update icon is moved to the fancy storage
equivalent proc
---
 code/game/objects/items/storage/fancy.dm   |  4 ++-
 code/game/objects/items/storage/storage.dm | 40 +++++++++++++---------
 2 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/code/game/objects/items/storage/fancy.dm b/code/game/objects/items/storage/fancy.dm
index 435bc64c41..a37ea9f8cf 100644
--- a/code/game/objects/items/storage/fancy.dm
+++ b/code/game/objects/items/storage/fancy.dm
@@ -58,7 +58,9 @@
 
 /obj/item/storage/fancy/remove_from_storage(obj/item/W, atom/new_location, burn = 0)
 	fancy_open = TRUE
-	return ..()
+	. = ..()
+	//Recall update icon  with the fancy item snowflake arg (ugh)
+	update_icon(1)
 
 /*
  * Donut Box
diff --git a/code/game/objects/items/storage/storage.dm b/code/game/objects/items/storage/storage.dm
index 9a3769206c..309479dd86 100644
--- a/code/game/objects/items/storage/storage.dm
+++ b/code/game/objects/items/storage/storage.dm
@@ -359,35 +359,43 @@
 	return 1
 
 
-//Call this proc to handle the removal of an item from the storage item. The item will be moved to the atom sent as new_target
+//Call this proc to handle the removal of an item from the storage item. The item will be moved to the new_location target, if that is null it's being deleted
 /obj/item/storage/proc/remove_from_storage(obj/item/W, atom/new_location)
 	if(!istype(W))
 		return 0
 
-	if(istype(src, /obj/item/storage/fancy))
-		var/obj/item/storage/fancy/F = src
-		F.update_icon(1)
-
-	for(var/mob/M in can_see_contents())
-		if(M.client)
-			M.client.screen -= W
+	//Cache this as it should be reusable down the bottom, will not apply if anyone adds a sleep to dropped
+	//or moving objects, things that should never happen
+	var/list/seeing_mobs = can_see_contents()
+	for(var/mob/M in seeing_mobs)
+		M.client.screen -= W
 
 	if(ismob(loc))
 		var/mob/M = loc
 		W.dropped(M)
-	W.layer = initial(W.layer)
-	W.plane = initial(W.plane)
-	W.forceMove(new_location)
+	
 
-	for(var/mob/M in can_see_contents())
+	if(new_location)
+		W.forceMove(new_location)
+		//Reset the items values
+		W.layer = initial(W.layer)
+		W.plane = initial(W.plane)
+		W.mouse_opacity = initial(W.mouse_opacity)
+		if(W.maptext)
+			W.maptext = ""
+		//We don't want to call this if the item is being destroyed
+		W.on_exit_storage(src)
+
+	else
+		//Being destroyed, just move to nullspace now (so it's not in contents for the icon update)
+		W.moveToNullspace()
+
+
+	for(var/mob/M in seeing_mobs)
 		orient2hud(M)
 		show_to(M)
 
-	if(W.maptext)
-		W.maptext = ""
-	W.on_exit_storage(src)
 	update_icon()
-	W.mouse_opacity = initial(W.mouse_opacity)
 	return 1
 
 /obj/item/storage/deconstruct(disassembled = TRUE)