Wizardry icon indicating copy to clipboard operation
Wizardry copied to clipboard

crash caused by missing check of infinite loop of crop magic

Open Largosaki opened this issue 3 years ago • 5 comments

4.3.4 - MC 1.12.2

wizardy:staff use some magic on plant,

some crop (like pam's harvestcraft)

It spreads horizontally and is always only one block high

In the open air will lead to infinite loops cause a crash

public class ModuleEffectThrive implements IModuleEffect { while (world.getBlockState(otherTargetPos.up()).getBlock() == block) { otherTargetPos = otherTargetPos.up(); state = world.getBlockState(otherTargetPos); block = state.getBlock(); }

should we introduce some checks like this? while (otherTargetPos.up().y < 255 || world.getBlockState(otherTargetPos.up()).getBlock() == block) { }

Largosaki avatar Apr 10 '21 06:04 Largosaki

Ah, I see what you're looking at - the otherTargetPos loop code would be run even if only targetPos was looking at the plant. Is this a crash you were actually able to have happen though? We've used Thrive many a time, and not actually ever seen this be a problem

murapix avatar Apr 10 '21 13:04 murapix

Yes, he keeps recurring crashes on my server

I tried to modify it to avoid the crash, although he might execute immediateBlockTick on the air block

currently working well

while (otherTargetPos.getY() < 255 && world.getBlockState(otherTargetPos.up()).getBlock() == block && !world.isAirBlock(otherTargetPos.up()))

<255 is just an additional check but I think it makes sense

otherTargetPos will check the top of the crop and its top for the first time. I guess this can correctly index things like sugar cane. (Because the first cast will make him at least two blocks high) But Pam Farm, which is always only one block high, is still only one block high after the first practice. This will cause repeated inspections of Air == Air to the sky.

taking into account the correct casting he should grow like this

while ( world.getBlockState(otherTargetPos.up()).getBlock() == block) { if (otherTargetPos.getY() > 254 || world.isAirBlock(otherTargetPos)) return true; }

Largosaki avatar Apr 11 '21 00:04 Largosaki

Would be better to fix this by blocking the otherTargetPos section if the targetPos section ran, but this works too. As for actually fixing this, I'll try to get it into the code, but having multiple versions in dev makes things screwy as hell. The 1.12 versions are officially on life support at this point though, so for a while you might just need to not use Thrive on blocks

murapix avatar Apr 11 '21 00:04 murapix

thanks for your reply I don’t know much about him just want the game to run normally there should be better code

Largosaki avatar Apr 11 '21 00:04 Largosaki

If you can verify that this solution works and causes no other issues, please create a Pull Request!

PhotonChaos avatar Apr 11 '21 01:04 PhotonChaos