Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makes it EVEN EASIER to work with atom item interactions ft. "Leaf and Branch" & "Death to Chains" #82625

Merged
merged 10 commits into from
Apr 18, 2024

Conversation

MrMelbert
Copy link
Contributor

@MrMelbert MrMelbert commented Apr 12, 2024

About The Pull Request

When I made the new item interaction chain, I did this pattern for items interacting with atoms:

/obj/item/proc/interact_with_atom(atom/interacting_with, mob/living/user, list/modifiers)
return NONE

This is VERY clean and makes it ridiculously easy to add item-to-atom interactions without worry about needing to fuss around with calling parent and checking parent return value, IE, dealing with the dreaded chain.

Of course for some reason when I did this I didn't do the same for atom-from-item interactions themselves, relying on people call parent and check for blockers. Which worked in my head but is not easy to pick up as a new contributor.

So here we are, hopefully the final version of the atom-item-non-combat-interact chain.

Base item_interaction that handles all the signals and calling relevant procs is now base_item_interaction

item_interaction is now a stub proc that atoms can override at whim without worrying about parent calls (unless subtypes implement it).

This results in MUCH cleaner and easier to work with code.

And as an added bonus, it's (technically) completely backwards compatible with existing code. I changed it just for posterity but for downstreams and such, they don't even need to change anything, and it'll work as it did before.

Changelog

🆑 Melbert
refactor: Atom-Item interactions have been refactored once more. Report any oddities.
/:cl:

@tgstation-server tgstation-server added the Refactor Makes the code harder to read label Apr 12, 2024
@MrMelbert MrMelbert marked this pull request as ready for review April 12, 2024 20:09
@jlsnow301 jlsnow301 mentioned this pull request Apr 13, 2024
san7890 pushed a commit that referenced this pull request Apr 16, 2024
## About The Pull Request
Rewrites how alt click works. 
Based heavily on #82625. What a cool concept, it flows nicely with
#82533.

Fixes #81242 
(tm bugs fixed)
Fixes #82668

<details><summary>More info for devs</summary>

Handy regex used for alt click s&r:
`AltClick\((.*).*\)(\n\t.*\.\.\(\))?`
`click_alt($1)` (yes I am aware this only copies the first arg. there
are no other args!)

### Obj reskins
No reason for obj reskin to check on every single alt click for every
object. It applies to only a few items.
- Moved to obj/item
- Made into signal
- Added screentips

### Ventcrawling
Every single atmospherics machine checked for ventcrawling capability on
alt click despite only 3 objects needing that functionality. This has
been moved down to those individual items.
</details>

## Why It's Good For The Game
For players: 
- Alt clicking should work more logically, not causing double actions
like eject disk and open item window
- Added context menus for reskinnable items
- Removed adjacency restriction on loot panel

For devs:
- Makes alt click interactions easier to work with, no more click chain
nonsense and redundant guard clauses.
- OOP hell reduced
- Pascal Case reduced
- Glorious snake case

## Changelog
:cl:
add: The lootpanel now works at range.
add: Screentips for reskinnable items.
fix: Alt click interactions have been refactored, which may lead to
unintentional changes to gameplay. Report any issues, please.
/:cl:
MrMelbert and others added 3 commits April 16, 2024 23:48
Item interaction now runs before all the tool_acts which means this interaction, which attempts to add ANY item to the machine, always blocks.
Copy link
Contributor

github-actions bot commented Apr 18, 2024

@Ketrai
Copy link
Contributor

Ketrai commented Apr 20, 2024

Just as an FYI this broke the tray interactions with ranges introduced in #82566

@Ketrai
Copy link
Contributor

Ketrai commented Apr 20, 2024

It's only the interaction when left clicking an oven with a tray.

I cannot for the life of me figure out why the proc isn't working, debugging right now.
image

It's just a missing ')' after all

@Ketrai
Copy link
Contributor

Ketrai commented Apr 20, 2024

#82787 Fixed

@1393F 1393F mentioned this pull request May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor Makes the code harder to read
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants