Cloned private variable is changing without trace

Hello. I'm creating a plugin to log chest access changes, and I just track Open/Close event to get the changes. When player opens a chest, I add them to the openedInventories variable, where key is Player and Value is new instance of my class Inventory. My problem is that when I try to get the previous state of chest when closing the chest, I get some strange result, not corresponding to the put value.

My code, EventListener.java:

Code:

package genesis;
 
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
 
import org.bukkit.Location;
import org.bukkit.Material;
import org.bukkit.block.Block;
import org.bukkit.entity.Creeper;
import org.bukkit.entity.EnderDragon;
import org.bukkit.entity.Fireball;
import org.bukkit.entity.Player;
import org.bukkit.entity.TNTPrimed;
import org.bukkit.event.EventHandler;
import org.bukkit.event.EventPriority;
import org.bukkit.event.Listener;
import org.bukkit.event.block.Action;
import org.bukkit.event.block.BlockBreakEvent;
import org.bukkit.event.block.BlockBurnEvent;
import org.bukkit.event.block.BlockPlaceEvent;
import org.bukkit.event.entity.EntityExplodeEvent;
import org.bukkit.event.entity.PlayerDeathEvent;
import org.bukkit.event.inventory.InventoryClickEvent;
import org.bukkit.event.inventory.InventoryCloseEvent;
import org.bukkit.event.inventory.InventoryEvent;
import org.bukkit.event.player.PlayerChatEvent;
import org.bukkit.event.player.PlayerInteractEvent;
import org.bukkit.event.player.PlayerJoinEvent;
import org.bukkit.event.player.PlayerQuitEvent;
import org.bukkit.inventory.InventoryHolder;
import org.bukkit.inventory.ItemStack;
 
public class EventListener implements Listener {
    BlockLog plugin;
    BlockLog pl;
 
    public EventListener(BlockLog pl) {
        this.pl = pl;
        this.plugin = pl;
    }
 
    HashMap<Player, Inventory> openedInventories = new HashMap<Player, Inventory>();
 
    @EventHandler
    public void chestClose1(PlayerDeathEvent e) {
        this.chestCloseInventory(e.getEntity());
    }
 
    @EventHandler
    public void chestClose2(PlayerQuitEvent e) {
        this.chestCloseInventory(e.getPlayer());
    }
 
    @EventHandler
    public void chestClose3(PlayerChatEvent e) {
        this.chestCloseInventory(e.getPlayer());
    }
 
    private void chestCloseInventory(Player player) {
        if (this.openedInventories.get(player) == null) {
            return;
        }
 
        ItemStack[] inventoryBefore = this.openedInventories.get(player).getInventoryBefore();
        ItemStack[] inventoryAfter = this.openedInventories.get(player).getInventoryNow();
        this.openedInventories.remove(player);
        
    }
 
    @EventHandler
    public void chestOpen(PlayerInteractEvent e) {
 
        if (e.getAction() == Action.RIGHT_CLICK_BLOCK) {
            if (Utils.isChest(e.getClickedBlock().getTypeId())) {
                this.openedInventories.put(e.getPlayer(), new Inventory(((InventoryHolder) e.getClickedBlock().getState()), this.pl));
                plugin.debug("opened inventory");
            }
        }
    }
}

Inventory.java

Code:

package genesis;
 
import org.bukkit.inventory.InventoryHolder;
import org.bukkit.inventory.ItemStack;
 
class Inventory {
    private InventoryHolder holder;
    private ItemStack[] itemsBefore;
    private String id;
    private BlockLog plugin;
 
    public Inventory(InventoryHolder holder, BlockLog plugin) {
        this.id = "" + Math.random();
        this.plugin = plugin;
        this.holder = holder;
        this.setInventoryBefore(this.getInventoryNow());
        plugin.debug("[" + id + "] initializating inventory: itemsBefore.amount == " + itemsBefore[0].getAmount());
    }
 
    public ItemStack[] getInventoryBefore() {
        plugin.debug("[" + id + "] getting inventory before" + this.itemsBefore[0].getAmount());
        return this.itemsBefore;
 
    }
 
    public void setInventoryBefore(ItemStack[] ib) {
        plugin.debug("[" + id + "] setting inventory before" + ib[0].getAmount());
        this.itemsBefore = ib.clone();
    }
 
    public ItemStack[] getInventoryNow() {
        plugin.debug("[" + id + "] getting iventory now" + holder.getInventory().getContents().clone()[0].getAmount());
        return holder.getInventory().getContents().clone();
    }
}

When I go to the chest and leave it imediatelly without taking any items, I get the correct debug results. However, when I take a stack of sponge from my inventory and immediately put it back*, and then I try to write a message (to trigger chest close), it shows me that getInventoryBefore()[0].getAmount() is zero, which makes no sense, because the setInventoryBefore() shown correct value (64), and there were no calls to setInventoryBefore() after it. I clone some of the variables to make sure that they're not referenced and that no action from Bukkit side is taken.

Is there something I missed?

* debug output -
[0.0132465465465] getting inventory now64
[0.0132465465465] setting inventory before64
[0.0132465465465] initializing inventory: itemsBefore.amount == 64
opened inventory
[0.0132465465465] getting inventory before0 <--- Here is the problem
[0.0132465465465] getting inventory now64

 

I'm not sure, but your problem might be that you aren't making a full copy of the inventory. inventoryBefore array elements can still be modified, because .clone() only copies the array.

Do a loop and clone each array element and see if it changes anything.

Speculation on my part on what might be happening:
You set inventoryBefore[0]: 64 x SPONGE
You take the item and bukkit sets inventoryBefore[0] amount: 0 (it is still the item in the chests inventory, only in a different array)
You put the item back and bukkit creates a new ItemStack and sets it to slot zero: 64 x SPONDGE
Because a new ItemStack was created inventoryBefore[0] is no longer changed and it remains: 0
You request item from chest slot zero: 64 x SPONDGE

 

This is genius, thanks! That was it! If I'd post this earlier I'd save myself a lot of time.

I actually think that inventory[0] is changed, but not on my side, because my array can't be modified (it is cloned)


Isn't there something called deep clone in java?

 

you can loop the elements by your own, and clone them by your self

 

I found a deep cloning library (which tells me that java doesn't have one by default):
http://code.google.com/p/cloning/

Haven't used it myself.