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

Fixes medical grippers not being usable in surgery #26311

Merged
merged 2 commits into from
Aug 7, 2024

Conversation

Migratingcocofruit
Copy link
Contributor

@Migratingcocofruit Migratingcocofruit commented Jul 27, 2024

What Does This PR Do

fixes #24594
Adds a missing check for grippers when attempting surgery steps with more than one option.

Why It's Good For The Game

fixes stuff

Images of changes

Testing

Spawned human, dragged it over a bunch of shrapnel and removed them via surgery as a medborg.
Implanted an IV bag in the human. Then and pulled it out as a borg.

Changelog

🆑
fix: Medical grabber now works in surgery
/:cl:

@ParadiseSS13-Bot ParadiseSS13-Bot added -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally -Status: Awaiting review This PR is awaiting review from the review team and removed -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally labels Jul 27, 2024
Copy link
Contributor

@Sirryan2002 Sirryan2002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting a hold on this PR. This is not the way to fix this issue... yes technically... this makes medical grippers work and doesn't break other things. However, this is snowflaking a type check into a very nicely written surgery system.

The problem is that the gripper is considered the tool here instead of just being treated as a standard hand like it would be for carbon surgeons. What needs to happen is that a tool should not be passed here (i.g. the gripper) and some procs like put_into_hand() should be refactored for silicons to be able to accept shrapnel.

@Sirryan2002 Sirryan2002 added the On Hold Its gonna be a while before this is reviewed label Jul 28, 2024
@Sirryan2002 Sirryan2002 requested a review from lewcc July 28, 2024 19:51
@Sirryan2002
Copy link
Contributor

@lewcc I would definitely like your two sense here

@Migratingcocofruit
Copy link
Contributor Author

Putting a hold on this PR. This is not the way to fix this issue... yes technically... this makes medical grippers work and doesn't break other things. However, this is snowflaking a type check into a very nicely written surgery system.

The problem is that the gripper is considered the tool here instead of just being treated as a standard hand like it would be for carbon surgeons. What needs to happen is that a tool should not be passed here (i.g. the gripper) and some procs like put_into_hand() should be refactored for silicons to be able to accept shrapnel.

This check existed already for normal surgery steps, but was missing from proxy surgery steps specifically. I do agree, however.

As things stand the tool used in surgery is given from get_active_hand(), which is used everywhere. If a gripper returns null like a hand would that would break a lot of code.
Maybe have a typecache of all things considered a hand for the purpose of surgery, then set tool to null if it's one of those?
It's still not very pretty and only covers surgery, but at least it condenses everything to one check against a list.

@S34NW S34NW added the Fix This PR will fix an issue in the game label Jul 29, 2024
Copy link
Contributor

@lewcc lewcc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is the Specialized Surgery tool, and we really only want to scope its behavior to surgery so it doesn't do more hand-like things than it should, I don't think we should really bring this further up the chain. In principle, I mostly agree with this change.

I'd suggest adding a roundstart trait to the gripper which gets checked here, something like TRAIT_SURGICAL_OPEN_HAND, instead of just checking the raw typepath.

@Fordoxia
Copy link
Contributor

Remember also that the Universal Gripper exists, and should also be capable of performing surgery.

@Migratingcocofruit
Copy link
Contributor Author

Given that this is the Specialized Surgery tool, and we really only want to scope its behavior to surgery so it doesn't do more hand-like things than it should, I don't think we should really bring this further up the chain. In principle, I mostly agree with this change.

I'd suggest adding a roundstart trait to the gripper which gets checked here, something like TRAIT_SURGICAL_OPEN_HAND, instead of just checking the raw typepath.

Added the traits and tested again; it works. Could you have a look at the code just to make sure it's up to standard?

@Sirryan2002 Sirryan2002 removed the On Hold Its gonna be a while before this is reviewed label Jul 30, 2024
@Sirryan2002 Sirryan2002 requested a review from lewcc July 31, 2024 23:42
@lewcc lewcc added this pull request to the merge queue Aug 7, 2024
Merged via the queue into ParadiseSS13:master with commit a0308e0 Aug 7, 2024
11 checks passed
@BAGELMENSK
Copy link

If you can make it so that the Mediborg can pick up organs for surgery, that'd really complete the experience. Engiborg already has their part gripper so its not without precedent.

@Fordoxia
Copy link
Contributor

Fordoxia commented Aug 7, 2024

If you can make it so that the Mediborg can pick up organs for surgery, that'd really complete the experience. Engiborg already has their part gripper so its not without precedent.

Feature freeze.

Also outside of that, design doesn't like the idea of mediborgs being able to grab organs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-Status: Awaiting review This PR is awaiting review from the review team Fix This PR will fix an issue in the game
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cyborg Medical Gripper Does Not Work as a Hand Substitute
7 participants