GTNewHorizons / GT-New-Horizons-Modpack

New Modpack with Gregtech, Thaumcraft and Witchery
https://www.gtnewhorizons.com/
Other
886 stars 275 forks source link

A fully charged GT++ Energy Buffer generates NPE on chunk load #16565

Open reobf opened 1 week ago

reobf commented 1 week ago

Your GTNH Discord Username

No response

Your Pack Version

2.6.1

Your Server

SP

Java Version

Java 8

Type of Server

Single Player

Your Expectation

Place a GT++ Energy Buffer and charge it to full energy. Save game and load game, there should be no ERROR log like:

[04:13:36] [Server thread/ERROR] [GregTech GTNH]: Encountered Exception while loading MetaTileEntity.
[04:13:36] [Server thread/ERROR] [GregTech GTNH]: java.lang.NullPointerException
    at gregtech.api.metatileentity.BaseTileEntity.markDirty(BaseTileEntity.java:589)
    at gregtech.api.metatileentity.CommonMetaTileEntity.markDirty(CommonMetaTileEntity.java:115)
    at gregtech.api.metatileentity.MetaTileEntity.markDirty(MetaTileEntity.java:1063)
    at gregtech.api.metatileentity.MetaTileEntity.setEUVar(MetaTileEntity.java:607)
    at gtPlusPlus.xmod.gregtech.common.tileentities.storage.GregtechMetaEnergyBuffer.loadNBTData(GregtechMetaEnergyBuffer.java:270)
    at gregtech.api.metatileentity.CommonMetaTileEntity.loadMetaTileNBT(CommonMetaTileEntity.java:97)
    at gregtech.api.metatileentity.BaseMetaTileEntity.setInitialValuesAsNBT(BaseMetaTileEntity.java:210)
    at gregtech.api.metatileentity.BaseMetaTileEntity.readFromNBT(BaseMetaTileEntity.java:170)
    at net.minecraft.tileentity.TileEntity.createAndLoadEntity(TileEntity.java:135)
    at net.minecraft.world.chunk.storage.AnvilChunkLoader.loadEntities(AnvilChunkLoader.java:525)
    at net.minecraftforge.common.chunkio.ChunkIOProvider.callStage2(ChunkIOProvider.java:41)
    at net.minecraftforge.common.chunkio.ChunkIOProvider.callStage2(ChunkIOProvider.java:12)
    at net.minecraftforge.common.util.AsynchronousExecutor.skipQueue(AsynchronousExecutor.java:344)
    at net.minecraftforge.common.util.AsynchronousExecutor.getSkipQueue(AsynchronousExecutor.java:302)
    at net.minecraftforge.common.chunkio.ChunkIOExecutor.syncChunkLoad(ChunkIOExecutor.java:12)
    at net.minecraft.world.gen.ChunkProviderServer.loadChunk(ChunkProviderServer.java:144)
    at net.minecraft.world.gen.ChunkProviderServer.loadChunk(ChunkProviderServer.java:119)
    at net.minecraft.server.MinecraftServer.initialWorldChunkLoad(MinecraftServer.java:305)
    at net.minecraft.server.integrated.IntegratedServer.loadAllWorlds(IntegratedServer.java:79)
    at net.minecraft.server.integrated.IntegratedServer.startServer(IntegratedServer.java:96)
    at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:445)
    at net.minecraft.server.MinecraftServer$2.run(MinecraftServer.java:752)

The Reality

It spams in log

Your Proposal

GT++ Energy Buffer stores 'aStoredEU' to NBT to keep EU energy when getting destroyed by a wrench. And it also stores 'aStoredEU' to NBT when saving chunk for some reason.

GregtechMetaEnergyBuffer.java

public void saveNBTData(final NBTTagCompound aNBT) {
        aNBT.setByte("aCurrentOutputAmperage", aCurrentOutputAmperage);
        long aEU = this.getBaseMetaTileEntity().getStoredEU();
        if (aEU > 0) {
            aNBT.setLong("aStoredEU", aEU);
            if (aNBT.hasKey("aStoredEU")) {
                Logger.WARNING("Set aStoredEU to NBT.");
            }
        }
    }

However, getStoredEU() will return MaxEUCapacity if the actual EU maxed out. For example, HV Energy Buffer’s max EU is 128,000,000 and you can charge its internal EU to a number like 128,000,004 and 'aStoredEU' will be 128,000,000 and the actual internal EU 'aStoredEnergy' (saved by BaseMetaTileEntity.java) will be 128,000,004 Problems happens when chunk loaded. GT++ Energy Buffer will call setEUVar() on chunk load.

MetaTileEntity.java
public void setEUVar(long aEnergy) {
        if (aEnergy != ((BaseMetaTileEntity) mBaseMetaTileEntity).mStoredEnergy) {
            markDirty();
            ((BaseMetaTileEntity) mBaseMetaTileEntity).mStoredEnergy = aEnergy;
        }
    }

Normally, aEnergy will be equivalent to mStoredEnergy and markDirty() will not be called. But if you over-charged the Buffer, 128,000,004!=128,000,000, markDirty() will be called Sadly, worldObj is not set at that time, and a NPE is what you will get This bug has no actual bad effect, because almost all data are loaded when NPE thrown, but the log spamming is annoying. Fix it PLS

Final Checklist