TheTemportalist / Compression

2 stars 0 forks source link

NPE Crash with ChromaticCraft #3

Closed Thiana closed 8 years ago

Thiana commented 8 years ago

I'm not sure if this is a bug with CC or Compression but since your mod is younger I'm reporting it here first.

Minecraft crashes on startup with an NPE error if ChromaticCraft and Compression are used together.

Crash details are at http://openeye.openmods.info/crashes/4d42fc1f786603a11d9cbbe8914e7109.

To replicate, install DragonAPI 9b, ChromaticCraft 9b, Origin 6.0.1b1 and Compression 0.0.3. Crash. Disable either mod and the instance works perfectly.

Thanks.

TheTemportalist commented 8 years ago

Please report this to Reika's DragonAPI, as that is the primary NPE thrower in the stacktrace. I will leave this issue open until this is solved because it may require action on my end.

ReikaKalseki commented 8 years ago

It is caused by your code calling player-specific functions but providing a null player.

TheTemportalist commented 8 years ago

Ok. Thanks for the input @ReikaKalseki. I will look into this ASAP.

TheTemportalist commented 8 years ago

So looking into my code, this is the function being called: https://github.com/TheTemportalist/Compression/blob/1.7.10/src/main/scala/com/temportalist/compression/common/init/CBlocks.scala#L202

def wrapInnerStack(stack: ItemStack, size: Long): ItemStack = {
    val retStack = if (WorldHelper.isBlock(stack.getItem))
            new ItemStack(this.compressed)
        else new ItemStack(CItems.compressed)
    val tag = new NBTTagCompound
    stack.getItem match {
        case food: ItemFood =>
            tag.setBoolean("canEat", true)
            retStack.getItem.asInstanceOf[IFood].setHealAmount(food.func_150905_g(stack))
            retStack.getItem.asInstanceOf[IFood].setSaturation(food.func_150906_h(stack))
        case food: IFood =>
            tag.setBoolean("canEat", true)
            stack.getItem.asInstanceOf[IFood].setHealAmount(food.getFoodAmount(stack))
            stack.getItem.asInstanceOf[IFood].setSaturation(food.getSaturationAmount(stack))
        case _ =>
            tag.setBoolean("canEat", false)
    }
    tag.setString("inner", NameParser.getName(stack, hasID = true, hasMeta = true))
    tag.setString("display", stack.getDisplayName)
    tag.setLong("stackSize", size)
    retStack.setTagCompound(tag)
    retStack
}

From there, the line causing the issue is (5th from the bottom):

tag.setString("display", stack.getDisplayName)

Now this is just calling plain old minecraft code. From what I can tell, this issue has something to do with NBT. All ItemStack.getDisplayName does is check the NBT for "display" and "name" keys.

I dont see where I am throwing nulls into the mix though.

TheTemportalist commented 8 years ago

@Thiana can you provide more of a log? One option would be for me to insert a try catch and spit out any blocks/items which cause issues. My suspicion is that 1 or more blocks/items in Chromaticraft doesnt like me calling getDisplayName before the game is fully initialized.

Thiana commented 8 years ago

The full log of the crash can be found at http://pastebin.com/UhQ66hqB

Thanks for trying to solve this :)

TheTemportalist commented 8 years ago

Alright. The log didn't indicate anything hugely important. I am going to test the chromaticcraft files you specified with latest origin and see what I get with my new try catch code.

ReikaKalseki commented 8 years ago

The code on my end getting called is something that ultimately calls player.getCompoundTag(). That throws the NPE, and thus player must be null.

https://github.com/ReikaKalseki/DragonAPI/blob/master/Libraries/ReikaPlayerAPI.java#L267

TheTemportalist commented 8 years ago

Okay. With that try catch code, I found that the game launched fine, and the things cause things to happen were:

[14:47:14] [Client thread/INFO] [compression]: Could not compress stack with item tile.block.crystalrune and size of 1 and meta of 0 and nbt of {}
[14:47:14] [Client thread/INFO] [compression]: Could not compress stack with item tile.dye.block and size of 1 and meta of 0 and nbt of {}
[14:47:14] [Client thread/INFO] [compression]: Could not compress stack with item tile.dye.grass and size of 1 and meta of 0 and nbt of {}
[14:47:14] [Client thread/INFO] [compression]: Could not compress stack with item tile.chroma.tieredore and size of 1 and meta of 0 and nbt of {}
TheTemportalist commented 8 years ago

@ReikaKalseki It is because here: https://github.com/ReikaKalseki/ChromatiCraft/blob/master/Items/ItemBlock/ItemBlockCrystalColors.java#L57 The game hasnt started yet, so the client side player is null. Im not passing a player :P

TheTemportalist commented 8 years ago

I will release the new code so that this issue doesn't persist (plus it will be helpful for other issues that may or may not occur in the future). When I do, I will close the issue.

TheTemportalist commented 8 years ago

Ill leave it unlocked for now :P

ReikaKalseki commented 8 years ago

OK, yes, but why are you trying to get an ItemStack display name outside of a world context? That is going to throw errors all over the place. A try-catch is probably the best way to deal with this, yes.

TheTemportalist commented 8 years ago

Only in the case of blocks and items which are trying to get nbt with a player, such in your case. Most blocks and items dont have that issue. Not that it is wrong or bad, but that I haven't had issue until now. If people start complaining that there are a lot of mods whose blocks/items arent being loaded because of this type of thing, then Ill change how i handle it. But until then, Im okay with how it is setup because it is just those 4 objects which wont be loaded as compressables.

TheTemportalist commented 8 years ago

The reason why I get item's display names is so that I dont have to create the inner stack and call getDisplayName every time that I want to get the display name of the compressed block/item. It would be a lot of overhead object creation and that would just be a hassle. Nor do I want to create a variable because it would only be used for this function.

TheTemportalist commented 8 years ago

And as to your suggestion to a try-catch, that is exactly what i did. If an object throws an error (such as your 4 objects), then I just dont load them as compressables. See: https://github.com/TheTemportalist/Compression/blob/1.7.10/src/main/scala/com/temportalist/compression/common/init/CBlocks.scala#L78

TheTemportalist commented 8 years ago

Alright. Now that we are all on the same page... I have uploaded a patch to curseforge. It will be available soon. http://minecraft.curseforge.com/projects/compression/files/2266221