alcatrazEscapee / tinkers-forging

A Minecraft mod that adds TFC Style Forging to 1.12+
https://minecraft.curseforge.com/projects/tinkers-forging
GNU General Public License v3.0
15 stars 5 forks source link

Trying to right click containers while holding charcoal places charcoal instead of opening container #15

Closed SeriousCreeper closed 5 years ago

SeriousCreeper commented 5 years ago

Heya,

placing charcoal on the ground seems to have a higher priority than interacting with inventories. In the first case i tried to right click the clay vessel from charcoal pit, but it placed the charcoal on the ground instead (without opening the inventory UI at all). clay vessel

Here is an example with a primal core shelf that doesn't have a UI, but instead the charcoal should just be placed on it when right clicking. Instead it places the charcoal on the ground. shelf

alcatrazEscapee commented 5 years ago

Yeah, I know about this - don't know how to fix it at the moment. (although I'm sure its doable).

SeriousCreeper commented 5 years ago

Looking at the code, you only allow adding charcoal if the player clicked on an up facing side. But in the 2nd check for the initial piece of charcoal, you allow placing the charcoal even when clicking the side of a block.

Perhaps only allowing to place the initial piece of charcoal on an up facing side would help (check if the face equals EnumFacing.UP in that 2nd if statement as well)? Unless you really want to be able to place the first piece by clicking on a side of a block.

alcatrazEscapee commented 5 years ago

The effect would still be there, just restricted to a different face though. Although this would probably solve most use cases.

SeriousCreeper commented 5 years ago

I'm guessing a lot of inventory container wouldn't match the isNormalCube() check right? I think it would only still try to lace if you try to right click on the top face of a container that returns isNormalCube as true. But yeah it's not ideal.

BigRenegade commented 5 years ago

Having checked thru the code, it looks like the issue might actually be in the Core and not the Mod itself. In the core (ContainerItemStack) looks like where the issue might be occurring from. I cannot be certain on this as I have not downloaded and run thru the code, but it is a logical place to start as I see nothing in the actual mod which might be causing this.

BigRenegade commented 5 years ago

Another spot to look is in the CoreHelpers code. This is where you have the dropItemInWorld code. Give me a couple of days to get thru this block in my code and I'll download this and run thru the code (ingame) and see if i can find where the issue actually is. No guarantees but I'm usually pretty good at finding bugs in code. If I do find it I will make a comment here so you can work on it.

alcatrazEscapee commented 5 years ago

Relevant spot is here. This should have nothing to do with dropItemInWorld / ContainerItemStack

BigRenegade commented 5 years ago

Ok. I see where you are working with it. I'll check into it more tomorrow and see if I can find the full issue and just what is causing it. Debugging my own code is going quicker and easier than I figured.

alcatrazEscapee commented 5 years ago

I know exactly what is causing it... The event checks if there charcoal placement is valid and then places, cancelling the right click event which opens the GUI. I don't know how to avoid said problem aside from what has already been suggested.

BigRenegade commented 5 years ago

try commenting out lines 187 - 200.  This is the part of the code that places it on the ground

alcatrazEscapee commented 5 years ago

That just removes the ability to place charcoal anywhere, which doesn't solve this issue at all.

LeoBeliik commented 5 years ago

You can replace the RightClickBlock event with RightClickItem event and work around that... Another solution could be using a charcoal/coal block to form the multiblock and keep the Rclick function to add fuel.

BigRenegade commented 5 years ago

you could also have it do a check to see if the right clicked item has an inventory, if so do not execute block placing code

alcatrazEscapee commented 5 years ago

I've added the check for placement above, should be fixed in 1.1.0. That should fix most use cases. If this is still an issue the I'll revisit this.