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 player interact issues #644

Merged
merged 4 commits into from Dec 31, 2017

Conversation

Projects
None yet
3 participants
@kaenganxt
Member

kaenganxt commented Dec 28, 2017

This PR fixes several issues with the PlayerInteractEvent and the rightClickBlock() and rightClickAir() methods of ItemType.

The following bugs are fixed:

  • Block clicks fired an additional interact event with left click air (this was checked in PlayerSwingArmHandler and failed because GlowLivingEntity#getTargetBlock returned wrong values)
  • Right clicking on a block with an unplaceable item (e.g. a nether star) fired an additional right click air event because the client sends the place block and use item packets in this case
  • When trying to place a placeable block and it fails (e.g. tall grass on stone), the client only sends a use item packet, which only fired a right click air interact. There's a right click block now fired instead (The block and block face values are computed by the UseItemHandler). This includes handling all internal right click actions.
  • Water behind a wall could be pickup up with a bucket
  • In some cases, interacting with blocks was possible even when sneaking and a block in the hand. (Solved by requiring both hands to be empty)

Other changes:

  • Emptying a bucket in survival mode now changes the item type to BUCKET
  • Implemented rightClickBlock handlers for all ItemTypes which previously depended on rightClickAir even if a block was clicked
  • Cleaned up boat placement

There still is an issue with this implementation:
When clicking next to an interactable block (e.g. a lever), the lever is still flipped because the current raytrace (BlockIterator) ignores the size of the blocks. Also, the clickedAt vector in the ItemType#rightClickBlock method doesn't have correct values in it because of this problem.
So a proper raytracing method has to be implemented in the future, which would be useful also for other things (like filling a bucket).

Everything regarding the PlayerInteractEvent has been tested in comparison to Spigot behaviour. The results were not equal in all cases, but that's because Spigot isn't handling everything correctly itself ;) (Some interacts even seem quite random).

One difference I noticed was that for right and left click air events, Spigot always used BlockFace.SOUTH whereas we are setting the value to null. So there may be plugins that don't expect a null value here. Should we also set the face to SOUTH (which doesn't really make sense), should we use something more general like BlockFace.SELF or should we still use null?

@Override
public void rightClickBlock(GlowPlayer player, GlowBlock target, BlockFace face, ItemStack holding, Vector clickedLoc, EquipmentSlot hand) {
// Two cases are handled here: Either the player clicked on a block on the land or beneath water

This comment has been minimized.

@mastercoms

mastercoms Dec 29, 2017

Member

I can't place boats under water in vanilla.

This comment has been minimized.

@kaenganxt

kaenganxt Dec 31, 2017

Member

I fixed boats being placeable underwater. The placement check is not perfect as vanilla seems to do a bounding box check, but I think it's good enough for now.

@@ -19,6 +41,8 @@ public void rightClickAir(GlowPlayer player, ItemStack holding) {
.getRecipeByKey(recipe), true);
}
}
holding.setAmount(0);

This comment has been minimized.

@mastercoms

mastercoms Dec 29, 2017

Member

It should also set the type to air.

This comment has been minimized.

@kaenganxt

kaenganxt Dec 30, 2017

Member

Both handlers create an empty stack if necessary (example) after executing rightClickAir or rightClickBlock.

This comment has been minimized.

@mastercoms

mastercoms Dec 31, 2017

Member

Then why did knowledge books stay in my inventory after use?

@mastercoms

This comment has been minimized.

Member

mastercoms commented Dec 29, 2017

We should be giving an air block with a face of SELF in the event.

@mastercoms mastercoms merged commit 8930aba into GlowstoneMC:dev Dec 31, 2017

2 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@kaenganxt kaenganxt deleted the kaenganxt:interactFixes branch Jan 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment