Mod interaction issue ToolProgression
tyra314 opened this issue · 9 comments
Hi,
I got reported an issue for my mod ToolProgression and I'm not sure, who's to blame. And because we have incompatible mappings used, I can't debug it right now. The issue is here: tyra314/ToolProgression#6
TL;DR Client Crash, when a chiseled minecraft:stone block is broken as a player.
- MC Version: 1.12
- C&B Version: 14.4
- ToolProgression: 1.2.4
- Do You have Optifine: no
I reproduced it with only the two mods loaded: https://gist.github.com/tyra314/b725e2ecbe0bbe252acd390c960d05a8
Have you seen something like this yet, or do you have an idea, where the problem is?
I've not seen the issue yet, but I have a vague understanding of what the issue is. C&B dose some pretend shenanigans to try and figure out weather a tool can break its block by pretending to be the most common block inside of it, which is obviously something its not.
https://github.com/AlgorithmX2/Chisels-and-Bits/blob/1.12/src/main/java/mod/chiselsandbits/chiseledblock/BlockChiseled.java#L1132
And its slightly varied twin a few lines down are likely the issue, state in that context is the block its pretending to be, while pos and the player, and the player's world is obviously a real thing.
Its all quite precarious to be sure, a lot of assumptions going on about how vanilla works, which simply don't hold up with your mod in play.
And yeah.. my mappings are practically ancient at this point, trying to keep merge-ability between versions has a cost.
This is probably going to require some more digging, I imagine that I wouldn't be passing the pretend state to getDigSpeed if it wasn't required, but at the same time if its required, then its going to crash with your mod.
Sounds like the only real solution at a glance is removing the C&B relative block hardness emulation.
It could work for this one case, however I fear that the problem will only re-emerge later. To be fair I'm basically throwing dynamite into the wind and hoping it won't explode.
I wish that dig speed wasn't so darn complicated, but nothing to be done about that. I should probably remove this emulation and try to re-create it a second way. If that's not doable then I suppose removal would be for the best, its just too dangerous to do when there are events in play.
Tbh, I don't think that what I'm doing there is only a sliver better. I'm doing evil things and hope for the best. Seems your dynamite has hit me :)
While I usually appreciate good solutions, I wonder how many other mods may mess with canHarvestBlock()
inside of the BreakSpeed
event.
Btw. not all blocks are affected, for instance chiseled dirt can be broken just fine.
I wonder if the reason for the non-crashing dirt is on your end? I wouldn't think that I treat it much differently then the other blocks.
I guess I can agree, that for the most part this sort of issue is rare, and has been, since the code in question has been in the wild for almost 2 years and so far this is the first I've seen of a problem from it; at least in this regard.
If you think that a fix on your end might be easy and solve the crash, maybe we can push off submitting to our dark overlords for a time longer.
I will admit I'm not sure when I'm going to find the time to really sit down into looking for an alternate solution regardless, its a pretty messy problem to be sure, and if one solution can be released into the wild sooner it might be for the best.
Well, I'd need two things for a quick workaround.
a) detect your fake BlockState, which should be easy, just check for the ChiseledBlock, right?
b) a way to get the required harvest level from the fake BlockState. Does ChiseledBlock::getHarvestLevel()
and ChiseledBlock::getHarvestTool()
the right™ thing?
Besides that issue, I'd need to check, whether overwrites work at all for chiseled blocks.
Basically the state I'm passing in, is for instance stone, or wool or what not, but the block in the world would be a chiselsandbits:*** state.
The level you'll want is based off the passed state, so stone, or wool, or whatever is passed.
The crash is of course because the state passed doesn't belong to the block in the world, so I guess thats as good a test as any to see if its fake?
This seems to fix it for now. tyra314/ToolProgression@ba70d87
That fix will be part of the upcoming releases of ToolProgression: 1.2.5 and 1.1.5
Thank you such much for your help!