From 67e29343b9dacc90905dc8d4cc780edefefe8231 Mon Sep 17 00:00:00 2001
From: SyncIt21 <110812394+SyncIt21@users.noreply.github.com>
Date: Thu, 11 May 2023 20:27:19 +0530
Subject: [PATCH] [NO GBP]Final Closet, Crate & Access Patches (#75331)

## About The Pull Request

1. Ensures the static list closet & crate paint job vars & the
`access_choices` list vars are initialized only once. Since they are
defined inside `Initialize()` they were initialized many times and need
to be set only once when null

2. Fixes #75313
Closets & Crates can be wrapped again

3. Fixes #75349
Personal closets can be claimed again by anyone without
ACCESS_ALL_PERSONAL_LOCKERS if it's currently unclaimed.

4. Fixes #75351
Wallet with combined access are accepted again

**Note:** To claim a personal closet swipe once to unlock it, then swipe
again to claim it

:cl:
fix: closets & crates can be wrapped with wrapping paper again
refactor: closet & crate paint jobs static list vars, access_choices
static list var are initialized only once during init
/:cl:

---------

Co-authored-by: Tim <timothymtorres@gmail.com>
---
 .../structures/crates_lockers/closets.dm      | 34 ++++++++++++++-----
 .../crates_lockers/closets/secure/personal.dm | 14 ++++++--
 .../structures/crates_lockers/crates.dm       |  6 ++--
 code/modules/jobs/access.dm                   | 12 ++++---
 4 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/code/game/objects/structures/crates_lockers/closets.dm b/code/game/objects/structures/crates_lockers/closets.dm
index 93460ba0f64..999863b9433 100644
--- a/code/game/objects/structures/crates_lockers/closets.dm
+++ b/code/game/objects/structures/crates_lockers/closets.dm
@@ -89,7 +89,9 @@
 /obj/structure/closet/Initialize(mapload)
 	. = ..()
 
-	var/static/list/closet_paint_jobs = list(
+	var/static/list/closet_paint_jobs
+	if(isnull(closet_paint_jobs))
+		closet_paint_jobs = list(
 		"Cargo" = list("icon_state" = "qm"),
 		"Engineering" = list("icon_state" = "ce"),
 		"Engineering Secure" = list("icon_state" = "eng_secure"),
@@ -107,9 +109,15 @@
 	if(paint_jobs)
 		paint_jobs = closet_paint_jobs
 
+	var/static/list/card_reader_choices
+	if(isnull(card_reader_choices))
+		card_reader_choices = list(
+			"Personal",
+			"Departmental",
+			"None"
+			)
 	if(access_choices)
-		var/static/list/choices = list("Personal", "Departmental", "None")
-		access_choices = choices
+		access_choices = card_reader_choices
 
 	// if closed, any item at the crate's loc is put in the contents
 	if (mapload && !opened)
@@ -819,7 +827,7 @@
 	else if(!user.combat_mode)
 		var/item_is_id = weapon.GetID()
 		if(!item_is_id)
-			return
+			return FALSE
 		if((item_is_id || !toggle(user)) && !opened)
 			togglelock(user)
 	else
@@ -999,6 +1007,17 @@
 		togglelock(user)
 		return SECONDARY_ATTACK_CANCEL_ATTACK_CHAIN
 
+/**
+ * returns TRUE if the closet is allowed to unlock
+ * * user: the player trying to unlock this closet
+ * * player_id: the id of the player trying to unlock this closet
+ * * registered_id: the id registered to this closet, null if no one registered
+ */
+/obj/structure/closet/proc/can_unlock(mob/living/user, obj/item/card/id/player_id, obj/item/card/id/registered_id)
+	if(isnull(registered_id))
+		return allowed(user)
+	return player_id == registered_id
+
 /obj/structure/closet/proc/togglelock(mob/living/user, silent)
 	if(!secure || broken)
 		return
@@ -1006,7 +1025,7 @@
 	if(locked) //only apply checks while unlocking else allow anyone to lock it
 		var/error_msg = ""
 		if(!isnull(id_card))
-			var/obj/item/card/id/advanced/prisoner/registered_id = id_card.resolve()
+			var/obj/item/card/id/registered_id = id_card.resolve()
 			if(!registered_id) //id was deleted at some point. make this closet public access again
 				name = initial(name)
 				desc = initial(desc)
@@ -1015,9 +1034,9 @@
 				req_one_access = null
 				togglelock(user, silent)
 				return
-			if(registered_id !=  user.get_idcard())
+			if(!can_unlock(user, user.get_idcard(), registered_id))
 				error_msg = "not your locker!"
-		else if(!allowed(user)) //allow anyone to lock the closet for safe keeping but apply checks only when unlocking
+		else if(!can_unlock(user, user.get_idcard()))
 			error_msg = "access denied!"
 		if(error_msg)
 			if(!silent)
@@ -1031,7 +1050,6 @@
 				span_notice("You [locked ? "locked" : "unlocked"] [src]."))
 	update_appearance()
 
-
 /obj/structure/closet/emag_act(mob/user)
 	if(secure && !broken)
 		if(user)
diff --git a/code/game/objects/structures/crates_lockers/closets/secure/personal.dm b/code/game/objects/structures/crates_lockers/closets/secure/personal.dm
index 93fea8a2343..2904c1d9a9c 100644
--- a/code/game/objects/structures/crates_lockers/closets/secure/personal.dm
+++ b/code/game/objects/structures/crates_lockers/closets/secure/personal.dm
@@ -1,14 +1,24 @@
 /obj/structure/closet/secure_closet/personal
-	desc = "It's a secure locker for personnel. The first person to open this closet gains control."
+	desc = "It's a secure locker for personnel. The first person to swipe their ID gains control."
 	name = "personal closet"
 	req_access = list(ACCESS_ALL_PERSONAL_LOCKERS)
 	card_reader_installed = TRUE
 
 /obj/structure/closet/secure_closet/personal/Initialize(mapload)
 	. = ..()
-	var/static/list/choices = list("Personal")
+	var/static/list/choices
+	if(isnull(choices))
+		choices = list("Personal")
 	access_choices = choices
 
+/obj/structure/closet/secure_closet/personal/can_unlock(mob/living/user, obj/item/card/id/player_id, obj/item/card/id/registered_id)
+	if(isnull(registered_id)) //first time anyone can unlock
+		return TRUE
+	else
+		if(allowed(user)) //players with ACCESS_ALL_PERSONAL_LOCKERS can override your ID
+			return TRUE
+		return player_id == registered_id
+
 /obj/structure/closet/secure_closet/personal/PopulateContents()
 	..()
 	if(prob(50))
diff --git a/code/game/objects/structures/crates_lockers/crates.dm b/code/game/objects/structures/crates_lockers/crates.dm
index cd7c701baef..0b8ec11582e 100644
--- a/code/game/objects/structures/crates_lockers/crates.dm
+++ b/code/game/objects/structures/crates_lockers/crates.dm
@@ -33,7 +33,9 @@
 	AddElement(/datum/element/climbable, climb_time = crate_climb_time, climb_stun = 0) //add element in closed state before parent init opens it(if it does)
 	. = ..()
 
-	var/static/list/crate_paint_jobs = list(
+	var/static/list/crate_paint_jobs
+	if(isnull(crate_paint_jobs))
+		crate_paint_jobs = list(
 		"Internals" = list("icon_state" = "o2crate"),
 		"Medical" = list("icon_state" = "medicalcrate"),
 		"Radiation" = list("icon_state" = "radiation"),
@@ -42,7 +44,7 @@
 		"Solar" = list("icon_state" = "engi_e_crate"),
 		"Engineering" = list("icon_state" = "engi_crate")
 	)
-	if(!isnull(paint_jobs))
+	if(paint_jobs)
 		paint_jobs = crate_paint_jobs
 
 /obj/structure/closet/crate/Destroy()
diff --git a/code/modules/jobs/access.dm b/code/modules/jobs/access.dm
index 021e5812d1e..451404d1d92 100644
--- a/code/modules/jobs/access.dm
+++ b/code/modules/jobs/access.dm
@@ -29,10 +29,14 @@
 	//If the mob is holding a valid ID, we let them in. get_active_held_item() is on the mob level, so no need to copypasta everywhere.
 	else if(check_access(accessor.get_active_held_item()))
 		return TRUE
-	//if they are carying a card that has access, that works
-	else if(isliving(accessor))
-		var/mob/living/being = accessor
-		if(check_access(being.get_idcard(TRUE)))
+	else if(ishuman(accessor))
+		var/mob/living/carbon/human/human_accessor = accessor
+		if(check_access(human_accessor.wear_id))
+			return TRUE
+	//if they have a hacky abstract animal ID with the required access, let them in i guess...
+	else if(isanimal(accessor))
+		var/mob/living/simple_animal/animal = accessor
+		if(check_access(animal.access_card))
 			return TRUE
 	else if(isbrain(accessor) && istype(accessor.loc, /obj/item/mmi))
 		var/obj/item/mmi/brain_mmi = accessor.loc