From 2d5f4480631b050652cf1817cb3dd7fb2f88cb66 Mon Sep 17 00:00:00 2001 From: _0Steven <42909981+00-Steven@users.noreply.github.com> Date: Thu, 16 May 2024 08:11:59 +0200 Subject: [PATCH] Assorted minor alt-click reskinning fixes, primarily context-based (#83105) ## About The Pull Request This is a collection of tiny alt-click context fixes that I found during testing #82920, but I felt were not right to put in there. Most of the following explanation is for posterity, like they're mostly one-liners, there's only so much explanation to do. First off, the emotion mask would reset `current_skin` for infinite reskinning, while we have the `INFINITE_RESKIN` flag: https://github.com/tgstation/tgstation/blob/0c562fd74299f8ce92a81c0a932b8ec4862189af/code/modules/clothing/masks/costume.dm#L16-L19 We set this to `INFINITE_RESKIN` for sanity's sake. Then, `/obj/item/clothing/under/add_context(...)` would call its parent, but sometimes return `NONE` when its parent returned `CONTEXTUAL_SCREENTIP_SET`: https://github.com/tgstation/tgstation/blob/0c562fd74299f8ce92a81c0a932b8ec4862189af/code/modules/clothing/under/_under.dm#L83 This is bad, because reskinning context is handled on the parent (`/obj/item`), and we have an item inheriting this which can be reskinned, the mech pilot's suit: https://github.com/tgstation/tgstation/blob/0c562fd74299f8ce92a81c0a932b8ec4862189af/code/modules/clothing/under/costume.dm#L224-L240 So we make this return the parent return value rather than `NONE`: ```dm return changed ? CONTEXTUAL_SCREENTIP_SET : . ``` Next up, `/obj/item/clothing/accessory/add_context(...)` would never actually call the parent and thus neither the reskinning context. It also checks for whether you have an item in your active hand when context is added, even though the context it adds actually only applies when the accessory itself is in your active hand. https://github.com/tgstation/tgstation/blob/0c562fd74299f8ce92a81c0a932b8ec4862189af/code/modules/clothing/under/accessories/_accessories.dm#L205-L210 So we instead make it call the parent first, check for whether the accessory itself is in our active hand, and return the parent value if not: ```dm /obj/item/clothing/accessory/add_context(atom/source, list/context, obj/item/held_item, mob/user) . = ..() if(held_item != source) return . (...) ``` This resolves our issue. We're almost there! `/obj/item/reagent_containers/spray/medical/add_context(...)` exists, but is entirely redundant due to this now being handled on the base item, and also misses some of the checks it has. https://github.com/tgstation/tgstation/blob/0c562fd74299f8ce92a81c0a932b8ec4862189af/code/modules/reagents/reagent_containers/spray.dm#L442-L447 So we just remove it. Finally, what is to me the funniest one: https://github.com/tgstation/tgstation/blob/9145ecb7e1e44635a1056fc704adfa3d764325e6/code/game/objects/items_reskin.dm#L8-L9 To add reskinning context, we check `item_flags` for `INFINITE_RESKIN`, while it is actually on `obj_flags`. So, instead, we were checking for the equivalent value in `item_flags`, being `IN_STORAGE`. https://github.com/tgstation/tgstation/blob/9145ecb7e1e44635a1056fc704adfa3d764325e6/code/__DEFINES/obj_flags.dm#L15 https://github.com/tgstation/tgstation/blob/9145ecb7e1e44635a1056fc704adfa3d764325e6/code/__DEFINES/obj_flags.dm#L34 And thus reskinning context for infinitely reskinnables would only show up if they were in storage. For now, we just update this to use `obj_flags` instead. That's everything I found so far, which this should all fix. ## Why It's Good For The Game Having working item usage context tends to be a good thing. ## Changelog :cl: fix: Emotion masks no longer use a janky workaround for infinite reskinning. fix: Mech pilot suit shows reskinning usage context correctly. fix: Accessories show "wear above/below suit" usage context appropriately. fix: Accessories don't block reskinning usage context when they shouldn't. fix: Showing reskinning usage context cares about the infinite reskinning flag, rather than whether it's in storage or not. del: Removed redundant reskinning usage context code from medical sprays, now shows reskinning usage context like other reskinnables. /:cl: --- code/game/objects/items.dm | 2 +- code/modules/clothing/masks/costume.dm | 2 +- code/modules/clothing/under/_under.dm | 3 ++- code/modules/clothing/under/accessories/_accessories.dm | 5 +++-- code/modules/reagents/reagent_containers/spray.dm | 9 --------- 5 files changed, 7 insertions(+), 14 deletions(-) diff --git a/code/game/objects/items.dm b/code/game/objects/items.dm index 497d1fa611d..1c2293746d6 100644 --- a/code/game/objects/items.dm +++ b/code/game/objects/items.dm @@ -288,7 +288,7 @@ if(!unique_reskin) return - if(current_skin && !(item_flags & INFINITE_RESKIN)) + if(current_skin && !(obj_flags & INFINITE_RESKIN)) return context[SCREENTIP_CONTEXT_ALT_LMB] = "Reskin" diff --git a/code/modules/clothing/masks/costume.dm b/code/modules/clothing/masks/costume.dm index 626d8ce4a65..844b823880a 100644 --- a/code/modules/clothing/masks/costume.dm +++ b/code/modules/clothing/masks/costume.dm @@ -4,6 +4,7 @@ icon_state = "joy" clothing_flags = MASKINTERNALS flags_inv = HIDESNOUT + obj_flags = parent_type::obj_flags | INFINITE_RESKIN unique_reskin = list( "Joy" = "joy", "Flushed" = "flushed", @@ -16,7 +17,6 @@ /obj/item/clothing/mask/joy/reskin_obj(mob/user) . = ..() user.update_worn_mask() - current_skin = null//so we can infinitely reskin /obj/item/clothing/mask/mummy name = "mummy mask" diff --git a/code/modules/clothing/under/_under.dm b/code/modules/clothing/under/_under.dm index a2e560743ff..a39a10d26f8 100644 --- a/code/modules/clothing/under/_under.dm +++ b/code/modules/clothing/under/_under.dm @@ -66,6 +66,7 @@ . = ..() var/changed = FALSE + if(isnull(held_item) && has_sensor == HAS_SENSORS) context[SCREENTIP_CONTEXT_RMB] = "Toggle suit sensors" changed = TRUE @@ -86,7 +87,7 @@ context[SCREENTIP_CONTEXT_ALT_LMB] = "Wear [adjusted == ALT_STYLE ? "normally" : "casually"]" changed = TRUE - return changed ? CONTEXTUAL_SCREENTIP_SET : NONE + return changed ? CONTEXTUAL_SCREENTIP_SET : . /obj/item/clothing/under/worn_overlays(mutable_appearance/standing, isinhands = FALSE) diff --git a/code/modules/clothing/under/accessories/_accessories.dm b/code/modules/clothing/under/accessories/_accessories.dm index fdaf666638d..bfc6c1293fe 100644 --- a/code/modules/clothing/under/accessories/_accessories.dm +++ b/code/modules/clothing/under/accessories/_accessories.dm @@ -210,8 +210,9 @@ . += "It can be worn above or below your suit. Right-click to toggle." /obj/item/clothing/accessory/add_context(atom/source, list/context, obj/item/held_item, mob/user) - if(!isnull(held_item)) - return NONE + . = ..() + if(held_item != source) + return . context[SCREENTIP_CONTEXT_RMB] = "Wear [above_suit ? "below" : "above"] suit" return CONTEXTUAL_SCREENTIP_SET diff --git a/code/modules/reagents/reagent_containers/spray.dm b/code/modules/reagents/reagent_containers/spray.dm index 389396ed3fc..be40215039d 100644 --- a/code/modules/reagents/reagent_containers/spray.dm +++ b/code/modules/reagents/reagent_containers/spray.dm @@ -438,15 +438,6 @@ "Yellow" = "sprayer_med_yellow", "Blue" = "sprayer_med_blue") - -/obj/item/reagent_containers/spray/medical/add_context(atom/source, list/context, obj/item/held_item, mob/user) - . = ..() - - if(!current_skin) - context[SCREENTIP_CONTEXT_ALT_LMB] = "Reskin" - return CONTEXTUAL_SCREENTIP_SET - - /obj/item/reagent_containers/spray/medical/reskin_obj(mob/M) ..() switch(icon_state)