SPIGOT-2871: Improve BlockStates + BlockStateMeta

nms-patches/ContainerEnchantTable.patch

@@ -1,13 +1,12 @@
 --- a/net/minecraft/server/ContainerEnchantTable.java
 +++ b/net/minecraft/server/ContainerEnchantTable.java
-@@ -3,9 +3,22 @@
+@@ -3,6 +3,17 @@
  import java.util.List;
  import java.util.Random;
  
 +// CraftBukkit start
 +import java.util.Map;
 +import org.bukkit.Location;
-+
 +import org.bukkit.craftbukkit.inventory.CraftInventoryEnchanting;
 +import org.bukkit.craftbukkit.inventory.CraftInventoryView;
 +import org.bukkit.craftbukkit.inventory.CraftItemStack;
@@ -18,21 +17,18 @@
 +
  public class ContainerEnchantTable extends Container {
  
--    public IInventory enchantSlots = new InventorySubcontainer("Enchant", true, 2) {
-+    // CraftBukkit - make type specific (changed from IInventory)
-+    public InventorySubcontainer enchantSlots = new InventorySubcontainer("Enchant", true, 2) {
-         public int getMaxStackSize() {
-             return 64;
-         }
-@@ -14,6 +27,11 @@
+     public IInventory enchantSlots = new InventorySubcontainer("Enchant", true, 2) {
+@@ -14,6 +25,13 @@
              super.update();
              ContainerEnchantTable.this.a((IInventory) this);
          }
 +
++        // CraftBukkit start
 +        @Override
 +        public Location getLocation() {
 +            return new org.bukkit.Location(world.getWorld(), position.getX(), position.getY(), position.getZ());
 +        }
++        // CraftBukkit end
      };
      public World world;
      private final BlockPosition position;

nms-patches/ItemReed.patch

@@ -0,0 +1,11 @@
+--- a/net/minecraft/server/ItemReed.java
++++ b/net/minecraft/server/ItemReed.java
+@@ -2,7 +2,7 @@
+ 
+ public class ItemReed extends Item {
+ 
+-    private final Block a;
++    public final Block a; // PAIL: private->public
+ 
+     public ItemReed(Block block) {
+         this.a = block;

src/main/java/org/bukkit/craftbukkit/block/CraftBlock.java

@@ -323,6 +323,16 @@ public class CraftBlock implements Block {
         case RED_SHULKER_BOX:
         case BLACK_SHULKER_BOX:
             return new CraftShulkerBox(this);
+        case ENCHANTMENT_TABLE:
+            return new CraftEnchantingTable(this);
+        case ENDER_CHEST:
+            return new CraftEnderChest(this);
+        case DAYLIGHT_DETECTOR:
+        case DAYLIGHT_DETECTOR_INVERTED:
+            return new CraftDaylightDetector(this);
+        case REDSTONE_COMPARATOR_OFF:
+        case REDSTONE_COMPARATOR_ON:
+            return new CraftComparator(this);
         default:
             return new CraftBlockState(this);
         }

src/main/java/org/bukkit/craftbukkit/block/CraftComparator.java

@@ -0,0 +1,33 @@
+package org.bukkit.craftbukkit.block;
+
+import net.minecraft.server.TileEntity;
+import net.minecraft.server.TileEntityComparator;
+import org.bukkit.Material;
+import org.bukkit.block.Block;
+import org.bukkit.block.EnderChest;
+import org.bukkit.craftbukkit.CraftWorld;
+
+public class CraftComparator extends CraftBlockState implements EnderChest {
+
+    private final CraftWorld world;
+    private final TileEntityComparator comparator;
+
+    public CraftComparator(final Block block) {
+        super(block);
+
+        world = (CraftWorld) block.getWorld();
+        comparator = (TileEntityComparator) world.getTileEntityAt(getX(), getY(), getZ());
+    }
+
+    public CraftComparator(final Material material, final TileEntityComparator te) {
+        super(material);
+
+        comparator = te;
+        world = null;
+    }
+
+    @Override
+    public TileEntity getTileEntity() {
+        return comparator;
+    }
+}

src/main/java/org/bukkit/craftbukkit/block/CraftContainer.java

@@ -1,7 +1,7 @@
 package org.bukkit.craftbukkit.block;
 
 import net.minecraft.server.ChestLock;
-import net.minecraft.server.TileEntity;
+import net.minecraft.server.ITileInventory;
 import net.minecraft.server.TileEntityContainer;
 import org.bukkit.Material;
 import org.bukkit.block.Block;
@@ -10,7 +10,7 @@ import org.bukkit.craftbukkit.CraftWorld;
 
 public class CraftContainer extends CraftBlockState implements Lockable {
 
-    private final TileEntityContainer container;
+    private final ITileInventory container;
 
     public CraftContainer(Block block) {
         super(block);
@@ -18,10 +18,10 @@ public class CraftContainer extends CraftBlockState implements Lockable {
         container = (TileEntityContainer) ((CraftWorld) block.getWorld()).getTileEntityAt(block.getX(), block.getY(), block.getZ());
     }
 
-    public CraftContainer(final Material material, TileEntity tileEntity) {
+    public CraftContainer(final Material material, ITileInventory tileEntity) {
         super(material);
 
-        container = (TileEntityContainer) tileEntity;
+        container = tileEntity;
     }
 
     @Override

src/main/java/org/bukkit/craftbukkit/block/CraftDaylightDetector.java

@@ -0,0 +1,33 @@
+package org.bukkit.craftbukkit.block;
+
+import net.minecraft.server.TileEntity;
+import net.minecraft.server.TileEntityLightDetector;
+import org.bukkit.Material;
+import org.bukkit.block.Block;
+import org.bukkit.block.EnderChest;
+import org.bukkit.craftbukkit.CraftWorld;
+
+public class CraftDaylightDetector extends CraftBlockState implements EnderChest {
+
+    private final CraftWorld world;
+    private final TileEntityLightDetector detector;
+
+    public CraftDaylightDetector(final Block block) {
+        super(block);
+
+        world = (CraftWorld) block.getWorld();
+        detector = (TileEntityLightDetector) world.getTileEntityAt(getX(), getY(), getZ());
+    }
+
+    public CraftDaylightDetector(final Material material, final TileEntityLightDetector te) {
+        super(material);
+
+        detector = te;
+        world = null;
+    }
+
+    @Override
+    public TileEntity getTileEntity() {
+        return detector;
+    }
+}

src/main/java/org/bukkit/craftbukkit/block/CraftEnchantingTable.java

@@ -0,0 +1,43 @@
+package org.bukkit.craftbukkit.block;
+
+import net.minecraft.server.TileEntity;
+import net.minecraft.server.TileEntityEnchantTable;
+import org.bukkit.Material;
+import org.bukkit.block.Block;
+import org.bukkit.craftbukkit.CraftWorld;
+import org.bukkit.block.EnchantingTable;
+
+public class CraftEnchantingTable extends CraftBlockState implements EnchantingTable {
+
+    private final CraftWorld world;
+    private final TileEntityEnchantTable enchant;
+
+    public CraftEnchantingTable(final Block block) {
+        super(block);
+
+        world = (CraftWorld) block.getWorld();
+        enchant = (TileEntityEnchantTable) world.getTileEntityAt(getX(), getY(), getZ());
+    }
+
+    public CraftEnchantingTable(final Material material, final TileEntityEnchantTable te) {
+        super(material);
+
+        enchant = te;
+        world = null;
+    }
+
+    @Override
+    public TileEntity getTileEntity() {
+        return enchant;
+    }
+
+    @Override
+    public String getCustomName() {
+        return enchant.hasCustomName() ? enchant.getName() : null;
+    }
+
+    @Override
+    public void setCustomName(String name) {
+        enchant.a(name); // PAIL: setCustomName
+    }
+}

src/main/java/org/bukkit/craftbukkit/block/CraftEnderChest.java

@@ -0,0 +1,33 @@
+package org.bukkit.craftbukkit.block;
+
+import net.minecraft.server.TileEntity;
+import net.minecraft.server.TileEntityEnderChest;
+import org.bukkit.Material;
+import org.bukkit.block.Block;
+import org.bukkit.block.EnderChest;
+import org.bukkit.craftbukkit.CraftWorld;
+
+public class CraftEnderChest extends CraftBlockState implements EnderChest {
+
+    private final CraftWorld world;
+    private final TileEntityEnderChest chest;
+
+    public CraftEnderChest(final Block block) {
+        super(block);
+
+        world = (CraftWorld) block.getWorld();
+        chest = (TileEntityEnderChest) world.getTileEntityAt(getX(), getY(), getZ());
+    }
+
+    public CraftEnderChest(final Material material, final TileEntityEnderChest te) {
+        super(material);
+
+        chest = te;
+        world = null;
+    }
+
+    @Override
+    public TileEntity getTileEntity() {
+        return chest;
+    }
+}

src/main/java/org/bukkit/craftbukkit/block/CraftFlowerPot.java

@@ -1,6 +1,7 @@
 package org.bukkit.craftbukkit.block;
 
 import net.minecraft.server.ItemStack;
+import net.minecraft.server.TileEntity;
 import net.minecraft.server.TileEntityFlowerPot;
 import org.bukkit.Material;
 import org.bukkit.block.Block;
@@ -26,6 +27,11 @@ public class CraftFlowerPot extends CraftBlockState implements FlowerPot {
         this.pot = pot;
     }
 
+    @Override
+    public TileEntity getTileEntity() {
+        return pot;
+    }
+
     @Override
     public MaterialData getContents() {
         return (pot.d() == null) ? null : CraftMagicNumbers.getMaterial(pot.getItem()).getNewData((byte) pot.getData()); // PAIL: rename

src/main/java/org/bukkit/craftbukkit/block/CraftLootable.java

@@ -1,6 +1,5 @@
 package org.bukkit.craftbukkit.block;
 
-import net.minecraft.server.TileEntity;
 import net.minecraft.server.TileEntityLootable;
 import org.bukkit.Material;
 import org.bukkit.Nameable;
@@ -17,10 +16,10 @@ public class CraftLootable extends CraftContainer implements Nameable {
         te = (TileEntityLootable) ((CraftWorld) block.getWorld()).getTileEntityAt(getX(), getY(), getZ());
     }
 
-    public CraftLootable(Material material, TileEntity tileEntity) {
+    public CraftLootable(Material material, TileEntityLootable tileEntity) {
         super(material, tileEntity);
 
-        te = (TileEntityLootable) tileEntity;
+        te = tileEntity;
     }
 
     @Override

src/main/java/org/bukkit/craftbukkit/block/CraftStructureBlock.java

@@ -1,5 +1,6 @@
 package org.bukkit.craftbukkit.block;
 
+import net.minecraft.server.TileEntity;
 import net.minecraft.server.TileEntityStructure;
 import org.bukkit.Material;
 import org.bukkit.block.Block;
@@ -20,4 +21,9 @@ public class CraftStructureBlock extends CraftBlockState {
 
         this.structure = structure;
     }
+
+    @Override
+    public TileEntity getTileEntity() {
+        return structure;
+    }
 }

src/main/java/org/bukkit/craftbukkit/inventory/CraftInventoryEnchanting.java

@@ -1,12 +1,11 @@
 package org.bukkit.craftbukkit.inventory;
 
+import net.minecraft.server.IInventory;
 import org.bukkit.inventory.EnchantingInventory;
 import org.bukkit.inventory.ItemStack;
 
-import net.minecraft.server.InventorySubcontainer;
-
 public class CraftInventoryEnchanting extends CraftInventory implements EnchantingInventory {
-    public CraftInventoryEnchanting(InventorySubcontainer inventory) {
+    public CraftInventoryEnchanting(IInventory inventory) {
         super(inventory);
     }
 
@@ -20,11 +19,6 @@ public class CraftInventoryEnchanting extends CraftInventory implements Enchanti
         return getItem(0);
     }
 
-    @Override
-    public InventorySubcontainer getInventory() {
-        return (InventorySubcontainer)inventory;
-    }
-
     @Override
     public void setSecondary(ItemStack item) {
         setItem(1, item);

src/main/java/org/bukkit/craftbukkit/inventory/CraftItemFactory.java

@@ -130,6 +130,7 @@ public final class CraftItemFactory implements ItemFactory {
         case GREEN_SHULKER_BOX:
         case RED_SHULKER_BOX:
         case BLACK_SHULKER_BOX:
+        case ENDER_CHEST:
             return new CraftMetaBlockState(meta, material);
         default:
             return new CraftMetaItem(meta);

src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java

@@ -387,6 +387,7 @@ public final class CraftItemStack extends ItemStack {
             case GREEN_SHULKER_BOX:
             case RED_SHULKER_BOX:
             case BLACK_SHULKER_BOX:
+            case ENDER_CHEST:
                 return new CraftMetaBlockState(item.getTag(), CraftMagicNumbers.getMaterial(item.getItem()));
             default:
                 return new CraftMetaItem(item.getTag());

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaBlockState.java

@@ -12,12 +12,16 @@ import net.minecraft.server.TileEntityBeacon;
 import net.minecraft.server.TileEntityBrewingStand;
 import net.minecraft.server.TileEntityChest;
 import net.minecraft.server.TileEntityCommand;
+import net.minecraft.server.TileEntityComparator;
 import net.minecraft.server.TileEntityDispenser;
 import net.minecraft.server.TileEntityDropper;
+import net.minecraft.server.TileEntityEnchantTable;
 import net.minecraft.server.TileEntityEndGateway;
+import net.minecraft.server.TileEntityEnderChest;
 import net.minecraft.server.TileEntityFlowerPot;
 import net.minecraft.server.TileEntityFurnace;
 import net.minecraft.server.TileEntityHopper;
+import net.minecraft.server.TileEntityLightDetector;
 import net.minecraft.server.TileEntityMobSpawner;
 import net.minecraft.server.TileEntityNote;
 import net.minecraft.server.TileEntityShulkerBox;
@@ -34,10 +38,14 @@ import org.bukkit.craftbukkit.block.CraftBlockState;
 import org.bukkit.craftbukkit.block.CraftBrewingStand;
 import org.bukkit.craftbukkit.block.CraftChest;
 import org.bukkit.craftbukkit.block.CraftCommandBlock;
+import org.bukkit.craftbukkit.block.CraftComparator;
 import org.bukkit.craftbukkit.block.CraftCreatureSpawner;
+import org.bukkit.craftbukkit.block.CraftDaylightDetector;
 import org.bukkit.craftbukkit.block.CraftDispenser;
 import org.bukkit.craftbukkit.block.CraftDropper;
+import org.bukkit.craftbukkit.block.CraftEnchantingTable;
 import org.bukkit.craftbukkit.block.CraftEndGateway;
+import org.bukkit.craftbukkit.block.CraftEnderChest;
 import org.bukkit.craftbukkit.block.CraftFlowerPot;
 import org.bukkit.craftbukkit.block.CraftFurnace;
 import org.bukkit.craftbukkit.block.CraftHopper;
@@ -199,6 +207,7 @@ public class CraftMetaBlockState extends CraftMetaItem implements BlockStateMeta
             case GREEN_SHULKER_BOX:
             case RED_SHULKER_BOX:
             case BLACK_SHULKER_BOX:
+            case ENDER_CHEST:
                 return true;
         }
         return false;
@@ -293,7 +302,7 @@ public class CraftMetaBlockState extends CraftMetaItem implements BlockStateMeta
                 te = new BlockJukeBox.TileEntityRecordPlayer();
             }
             return new CraftJukebox(material, (BlockJukeBox.TileEntityRecordPlayer) te);
-        case BREWING_STAND:
+        case BREWING_STAND_ITEM:
             if (te == null) {
                 te = new TileEntityBrewingStand();
             }
@@ -353,6 +362,27 @@ public class CraftMetaBlockState extends CraftMetaItem implements BlockStateMeta
                 te = new TileEntityShulkerBox();
             }
             return new CraftShulkerBox(material, (TileEntityShulkerBox) te);
+        case ENCHANTMENT_TABLE:
+            if (te == null) {
+                te = new TileEntityEnchantTable();
+            }
+            return new CraftEnchantingTable(material, (TileEntityEnchantTable) te);
+        case ENDER_CHEST:
+            if (te == null){
+                te = new TileEntityEnderChest();
+            }
+            return new CraftEnderChest(material, (TileEntityEnderChest) te);
+        case DAYLIGHT_DETECTOR:
+        case DAYLIGHT_DETECTOR_INVERTED:
+            if (te == null){
+                te = new TileEntityLightDetector();
+            }
+            return new CraftDaylightDetector(material, (TileEntityLightDetector) te);
+        case REDSTONE_COMPARATOR:
+            if (te == null){
+                te = new TileEntityComparator();
+            }
+            return new CraftComparator(material, (TileEntityComparator) te);
         default:
             throw new IllegalStateException("Missing blockState for " + material);
         }
@@ -362,7 +392,7 @@ public class CraftMetaBlockState extends CraftMetaItem implements BlockStateMeta
     public void setBlockState(BlockState blockState) {
         Validate.notNull(blockState, "blockState must not be null");
         TileEntity te = ((CraftBlockState) blockState).getTileEntity();
-        Validate.notNull(te, "Invalid blockState");
+        Validate.notNull(te, "Invalid tile for " + blockState);
 
         boolean valid;
         switch (material) {
@@ -400,7 +430,7 @@ public class CraftMetaBlockState extends CraftMetaItem implements BlockStateMeta
         case JUKEBOX:
             valid = te instanceof BlockJukeBox.TileEntityRecordPlayer;
             break;
-        case BREWING_STAND:
+        case BREWING_STAND_ITEM:
             valid = te instanceof TileEntityBrewingStand;
             break;
         case SKULL:
@@ -444,6 +474,19 @@ public class CraftMetaBlockState extends CraftMetaItem implements BlockStateMeta
         case BLACK_SHULKER_BOX:
             valid = te instanceof TileEntityShulkerBox;
             break;
+        case ENCHANTMENT_TABLE:
+            valid = te instanceof TileEntityEnchantTable;
+            break;
+        case ENDER_CHEST:
+            valid = te instanceof TileEntityEnderChest;
+            break;
+        case DAYLIGHT_DETECTOR:
+        case DAYLIGHT_DETECTOR_INVERTED:
+            valid = te instanceof TileEntityLightDetector;
+            break;
+        case REDSTONE_COMPARATOR:
+            valid = te instanceof TileEntityComparator;
+            break;
         default:
             valid = false;
             break;

src/test/java/org/bukkit/craftbukkit/inventory/ItemMetaTest.java

@@ -5,6 +5,11 @@ import static org.hamcrest.Matchers.*;
 
 import java.util.Arrays;
 import java.util.List;
+import net.minecraft.server.Block;
+import net.minecraft.server.ITileEntity;
+import net.minecraft.server.Item;
+import net.minecraft.server.ItemBlock;
+import net.minecraft.server.ItemReed;
 
 import org.bukkit.Bukkit;
 import org.bukkit.Color;
@@ -21,6 +26,7 @@ import org.bukkit.craftbukkit.inventory.ItemStackTest.CraftWrapper;
 import org.bukkit.enchantments.Enchantment;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.meta.BannerMeta;
+import org.bukkit.inventory.meta.BlockStateMeta;
 import org.bukkit.inventory.meta.BookMeta;
 import org.bukkit.inventory.meta.EnchantmentStorageMeta;
 import org.bukkit.inventory.meta.FireworkEffectMeta;
@@ -127,6 +133,38 @@ public class ItemMetaTest extends AbstractTestingBase {
         assertThat(bukkit, is((ItemStack) craft));
     }
 
+    @Test
+    public void testBlockStateMeta() {
+        for (Item item : (Iterable<Item>) Item.REGISTRY) {
+            Block block = null;
+
+            if (item instanceof ItemBlock) {
+                block = ((ItemBlock) item).getBlock();
+            } else if (item instanceof ItemReed) {
+                block = ((ItemReed) item).a;
+            }
+
+            if (block != null) {
+                if (block instanceof ITileEntity) {
+                    ItemStack stack = CraftItemStack.asNewCraftStack(Item.getItemOf(block));
+
+                    // Command blocks aren't unit testable atm
+                    if (stack.getType() == Material.AIR || stack.getType() == Material.COMMAND || stack.getType() == Material.COMMAND_CHAIN || stack.getType() == Material.COMMAND_REPEATING) {
+                        return;
+                    }
+
+                    ItemMeta meta = stack.getItemMeta();
+                    assertTrue(stack + " has meta of type " + meta + " expected BlockStateMeta", meta instanceof BlockStateMeta);
+
+                    BlockStateMeta blockState = (BlockStateMeta) meta;
+                    assertNotNull(stack + " has null block state", blockState.getBlockState());
+
+                    blockState.setBlockState(blockState.getBlockState());
+                }
+            }
+        }
+    }
+
     @Test
     public void testEachExtraData() {
         final List<StackProvider> providers = Arrays.asList(
@@ -209,7 +247,7 @@ public class ItemMetaTest extends AbstractTestingBase {
                     return cleanStack;
                 }
             },
-            new StackProvider(Material.BANNER) {                    
+            new StackProvider(Material.BANNER) {
                 @Override ItemStack operate(ItemStack cleanStack) {
                     final BannerMeta meta = (BannerMeta) cleanStack.getItemMeta();
                     meta.setBaseColor(DyeColor.CYAN);