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 grab delay after performing an action #4494

Closed

Conversation

Git-Nivrak
Copy link
Contributor

@Git-Nivrak Git-Nivrak commented Sep 24, 2023

About the pull request

First there was an issue where you could grab a marine behind a cade to bypass grab delay and hit it faster than intended.
Which was then fixed by #4204 which in turn fucked your ability to grab things without delay and then attempted to be fixed by #4246 which only partially fixed it.

This PR fixes it with a simpler solution that doesn't affect any other click delays, or actions. By removing a mechanic which automatically attacks the "barricade" (can also be a mob, just the general name for something in the way of you grabbing someone else) when attempting to grab it fails.

Explain why it's good for the game

Fixes an annoying bug that's been up for a while

Testing Photographs and Procedure

Screenshots & Videos

Put screenshots and videos here with an empty line between the screenshots and the <details> tags.

Changelog

🆑
balance: xenos no longer have grab delay via any grab method
/:cl:

@github-actions github-actions bot added the Fix Fix one bug, make ten more label Sep 24, 2023
@Drulikar
Copy link
Contributor

Drulikar commented Sep 24, 2023

This should also has the side effect of removing a few ways xenos bonk into barricades.

Edit
In testing though runner/lurker pounce still has the bonk handled a different way so that was my main concern. Warrior lunge now just doesn't hit the barricade.

@Drulikar Drulikar added the Balance You need to be a professional veteran game maintainer to comprehend what is being done here. label Sep 24, 2023
@Git-Nivrak
Copy link
Contributor Author

This should also has the side effect of removing a few ways xenos bonk into barricades.

Edit In testing though runner/lurker pounce still has the bonk handled a different way so that was my main concern. Warrior lunge now just doesn't hit the barricade.

If that's the case imo it's fine, About warrior I think that not having an unintentional slash is actually a good thing all in all

Copy link
Member

@morrowwolf morrowwolf left a comment

Choose a reason for hiding this comment

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

Alright so before this is merged we need to make sure ctrl-click and grab intent click are functioning effectively the same way.

My current understanding with this change is grab intent click is still going to have the delay.

Still thinking about it otherwise. I don't have a huge issue with the ctrl-click delay 🤷

@morrowwolf morrowwolf marked this pull request as draft September 26, 2023 10:45
@Git-Nivrak
Copy link
Contributor Author

That is the way it was before the initial bugfix for the infinite slash problem, that is ctrl + click not having delay and grab intent click having delay, so I brought it back in line.
Now either way I think it's important to either leave it like that or make grab click have no delay as well because grabbing + disarming is something that is used a lot and the delay makes it feel really bad (Suppose you disarm someone, then you grab them to devour then by the time you are able to click them again you lost a disarm) or if you are trying to grab someone mid combat which is already hard when moving then it just doesn't go through because you have a delay from disarming / slashing.
Let me know what you decide

@Drulikar
Copy link
Contributor

Drulikar commented Sep 26, 2023

Morrow is saying if Ctrl+click grab is not going to have a delay, then grab intent needs to not have a delay. Currently without this pr they behave the same. If this pr would be merged as is, then once again Ctrl click would be delayless and grab intent would have a delay.

@Git-Nivrak
Copy link
Contributor Author

Now they are both delay-less

@Git-Nivrak Git-Nivrak marked this pull request as ready for review October 2, 2023 19:16
@morrowwolf morrowwolf removed the Fix Fix one bug, make ten more label Oct 4, 2023
code/_onclick/click.dm Outdated Show resolved Hide resolved
@Drulikar Drulikar marked this pull request as draft October 5, 2023 03:22
@Git-Nivrak Git-Nivrak marked this pull request as ready for review October 5, 2023 11:43
@Drulikar Drulikar marked this pull request as draft October 6, 2023 12:41
@Drulikar
Copy link
Contributor

Drulikar commented Oct 6, 2023

Previous feedback is not resolved.

@Git-Nivrak
Copy link
Contributor Author

Shouuuld be all good now
Tested the following:
Grab intent into disarm on xeno -> No delay
Disarm into grab intent on xeno -> No delay
Disarm into grab into disarm again -> Delay on second disarm
No infinite bashing with items on grab intent

@Git-Nivrak Git-Nivrak marked this pull request as ready for review October 6, 2023 16:17
Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

Issue above no longer appears to be reproduceable. This should now be suitable to TM.

code/_onclick/click.dm Outdated Show resolved Hide resolved
@Drulikar Drulikar added the Testmerge Candidate we'll test this while you're asleep and the server has 10 players label Oct 6, 2023
Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

Single letter vars bug me but otherwise this needs testing to make sure there isn't something else I didn't catch.

Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in getting this TM'd and tested more, but it seems the grab intent rapid attacking of cades and fences is back. I can see about getting a reproduction in a bit if you like.

@Drulikar Drulikar marked this pull request as draft October 14, 2023 13:23
@Drulikar
Copy link
Contributor

spam.mp4

Should be pretty easy to reproduce just using the grab intent and click objects.

@Drulikar
Copy link
Contributor

Please work out a way that both ctrl+click and grab intent behave the same - not that one way is superior to the other.

@github-actions
Copy link
Contributor

This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being auto closed in ~ 7 days, please address any outstanding review items and ensure your PR is finished, if these are all true and you are auto-staled anyway, you need to actively ask maintainers if your PR will be merged. Once you have done any of the previous actions then you should request a maintainer remove the stale label on your PR, to reset the stale timer. If you feel no maintainer will respond in that time, you may wish to close this PR youself, while you seek maintainer comment, as you will then be able to reopen the PR yourself

@github-actions github-actions bot added the Stale beg a maintainer to review your PR label Oct 23, 2023
@github-actions github-actions bot closed this Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Balance You need to be a professional veteran game maintainer to comprehend what is being done here. Stale beg a maintainer to review your PR Testmerge Candidate we'll test this while you're asleep and the server has 10 players
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants