SlimeKnights / TinkersConstruct

Tinker a little, build a little, tinker a little more...
MIT License
1.23k stars 778 forks source link

Custom Block hardness from IguanaTweaks Reborn messes up the mining speed of tools #4840

Closed Insane96 closed 2 years ago

Insane96 commented 2 years ago

Minecraft Version

1.18.1

Forge Version

39.1.2

Mantle Version

1.8.37

Tinkers' Construct Version

3.4.2.60

Describe your issue

When using IguanaTweaks Reborn to change block hardnesses, blocks can change how they're mined with tool. For example (in the video), I changed Nether Wart Blocks hardness and even if you shouldn't be able to mine it like stone and logs, you can still mine it.

https://youtu.be/XWjEXwvF7Tw

Crash Report

No response

Other mods

IguanaTweaksReborn-2.9.3-mc1.18.1.jar
InsaneLib-1.4.3-mc1.18.1.jar

Tried reproducing with just Tinkers?

No, Tinker + Iguana Tweaks Reborn + Libs

Performance Enchancers

None of the above

Searched for known issues?

Checked pinned issues, Searched open issues, Searched closed issues, Checked the FAQ

Insane96 commented 2 years ago

Definitely a mod iteraction issue with IguanaTweaks Reborn, when hardness of blocks is changed

https://github.com/Insane96/IguanaTweaksReborn/blob/9bca167f167f7ca87b1c91c7f03c6a832b70345a/src/main/java/insane96mcp/iguanatweaksreborn/module/mining/feature/CustomHardness.java#L95

KnightMiner commented 2 years ago

I'd call this a bug in iguana tweaks, other mods successfully change hardness in the proper getter, changing it using the event is a really awkward approach. At the least iguana should be setting an event priority.

Anyways, as with all mod interaction bugs you should crosspost to their tracker

Insane96 commented 2 years ago

I wanted to use events to prevent changing something that's not public and since Forge devs always advertise to use events whenever possible. Well, gonna change that.

KnightMiner commented 2 years ago

I'd argue use events where practical is a better approach. The goal of modding should be maximizing compatibility, and in this case an awkward event implementation is worse than simply an AT making the field non-final. There are a lot of edge cases with changing hardness via ratio, more if some of my plans end up going through.

Anyways, if you have to use the event, use it properly. At normal priority your handler is running at unknown timing with many mods also using the event. The logic you had implied it should be the last logic running to me

Insane96 commented 2 years ago

Well, since I no longer have the 1.18.1 code gonna wait 1.18.2 and see if a priority fixes it

KnightMiner commented 2 years ago

Going to close this as there is nothing to fix on our side. Event priority is probably the solution to solve it if you don't want to override the property, but as I said setting the prop directly will give you better overall compat (especially with mods that display hardness values)