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 EntityInteractCallback not reaching server if canceled on client #139

Open
wants to merge 1 commit into
base: 1.20.1
Choose a base branch
from

Conversation

PssbleTrngle
Copy link

This is a proposed fix for #135

This changes the cancelation of the injected method to happen after the ServerboundInteractPacket has been send to the server instead of before. This means that returning a non-null InteractionResult on client now still triggers the EntityInteractCallback on the server too instead of omitting it.

I have also added an example callback to the test mod to check if this actually solves the issue, I have left it in, but can also remove it again if you want to.

@AlphaMode
Copy link
Member

I believe not sending the packet is intentional, but I would have to double check the forge patch

@PssbleTrngle
Copy link
Author

PssbleTrngle commented Oct 17, 2024

I very much believe it is not intentional or should at least not be, because it leads to the following problem:

  1. When adding a custom interaction using the callback, the developer can either return null on client side, causing the event to be propagated to the server (were it usually is meant to be because most of the logic is happening there).
  2. Doing this will cause the client not to display the correct interaction animation (none at all)
  3. it will also not only be the only way to get the EntityInteractCallback to run on server-side, but also still run the default interaction logic on client side, since that is now not cancelled in any way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants