squeek502 / WailaHarvestability

Minecraft mod that adds required harvest level and the effective tool to Waila tooltips of harvestable blocks
The Unlicense
17 stars 21 forks source link

[1.10.2] <ERROR> in tooltip with Tinkers tools #49

Closed Nargath closed 7 years ago

Nargath commented 7 years ago

Mod Versions: Minecraft: 1.10.2 Tinkers: 1.10.2-2.5.2 WAILA: 1.7.0-B3_1.9.4 WAILA Harvestibility: 1.10.2-1.1.8

When I have any Tinkers tool equipped and look at nearly any block, I get the WAILA tooltip, then says , then the source underneath.

waila harvest tinkers error

GirafiStudios commented 7 years ago

Thanks for the report! This is caused by that Tinkers Construct have recently added support for that certain tools can mine blocks from your off hand. I will talk to Squeek about what the best approach to fix this is :)

squeek502 commented 7 years ago

@GirafiStudios I'll be unavailable on IRC until Sunday, so let's discuss the possible solutions here. Do you know where the error is coming from exactly?

GirafiStudios commented 7 years ago

Yeah. It is the harvest level. https://github.com/squeek502/WailaHarvestability/blob/1.10.x/java/squeek/wailaharvestability/WailaHandler.java#L119 , ToolHelper.canToolHarvestLevel

squeek502 commented 7 years ago

That's just a proxy method that calls ForgeHooks.canToolHarvestBlock(blockAccess, blockPos, tool), so somehow that's erroring?

GirafiStudios commented 7 years ago

It is because of the itemHeld parameter, which only checks for "player.getHeldItemMainhand()". Tinkers Construct checks both hands. Here is the error it throws: https://gist.github.com/GirafiStudios/cfceaacb90e0dcd614e4722601047ba0 , so it is a bit easier to track down :)

squeek502 commented 7 years ago

How would that be fixed on our end? Wouldn't this be a TiC bug regardless?

GirafiStudios commented 7 years ago

I'm pretty sure this is intended behaviour from TiCons side, since some of their tools now can harvest the block in the off hand.

squeek502 commented 7 years ago

Do you mean "it's now possible" or "you HAVE to harvest using the offhand". If it's the former, then this is a TiC bug. If it's the latter, then it should still be fixed on TiC's end (throwing an error is not the way to handle this).

EDIT: Mixed up former/latter

GirafiStudios commented 7 years ago

It is now possible for some tools. The warning is actually thrown because of some Waila code: http://prntscr.com/cl4u0p

squeek502 commented 7 years ago

Right, it's just catching errors so they don't crash the game.

Reporting this on TiC's tracker; we should always be able to expect ForgeHooks.canToolHarvestBlock to be safe to call with the item in the main hand.

squeek502 commented 7 years ago

So turns out that this is caused by ForgeHooks.canToolHarvestBlock giving a null player object when calling Item.getHarvestLevel, and TiC not doing a null check on it.

However, we should probably not use that Forge function and instead write our own version (it's a pretty simple function) that does use the the player parameter, as that could give more accurate results (although this sort of mainhand/offhand stuff might get confusing).

GirafiStudios commented 7 years ago

I will look into that now :)

GirafiStudios commented 7 years ago

There we go :) Did a few other minor things while I were at it, listed them in the commit message. Is there any other places to contact you @squeek502, when you don't have access to IRC ? :)