[Bleeding] Fix the openInventory methods for custom inventories. Fixes BUKKIT-1248

Details:
- The attributes of custom inventory views are no longer ignored
- Enchanting or crafting inventories no longer ignore the passed inventory and open a new one
- Inventories associated with tile entities no longer raise a class cast exception if there was no associated tile entity
- InventoryOpenEvent and InventoryCloseEvent (if they already had some other inventory open) now fire in all cases
- If for any reason the inventory failed to open, the method now returns null instead of returned the previous inventory they had open (or the default inventory, if none)

2 files changed, 100 insertions, 33 deletions

diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftHumanEntity.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftHumanEntity.java
index e7beb2e6..a0dbf414 100644
--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftHumanEntity.java
+++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftHumanEntity.java
@@ -3,6 +3,8 @@ package org.bukkit.craftbukkit.entity;
 import java.util.Set;
 
 import net.minecraft.server.Container;
+import net.minecraft.server.ContainerBrewingStand;
+import net.minecraft.server.ContainerWorkbench;
 import net.minecraft.server.EntityHuman;
 import net.minecraft.server.EntityPlayer;
 import net.minecraft.server.ICrafting;
@@ -23,6 +25,7 @@ import org.bukkit.inventory.InventoryView;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
 import org.bukkit.block.Block;
+import org.bukkit.craftbukkit.event.CraftEventFactory;
 import org.bukkit.craftbukkit.inventory.CraftContainer;
 import org.bukkit.craftbukkit.inventory.CraftInventory;
 import org.bukkit.craftbukkit.inventory.CraftInventoryPlayer;
@@ -168,7 +171,10 @@ public class CraftHumanEntity extends CraftLivingEntity implements HumanEntity {
     }
 
     public InventoryView openInventory(Inventory inventory) {
+        if(!(getHandle() instanceof EntityPlayer)) return null;
+        EntityPlayer player = (EntityPlayer) getHandle();
         InventoryType type = inventory.getType();
+        Container formerContainer = getHandle().activeContainer;
         // TODO: Should we check that it really IS a CraftInventory first?
         CraftInventory craftinv = (CraftInventory) inventory;
         switch(type) {
@@ -177,28 +183,57 @@ public class CraftHumanEntity extends CraftLivingEntity implements HumanEntity {
             getHandle().openContainer(craftinv.getInventory());
             break;
         case DISPENSER:
-            getHandle().openDispenser((TileEntityDispenser)craftinv.getInventory());
+            if (craftinv.getInventory() instanceof TileEntityDispenser) {
+                getHandle().openDispenser((TileEntityDispenser)craftinv.getInventory());
+            } else {
+                openCustomInventory(inventory, player, 3);
+            }
             break;
         case FURNACE:
-            getHandle().openFurnace((TileEntityFurnace)craftinv.getInventory());
+            if (craftinv.getInventory() instanceof TileEntityFurnace) {
+                getHandle().openFurnace((TileEntityFurnace)craftinv.getInventory());
+            } else {
+                openCustomInventory(inventory, player, 2);
+            }
             break;
         case WORKBENCH:
-            getHandle().startCrafting(getLocation().getBlockX(), getLocation().getBlockY(), getLocation().getBlockZ());
+            openCustomInventory(inventory, player, 1);
             break;
         case BREWING:
-            getHandle().openBrewingStand((TileEntityBrewingStand)craftinv.getInventory());
+            if (craftinv.getInventory() instanceof TileEntityBrewingStand) {
+                getHandle().openBrewingStand((TileEntityBrewingStand)craftinv.getInventory());
+            } else {
+                openCustomInventory(inventory, player, 5);
+            }
             break;
         case ENCHANTING:
-            getHandle().startEnchanting(getLocation().getBlockX(), getLocation().getBlockY(), getLocation().getBlockZ());
+                openCustomInventory(inventory, player, 4);
             break;
         case CREATIVE:
         case CRAFTING:
             throw new IllegalArgumentException("Can't open a " + type + " inventory!");
         }
+        if (getHandle().activeContainer == formerContainer) {
+            return null;
+        }
         getHandle().activeContainer.checkReachable = false;
         return getHandle().activeContainer.getBukkitView();
     }
 
+    private void openCustomInventory(Inventory inventory, EntityPlayer player, int windowType) {
+        Container container = new CraftContainer(inventory, this, player.nextContainerCounter());
+
+        container = CraftEventFactory.callInventoryOpenEvent(player, container);
+        if(container == null) return;
+
+        String title = container.getBukkitView().getTitle();
+        int size = container.getBukkitView().getTopInventory().getSize();
+
+        player.netServerHandler.sendPacket(new Packet100OpenWindow(container.windowId, windowType, title, size));
+        getHandle().activeContainer = container;
+        getHandle().activeContainer.addSlotListener(player);
+    }
+
     public InventoryView openWorkbench(Location location, boolean force) {
         if (!force) {
             Block block = location.getBlock();
@@ -248,18 +283,19 @@ public class CraftHumanEntity extends CraftLivingEntity implements HumanEntity {
         }
 
         // Trigger an INVENTORY_OPEN event
-        InventoryOpenEvent event = new InventoryOpenEvent(inventory);
-        player.activeContainer.transferTo(container, this);
-        server.getPluginManager().callEvent(event);
-        if (event.isCancelled()) {
-            container.transferTo(player.activeContainer, this);
+        container = CraftEventFactory.callInventoryOpenEvent(player, container);
+        if (container == null) {
             return;
         }
 
         // Now open the window
-        player.netServerHandler.sendPacket(new Packet100OpenWindow(container.windowId, 1, "Crafting", 9));
+        InventoryType type = inventory.getType();
+        int windowType = CraftContainer.getNotchInventoryType(type);
+        String title = inventory.getTitle();
+        int size = inventory.getTopInventory().getSize();
+        player.netServerHandler.sendPacket(new Packet100OpenWindow(container.windowId, windowType, title, size));
         player.activeContainer = container;
-        player.activeContainer.addSlotListener((ICrafting) player);
+        player.activeContainer.addSlotListener(player);
     }
 
     public void closeInventory() {
diff --git a/src/main/java/org/bukkit/craftbukkit/inventory/CraftContainer.java b/src/main/java/org/bukkit/craftbukkit/inventory/CraftContainer.java
index 58310061..52696e32 100644
--- a/src/main/java/org/bukkit/craftbukkit/inventory/CraftContainer.java
+++ b/src/main/java/org/bukkit/craftbukkit/inventory/CraftContainer.java
@@ -1,7 +1,9 @@
 package org.bukkit.craftbukkit.inventory;
 
 import org.bukkit.craftbukkit.entity.CraftPlayer;
+import org.bukkit.entity.HumanEntity;
 import org.bukkit.event.inventory.InventoryType;
+import org.bukkit.inventory.Inventory;
 import org.bukkit.inventory.InventoryView;
 
 import net.minecraft.server.Container;
@@ -28,6 +30,30 @@ public class CraftContainer extends Container {
         setupSlots(top, bottom);
     }
 
+    public CraftContainer(final Inventory inventory, final HumanEntity player, int id) {
+        this(new InventoryView() {
+            @Override
+            public Inventory getTopInventory() {
+                return inventory;
+            }
+
+            @Override
+            public Inventory getBottomInventory() {
+                return player.getInventory();
+            }
+
+            @Override
+            public HumanEntity getPlayer() {
+                return player;
+            }
+
+            @Override
+            public InventoryType getType() {
+                return inventory.getType();
+            }
+        }, id);
+    }
+
     @Override
     public InventoryView getBukkitView() {
         return view;
@@ -50,27 +76,7 @@ public class CraftContainer extends Container {
         cachedTitle = view.getTitle();
         if (view.getPlayer() instanceof CraftPlayer) {
             CraftPlayer player = (CraftPlayer) view.getPlayer();
-            int type;
-            switch(cachedType) {
-            case WORKBENCH:
-                type = 1;
-                break;
-            case FURNACE:
-                type = 2;
-                break;
-            case DISPENSER:
-                type = 3;
-                break;
-            case ENCHANTING:
-                type = 4;
-                break;
-            case BREWING:
-                type = 5;
-                break;
-            default:
-                type = 0;
-                break;
-            }
+            int type = getNotchInventoryType(cachedType);
             IInventory top = ((CraftInventory)view.getTopInventory()).getInventory();
             IInventory bottom = ((CraftInventory)view.getBottomInventory()).getInventory();
             this.d.clear();
@@ -85,6 +91,31 @@ public class CraftContainer extends Container {
         return true;
     }
 
+    public static int getNotchInventoryType(InventoryType type) {
+        int typeID;
+        switch(type) {
+        case WORKBENCH:
+            typeID = 1;
+            break;
+        case FURNACE:
+            typeID = 2;
+            break;
+        case DISPENSER:
+            typeID = 3;
+            break;
+        case ENCHANTING:
+            typeID = 4;
+            break;
+        case BREWING:
+            typeID = 5;
+            break;
+        default:
+            typeID = 0;
+            break;
+        }
+        return typeID;
+    }
+
     private void setupSlots(IInventory top, IInventory bottom) {
         switch(cachedType) {
         case CREATIVE: