mirror of
https://github.com/Bubberstation/Bubberstation.git
synced 2026-01-16 12:07:48 +00:00
aa5eddb99b
## About The Pull Request **Edit: Since writing, this pr has been updated to address failing CI based on code-general suggestions, invalidating the previous descriptions. The previous descriptions has been included as spoilers for posterity** Right, so, this has gone from just a simple pride pin fix to realizing CI fails with it to doing a more complex lasting fix based on suggestions. Recap time. Objects get reskinning set up if they have `unique_reskin` set when `Initialize(...)` runs.9145ecb7e1/code/game/objects/items.dm (L267-L269)Because pride pins use a global list, we set it in `Initialize(...)`... After we call the parent.9145ecb7e1/code/modules/clothing/under/accessories/badges.dm (L196-L198)Obviously this fails. However, moving this *before* `Initialize(...)`, while fixing the issue, causes CI to fail due to calling `register_context()` twice. Why? Well, it's simple. We automatically call `register_context()` if we have `unique_reskin` set, as seen above, but we *also* call it on accessory `Initialize(...)` due to it having its own context.0c562fd742/code/modules/clothing/under/accessories/_accessories.dm (L29-L31)This causes it to try register the same thing twice, which doesn't _break_ things, but it sure as hell isn't clean. So talking about this with San in code general, we decided to try go with the following: We add two new procs, `setup_reskinning()` and `check_setup_reskinning()`, and handle all this fuckery within those. This lets subtypes override them with their own new checks or differences in setup. Then we override `setup_reskinning()` for `/obj/item/clothing/under` and `/obj/item/clothing/accessory` to not register context again, and do the same for `/obj/item/clothing/accessory/pride` but while also setting `unique_reskin`. This fixes it. <details> <summary>Previous implementation for posterity</summary> Back from my short code break, time to fix some of the things I've been annoyed by. Firstly, I noticed pride pins could no longer be reskinned since the alt-click refactor. Looking into it, this seems to be because we now only register this on `Initialize(...)` if `unique_reskin` has been set:9145ecb7e1/code/game/objects/items.dm (L267-L269)While due to using a global list we don't set this in the item definition, but in `Initialize(...)` :9145ecb7e1/code/modules/clothing/under/accessories/badges.dm (L196-L198)Where we call the parent proc _before_ setting `unique_reskin`, and thus not registering our ability to reskin. So all we do is set this to our global list _before_ we call the parent proc. ```dm /obj/item/clothing/accessory/pride/Initialize(mapload) unique_reskin = GLOB.pride_pin_reskins // Set before parent proc checks for it. . = ..() ``` This fixes it. </details> ## Why It's Good For The Game Fixes pride pin reskinning. Theoretically makes it easier to avoid this happening in the future, and allows `setup_reskinning()` to be manually called in the case of values being edited post-initialize. <details> <summary>Previous pitch for posterity</summary> Fixes pride pin reskinning. </details> ## Changelog 🆑 fix: Pride pins can be reskinned again with alt-click. /🆑
87 lines
2.2 KiB
Plaintext
87 lines
2.2 KiB
Plaintext
/// Called when alt clicked and the item has unique reskin options
|
|
/obj/item/proc/on_click_alt_reskin(datum/source, mob/user)
|
|
SIGNAL_HANDLER
|
|
|
|
if(!user.can_perform_action(src, NEED_DEXTERITY))
|
|
return NONE
|
|
|
|
if(!(obj_flags & INFINITE_RESKIN) && current_skin)
|
|
return NONE
|
|
|
|
INVOKE_ASYNC(src, PROC_REF(reskin_obj), user)
|
|
return CLICK_ACTION_SUCCESS
|
|
|
|
/**
|
|
* Checks if we should set up reskinning,
|
|
* by default if unique_reskin is set.
|
|
*
|
|
* Called on setup_reskinning().
|
|
* Inheritors should override this to add their own checks.
|
|
*/
|
|
/obj/item/proc/check_setup_reskinning()
|
|
SHOULD_CALL_PARENT(TRUE)
|
|
if(unique_reskin)
|
|
return TRUE
|
|
|
|
return FALSE
|
|
|
|
/**
|
|
* Registers signals and context for reskinning,
|
|
* if check_setup_reskinning() passes.
|
|
*
|
|
* Called on Initialize(...).
|
|
* Inheritors should override this to add their own setup steps,
|
|
* or to avoid double calling register_context().
|
|
*/
|
|
/obj/item/proc/setup_reskinning()
|
|
SHOULD_CALL_PARENT(FALSE)
|
|
if(!check_setup_reskinning())
|
|
return
|
|
|
|
RegisterSignal(src, COMSIG_CLICK_ALT, PROC_REF(on_click_alt_reskin))
|
|
register_context()
|
|
|
|
/**
|
|
* Reskins object based on a user's choice
|
|
*
|
|
* Arguments:
|
|
* * user The mob choosing a reskin option
|
|
*/
|
|
/obj/item/proc/reskin_obj(mob/user)
|
|
if(!LAZYLEN(unique_reskin))
|
|
return
|
|
|
|
var/list/items = list()
|
|
for(var/reskin_option in unique_reskin)
|
|
var/image/item_image = image(icon = src.icon, icon_state = unique_reskin[reskin_option])
|
|
items += list("[reskin_option]" = item_image)
|
|
sort_list(items)
|
|
|
|
var/pick = show_radial_menu(user, src, items, custom_check = CALLBACK(src, PROC_REF(check_reskin_menu), user), radius = 38, require_near = TRUE)
|
|
if(!pick)
|
|
return
|
|
if(!unique_reskin[pick])
|
|
return
|
|
current_skin = pick
|
|
icon_state = unique_reskin[pick]
|
|
to_chat(user, "[src] is now skinned as '[pick].'")
|
|
SEND_SIGNAL(src, COMSIG_OBJ_RESKIN, user, pick)
|
|
|
|
|
|
/**
|
|
* Checks if we are allowed to interact with a radial menu for reskins
|
|
*
|
|
* Arguments:
|
|
* * user The mob interacting with the menu
|
|
*/
|
|
/obj/item/proc/check_reskin_menu(mob/user)
|
|
if(QDELETED(src))
|
|
return FALSE
|
|
if(!(obj_flags & INFINITE_RESKIN) && current_skin)
|
|
return FALSE
|
|
if(!istype(user))
|
|
return FALSE
|
|
if(user.incapacitated())
|
|
return FALSE
|
|
return TRUE
|