[1.12.2] Tile Inventory Not Saving

[unassigned]

Hello, I have a tile entity with an Inventory, however, when saving and exiting, then re-entering, the item disappears. As well, the client is never actually aware of the item, its all stored on the server, which makes me believe that there is some kind of desync issue. Here is the my TE base class, which handles capabilities and nbt syncing/saving:

Spoiler

public class TileBase extends TileEntity implements ITickable {

    protected String name;
    protected TileEntity[] tilesNear = new TileEntity[6];
    protected int ticksElapsed;

    public TileBase(String name) {
        this.name = name;
    }

    public String getRawName() { return this.name; }

    @Override
    public void update() { //todo energy interaction
        ticksElapsed++;
        this.sendIntervalUpdate();
    }

    @Override
    public final NBTTagCompound writeToNBT(NBTTagCompound compound){
        this.writeSyncNBT(compound, SaveType.SAVE_TILE);
        return compound;
    }

    @Override
    public final void readFromNBT(NBTTagCompound compound){
        this.readSyncNBT(compound, SaveType.SAVE_TILE);
    }

    @Override
    public final SPacketUpdateTileEntity getUpdatePacket(){
        NBTTagCompound compound = new NBTTagCompound();
        this.writeSyncNBT(compound, SaveType.SYNC);
        return new SPacketUpdateTileEntity(this.pos, -1, compound);
    }

    @Override
    public final void onDataPacket(NetworkManager net, SPacketUpdateTileEntity pkt){
        this.readSyncNBT(pkt.getNbtCompound(), SaveType.SYNC);
    }

    @Override
    public final NBTTagCompound getUpdateTag(){
        NBTTagCompound compound = new NBTTagCompound();
        this.writeSyncNBT(compound, SaveType.SYNC);
        return compound;
    }

    @Override
    public final void handleUpdateTag(NBTTagCompound compound){
        this.readSyncNBT(compound, SaveType.SYNC);
    }

    public void writeSyncNBT(NBTTagCompound compound, SaveType type){
        if(type != SaveType.SAVE_BLOCK) super.writeToNBT(compound);

        if(type == SaveType.SAVE_TILE){
            compound.setInteger("TicksElapsed", this.ticksElapsed);
        }

    }

    public void readSyncNBT(NBTTagCompound compound, SaveType type){
        if(type != SaveType.SAVE_BLOCK) super.readFromNBT(compound);

        if(type == SaveType.SAVE_TILE){
            this.ticksElapsed = compound.getInteger("TicksElapsed");
        }

    }

    @Override
    public boolean shouldRefresh(World world, BlockPos pos, IBlockState oldState, IBlockState newState){
        return !oldState.getBlock().isAssociatedBlock(newState.getBlock());
    }

    @Nullable
    @Override
    public ITextComponent getDisplayName() {
        return new TextComponentTranslation("container."+ VoidUtils.MODID+"."+this.name+".name");
    }

    protected void sendIntervalUpdate() {
        if(this.ticksElapsed%5 == 0){ //1/5 update per second todo: configs
            this.sendNetworkUpdate();
        }
    }

    public final void sendNetworkUpdate(){ if(world != null && !world.isRemote) TilePacketDispatcher.dispatchTEPacket(this);}

    @Override
    public boolean hasCapability(Capability<?> capability, EnumFacing facing){
        return this.getCapability(capability, facing) != null;
    }

    @SuppressWarnings("unchecked")
    @Override
    public <T> T getCapability(Capability<T> capability, EnumFacing facing){
        if(capability == CapabilityItemHandler.ITEM_HANDLER_CAPABILITY){
            IItemHandler handler = this.getItemHandler(facing);
            if(handler != null){
                return (T)handler;
            }
        }
        else if(capability == CapabilityFluidHandler.FLUID_HANDLER_CAPABILITY){
            IFluidHandler tank = this.getFluidHandler(facing);
            if(tank != null){
                return (T)tank;
            }
        }
        else if(capability == CapabilityEnergy.ENERGY){
            IEnergyStorage storage = this.getEnergyStorage(facing);
            if(storage != null){
                return (T)storage;
            }
        }
        return super.getCapability(capability, facing);
    }

    /**
     * Override these when you want the Tile to have the capability
     * @param facing side that the capability exists
     * @return the handler
     */
    @Nullable
    public IItemHandler getItemHandler(EnumFacing facing){
        return null;
    }

    /**
     * Override these when you want this tile to have the capability
     * @param facing side that the capability exists
     * @return the handler
     */
    @Nullable
    public IEnergyStorage getEnergyStorage(EnumFacing facing){
        return null;
    }

    /**
     * Override these when you want this tile to have the capability
     * @param facing side that the capability exists
     * @return the handler
     */
    @Nullable
    public IFluidHandler getFluidHandler(EnumFacing facing){
        return null;
    }

    public enum SaveType {
        SAVE_TILE,
        SYNC,
        SAVE_BLOCK
    }
}

 

 

And here is the extension of this that adds an inventory:

Spoiler

public class TileInventoryBase extends TileBase {

    public final ItemStackHandler inv;

    public TileInventoryBase(String name, int slots){
        super(name);
        inv = new TileStackHandler(slots);
    }

    @Override
    public void writeSyncNBT(NBTTagCompound compound, SaveType type) {
        super.writeSyncNBT(compound, type);
        if(type == SaveType.SAVE_TILE || (type == SaveType.SYNC && this.shouldSyncSlots())) this.inv.serializeNBT();
    }

    @Override
    public void readSyncNBT(NBTTagCompound compound, SaveType type) {
        super.readSyncNBT(compound, type);
        if(type == SaveType.SAVE_TILE || (type == SaveType.SYNC && this.shouldSyncSlots())) this.inv.deserializeNBT(compound);
    }

    @Override
    public void markDirty() {
        super.markDirty();
        if(this.shouldSyncSlots()){
            this.sendNetworkUpdate();
        }
    }

    @Nullable
    @Override
    public IItemHandler getItemHandler(EnumFacing facing) {
        return this.inv;
    }

    public int getMaxStackSize(int slot) { return 64; }

    public boolean shouldSyncSlots() { return false; }

    protected class TileStackHandler extends ItemStackHandler {

        protected TileStackHandler(int slots) {
            super(slots);
        }

        @Override
        public int getSlotLimit(int slot) {
            return TileInventoryBase.this.getMaxStackSize(slot);
        }

        @Override
        protected void onContentsChanged(int slot) {
            super.onContentsChanged(slot);
            TileInventoryBase.this.markDirty();
        }
    }
}

 

 

I have stepped through where I dispatch the vanilla packet, and during inventory changed, the function is called and sent to the client, however, nothing actually happens past that, and the item is never synced, thus lost upon restarting. Here is where I'm sending this vanilla packet:

Spoiler

   public static void dispatchTEPacket(TileEntity tileEntity){
        WorldServer worldServer = (WorldServer) tileEntity.getWorld();
        PlayerChunkMapEntry entry = worldServer.getPlayerChunkMap().getEntry(tileEntity.getPos().getX() >> 4, tileEntity.getPos().getZ() >> 4);
        if(entry == null) return;

        for(EntityPlayerMP player : entry.getWatchingPlayers()){
            if(tileEntity.getUpdatePacket() != null)
                player.connection.sendPacket(tileEntity.getUpdatePacket());
        }
    }

 

 

Thank you.

[diesieben07]

You should not sync the inventory to the client manually. This happens automatically through the Container mechanisms.

Syncing to the client also has nothing to do with things saving to disk (i.e. persisting through restarts).

 

In general I can only recommend to keep "saving to disk" and "syncing to the client" clearly separated, as they very rarely do the same thing.

 

Also, why is dispatchTEPacket a thing? Vanilla already does that.

[unassigned]

2 hours ago, diesieben07 said:

You should not sync the inventory to the client manually. This happens automatically through the Container mechanisms.

Syncing to the client also has nothing to do with things saving to disk (i.e. persisting through restarts).

 

My block doesn't actually have a container, nor does it have a GUI. It's a block that stores internally and displays it in the world in a render. I don't think I have to provide a container for this, as nothing is being displayed/interacted with, how I know that these are desynced is from a simple logger within the update function fired on both sides:

[16:57:18] [Client thread/INFO] [STDOUT]: [com.github.unassingedxd.voidutils.main.tile.TileVoidInfuser:update:32]: 1xtile.air@0
[16:57:18] [Server thread/INFO] [STDOUT]: [com.github.unassingedxd.voidutils.main.tile.TileVoidInfuser:update:32]: 1xitem.diamond@0

As you can see, the client thread still thinks that there is nothing in it, while the server thread has the correct information being displayed.

 

2 hours ago, diesieben07 said:

In general I can only recommend to keep "saving to disk" and "syncing to the client" clearly separated, as they very rarely do the same thing.

 

Yeah, that's what I attempted to separate with my SaveType stuff, to cut down on saving/loading every tick when nothing is actually being done. I think I implemented it correctly, as nothing is being saved atm as nothing is needed synced (within the TileBase). However, on the inventory sided of things, if my 'shouldSyncSlots' is true, then it will send every sync update, however, as you see, its just serializing/deserializing nbt's from the inventory, which I think is what you're getting at. Is there any way to manually send this information without a container? As I still believe that a container isn't necessary for the purposes I have for the block right now. 

 

2 hours ago, diesieben07 said:

You should not sync the inventory to the client manually. This happens automatically through the Container mechanisms.

Syncing to the client also has nothing to do with things saving to disk (i.e. persisting through restarts).

 

In general I can only recommend to keep "saving to disk" and "syncing to the client" clearly separated, as they very rarely do the same thing.

  

Also, why is dispatchTEPacket a thing? Vanilla already does that.

This was my attempt to send that packet within intervals to cut down in network traffic, is there a better way to achieve this? Or do I have the completely wrong idea on sending updates on an interval?

 

Thank you for your time.

[diesieben07]

18 hours ago, unassigned said:

Yeah, that's what I attempted to separate with my SaveType stuff, to cut down on saving/loading every tick when nothing is actually being done. I think I implemented it correctly, as nothing is being saved atm as nothing is needed synced (within the TileBase). However, on the inventory sided of things, if my 'shouldSyncSlots' is true, then it will send every sync update, however, as you see, its just serializing/deserializing nbt's from the inventory, which I think is what you're getting at. Is there any way to manually send this information without a container? As I still believe that a container isn't necessary for the purposes I have for the block right now. 

I recommend just keeping the methods separate, like vanilla already does. writeToNbt and readFromNbt should be kept for saving to disk, don't mix in syncing to the client. If you want to use the vanilla mechanism for tile entity syncing, I explained that here.

 

19 hours ago, unassigned said:

This was my attempt to send that packet within intervals to cut down in network traffic, is there a better way to achieve this? Or do I have the completely wrong idea on sending updates on an interval?

Call World::notifyBlockUpdate to re-send the vanilla tile entity sync packet (see above).

[unassigned]

4 minutes ago, diesieben07 said:

I recommend just keeping the methods separate, like vanilla already does. writeToNbt and readFromNbt should be kept for saving to disk, don't mix in syncing to the client. If you want to use the vanilla mechanism for tile entity syncing, I explained that here.

 

Call World::notifyBlockUpdate to re-send the vanilla tile entity sync packet (see above).

Okay, I rearranged my stuff (ex. separating NBT from Syncing) and it seems to be working now. Thanks!