[MIRROR] Refactor renaming UNIQUE_RENAME items from the pen to an element #2849
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ment (#1935) * Refactor renaming UNIQUE_RENAME items from the pen to an element (#82491) ## About The Pull Request So a bit ago someone in code_general wanted to make plushies renamable, but learnt that just adding the `UNIQUE_RENAME` flag wouldn't work as pens would murder the plushie and only THEN let you rename it. I noted refactoring both pens and plushies to use the new `item_interaction(...)` procs would Just Solve This, but, well, they didn't really have any coding experience. But, hey, renaming being hardcoded to the pens has annoyed me ever since I laid my eyes upon the hot mess that is paperwork code. So here we are! ### We're making it an element. There's not really much to this, this is mostly the same code but moved to an element and with some minor cleanups. First, we move it all from `/obj/item/pen` to a new element we called `/datum/element/tool_renaming`. With this, instead of having it proc on `/obj/item/pen/afterattack(...)`, we register it to proc on the `COMSIG_ITEM_INTERACTING_WITH_ATOM` signal. https://github.com/tgstation/tgstation/blob/6e36ed984070d53e16bed4fa43eb0cbe6460deed/code/__DEFINES/dcs/signals/signals_atom/signals_atom_x_act.dm#L59-L62 Secondly, we realize the code is just going through each if statement regardless of whether the previous was correct. https://github.com/tgstation/tgstation/blob/6e36ed984070d53e16bed4fa43eb0cbe6460deed/code/modules/paperwork/pen.dm#L225-L258 And, as we're dealing with text, just make it a switch statement instead. ```dm switch(pen_choice) if("Rename") (...) if("Description") (...) if("Reset") (...) ``` Then, we replace all single letter variables with descriptive ones, replace the if-elses with early returns, and make it actually return item interaction flags. Finally, we slap this onto the pen, and we're done. Now we can slap it onto other fitting renaming tools, and it uses the proper item interaction system. ## Why It's Good For The Game I feel it's generally better to not hardcode this to just pens, we have plenty other writing utensils and possible renaming tools. It's also a bit cleaner than before. Apart from that, moves it from using `afterattack(...)` to the proper item interaction chain by using `COMSIG_ITEM_INTERACTING_WITH_ATOM`, which should reduce janky interactions. ## Changelog :cl: refactor: Instead of being hardcoded to the pen, renaming items is now an element. Currently only pens have this, and functionality should be the same, but please report it if you find any items that were renamable but now aren't. /:cl: * Refactor renaming UNIQUE_RENAME items from the pen to an element --------- Co-authored-by: _0Steven <[email protected]>
AnywayFarus
added a commit
that referenced
this pull request
Apr 12, 2024
Iajret
pushed a commit
that referenced
this pull request
Jun 5, 2024
…lso halo stage [MDB IGNORE] (#2849) * cult stun gets weaker at when cult gets red eyes, and then also halo stage (#83659) ## About The Pull Request cult stun gets weaker with cult stage at red eyes, stun lasts 9.6 seconds from 16 seconds by default at halo, stun lasts 1.9 seconds ## Why It's Good For The Game getting stinky handed in the middle of a fight with a halo cultist just isnt really fun because then the cultist has like 16 seconds to beat you to death with this you may have a chance of getting back on your feet and showing the narsie scum whos boss ## Changelog :cl: balance: cult stun gets weaker when they get red eyes and later more when they have halos /:cl: * cult stun gets weaker at when cult gets red eyes, and then also halo stage --------- Co-authored-by: jimmyl <[email protected]> Co-authored-by: NovaBot13 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Mirrored on Nova: NovaSector/NovaSector#1935
Original PR: tgstation/tgstation#82491
About The Pull Request
So a bit ago someone in code_general wanted to make plushies renamable, but learnt that just adding the
UNIQUE_RENAMEflag wouldn't work as pens would murder the plushie and only THEN let you rename it. I noted refactoring both pens and plushies to use the newitem_interaction(...)procs would Just Solve This, but, well, they didn't really have any coding experience.But, hey, renaming being hardcoded to the pens has annoyed me ever since I laid my eyes upon the hot mess that is paperwork code.
So here we are!
We're making it an element.
There's not really much to this, this is mostly the same code but moved to an element and with some minor cleanups.
First, we move it all from
/obj/item/pento a new element we called/datum/element/tool_renaming. With this, instead of having it proc on/obj/item/pen/afterattack(...), we register it to proc on theCOMSIG_ITEM_INTERACTING_WITH_ATOMsignal.https://github.com/tgstation/tgstation/blob/6e36ed984070d53e16bed4fa43eb0cbe6460deed/code/__DEFINES/dcs/signals/signals_atom/signals_atom_x_act.dm#L59-L62
Secondly, we realize the code is just going through each if statement regardless of whether the previous was correct.
https://github.com/tgstation/tgstation/blob/6e36ed984070d53e16bed4fa43eb0cbe6460deed/code/modules/paperwork/pen.dm#L225-L258
And, as we're dealing with text, just make it a switch statement instead.
Then, we replace all single letter variables with descriptive ones, replace the if-elses with early returns, and make it actually return item interaction flags.
Finally, we slap this onto the pen, and we're done.
Now we can slap it onto other fitting renaming tools, and it uses the proper item interaction system.
Why It's Good For The Game
I feel it's generally better to not hardcode this to just pens, we have plenty other writing utensils and possible renaming tools.
It's also a bit cleaner than before.
Apart from that, moves it from using
afterattack(...)to the proper item interaction chain by usingCOMSIG_ITEM_INTERACTING_WITH_ATOM, which should reduce janky interactions.Changelog
🆑 00-Steven
refactor: Instead of being hardcoded to the pen, renaming items is now an element. Currently only pens have this, and functionality should be the same, but please report it if you find any items that were renamable but now aren't.
/:cl: