mirror of
https://github.com/Bubberstation/Bubberstation.git
synced 2025-12-27 10:02:12 +00:00
eb2bc00b7e
* Rack sounds pr turned rack code refactor (#81973) ## About The Pull Request ### Alternate title: "Label Hoarding PR" You ever just want to add sounds to a thing and then end up just refactoring half of the damn thing? Yeah. Anyhow. So in rough chronological order! ### Putting items in racks actually plays the sounds Picking things up from racks plays their pickup sound, but putting them on them doesn't. Just dropping it or putting it on tables does make the right sounds. This seems to be because of a `silent` parameter in `transferItemToLoc` that's set by tables but not racks. ```dm (/code/game/objects/structures/tables_racks.dm, line 273) if(user.transferItemToLoc(I, drop_location(), silent = FALSE)) (/code/game/objects/structures/tables_racks.dm, line 867) if(user.transferItemToLoc(W, drop_location())) ``` Adding this makes it work just fine. ```dm (/code/game/objects/structures/tables_racks.dm, line 867) if(user.transferItemToLoc(W, drop_location(), silent = FALSE)) ``` ### Attackby single letter parameters, 1 instead of TRUE Then, I noticed `attackby` just returns `1` to mean true after calling `transferItemToLoc`, when we just have the more readable `TRUE`. Similarly it uses a single letter parameter `W`, which on its own is already unreadable, but is also mismatched with the parent proc using `attacking_item`. ```dm (/code/game/objects/structures/tables_racks.dm, line 859-868) /obj/structure/rack/attackby(obj/item/W, mob/living/user, params) var/list/modifiers = params2list(params) if (W.tool_behaviour == TOOL_WRENCH && !(obj_flags & NO_DECONSTRUCTION) && LAZYACCESS(modifiers, RIGHT_CLICK)) W.play_tool_sound(src) deconstruct(TRUE) return if(user.combat_mode) return ..() if(user.transferItemToLoc(W, drop_location())) return 1 (/code/_onclick/item_attack.dm, line 133-136) /atom/proc/attackby(obj/item/attacking_item, mob/user, params) if(SEND_SIGNAL(src, COMSIG_ATOM_ATTACKBY, attacking_item, user, params) & COMPONENT_NO_AFTERATTACK) return TRUE return FALSE ``` So we change it `1` to `TRUE` and `W` to `attacking_item` to make it more readable! ### Don't let them try to put Abstract items While looking over the table code, though, I also noticed it has a check for the `ABSTRACT` item flag when placing items. Having tested this, we add this too for consistency and to avoid awkward situations down the line. ```dm (/code/game/objects/structures/tables_racks.dm, line 272-273) if(!user.combat_mode && !(I.item_flags & ABSTRACT)) if(user.transferItemToLoc(I, drop_location(), silent = FALSE)) (/code/game/objects/structures/tables_racks.dm, line 859-868) /obj/structure/rack/attackby(obj/item/attacking_item mob/living/user, params) var/list/modifiers = params2list(params) if (attacking_item.tool_behaviour == TOOL_WRENCH && !(obj_flags & NO_DECONSTRUCTION) && LAZYACCESS(modifiers, RIGHT_CLICK)) attacking_item.play_tool_sound(src) deconstruct(TRUE) return if(user.combat_mode || attacking_item.item_flags & ABSTRACT) return ..() if(user.transferItemToLoc(W, drop_location())) return TRUE ``` ### Split off rack structure attackby wrenching into wrench_act procs But that's still kind of bad! But wait, we have procs for wrenching actions, so it should really be in there. So we move this to its own proc. ```dm (/code/game/objects/structures/tables_racks.dm, line 850-855) /obj/structure/rack/wrench_act_secondary(mob/living/user, obj/item/tool) if(obj_flags & NO_DECONSTRUCTION) return FALSE tool.play_tool_sound(src) deconstruct(TRUE) return ITEM_INTERACT_SUCCESS (/code/game/objects/structures/tables_racks.dm, line 857-861) /obj/structure/rack/attackby(obj/item/attacking_item mob/living/user, params) if(user.combat_mode || attacking_item.item_flags & ABSTRACT) return ..() if(user.transferItemToLoc(W, drop_location())) return TRUE ``` ### Split off rack item attackby wrenching into wrench_act procs, include sounds But the _item_ can also be deconstructed, and sure enough, it does the same thing. ```dm (/code/game/objects/structures/tables_racks.dm, line 920-925) /obj/item/rack_parts/attackby(obj/item/W, mob/user, params) if (W.tool_behaviour == TOOL_WRENCH) new /obj/item/stack/sheet/iron(user.loc) qdel(src) else . = ..() ``` So we give this the same treatment, and include a `deconstruct` method rather than just having it be separate. We also play the tool sound for consistency with deconstructing the rack structure. ```dm (/code/game/objects/structures/tables_racks.dm, line 948-953) /obj/item/rack_parts/wrench_act(mob/living/user, obj/item/tool) if(obj_flags & NO_DECONSTRUCTION) return FALSE tool.play_tool_sound(src) deconstruct(TRUE) return ITEM_INTERACT_SUCCESS (/code/game/objects/structures/tables_racks.dm, line 955-958) /obj/item/rack_parts/deconstruct(disassembled = TRUE) if(!(obj_flags & NO_DECONSTRUCTION)) new /obj/item/stack/sheet/iron(drop_location()) return ..() ``` Note: this makes it so it only deconstructs rack items on left click. I think that's perfectly fine. ### Ancient code removal Now we get to the fun part! Ancient code. When removing the single letter parameters from `attackby` previously, I thought I might as well remove other such instances while we're at it. This gets us to `MouseDrop_T`. ```dm (/code/game/objects/structures/tables_racks.dm, line 850-857) /obj/structure/rack/MouseDrop_T(obj/O, mob/user) . = ..() if ((!( isitem(O) ) || user.get_active_held_item() != O)) return if(!user.dropItemToGround(O)) return if(O.loc != src.loc) step(O, get_dir(O, src)) ``` What the fuck? Right so, this lets us click-drag-drop an item onto the rack, but only for our active item. And it just drops it and steps it in the right direction. So we just, we just kill it. We just kill it. You can just click on the rack to do functionally the exact same thing, so we just kill it. It's blocking us from dumping storage item contents, so just. We Just Kill It. We Just Fucking Kill It. ### Combat mode kicking Anyhow! With that out of the way, we move to the finishing touches: usage context and kicking! While writing up context I was reminded that currently clicking on a rack with an empty hand would just, kick it, regardless of combat mode. ```dm (/code/game/objects/structures/tables_racks.dm, line 873-882) /obj/structure/rack/attack_hand(mob/living/user, list/modifiers) . = ..() if(.) return if(user.body_position == LYING_DOWN || user.usable_legs < 2) return user.changeNext_move(CLICK_CD_MELEE) user.do_attack_animation(src, ATTACK_EFFECT_KICK) user.visible_message(span_danger("[user] kicks [src]."), null, null, COMBAT_MESSAGE_RANGE) take_damage(rand(4,8), BRUTE, MELEE, 1) ``` This is awkward, misclick a few times and you've kicked it back into item form. So we make it only kick it while in combat mode to avoid this. ```dm (/code/game/objects/structures/tables_racks.dm, line 880-889) /obj/structure/rack/attack_hand(mob/living/user, list/modifiers) (...) if(!user.combat_mode || user.body_position == LYING_DOWN || user.usable_legs < 2) return (...) ``` ### Usage context Then finally! Usage context! This part's easy, just copying over the logic from tables and making it show deconstruct context for the rack structure and item and construction context for the item. ```dm (/code/game/objects/structures/tables_racks.dm, line 840-851) /obj/structure/rack/add_context(atom/source, list/context, obj/item/held_item, mob/living/user) . = ..() if(isnull(held_item)) return NONE if(!(obj_flags & NO_DECONSTRUCTION)) if(held_item.tool_behaviour == TOOL_WRENCH) context[SCREENTIP_CONTEXT_RMB] = "Deconstruct" . = CONTEXTUAL_SCREENTIP_SET return . || NONE (/code/game/objects/structures/tables_racks.dm, line 931-946) /obj/item/rack_parts/add_context(atom/source, list/context, obj/item/held_item, mob/living/user) . = ..() if(isnull(held_item)) return NONE if(held_item == src) context[SCREENTIP_CONTEXT_LMB] = "Construct Rack" . = CONTEXTUAL_SCREENTIP_SET if(!(obj_flags & NO_DECONSTRUCTION)) if(held_item.tool_behaviour == TOOL_WRENCH) context[SCREENTIP_CONTEXT_LMB] = "Deconstruct" . = CONTEXTUAL_SCREENTIP_SET return . || NONE ``` And then don't forget to register said context. ```dm (/code/game/objects/structures/tables_racks.dm, line 834-838) /obj/structure/rack/Initialize(mapload) . = ..() AddElement(/datum/element/climbable) AddElement(/datum/element/elevation, pixel_shift = 12) register_context() (/code/game/objects/structures/tables_racks.dm, line 927-929) /obj/item/rack_parts/Initialize(mapload) . = ..() register_context() ``` And that's all! Now please remind me not to be _this_ comprehensive again. Not for something that's less than thirty lines, at least. Thanks. ## Why It's Good For The Game Doing this in chronological order. First off, it's wonky that putting say a toolbox on a table or the ground makes sound but putting it in a rack is perfectly silent, especially when taking it from the rack isn't. This makes it consistent with tables. Returning TRUE is more readable than returning 1. Single letter parameters are awful for readability, _especially_ when inconsistent with the parent parameters. This resolves those. It shouldn't let you attempt to place items tagged with `ABSTRACT`, as you shouldn't be able to place those. This makes it consistent with tables. We have procs for wrench actions, better to use those than implementing your own wrench checks in `attackby`. This makes it do so. It's just nice to have a deconstruction sound, so this makes deconstructing the item also play the sound of the used tool. The ancient rack code we're removing is entirely just a more awkward way of doing what we can already do without it, it only let you drag your active item onto them, _but you can just click_. It was awkwardly implemented, and blocked anything from doing its own click-drag onto surface thing like say storage dumping contents. Usage context is just nice to have. This adds context for deconstructing to the rack structure and for deconstructing and constructing to the rack item. ## Changelog 🆑 refactor: Touched most of the code for racks. It should function almost entirely the same save for what's noted here, please report any issues. code: Wrenching moved to wrenching procs. Side-effect, rack items are only deconstructed on left-click sound: Items that have sounds make them when placed on racks, much like when placed on tables. sound: Rack items now make a sound upon deconstructing them. fix: Racks no longer let you attempt to place abstract items like the slap hand or water tank spray nozzles on them. qol: Clicking on a rack no longer kicks the shit out of it if you don't actually have combat mode on. qol: Racks and rack items have hover tooltip usage context. del: Killed ancient rack code for dragging your active item onto a rack. Just click it does the same thing. This allows you to actually dump items onto racks, though currently disorderly. /🆑 * Rack sounds pr turned rack code refactor --------- Co-authored-by: _0Steven <42909981+00-Steven@users.noreply.github.com>