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

Grab #30011

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

Grab #30011

wants to merge 23 commits into from

Conversation

FaDeOkno
Copy link

@FaDeOkno FaDeOkno commented Jul 13, 2024

About the PR

This PR adds Grab (Yellow intent system) into the game. You can grab someone softly, hardly and start choking them. Also you can throw grabbed entity.

Why / Balance

Grab is a core mechanic in SS13, so I've decided to move it in SS14

Technical details

The main system was made in the PullingSystem.cs. It checks when you are trying to pull someone, and if you have combat mode on, changes GrabStage to higher. When you're trying to unequip VirtualItem, it checks combat mode and lowers grab stage if on

Media

First version of Grab:
https://youtu.be/E_pl2XdckM4

Final version of Grab:
https://youtu.be/ZCQ1AKkN9tY

  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

VirtualItemDeletedEvent got Virtual Item uid field
Added VirtualItemThrownEvent. It is rised when user attempted to throw Virtual Item and contains the direction.
Added VirtualItemDropAttemptEvent. It is rised when user attempts to drop OR throw Virtual Item. This event is cancellable, and contains bool if this was a throw or just drop attempt.

Changelog

🆑

  • add: Added grab system. You can grab people by clicking on pulled by you player in combat mode to start grabbing them. There are 3 grab stages - soft, hard and choke. For the last one you need both hands to be used even if you are reptilian.
  • add: Added ability to throw grabbed people. You have to hardly grab someone or be choking them to perform throw. Use Ctrl+Q to throw them.

@github-actions github-actions bot added Status: Needs Review This PR requires new reviews before it can be merged. labels Jul 13, 2024
@SebaenSah
Copy link

Yes please! I can't wait to forget and accidentally start choking the secoffs!

@FaDeOkno
Copy link
Author

Uh... Whats wrong with Test Packaging test?..

@Plykiya
Copy link
Contributor

Plykiya commented Jul 13, 2024

Not sure if this would be a blocker for the PR but a doc exists for Grapple Systems which was approved but not merged. space-wizards/docs#213

It'd be nice if a maintainer could weigh on in that.

@FaDeOkno
Copy link
Author

Oh, okay

@Errant-4
Copy link
Contributor

Errant-4 commented Jul 13, 2024

I tried it and I have some UX and maybe design observations:

Reptilians can grab without using hands at all levels, including apparently choking other players hands-free with their tail. They should probably use hands for the higher tier grabs like others?

When Player1 releases Player2, Player2 gets a popup saying "You are trying to escape" rather than telling them that Player1 released them

When a player tries to escape a grab, there is no proper communication from the game about what's happening, is this actually taking some time? Did it succeed did it fail?

The grabbing player is also not notified in any way about attempted and succesful escapes

There does not seem to be a way for the Grabbing player to reduce the level of the grab (ie stop choking the target)

The PULLED and PULLING alert icons should display the current grab level

Edit
Also, while obviously you can't fully describe a change this big in the changelog, you can definitely do better than "Added grab system". Consider that many players have never played Space Station 13, what does this tell them about what this is and how to use it? nothing

@FaDeOkno
Copy link
Author

FaDeOkno commented Jul 13, 2024

Reptilians can grab without using hands at all levels, including apparently choking other players hands-free with their tail. They should probably use hands for the higher tier grabs like others?

Im not sure about it

When Player1 releases Player2, Player2 gets a popup saying "You are trying to escape" rather than telling them that Player1 released them

oh. will fix that rn

When a player tries to escape a grab, there is no proper communication from the game about what's happening, is this actually taking some time? Did it succeed did it fail?

There are no popup showing that you're trying to escape?
You have random chance to escape, but also there are 1 second delay for avoiding too fast clicks

The grabbing player is also not notified in any way about attempted and succesful escapes

Would be fixed

There does not seem to be a way for the Grabbing player to reduce the level of the grab (ie stop choking the target)

Maybe it would be good to be able to lower the level by dropping virtualitem and clicking on the alert while in combat mode, but Im not sure

The PULLED and PULLING alert icons should display the current grab level

I don't have such icons yet. I can draw them to display the type, but it probably will be awful

@bruhmogus

This comment was marked as off-topic.

@ninruB

This comment was marked as off-topic.

@Errant-4
Copy link
Contributor

OH DEAR GOD NO INTENTS fucking dies

Ah yeah that's a somewhat fair point, the title should probably be changed given that no literal intents are actually involved (which is good)

@FaDeOkno FaDeOkno changed the title Grab intent Grab Jul 13, 2024
@TheDoctor1977
Copy link

TheDoctor1977 commented Jul 13, 2024

Whats so bad about intents? I personally like them and we did have a dedicated disarm intent action a while ago.

I personally just find them to be unnecessarily clunky for things that could be done contextually (in the same way that we killed disarm intent by just making it right click to disarm)

@bruhmogus
Copy link

Whats so bad about intents? I personally like them and we did have a dedicated disarm intent action a while ago.

I personally just find them to be unnecessarily clunky for things that could be done contextually (in the same way that we killed disarm intent by just making it right click to disarm)

we could also make it so that trying to ctrl click on someone you're already pulling increases the grab level, so like one click to soft, two clicks to hard, three to fucking murderise

@lzk228
Copy link
Contributor

lzk228 commented Jul 13, 2024

should be an animation when you trying to grab (like punch/push animation)

@FaDeOkno
Copy link
Author

FaDeOkno commented Jul 13, 2024

Whats so bad about intents? I personally like them and we did have a dedicated disarm intent action a while ago.

I personally just find them to be unnecessarily clunky for things that could be done contextually (in the same way that we killed disarm intent by just making it right click to disarm)

we could also make it so that trying to ctrl click on someone you're already pulling increases the grab level, so like one click to soft, two clicks to hard, three to fucking murderise

But that's literally how it works here

@bruhmogus
Copy link

Whats so bad about intents? I personally like them and we did have a dedicated disarm intent action a while ago.

I personally just find them to be unnecessarily clunky for things that could be done contextually (in the same way that we killed disarm intent by just making it right click to disarm)

we could also make it so that trying to ctrl click on someone you're already pulling increases the grab level, so like one click to soft, two clicks to hard, three to fucking murderise

But that's literally how it works here

i didnt read any of that i just saw intent and had an allergic reaction

@FaDeOkno
Copy link
Author

Whats so bad about intents? I personally like them and we did have a dedicated disarm intent action a while ago.

I personally just find them to be unnecessarily clunky for things that could be done contextually (in the same way that we killed disarm intent by just making it right click to disarm)

we could also make it so that trying to ctrl click on someone you're already pulling increases the grab level, so like one click to soft, two clicks to hard, three to fucking murderise

But that's literally how it works here

i didnt read any of that i just saw intent and had an allergic reaction

lol

@FaDeOkno
Copy link
Author

Edit Also, while obviously you can't fully describe a change this big in the changelog, you can definitely do better than "Added grab system". Consider that many players have never played Space Station 13, what does this tell them about what this is and how to use it? nothing

uh
i can try
maybe.

@FaDeOkno
Copy link
Author

virtual items are cursed

@FaDeOkno
Copy link
Author

image
I guess this looks pretty ok for the alert

@deepdarkdepths
Copy link
Contributor

How do you throw people using this?

@slarticodefast
Copy link
Member

should be an animation when you trying to grab

I agree that this needs better visual cues or people will start choking each other by accident without noticing.

There should also be popups for all stages for the grabber, the grabbed person and third persons (at least if the animation does not show this clearly).
"You are being grabbed by Urist McHands"
"You grab Urist McScales"
"Urist McHands grabs Urist McScales"

"Urist McHands starts choking you"
"You start choking Urist McScales"
"Urist McHands starts choking Urist McScales"

Especially throwing ist not visually distinct from pulling. Maybe throwing someone should rotate his sprite like it does for objects?

If I am a security officer watching a fight I should be able to see if they are just pulling each other or choking/throwing to be able to decide if I need to step in.

@FaDeOkno
Copy link
Author

should be an animation when you trying to grab

I agree that this needs better visual cues or people will start choking each other by accident without noticing.

There should also be popups for all stages for the grabber, the grabbed person and third persons (at least if the animation does not show this clearly). "You are being grabbed by Urist McHands" "You grab Urist McScales" "Urist McHands grabs Urist McScales"

"Urist McHands starts choking you" "You start choking Urist McScales" "Urist McHands starts choking Urist McScales"

Especially throwing ist not visually distinct from pulling. Maybe throwing someone should rotate his sprite like it does for objects?

If I am a security officer watching a fight I should be able to see if they are just pulling each other or choking/throwing to be able to decide if I need to step in.

There are these popups. Video has first version of grab so it doesnt have them.

I'm currently trying to rewrite throwing because its awfully made. But rotating thrown person will be looking... Not really good

@metalgearsloth metalgearsloth added the Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. label Jul 18, 2024
@FaDeOkno
Copy link
Author

  1. I would recommend splitting the PR out if you want to get it through review faster.

Is it necessary?

  1. Please read through https://docs.spacestation14.com/en/general-development/codebase-info/conventions.html

Okay, I'll structure my files soon

@EmoGarbage404
Copy link
Member

Yeah please move all the CQC stuff out as it'll make reviewing significantly easier

@FaDeOkno
Copy link
Author

Yeah please move all the CQC stuff out as it'll make reviewing significantly easier

okay

@FaDeOkno FaDeOkno changed the title Grab & Martial Arts - Ready for review Grab Jul 19, 2024
@FaDeOkno
Copy link
Author

what
why
It exsists.
image

@deltanedas
Copy link
Contributor

its private not protected

@FaDeOkno
Copy link
Author

its private not protected

I've never had such problem without making it protected. And checks are not finding it for some reason

@M1and1B
Copy link

M1and1B commented Jul 22, 2024

Heh, good job, my friend

@FaDeOkno
Copy link
Author

  1. Needs a design doc.

Well I've made a small doc, but there are no any reaction to it yet

@FaDeOkno
Copy link
Author

Waiting.

@deepdarkdepths
Copy link
Contributor

Give me grab or give me death

@Tornado-Technology
Copy link
Contributor

Okay all shit with netPredict fixed, xd
Link: https://github.com/Tornado-Technology/space-station-14/tree/grab-intent

@Tornado-Technology
Copy link
Contributor

Okay all shit with netPredict fixed, xd Link: https://github.com/Tornado-Technology/space-station-14/tree/grab-intent

I really changed only one system and two components, but I don't give a fuck, just like I don't give a fuck about creating client systems for the sake of popups.

@FaDeOkno you can take my solution

@Tornado-Technology
Copy link
Contributor

Tornado-Technology commented Aug 2, 2024

  1. Needs a design doc.
  2. I would recommend splitting the PR out if you want to get it through review faster.
  3. Please read through https://docs.spacestation14.com/en/general-development/codebase-info/conventions.html

@metalgearsloth I'll ping you since the author didn't provide a link to his docs: space-wizards/docs#261

Copy link
Contributor

@Tornado-Technology Tornado-Technology left a comment

Choose a reason for hiding this comment

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

It's best not to make changes to code you haven't gotten into

Content.Server/Movement/Components/PullMovingComponent.cs Outdated Show resolved Hide resolved
@@ -1,4 +1,4 @@
using System.Linq;
using System.Linq;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think u dont need linq for this

Copy link
Author

Choose a reason for hiding this comment

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

I didnt touch this line

Resources/Prototypes/Roles/Jobs/Civilian/chef.yml Outdated Show resolved Hide resolved
Content.Shared/Weapons/Melee/SharedMeleeWeaponSystem.cs Outdated Show resolved Hide resolved
@github-actions github-actions bot added Status: Needs Review This PR requires new reviews before it can be merged. and removed Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. labels Aug 2, 2024
@Pumkin69
Copy link

Pumkin69 commented Aug 2, 2024

Instead of doing a full RNG % system to break out of grabs i recommend making it a short do-after that is uninterruptible and making it similar to how league of legends does crit chance and doing a markov chain to do RNG instead of just a straight % chance. This would make it so its not just lottery on if you get out in the first try or the 20th (or at least it will help a lot). This will be hard to code i assume but i have no fucking clue i just think its a better system than full RNG cause its not fun imo to just gamble on stuff like shoves for example it feels skilless and unfun. So to better state this system simply makes it so everytime you try to break free you gain more % on the next try. So for example on that it would be X% for the first time and then go up and up every time you try and break out by whatever arbitrary X% you think would be balanced.

Copy link
Contributor

github-actions bot commented Aug 7, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the Merge Conflict This PR currently has conflicts that need to be addressed. label Aug 7, 2024
@deathride58 deathride58 added the Undergoing Maintainer Discussion This PR is currently going through the 72-hour discussion window as per maintainer policy label Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: Sprites Should be reviewed or fixed by people who are knowledgeable with spriting or visual design. Merge Conflict This PR currently has conflicts that need to be addressed. Status: Needs Review This PR requires new reviews before it can be merged. Undergoing Maintainer Discussion This PR is currently going through the 72-hour discussion window as per maintainer policy
Projects
None yet
Development

Successfully merging this pull request may close these issues.