Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crash caused by missing check of infinite loop of crop magic #341

Open
Largosaki opened this issue Apr 10, 2021 · 5 comments
Open

crash caused by missing check of infinite loop of crop magic #341

Largosaki opened this issue Apr 10, 2021 · 5 comments

Comments

@Largosaki
Copy link

Largosaki commented Apr 10, 2021

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

@murapix
Copy link
Collaborator

murapix commented Apr 10, 2021

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

@Largosaki
Copy link
Author

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

@murapix
Copy link
Collaborator

murapix commented Apr 11, 2021

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

@Largosaki
Copy link
Author

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

@PhotonChaos
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants