SlimeVoid / LittleBlocks-FML

LittleBlocks for FML
http://www.slimevoid.net/LittleBlocks
GNU Lesser General Public License v3.0
18 stars 13 forks source link

GetParentWorld is used after it is determined to be null #93

Open Korynkai opened 9 years ago

Korynkai commented 9 years ago

public long getTotalWorldTime() { if (this.getParentWorld() != null) { return this.getParentWorld().getWorldInfo().getWorldTotalTime(); } else { LoggerLittleBlocks.getInstance(Logger.filterClassName(this.getClass().toString())).write(this.getParentWorld().isRemote, "getTotalWorldTime().[null]", LoggerLittleBlocks.LogLevel.DEBUG); return 0; } }

note: if you don't see it, you're testing if this.getParentWorld() is or is not null, then when it is null, you're calling write from a logger instance with the isRemote value of this.getParentWorld(), which has already been determined to be null, so it's looking for null.isRemote.

Attempting to use a value even though it's already determined as null? No wonder I've been having problems getting this mod to work! (every version I've tried for 1.7.x from the site and github)

This error was stumbled upon with the following crash report, LittleWorld.java line 255 is the code above

---- Minecraft Crash Report ---- // Sorry :(

Time: 11/6/14 2:52 PM Description: Unexpected error

java.lang.NullPointerException: Unexpected error at net.slimevoid.littleblocks.world.LittleWorld.func_82737_E(LittleWorld.java:255) at net.slimevoid.littleblocks.world.LittleWorldClient.func_82737_E(LittleWorldClient.java:97) at codechicken.multipart.TickScheduler$WorldTickScheduler.loadTag(TickScheduler.scala:108) at codechicken.multipart.TickScheduler$WorldTickScheduler.load(TickScheduler.scala:100) at codechicken.lib.world.WorldExtensionManager.onWorldLoad(WorldExtensionManager.java:165) at codechicken.lib.world.WorldExtensionManager.access$100(WorldExtensionManager.java:22) at codechicken.lib.world.WorldExtensionManager$WorldExtensionEventHandler.onWorldLoad(WorldExtensionManager.java:85) at cpw.mods.fml.common.eventhandler.ASMEventHandler_627_WorldExtensionEventHandler_onWorldLoad_Load.invoke(.dynamic) at cpw.mods.fml.common.eventhandler.ASMEventHandler.invoke(ASMEventHandler.java:54) at cpw.mods.fml.common.eventhandler.EventBus.post(EventBus.java:138) at net.slimevoid.littleblocks.world.LittleWorldClient.(LittleWorldClient.java:44) at net.slimevoid.littleblocks.client.events.WorldClientEvent.onWorldLoad(WorldClientEvent.java:25) at cpw.mods.fml.common.eventhandler.ASMEventHandler_543_WorldClientEvent_onWorldLoad_Load.invoke(.dynamic) at cpw.mods.fml.common.eventhandler.ASMEventHandler.invoke(ASMEventHandler.java:54) at cpw.mods.fml.common.eventhandler.EventBus.post(EventBus.java:138) at net.minecraft.client.multiplayer.WorldClient.(WorldClient.java:61) at net.minecraft.client.network.NetHandlerPlayClient.func_147282_a(NetHandlerPlayClient.java:237) at net.minecraft.network.play.server.S01PacketJoinGame.func_148833_a(SourceFile:70) at net.minecraft.network.play.server.S01PacketJoinGame.func_148833_a(SourceFile:13) at net.minecraft.network.NetworkManager.func_74428_b(NetworkManager.java:212) at net.minecraft.client.Minecraft.func_71407_l(Minecraft.java:2050) at net.minecraft.client.Minecraft.func_71411_J(Minecraft.java:962) at net.minecraft.client.Minecraft.func_99999_d(Minecraft.java:887) at net.minecraft.client.main.Main.main(SourceFile:148) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at net.minecraft.launchwrapper.Launch.launch(Launch.java:135) at net.minecraft.launchwrapper.Launch.main(Launch.java:28)

Tarig0 commented 9 years ago

And there are other issues with your setup causing this to be null, we will fix this one line of code, which by the way you could have made a PR instead of this patronizing issue.

Tarig0 commented 9 years ago

List of mods will be nice to see why the parent world was null

Korynkai commented 9 years ago

Sorry, suppose I could've done that, but we had believed it to be a major "derp" and the tone of the original post was more of a friend's idea than my own.. you see we're working on our own modpack, some of which includes reworked mods, fixes and even newly written mods by us in some cases (all to be kept private until we determine everything to be fairly stable, at which point we will release the source to any and all modified open-source mods)... but a fix I implemented actually made it worse... here's the pastebin of the original dump including mod list (note, if ALL these aren't changed, it will still error in the same way on each corresponding line as they're called, not just the one): http://pastebin.com/Q19YGVSf

patch (removing the logger line that tries to access such variable and adding some null checks): http://pastebin.com/YNfAp4Ae post-patch crash (obviously crashing further within MC and Forge): http://pastebin.com/NF6e9nHx

Once again, about the tone, initially we were only trying to migrate from 1.6.4 to 1.7.10 for our server's setup (a bug we ran into with OC not working at all on Windows forced us to do this, regardless of whether our world would convert or not) but it ended up being a full-blown modpack + patches project, and each bug we've run into makes us that much further from reaching our goal. I'm sure you can understand.

Watch, then the post-patch crash is a derp caused by something I overlooked in my patch, wouldn't surprise me... xP

Hey, we all make these mistakes, Lord knows both I and aforementioned friend have been hit by even worse mistakes.

Tarig0 commented 9 years ago

O.K. lets get down to the post-patch error.

does this happen as you join the world or when a gui opens and which gui if the later

Tarig0 commented 9 years ago

btw how do you inject that patch file? I didn't know that was possible outside of forge

Korynkai commented 9 years ago

It was a diff-style patch that can be injected with the standard GNU/Linux patch command directly into the source tree... Funny thing is I've been working alot on other mods lately anyway, I'm thinking of just forking LittleBlocks, taking a long, hard look at it, and seeing if I can't fix the issue I was having before and submit a pull request... The origin of the issue revolves around adding a block with a tick handler into the LittleBlocks chunk and it would crash at that point and whenever one tried to load the world, though I'm not 100% sure if that was the actual case in this report (I need to recreate the test environment, haven't had a chance to as I've been working on other projects)

Korynkai commented 9 years ago

Actually, just tried applying the patch, something must've been different with my source because two hunks failed... Should be an easy fix, seems to just be the removal of a conditional (and modification of indentations) that it failed on.

Eurymachus commented 9 years ago

Yes this is quite a patronizing way to log and issue.

I will modify the Logger to use the correct this.isRemote

Eurymachus commented 9 years ago

That said, parentWorld should never EVER be null. I'd be interested to see what exactly it is that you're doing to cause that.

Korynkai commented 9 years ago

I don't quite understand it myself (as of yet), nothing I can think of that I did should've caused this to be null. This occurred in my test world when I was attempting to test whether a ticking machine would function correctly as a LittleBlock (though we have an additional solution to this task in our modpack). The results of this test were the errors thrown.