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

Fix hardcoded diggingface for cancel digging #3322

Merged

Conversation

Vinciepincie
Copy link
Contributor

Previously the packet that cancels the digging was hardcoded to face 1. I don't see a reason why the cancelling face would be different than the digging face.

On a server i noticed the block often doesnt break anymore when breaking something with hand again after cancelling. With this change this issue doesnt seem to happen so it possibly triggered anticheat or some glitch before.

@zardoy
Copy link
Contributor

zardoy commented Mar 3, 2024

I don't see a reason why the canceling face would be different than the digging face.

IMO MC protocol is absolutely nonsense in some scenarios, many things are hardcoded at the protocol level. Its better to look at the actual packets that the original client sends. In the case of block digging canceling (status 1) the face should always be 0 and not 1. Most probably some plugin on the server just checks the face against 1 and then kicks you for obvious reasons.

@zardoy
Copy link
Contributor

zardoy commented Mar 5, 2024

@Vinciepincie did you get a chance to test with the proposed change? I had the same issue and imo this issue should be prioritized

@Vinciepincie
Copy link
Contributor Author

@Vinciepincie did you get a chance to test with the proposed change? I had the same issue and imo this issue should be prioritized

Yes it seems to works fine now.
I also tested by sniffing packets with a vanilla client (1.20.4) with this tool https://github.com/adepierre/SniffCraft
and it's indeed hardcoded to 0 as you said

@zardoy
Copy link
Contributor

zardoy commented Mar 13, 2024

@Vinciepincie so is this ready now?

@Vinciepincie
Copy link
Contributor Author

@zardoy
yes

@rom1504
Copy link
Member

rom1504 commented Mar 17, 2024

seems valid to me

we need a lot more tests for this though

@rom1504 rom1504 merged commit ab78bf8 into PrismarineJS:master Mar 17, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

None yet

3 participants