AutoStoring ItemDrops in TileEntity Inventory - Duplication Glitch

[Reika]

I made an item vacuum that absorbs the item drops around it and stores them in its own internal inventory.

 

It works, but with one bug: When given multiple different stacks of the same ID, it will start losing items or getting extras. The gain rate is about 0.2 - 2%, whereas the loss rate, while usually zero, has, rarely, exceeded 76000% (36 stacks -> 3 items).

 

Why is it doing this? Also worth noting: it was duplicating everything (running the method twice per call, for a total of 4 times (2x server + 2x client) when I let it run the absorb code every tick, but when I switched that to every 2 ticks, that bug stopped and this new subtler one arose.

 

Here is my code:

 

    public void updateEntity() {
        tickcount++;
        if (this.power < MINPOWER)
            return;
        if (tickcount < 2)
            return;
        tickcount = 0;
        this.suck(this.worldObj, this.xCoord, this.yCoord, this.zCoord);
        this.absorb(this.worldObj, this.xCoord, this.yCoord, this.zCoord);
    }
    
    public void suck(World world, int x, int y, int z) {
        AxisAlignedBB box = this.getBox(world, x, y, z);
        List inbox = world.getEntitiesWithinAABB(EntityItem.class, box);
        for (int i = 0; i < inbox.size(); i++) {
            EntityItem ent = (EntityItem)inbox.get(i);
            double dd = 8.0D;
            double dx = (this.xCoord - ent.posX) / dd;
            double dy = (this.yCoord - ent.posY) / dd;
            double dz = (this.zCoord - ent.posZ) / dd;
            double ddt = ReikaMathLibrary.py3d(dx, dy, dz);
            double dd1 = 1.0D - ddt;

            if (dd1 > 0.0D)
            {
                dd1 *= dd1;
                ent.motionX += dx / ddt * dd1 * 0.2D;
                ent.motionY += dy / ddt * dd1 * 0.2D;
                ent.motionZ += dz / ddt * dd1 * 0.2D;
            }
        }
    }

    public void absorb(World world, int x, int y, int z) {
        AxisAlignedBB close = AxisAlignedBB.getBoundingBox(this.xCoord, this.yCoord, this.zCoord, this.xCoord+1, this.yCoord+1, this.zCoord+1).expand(0.25D, 0.25D, 0.25D);
        List closeitems = world.getEntitiesWithinAABB(EntityItem.class, close);
        for (int i = 0; i < closeitems.size(); i++) {
            EntityItem ent = (EntityItem)closeitems.get(i);
            ItemStack is = ent.getEntityItem();
            int targetslot = this.checkForStack(is);
            if (targetslot != -1) {
                if (this.inventory[targetslot] == null)
                    this.inventory[targetslot] = new ItemStack(is.itemID, is.stackSize, is.getItemDamage());
                else
                    this.inventory[targetslot].stackSize += is.stackSize;
            }
            else {
                return;
            }
            ent.setDead();
         
                world.playSoundEffect(x+0.5, y+0.5, z+0.5, "random.pop", 0.1F+0.5F*par5Random.nextFloat(), par5Random.nextFloat());
        }
    }
    
    public int checkForStack(ItemStack is) {
        int target = -1;
        int id = is.itemID;
        int meta = is.getItemDamage();
        int size = is.stackSize;
        int firstempty = -1;
        
        for (int k = 0; k < this.inventory.length; k++) { //Find first empty slot
            if (inventory[k] == null) {
                firstempty = k;
                k = inventory.length;
            }
        }
        for (int j = 0; j < this.inventory.length; j++) {
            if (inventory[j] != null) {
                if (inventory[j].itemID == id && inventory[j].getItemDamage() == meta) {
                    if (inventory[j].stackSize+size <= is.getMaxStackSize()) {
                        target = j;
                        j = inventory.length;
                    }
                    else {
                        int diff = is.getMaxStackSize() - inventory[j].stackSize;
                        inventory[j].stackSize += diff;
                        is.stackSize -= diff;
                    }
                }
            }
        }
        
        if (target == -1)
            target = firstempty;
        return target;
    }
    
    public AxisAlignedBB getBox(World world, int x, int y, int z) {
        int expand = 5;
        AxisAlignedBB box = AxisAlignedBB.getBoundingBox(this.xCoord, this.yCoord, this.zCoord, this.xCoord+1, this.yCoord+1, this.zCoord+1).expand(expand, expand, expand);
        return box;
    }

[Reika]

It seems to me that you are running the pickup code on client & server. That may cause conflicts between the two.

I am, yes, but I got bigger problems when I was not. If server-only, it was leaving ghost items on the ground, and with client only, it did not even absorb them.

[Reika]

I'd say you should only do it on the server. The "ghost entities" should not occur, do you mind posting your code with the "only serverside" option?

What I notice though is that your are making the Items move towards your TE via motionX, etc. The "normal" pickup animation is an EntityFX, you might look into that. The motion of Item entities is always a bit weird.

 

The code is exactly the same as posted, with the only change being the addition of @SideOnly(Side.SERVER) before absorb().

 

Also, the motion is not for animation - it is to actually move the items physically closer; there are two bounding boxes - a large "Area of Effect" one and a small "absorb within" one.

[Reika]

You are misunderstanding @SideOnly

@SideOnly and the Side in general is always about if you are on the Client or on the Dedicated server. If you have @SideOnly(Side=SERVER) it will only exist for the Dedicated server, the Integrated Server will not have that method.

This explains the NoSuchMethodError crashes I often get when using it.

 

Use the worldObj of the TileEntity to check if you are on the Server Side (Integrated or Dedicated). (hint: World.isRemote is always false on the server, true on the client).

This stands in direct contradiction of what I have been told - that in 1.3+, it ALWAYS returns true...

 

 

EDIT: Adding your suggestion has mitigated the problem - ordinary operation no longer duplicates items, but breaking and placing the machine will duplicate or delete items, with greater probability and greater numbers as you do it more rapidly. I am using default furnace drop-on-break code:

    public void breakBlock(World par1World, int par2, int par3, int par4, int par5, int par6)
    {
            TileEntityVacuum tileentityVacuum = (TileEntityVacuum)par1World.getBlockTileEntity(par2, par3, par4);

            if (tileentityVacuum != null)
            {
                label0:

                for (int i = 0; i < tileentityVacuum.getSizeInventory(); i++)
                {
                    ItemStack itemstack = tileentityVacuum.getStackInSlot(i);

                    if (itemstack == null)
                    {
                        continue;
                    }

                    float f = par5Random.nextFloat() * 0.8F + 0.1F;
                    float f1 = par5Random.nextFloat() * 0.8F + 0.1F;
                    float f2 = par5Random.nextFloat() * 0.8F + 0.1F;

                    do
                    {
                        if (itemstack.stackSize <= 0)
                        {
                            continue label0;
                        }

                        int j = par5Random.nextInt(21) + 10;

                        if (j > itemstack.stackSize)
                        {
                            j = itemstack.stackSize;
                        }

                        itemstack.stackSize -= j;
                        EntityItem entityitem = new EntityItem(par1World, (float)par2 + f, (float)par3 + f1, (float)par4 + f2, new ItemStack(itemstack.itemID, j, itemstack.getItemDamage()));

                        if (itemstack.hasTagCompound())
                        {
                            entityitem.getEntityItem().setTagCompound((NBTTagCompound)itemstack.getTagCompound().copy());
                        }

                        float f3 = 0.05F;
                        entityitem.motionX = (float)par5Random.nextGaussian() * f3;
                        entityitem.motionY = (float)par5Random.nextGaussian() * f3 + 0.2F;
                        entityitem.motionZ = (float)par5Random.nextGaussian() * f3;
                        par1World.spawnEntityInWorld(entityitem);
                    }
                    while (true);
                }
            }
  
        super.breakBlock(par1World, par2, par3, par4, par5, par6);
    }

 

I added the check-for-item-ready-to-be-picked-up code since writing the original post - that may be causing some of the duplication. Time shall tell.

 

EDIT 2: It did help, but it did not completely eliminate it (adding entityitem.delayBeforeCanPickup = 10). Also, with multiple vacuums in close proximity, they fight over the drops (expected) but often each one will get a "copy" and this will result in massive duplication.

[Reika]

That is odd - I have used that last one a few times and had no problems with it. Then again, I am not doing anything major with it.

 

Also, Server checking and adding entityitem.delayBeforeCanPickup = 10 did help, but it did not completely eliminate the glitch. Also, with multiple vacuums in close proximity, they fight over the drops (expected) but often each one will get a "copy" and this will result in massive duplication.

[Reika]

That is odd - I have used that last one a few times and had no problems with it. Then again, I am not doing anything major with it.

Sure, it's not slow, but its not the most effective way

Also, Server checking and adding entityitem.delayBeforeCanPickup = 10 did help, but it did not completely eliminate the glitch. Also, with multiple vacuums in close proximity, they fight over the drops (expected) but often each one will get a "copy" and this will result in massive duplication.

That seems odd to me. Are you sure your whole updateEntity method only execute on the server?

I tried making it server-only, but then the drops, from the player's point of view, just disappear then appear inside the internal inventory. I want the player to see the items being literally sucked into the machine.

[Reika]

Then try setting entity.velocityChanged to true when you change the entity motion. Still, only do it serverside, otherwise there is no way to completely avoid duplication.

It did not help - duplication is still occurring. That said, this technique is going to fix some glitches with the Fan and PileDriver.

[Reika]

package Reika.RotaryCraft;

import java.util.List;
import java.util.Random;

import net.minecraft.entity.item.EntityItem;
import net.minecraft.entity.player.EntityPlayer;
import net.minecraft.inventory.IInventory;
import net.minecraft.item.ItemStack;
import net.minecraft.nbt.NBTTagCompound;
import net.minecraft.nbt.NBTTagList;
import net.minecraft.src.ModLoader;
import net.minecraft.util.AxisAlignedBB;
import net.minecraft.world.World;
import Reika.DragonAPI.ReikaMathLibrary;
import cpw.mods.fml.relauncher.Side;
import cpw.mods.fml.relauncher.SideOnly;

public class TileEntityVacuum extends TileEntityPowerReceiver implements IInventory {

public static final int MINPOWER = 32768;
public static final int MAXRANGE = RotaryConfig.maxvacuumrange;

public ItemStack[] inventory = new ItemStack[54]; 

private int tickcount = 0;

public Random par5Random = new Random();

public boolean canUpdate() {
	return true;
}

public void updateEntity() {
	if (this.worldObj.isRemote)
		return;
	this.getPower4Sided();
	this.power = this.torque*this.omega;
	tickcount++;
	if (this.power < MINPOWER)
		return;
	if (tickcount < 2)
		return;
	tickcount = 0;
	this.suck(this.worldObj, this.xCoord, this.yCoord, this.zCoord);
	this.absorb(this.worldObj, this.xCoord, this.yCoord, this.zCoord);
}

public void suck(World world, int x, int y, int z) {
	AxisAlignedBB box = this.getBox(world, x, y, z);
	List inbox = world.getEntitiesWithinAABB(EntityItem.class, box);
	for (int i = 0; i < inbox.size(); i++) {
		EntityItem ent = (EntityItem)inbox.get(i);
		double dd = 8.0D;
            double dx = (this.xCoord - ent.posX) / dd;
            double dy = (this.yCoord - ent.posY) / dd;
            double dz = (this.zCoord - ent.posZ) / dd;
            double ddt = ReikaMathLibrary.py3d(dx, dy, dz);
            double dd1 = 1.0D - ddt;

            if (dd1 > 0.0D)
            {
                dd1 *= dd1;
                ent.motionX += dx / ddt * dd1 * 0.2D;
                ent.motionY += dy / ddt * dd1 * 0.2D;
                ent.motionZ += dz / ddt * dd1 * 0.2D;
                if (!world.isRemote)
                	ent.velocityChanged = true;
            }
	}
}

public void absorb(World world, int x, int y, int z) {
	if (world.isRemote)
		return;
	AxisAlignedBB close = AxisAlignedBB.getBoundingBox(this.xCoord, this.yCoord, this.zCoord, this.xCoord+1, this.yCoord+1, this.zCoord+1).expand(0.25D, 0.25D, 0.25D);
	List closeitems = world.getEntitiesWithinAABB(EntityItem.class, close);
	//ModLoader.getMinecraftInstance().thePlayer.addChatMessage(String.format("%d", closeitems.size()));
	for (int i = 0; i < closeitems.size(); i++) {
		EntityItem ent = (EntityItem)closeitems.get(i);
		if (ent.delayBeforeCanPickup <= 0) {
			ItemStack is = ent.getEntityItem();
			int targetslot = this.checkForStack(is);
			// Keep note: the checkForStack may not decr the size of the "real" stack, just
			// the projected copy inside itself - watch to see if "extra" items appearing
			if (targetslot != -1) {
				if (this.inventory[targetslot] == null)
					this.inventory[targetslot] = new ItemStack(is.itemID, is.stackSize, is.getItemDamage());
				else
					this.inventory[targetslot].stackSize += is.stackSize;
			}
			else {
				return;
			}
			//ModLoader.getMinecraftInstance().thePlayer.addChatMessage(String.format("%f", par5Random.nextFloat()));
			ent.setDead();
			//ModLoader.getMinecraftInstance().thePlayer.addChatMessage(String.valueOf(FMLCommonHandler.instance().getEffectiveSide()));
			//if (FMLCommonHandler.instance().getEffectiveSide() == Side.SERVER) {
				world.playSoundEffect(x+0.5, y+0.5, z+0.5, "random.pop", 0.1F+0.5F*par5Random.nextFloat(), par5Random.nextFloat());
				//ModLoader.getMinecraftInstance().thePlayer.addChatMessage("FD2");
			//}
		}
	}
}

public int checkForStack(ItemStack is) {
	int target = -1;
	int id = is.itemID;
	int meta = is.getItemDamage();
	int size = is.stackSize;
	int firstempty = -1;

	for (int k = 0; k < this.inventory.length; k++) { //Find first empty slot
		if (inventory[k] == null) {
			firstempty = k;
			k = inventory.length;
		}
	}
	for (int j = 0; j < this.inventory.length; j++) {
		if (inventory[j] != null) {
			if (inventory[j].itemID == id && inventory[j].getItemDamage() == meta) {
				if (inventory[j].stackSize+size <= is.getMaxStackSize()) {
					target = j;
					j = inventory.length;
				}
				else {
					int diff = is.getMaxStackSize() - inventory[j].stackSize;
					inventory[j].stackSize += diff;
					is.stackSize -= diff;
				}
			}
		}
	}

	if (target == -1)
		target = firstempty;
	return target;
}

public AxisAlignedBB getBox(World world, int x, int y, int z) {
	int expand = ReikaMathLibrary.extrema((int)(this.power/MINPOWER), MAXRANGE, "min");
	AxisAlignedBB box = AxisAlignedBB.getBoundingBox(this.xCoord, this.yCoord, this.zCoord, this.xCoord+1, this.yCoord+1, this.zCoord+1).expand(expand, expand, expand);
	return box;
}

 /**
     * Returns the number of slots in the inventory.
     */
    public int getSizeInventory()
    {
        return inventory.length;
    }
    
    public static boolean func_52005_b(ItemStack par0ItemStack)
    {
        return true;
    }

    /**
     * Returns the stack in slot i
     */
    public ItemStack getStackInSlot(int par1)
    {
        return inventory[par1];
    }

    /**
     * Decrease the size of the stack in slot (first int arg) by the amount of the second int arg. Returns the new
     * stack.
     */
    public ItemStack decrStackSize(int par1, int par2)
    {
        if (inventory[par1] != null)
        {
            if (inventory[par1].stackSize <= par2)
            {
                ItemStack itemstack = inventory[par1];
                inventory[par1] = null;
                return itemstack;
            }

            ItemStack itemstack1 = inventory[par1].splitStack(par2);

            if (inventory[par1].stackSize == 0)
            {
                inventory[par1] = null;
            }

            return itemstack1;
        }
        else
        {
            return null;
        }
    }

    /**
     * When some containers are closed they call this on each slot, then drop whatever it returns as an EntityItem -
     * like when you close a workbench GUI.
     */
    public ItemStack getStackInSlotOnClosing(int par1)
    {
        if (inventory[par1] != null)
        {
            ItemStack itemstack = inventory[par1];
            inventory[par1] = null;
            return itemstack;
        }
        else
        {
            return null;
        }
    }

    /**
     * Sets the given item stack to the specified slot in the inventory (can be crafting or armor sections).
     */
    public void setInventorySlotContents(int par1, ItemStack par2ItemStack)
    {
        inventory[par1] = par2ItemStack;

        if (par2ItemStack != null && par2ItemStack.stackSize > getInventoryStackLimit())
        {
            par2ItemStack.stackSize = getInventoryStackLimit();
        }
    }

    /**
     * Returns the name of the inventory.
     */
    public String getInvName()
    {
        return "Item Vacuum";
    }

    /**
     * Reads a tile entity from NBT.
     */
    public void readFromNBT(NBTTagCompound par1NBTTagCompound)
    {
        super.readFromNBT(par1NBTTagCompound);
        NBTTagList nbttaglist = par1NBTTagCompound.getTagList("Items");
        inventory = new ItemStack[getSizeInventory()];

        for (int i = 0; i < nbttaglist.tagCount(); i++)
        {
            NBTTagCompound nbttagcompound = (NBTTagCompound)nbttaglist.tagAt(i);
            byte byte0 = nbttagcompound.getByte("Slot");

            if (byte0 >= 0 && byte0 < inventory.length)
            {
                inventory[byte0] = ItemStack.loadItemStackFromNBT(nbttagcompound);
            }
        }
        this.torque = par1NBTTagCompound.getInteger("torque");
        this.omega = par1NBTTagCompound.getInteger("omega");
    }

    /**
     * Writes a tile entity to NBT.
     */
    public void writeToNBT(NBTTagCompound par1NBTTagCompound)
    {
        super.writeToNBT(par1NBTTagCompound);
        par1NBTTagCompound.setInteger("torque", this.torque);
        par1NBTTagCompound.setInteger("omega", this.omega);
        NBTTagList nbttaglist = new NBTTagList();

        for (int i = 0; i < inventory.length; i++)
        {
            if (inventory[i] != null)
            {
                NBTTagCompound nbttagcompound = new NBTTagCompound();
                nbttagcompound.setByte("Slot", (byte)i);
                inventory[i].writeToNBT(nbttagcompound);
                nbttaglist.appendTag(nbttagcompound);
            }
        }

        par1NBTTagCompound.setTag("Items", nbttaglist);
    }

    /**
     * Returns the maximum stack size for a inventory slot. Seems to always be 64, possibly will be extended. *Isn't
     * this more of a set than a get?*
     */
    public int getInventoryStackLimit()
    {
        return 64;
    }
    
    public void openChest() {
    	
    }

    public void closeChest() {
    	
    }
    
    public boolean isUseableByPlayer(EntityPlayer par1EntityPlayer) {
        if (worldObj.getBlockTileEntity(xCoord, yCoord, zCoord) != this)
            return false;	 
        return par1EntityPlayer.getDistanceSq((double)xCoord + 0.5D, (double)yCoord + 0.5D, (double)zCoord + 0.5D) <= 64D;
    }
}

[Reika]

Hm. If it really duplicates things I am out of things to check what causes it. Your TileEntity seems fine to me.

My best guess is that the 2+ TEs are running the absorb code simultaneously, so all get a copy of the entityitem. This makes sense, as they would all detect the item simultaneously, but I have no idea how to fix it.

[Reika]

Almost everything in MC runs in a single Thread, so TEs, Entities, etc. are ticked after each other, not simultaneously.

Really? Well, that is interesting. Which leaves me at an even greater loss for an explanation.