Clarify dye and wool color datas in DyeColor. Addresses BUKKIT-2786

DyeColor used the wool data for getData(), which is very misleading based
on class name. The old method has been deprecated and replaced with
getWoolData() and getDyeData() for the appropriate types of data values.

The MaterialData classes Dye and Wool were updated appropriately,
especially Dye innapropriately using a DyeColor data value compensation.

Unit tests were added for the new methods, as well as the getColor on Dye
and Wool.

4 files changed, 152 insertions, 38 deletions

diff --git a/src/main/java/org/bukkit/DyeColor.java b/src/main/java/org/bukkit/DyeColor.java
index 343e86c8..c176dfd3 100644
--- a/src/main/java/org/bukkit/DyeColor.java
+++ b/src/main/java/org/bukkit/DyeColor.java
@@ -12,88 +12,115 @@ public enum DyeColor {
     /**
      * Represents white dye
      */
-    WHITE(0x0, Color.WHITE, Color.fromRGB(0x1E1B1B)),
+    WHITE(0x0, 0xF, Color.WHITE, Color.fromRGB(0x1E1B1B)),
     /**
      * Represents orange dye
      */
-    ORANGE(0x1, Color.fromRGB(0xD87f33), Color.fromRGB(0xB3312C)),
+    ORANGE(0x1, 0xE, Color.fromRGB(0xD87f33), Color.fromRGB(0xB3312C)),
     /**
      * Represents magenta dye
      */
-    MAGENTA(0x2, Color.fromRGB(0xB24CD8), Color.fromRGB(0x3B511A)),
+    MAGENTA(0x2, 0xD, Color.fromRGB(0xB24CD8), Color.fromRGB(0x3B511A)),
     /**
      * Represents light blue dye
      */
-    LIGHT_BLUE(0x3, Color.fromRGB(0x6699D8), Color.fromRGB(0x51301A)),
+    LIGHT_BLUE(0x3, 0xC, Color.fromRGB(0x6699D8), Color.fromRGB(0x51301A)),
     /**
      * Represents yellow dye
      */
-    YELLOW(0x4, Color.fromRGB(0xE5E533), Color.fromRGB(0x253192)),
+    YELLOW(0x4, 0xB, Color.fromRGB(0xE5E533), Color.fromRGB(0x253192)),
     /**
      * Represents lime dye
      */
-    LIME(0x5, Color.fromRGB(0x7FCC19), Color.fromRGB(0x7B2FBE)),
+    LIME(0x5, 0xA, Color.fromRGB(0x7FCC19), Color.fromRGB(0x7B2FBE)),
     /**
      * Represents pink dye
      */
-    PINK(0x6, Color.fromRGB(0xF27FA5), Color.fromRGB(0x287697)),
+    PINK(0x6, 0x9, Color.fromRGB(0xF27FA5), Color.fromRGB(0x287697)),
     /**
      * Represents gray dye
      */
-    GRAY(0x7, Color.fromRGB(0x4C4C4C), Color.fromRGB(0xABABAB)),
+    GRAY(0x7, 0x8, Color.fromRGB(0x4C4C4C), Color.fromRGB(0xABABAB)),
     /**
      * Represents silver dye
      */
-    SILVER(0x8, Color.fromRGB(0x999999), Color.fromRGB(0x434343)),
+    SILVER(0x8, 0x7, Color.fromRGB(0x999999), Color.fromRGB(0x434343)),
     /**
      * Represents cyan dye
      */
-    CYAN(0x9, Color.fromRGB(0x4C7F99), Color.fromRGB(0xD88198)),
+    CYAN(0x9, 0x6, Color.fromRGB(0x4C7F99), Color.fromRGB(0xD88198)),
     /**
      * Represents purple dye
      */
-    PURPLE(0xA, Color.fromRGB(0x7F3FB2), Color.fromRGB(0x41CD34)),
+    PURPLE(0xA, 0x5, Color.fromRGB(0x7F3FB2), Color.fromRGB(0x41CD34)),
     /**
      * Represents blue dye
      */
-    BLUE(0xB, Color.fromRGB(0x334CB2), Color.fromRGB(0xDECF2A)),
+    BLUE(0xB, 0x4, Color.fromRGB(0x334CB2), Color.fromRGB(0xDECF2A)),
     /**
      * Represents brown dye
      */
-    BROWN(0xC, Color.fromRGB(0x664C33), Color.fromRGB(0x6689D3)),
+    BROWN(0xC, 0x3, Color.fromRGB(0x664C33), Color.fromRGB(0x6689D3)),
     /**
      * Represents green dye
      */
-    GREEN(0xD, Color.fromRGB(0x667F33), Color.fromRGB(0xC354CD)),
+    GREEN(0xD, 0x2, Color.fromRGB(0x667F33), Color.fromRGB(0xC354CD)),
     /**
      * Represents red dye
      */
-    RED(0xE, Color.fromRGB(0x993333), Color.fromRGB(0xEB8844)),
+    RED(0xE, 0x1, Color.fromRGB(0x993333), Color.fromRGB(0xEB8844)),
     /**
      * Represents black dye
      */
-    BLACK(0xF, Color.fromRGB(0x191919), Color.fromRGB(0xF0F0F0));
+    BLACK(0xF, 0x0, Color.fromRGB(0x191919), Color.fromRGB(0xF0F0F0));
 
-    private final byte data;
+    private final byte woolData;
+    private final byte dyeData;
     private final Color color;
     private final Color firework;
-    private final static DyeColor[] BY_DATA;
+    private final static DyeColor[] BY_WOOL_DATA;
+    private final static DyeColor[] BY_DYE_DATA;
     private final static Map<Color, DyeColor> BY_COLOR;
     private final static Map<Color, DyeColor> BY_FIREWORK;
 
-    private DyeColor(final int data, Color color, Color firework) {
-        this.data = (byte) data;
+    private DyeColor(final int woolData, final int dyeData, Color color, Color firework) {
+        this.woolData = (byte) woolData;
+        this.dyeData = (byte) dyeData;
         this.color = color;
         this.firework = firework;
     }
 
     /**
-     * Gets the associated data value representing this color
+     * Gets the associated (wool) data value representing this color.
      *
-     * @return A byte containing the data value of this color
+     * @return A byte containing the (wool) data value of this color
+     * @deprecated The name is misleading. It would imply {@link Material#INK_SACK} but uses {@link Material#WOOL}
+     * @see #getWoolData()
+     * @see #getDyeData()
      */
+    @Deprecated
     public byte getData() {
-        return data;
+        return getWoolData();
+    }
+
+    /**
+     * Gets the associated wool data value representing this color.
+     *
+     * @return A byte containing the wool data value of this color
+     * @see #getDyeData()
+     */
+    public byte getWoolData() {
+        return woolData;
+    }
+
+    /**
+     * Gets the associated dye data value representing this color.
+     *
+     * @return A byte containing the dye data value of this color
+     * @see #getWoolData()
+     */
+    public byte getDyeData() {
+        return dyeData;
     }
 
     /**
@@ -115,17 +142,47 @@ public enum DyeColor {
     }
 
     /**
-     * Gets the DyeColor with the given data value
+     * Gets the DyeColor with the given (wool) data value.
      *
-     * @param data Data value to fetch
+     * @param data (wool) data value to fetch
      * @return The {@link DyeColor} representing the given value, or null if it doesn't exist
+     * @deprecated The name is misleading. It would imply {@link Material#INK_SACK} but uses {@link Material#WOOL}
+     * @see #getByDyeData(byte)
+     * @see #getByWoolData(byte)
      */
+    @Deprecated
     public static DyeColor getByData(final byte data) {
+        return getByWoolData(data);
+    }
+
+    /**
+     * Gets the DyeColor with the given wool data value.
+     *
+     * @param data Wool data value to fetch
+     * @return The {@link DyeColor} representing the given value, or null if it doesn't exist
+     * @see #getByDyeData(byte)
+     */
+    public static DyeColor getByWoolData(final byte data) {
+        int i = 0xff & data;
+        if (i > BY_WOOL_DATA.length) {
+            return null;
+        }
+        return BY_WOOL_DATA[i];
+    }
+
+    /**
+     * Gets the DyeColor with the given dye data value.
+     *
+     * @param data Dye data value to fetch
+     * @return The {@link DyeColor} representing the given value, or null if it doesn't exist
+     * @see #getByWoolData(byte)
+     */
+    public static DyeColor getByDyeData(final byte data) {
         int i = 0xff & data;
-        if (i > BY_DATA.length) {
+        if (i > BY_DYE_DATA.length) {
             return null;
         }
-        return BY_DATA[i];
+        return BY_DYE_DATA[i];
     }
 
     /**
@@ -149,12 +206,14 @@ public enum DyeColor {
     }
 
     static {
-        BY_DATA = values();
+        BY_WOOL_DATA = values();
+        BY_DYE_DATA = values();
         ImmutableMap.Builder<Color, DyeColor> byColor = ImmutableMap.builder();
         ImmutableMap.Builder<Color, DyeColor> byFirework = ImmutableMap.builder();
 
         for (DyeColor color : values()) {
-            BY_DATA[color.data & 0xff] = color;
+            BY_WOOL_DATA[color.woolData & 0xff] = color;
+            BY_DYE_DATA[color.dyeData & 0xff] = color;
             byColor.put(color.getColor(), color);
             byFirework.put(color.getFireworkColor(), color);
         }
diff --git a/src/main/java/org/bukkit/material/Dye.java b/src/main/java/org/bukkit/material/Dye.java
index e053c7bb..e7c07eae 100644
--- a/src/main/java/org/bukkit/material/Dye.java
+++ b/src/main/java/org/bukkit/material/Dye.java
@@ -33,7 +33,7 @@ public class Dye extends MaterialData implements Colorable {
      * @return DyeColor of this dye
      */
     public DyeColor getColor() {
-        return DyeColor.getByData((byte) (15 - getData()));
+        return DyeColor.getByDyeData(getData());
     }
 
     /**
@@ -42,7 +42,7 @@ public class Dye extends MaterialData implements Colorable {
      * @param color New color of this dye
      */
     public void setColor(DyeColor color) {
-        setData((byte) (15 - color.getData()));
+        setData(color.getDyeData());
     }
 
     @Override
diff --git a/src/main/java/org/bukkit/material/Wool.java b/src/main/java/org/bukkit/material/Wool.java
index 00f2a2ca..62f63bd6 100644
--- a/src/main/java/org/bukkit/material/Wool.java
+++ b/src/main/java/org/bukkit/material/Wool.java
@@ -38,7 +38,7 @@ public class Wool extends MaterialData implements Colorable {
      * @return DyeColor of this dye
      */
     public DyeColor getColor() {
-        return DyeColor.getByData(getData());
+        return DyeColor.getByWoolData(getData());
     }
 
     /**
@@ -47,7 +47,7 @@ public class Wool extends MaterialData implements Colorable {
      * @param color New color of this dye
      */
     public void setColor(DyeColor color) {
-        setData(color.getData());
+        setData(color.getWoolData());
     }
 
     @Override
diff --git a/src/test/java/org/bukkit/DyeColorTest.java b/src/test/java/org/bukkit/DyeColorTest.java
index 70049d7f..9e30fbfc 100644
--- a/src/test/java/org/bukkit/DyeColorTest.java
+++ b/src/test/java/org/bukkit/DyeColorTest.java
@@ -1,15 +1,70 @@
 package org.bukkit;
 
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.*;
+import static org.hamcrest.Matchers.*;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import org.bukkit.material.Colorable;
+import org.bukkit.material.Dye;
+import org.bukkit.material.Wool;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 
+@RunWith(Parameterized.class)
 public class DyeColorTest {
+
+    @Parameters(name= "{index}: {0}")
+    public static List<Object[]> data() {
+        List<Object[]> list = new ArrayList<Object[]>();
+        for (DyeColor dye : DyeColor.values()) {
+            list.add(new Object[] {dye});
+        }
+        return list;
+    }
+
+    @Parameter public DyeColor dye;
+
     @Test
+    @SuppressWarnings("deprecation")
     public void getByData() {
-        for (DyeColor dyeColor : DyeColor.values()) {
-            assertThat(DyeColor.getByData(dyeColor.getData()), is(dyeColor));
-        }
+        byte data = dye.getData();
+
+        DyeColor byData = DyeColor.getByData(data);
+        assertThat(byData, is(dye));
+    }
+
+    @Test
+    public void getByWoolData() {
+        byte data = dye.getWoolData();
+
+        DyeColor byData = DyeColor.getByWoolData(data);
+        assertThat(byData, is(dye));
+    }
+
+    @Test
+    public void getByDyeData() {
+        byte data = dye.getDyeData();
+
+        DyeColor byData = DyeColor.getByDyeData(data);
+        assertThat(byData, is(dye));
+    }
+
+    @Test
+    public void getDyeDyeColor() {
+        testColorable(new Dye(Material.INK_SACK, dye.getDyeData()));
+    }
+
+    @Test
+    public void getWoolDyeColor() {
+        testColorable(new Wool(Material.WOOL, dye.getWoolData()));
+    }
+
+    private void testColorable(final Colorable colorable) {
+        assertThat(colorable.getColor(), is(this.dye));
     }
 }