Null Pointer Exception in TileEntityBackpack

[radfast]

java.lang.NullPointerException at net.mcft.copy.betterstorage.tile.entity.TileEntityBackpack.func_70319_e(TileEntityBackpack.java:127)

I took at look at the code, and seems like it's a problem with 'stack' being null in line 127, and you might see exact the same issue at line 148 I would think.

When you fix this, if it's not too much trouble, please can you also release an update of the 1.6.4 version of your mod so that the fix can be picked up by the various modpacks which use that (otherwise a lot of servers will keep on crashing or will have to ban Backpacks...)

[copygirl]

There's no point at which the backpack stack should be null, unless an error has not occurred before. Was there an error when loading the backpack? Has it been tampered with by another mod? Forge log would be nice, telling me how to reproduce the error would be even better.

[radfast]

Hi copygirl and thank you for looking at this issue, thank you also for coding a decent mod and making it available to the community.

I looked at your code in detail before posting this issue and I agree with you that theoretically the stack "shouldn't" ever be null, at least, not in code theory land. With no disrespect, and some humour intended, here is how to reproduce the problem in reality:

1. Set up a real public server running one of the popular modpacks which includes BetterStorage - for example Attack of the B Team, that's a nice modpack - and make sure to have 50+ players playing on the server at peak times doing all the creative things which players might do
2. This server is a public multiplayer server so naturally it is running MCPC+ (remember I said it is a real server) so that's Spigot, Bukkit, Essentials and everything else that goes with that, also MCMyAdmin or something similar is controlling the server
3. The server has some Bukkit plugins which, in order to allow the server staff to maintain the server properly and keep it playable for the players, can place or move blocks in the world, or undo placement of blocks. These plugins include for example Grief Prevention and WorldEdit, I expect there are others too.
4. There are also other mods in the modpack, and some of these mods can move blocks around in the world or place blocks - for example due to explosions or specific block moving mods
5. Sometimes this server (remember it's a real server) will crash completely, due to buggy code in one of the mods - could be any of the mods in the modpack crashing, this NPE in your code is an example but there are of course a few other examples too, there can also be 'server stopped responding' type crashes due to something getting itself in an endless loop. Following a crash, the world region files may well not have been saved correctly, or player data files may not be synchronised with world region files.

I would think any of factors 3,4, or 5 could lead to the NPE error. I can't tell you which it is here, though I guess there is a few GB of Prism data saved somewhere which might or might contain a few clues. It seems to me very likely that one of these factors 3, 4 or 5 - 5 being by far the most likely - has caused a TileEntityBackpack to be in one of the server's world region files without the full associated NBT data it should normally have, so when the server loads that chunk it contains a TileEntityBackpack with a null stack, in which case your code causes the server to crash in the circumstances we are seeing.

I hope that you won't be tempted to flag this issue "CrossMod / Won't fix". As things are, your mod is causing real public servers to crash, and therefore in fact aggravating the situation I described in (5) above. Yours is - of course - not the only code which does that, there is a handful of other mods with a similar coding approach (i.e. TileEntities where the code "assumes" that fields will always be correctly populated - the Volleyball Court master block in Tropicraft is another example).

It needs 2 small changes to add a null check to your code here, so that it will play nice when it's being used in a real server environment like the one I described. Generally I would suggest that coding for robustness is a sensible approach which all mod authors should take when coding mods, assuming they are willing for their mods to be used on real public servers of course.

Thanks for reading :)

[copygirl]

Adding null checks would only hide the actual problem. I need to know what's the cause before deciding what the proper way to fix this is. Again, I will ask for a proper crash log, or even better the forge log (or at least up to a few 100 lines before the crash). I doubt the crash log would help much in this situation, but I'm not sure.

[radfast]

Here's the crash log. Forge log I'm not sure if I can still get from that far back, I will try. I'll also take a look at the region file on the server to see if there's still a backpack tile entity there.

---- Minecraft Crash Report ----
// Hey, that tickles! Hehehe!

Time: 3/29/14 1:19 PM
Description: Ticking entity

java.lang.NullPointerException
    at net.mcft.copy.betterstorage.tile.entity.TileEntityBackpack.func_70319_e(TileEntityBackpack.java:127)
    at net.minecraft.entity.player.EntityPlayerMP.func_71119_a(EntityPlayerMP.java:694)
    at net.minecraft.entity.player.EntityPlayerMP.func_70071_h_(EntityPlayerMP.java:399)
    at micdoodle8.mods.galacticraft.core.entities.player.GCCorePlayerMP.func_70071_h_(GCCorePlayerMP.java:186)
    at net.minecraft.world.World.func_72866_a(World.java:2944)
    at net.minecraft.world.WorldServer.func_72866_a(WorldServer.java:1006)
    at net.minecraft.world.World.func_72870_g(World.java:2890)
    at net.minecraft.world.World.func_72939_s(World.java:2704)
    at net.minecraft.server.MinecraftServer.func_71190_q(MinecraftServer.java:883)
    at net.minecraft.server.dedicated.DedicatedServer.func_71190_q(DedicatedServer.java:330)
    at net.minecraft.server.MinecraftServer.func_71217_p(MinecraftServer.java:777)
    at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:659)
    at net.minecraft.server.ThreadMinecraftServer.run(ThreadMinecraftServer.java:16)

A detailed walkthrough of the error, its code path and all known details is as follows:
---------------------------------------------------------------------------------------

-- Head --
Stacktrace:
    at net.mcft.copy.betterstorage.tile.entity.TileEntityBackpack.func_70319_e(TileEntityBackpack.java:127)
    at net.minecraft.entity.player.EntityPlayerMP.func_71119_a(EntityPlayerMP.java:694)
    at net.minecraft.entity.player.EntityPlayerMP.func_70071_h_(EntityPlayerMP.java:399)
    at micdoodle8.mods.galacticraft.core.entities.player.GCCorePlayerMP.func_70071_h_(GCCorePlayerMP.java:186)
    at net.minecraft.world.World.func_72866_a(World.java:2944)
    at net.minecraft.world.WorldServer.func_72866_a(WorldServer.java:1006)
    at net.minecraft.world.World.func_72870_g(World.java:2890)

-- Entity being ticked --
Details:
    Entity Type: null (micdoodle8.mods.galacticraft.core.entities.player.GCCorePlayerMP)
    Entity ID: 3252
    Entity Name: egilsen2001
    Entity's Exact location: 220.25, 70.00, 925.44
    Entity's Block location: World: (220,70,925), Chunk: (at 12,4,13 in 13,57; contains blocks 208,0,912 to 223,255,927), Region: (0,1; contains chunks 0,32 to 31,63, blocks 0,0,512 to 511,255,1023)
    Entity's Momentum: 0.00, -0.08, 0.00
Stacktrace:
    at net.minecraft.world.World.func_72939_s(World.java:2704)

[copygirl]

Yeah, looks like someone just walked or tried to log in near one of the broken backpacks. The crash log doesn't show why it's broken. While thinking about it, the backpack might've crashed from the backpack ID having changed. I suppose I should account for that and remove the backpack on load. I suppose it'll also fix this error from happening repeatedly. Actually, nevermind. The backpack item and block share the same ID. If the backpack ID changes, the block would disappear on its own.

[copygirl]

Closing this for now since we haven't been able to reproduce this.