SpongeAPI
SpongeAPI copied to clipboard
Inconsistent behavior with InteractItemEvent interaction point
The interaction point of an InteractItemEvent
when clicking on a block has two related issues:
- Though the interaction point is a
Vector3d
, components are always rounded down. My expectation is that, as aVector3d
, it would contain the actual hit position of the raycast rather than only the block position as a double. This may be more of an implementation issue than api. - As a result of the above, it is not possible to obtain the block being interacted with by the item reliably. If the interaction point is on the north, east, or bottom faces, the interaction point matches the block position of the targetted block. Otherwise, the position is the block position next to the target block along the face clicked, which is caused by the rounding of the position.
After speaking with @bloodmc, the intended way to handle this is through the InteractBlockEvent
directly. This requires canceling InteractItemEvent
s for items that don't have an interaction point, then handle it again in the InteractBlockEvent
if the interacted block doesn't match and the item does. This is a workable solution, however does require checking the item data twice (as well as being less intuitive).
Additionally, there might be a potential issue here with Forge compatibility. The javadocs for the PlayerInteractEvent.RightClickBlock follow both a different order (block them item) as well as passing any subsequent events if it's canceled. I don't know much about the status of this, but I think it's worth bringing up as there definitely seems to be room for error here.
On the original topic, I would propose the following as solutions:
- Have the interaction point for
InteractItemEvent
use the correctVector3d
point (I'm not sure why this is being changed, so this may be a bit complex) - Add a
getInteractionBlockPosition
(or something) method to the event that returns aVector3i
corresponding to the position of the block being interacted with. As from above, this is not the same as the floor of the interaction point.
@SimonFlash
The first point is an implementation issue. Namely where this is called is basically passing off what Vanilla is giving us at this time. We could raytrace always, that has came up before but we never settled on either way.
Secondly, I don't want to add a getInteractionBlockPosition namely because we're not trying to make the assumption here that an InteractItem means a block position is in play. The API doesn't enforce that contract and MCP isn't the only implementation.
@Zidane
Could we make ray tracing easier for plugins that need it then? I'm not sure what a plugin attempting to do it manually would look like at the moment.
Adding a ray trace from the plugin end is easy enough, but it has to be done carefully to consider things like the player's reach. Either way, I don't believe this is something the developer should be responsible for; it should be on the API to provide that location.
If the worry is that the name getInteractionBlockPosition
implies that an actual block is being interacted with, then change it. I personally don't see this being an issue as it matches the naming conventions of Location#getBlockPosition
and many other parts of the API. As in the original post, this position is not necessarily the same as the interaction point due to rounding with directions. The only other way to get this would be through a raytrace, which shouldn't be necessary.
I believe part of the consideration here too needs to be on the order the events are fired in. From what I can tell, Forge doesn't have this because their RightClickBlock
event is fired before RightClickItem
, which actually isn't called if a block is interacted with successfully (see here). This is something I think needs to be addressed because right now I have no idea what the order of these events is intended to be and to what extent they affect eachother.
Does the item being interacted with get to choose the interaction point at all? I could see different tools/blocks acting differently.
Edit: See https://github.com/SpongePowered/SpongeCommon/issues/1799 for an opposing view point on why ray tracing isn't ideal.