TerraFirmaCraft

TerraFirmaCraft

2M Downloads

Knapping GUI overwrites right click events

UnlimatedStone9 opened this issue ยท 2 comments

commented

Describe the bug

  1. What did you expect to happen?
    Be able to place clay (or any other knappable item) into a container using right click (examples being Tinkers' Construct drying racks, PyroTech Compacting Bins or Immersive Craft chests)

  2. What actually happened instead (i.e. what was the bug)
    The knapping GUI opened when it shouldn't

To Reproduce
Find a container that you can place items into using right click.
Attempt to place a knappable object into that container

Meta Info

  • TFC Version: 1.12.2-0.23.2.70

  • If necessary, what other mods (including versions) MUST be present to experience the bug?
    Any mod that has a container you can place items into using right click rather than opening a GUI, examples being;
    Tinkers' Construct's drying racks
    PyroTech's compacting bins
    Immersive Craft's chests, etc...

commented

From a little testing, it appears only clay balls have this issue but I could be wrong.
Any clay from 1-4 can be placed, but once you have 5 (the required amount to knapp), you always open the GUI, this is tedious.

commented

This is likely due to the fact that we hook clay by intercepting PlayerInteractEvent.RightClickBlock when using vanilla clay. Based on that result, Block#onBlockActivated is called next, and ItemStack#onUseItem is called second. As there's no additional events for intercepting logic of non-TFC items usage, we end up superseding the onBlockActivated call.

This might also apply to charcoal, as it can be used to create charcoal piles which again, will happen at a higher priority than interacting with a block.

One possible way to solve this is to move from an event handler to a registry override. This would fix the issue, only with vanilla clay, and provided no other mod registry replaces vanilla clay.

Alternatively, we could export and duplicate the logic from the call paths of ForgeHooks.onRightClickBlock in the placement logic. Thus, when the event is fired, redirect to our own placement logic with additional handlers for clay at the appropriate times. This is probably the better solution overall, although might be more difficult to get working correctly.