Chisels & Bits - For Fabric

Chisels & Bits - For Fabric

2M Downloads

Mod interaction issue ToolProgression

tyra314 opened this issue · 9 comments

commented

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?

commented

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.

commented

I'd happily add a special case for the fake blockstate.

commented

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.

commented

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.

commented

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.

commented

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.

commented

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?

commented

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!

commented

No problem, Thanks for fixing that on your end, at least for now, I'll mark it for this in hopes that I'll get around to hopefully figuring something out before something else comes up.