TehNut-Mods / ResourcefulCrops

Simply JSON based resource crops
MIT License
12 stars 7 forks source link

Crash when planting on waterfree farming block #58

Closed Knito58 closed 6 years ago

Knito58 commented 6 years ago

I planted 16 iron seed on the left side of a farm and accidently also a redstone seed. I hit the redstone seed, took it and went to the right side of the same 9x9 farming spot. Planting it there then crashed the game. The farming blocks are "Redstone Infused Fertilized Dirt" from the mod "Waterfree Farming" which is an auto-farm in short.

Crash Report

A reload of that world now always crashes the game with the same error in the crash report.

MC 1.12.2, Forge 14.23.1.2556, Resourceful Crops 1.12-2.1.2-63, Waterfree Farming 2.0.4 Gamemode SSP, no Cheats

TehNut commented 6 years ago

The other mod is closed source so I can't look on my phone, but it appears he is removing the TileEntity before the block. If so, this is on his side.

Knito58 commented 6 years ago

I also gave a note on the mod's page on curseforge https://minecraft.curseforge.com/projects/waterfree-farming

TehNut commented 6 years ago

You should probably link them here too

Knito58 commented 6 years ago

At tehnut.resourceful.crops.block.BlockResourcefulCrop.func_176221_a(BlockResourcefulCrop.java:49) Function getActualState( state, world, pos ) I assume "pos" is NULL or points to an empty space with no crop. I deleted a crop by hitting it and went away. I believe the farm block with the deleted crop on it ticks now. After loading the game I can play for about 1 second. Whatever I do then, the game crashes. It is always the same error. I guess it would be a good idea if the function would catch errors coming from invalid positions.

TehNut commented 6 years ago

Line 122 in RedstoneTilledFertilizedDirt

if (newCrops.func_185525_y(newCrops.func_176221_a(worldIn.func_180495_p(pos.func_177984_a()), worldIn, pos)))

If deobfuscated, this would be mapped as

if (newCrops.isMaxAge(newCrops.getActualState(worldIn.getBlockState(pos.up()), worldIn, pos)))

The last pos there should also be offset up. This is the fault of Waterfree Farming. Also, he is duplicating lots of getBlockState calls, which is very inefficient.

Knito58 commented 6 years ago

It is the same error in both programmers code:

Both assume that there is a valid crop in "pos" when the getActualState() is called.

You say: "The other code bad!" And you are right. RaphiiWarren could say the same and would be right, too.

If both of you close this issue, then nothing gets fixed.

TehNut commented 6 years ago

No. He is giving the wrong position to a method that expects the proper one. This is 100% not my issue. I am not going to bandaid my mod because somebody else's is broken. That is not how this works.

Knito58 commented 6 years ago

OK. I was wrong. I did a new creative world and installed a "Waterfree Autofarm" 9x9. I put on it: Regular seeds, potatoes, carrots, actually additions crops like flax and rice. Potatoes and Carrots were successfully installed with "Ex Nihilo" seeds. I waited some time and then deleted one of each crop. Nothing happened. Everything fine. Waiting.

Then I planted redstone seeds from "Resourceful Crops" on it and wanted to wait some time. The game crashed after a second without having deleted any crop at all. So deleting a crop is not necessary to crash the session. That is why I believe that you are right in the sense of that there is an error when adressing getActualState() with the wrong pos on the waterfree side.

But it is only your crop which crashes.

If you want to replay this to see that error:

Build a 9x9 field of "Redstone Infused Fertilized Dirt". Replace the center block with a "Controller Fertilized Dirt". Till the area with a "Clay Hoe" from the waterfree mod.

That is the playground. Now this farm would collect seeds and drops in the center block and replant harvested crops. It is hard to make on Skyblock but I like it because it does not need RF to work.

And that is why I am insisting: I am losing a world which took weeks to make, which was "running" and now is ruined.

I am very sorry about this whole thing. I had alot of activities on GitHub with crashing mods. But none of those crashes was a game killer in the sense that the world was lost.

Greetings, Konitor

RaphiiWarren commented 6 years ago

INITIAL THOUGHTS:

Hey @TehNut I understand your point, I am feeling the same way. Fixing ones mod for just another mod seems weird, but basically if players want to combine the mods it should be possible, ain't it?

Like @Knito58 said, it's not a problem with giving the wrong position, because basically the normal crops wouldn't get harvested then too. There is something with your GetActualBlockstate which gets called by my mod to determine if a crop is MaxAge or not. I don't want to attack your code, but somehow there is a NullPointer reference. Your code looks like this:

public IBlockState getActualState(IBlockState state, IBlockAccess world, BlockPos pos) { Seed seed = RegistrarResourcefulCrops.SEEDS.getValue(Util.getTile(TileSeedContainer.class, world, pos).getSeedKey()); return state.withProperty(SEED_TYPE, seed.getRegistryName().toString().replace(":", "_")); }

And minecraft vanilla crops use the inherited function getActualState from the Block.class, which looks like that:

public IBlockState getActualState(IBlockState state, IBlockAccess worldIn, BlockPos pos) { return state; }

So somehow the line starting with "Seed seed= ..." is getting a null pointer exception. And I understand your idea of a wrong pos, but basically even if it isn't pos.up() like you said, this would not be a null pointer because pos is the Position of the block which can't be null, because it exists in the world. So I can't tell you which of the other parts is throwing the null pointer exception, but it isn't the pos.

There is a bug on my side too, because the block shouldn't tick when there is no block on it, so I need to look into the code too and I will try to recreate the bug @Knito58 to see if I may find a solution.

@TehNut I know your mod has way more downloads then mine, but don't be salty against me just because my mod isn't as big as yours. I know my programming, I studied that.

READ HERE FOR SOLUTION:

UPDATE: After testing the two mods together I can tell, that @TehNut was right. I updated the cod, so there is two times pos.up(). The point is, that other crops (like vanilla or pams harvest craft) don't use the second pos-parameter of getActualState, so they don't throw this exception, because they don't use it. So basically the line throws an exception because it doesnt find a seed at the given position, because it is a block of my mod. I couldn't recreate the seeds, because I didn't look that much into this mod, but it happened with normal seeds too. At this point I am not even sure if the automation works with this mod, because I can't use bonemeal on the seeds. I will let minecraft run in the background and will give an update to that later.

UPDATE 2: Seeds from this mod get harvested, it seems to take a bit longer than normal though.

Sorry for the inconvenience @Knito58, update will be released soon. And sorry to you @TehNut, but your answers seemed a bit salty, but you were right all along.

Knito58 commented 6 years ago

Thank you, guys!

TehNut commented 6 years ago

Not salty, just blunt. shrug