Wizardry Mod

Wizardry Mod

6M Downloads

crash caused by missing check of infinite loop of crop magic

Largosaki opened this issue · 5 comments

commented

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) {
}

commented

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

commented

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;
}

commented

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

commented

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

commented

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