2 files changed, 63 insertions, 32 deletions

diff --git a/src/main/java/org/bukkit/potion/Potion.java b/src/main/java/org/bukkit/potion/Potion.java
index 18a03ae2..0320bc7d 100644
--- a/src/main/java/org/bukkit/potion/Potion.java
+++ b/src/main/java/org/bukkit/potion/Potion.java
@@ -31,6 +31,9 @@ public class Potion {
         if (type != null) {
             this.name = type.getDamageValue();
         }
+        if (type == null || type == PotionType.WATER) {
+            this.level = 0;
+        }
     }
 
     /** @deprecated In favour of {@link #Potion(PotionType, int)} */
@@ -104,7 +107,7 @@ public class Potion {
     public Potion(int name) {
         this(PotionType.getByDamageValue(name & POTION_BIT));
         this.name = name & NAME_BIT;
-        if (name == 0) {
+        if ((name & POTION_BIT) == 0) {
             // If it's 0 it would've become PotionType.WATER, but it should actually be mundane potion
             this.type = null;
         }
@@ -363,7 +366,7 @@ public class Potion {
     public static Potion fromDamage(int damage) {
         PotionType type = PotionType.getByDamageValue(damage & POTION_BIT);
         Potion potion;
-        if (type == null) {
+        if (type == null || (type == PotionType.WATER && damage != 0)) {
             potion = new Potion(damage & NAME_BIT);
         } else {
             int level = (damage & TIER_BIT) >> TIER_SHIFT;
@@ -406,4 +409,8 @@ public class Potion {
             throw new IllegalArgumentException("brewer can only be set internally");
         brewer = other;
     }
+
+    public int getNameId() {
+        return name;
+    }
 }
\ No newline at end of file
diff --git a/src/test/java/org/bukkit/potion/PotionTest.java b/src/test/java/org/bukkit/potion/PotionTest.java
index afc54352..58252aeb 100644
--- a/src/test/java/org/bukkit/potion/PotionTest.java
+++ b/src/test/java/org/bukkit/potion/PotionTest.java
@@ -85,38 +85,62 @@ public class PotionTest {
         assertTrue((potion.toDamageValue() & 0x3F) == (PotionType.POISON.getDamageValue() | 0x20));
     }
 
-    @Test
-    public void useNulls() {
-        try {
-            new Potion(null, 2);
-            fail("cannot use null type in constructor with a level");
-        } catch (IllegalArgumentException ex) {
-        }
-
-        try {
-            new Potion(PotionType.POISON, 3);
-            fail("level must be less than 3 in constructor");
-        } catch (IllegalArgumentException ex) {
-        }
+    @Test(expected=IllegalArgumentException.class)
+    public void nullType() {
+        new Potion(null, 2);
+    }
+
+    @Test(expected=IllegalArgumentException.class)
+    public void maxLevelConstruct() {
+        new Potion(PotionType.POISON, 3);
+    }
+
+    @Test(expected=IllegalArgumentException.class)
+    public void maxLevelSet() {
+        Potion potion = new Potion(PotionType.POISON);
+        potion.setLevel(3);
+    }
+
+    @Test(expected=IllegalArgumentException.class)
+    public void nullStack() {
+        Potion potion = new Potion(PotionType.POISON);
+        potion.apply((ItemStack) null);
+    }
 
+    @Test(expected=IllegalArgumentException.class)
+    public void nullEntity() {
         Potion potion = new Potion(PotionType.POISON);
-        try {
-            potion.setLevel(3);
-            fail("level must be set less than 3");
-        } catch (IllegalArgumentException ex) {
-        }
-
-        try {
-            potion.apply((ItemStack) null);
-            fail("cannot apply to a null itemstack");
-        } catch (IllegalArgumentException ex) {
-        }
-
-        try {
-            potion.apply((LivingEntity) null);
-            fail("cannot apply to a null entity");
-        } catch (IllegalArgumentException ex) {
-        }
+        potion.apply((LivingEntity) null);
+    }
+
+    @Test
+    public void water() {
+        Potion potion = new Potion(PotionType.WATER);
+        assertEquals(0, potion.getLevel());
+        assertFalse(potion.isSplash());
+        assertFalse(potion.hasExtendedDuration());
+        assertEquals(0, potion.toDamageValue());
+    }
+
+    @Test
+    public void mundane() {
+        Potion potion = new Potion(0);
+        assertFalse(potion.getType() == PotionType.WATER);
+        assertFalse(potion.toDamageValue() == 0);
+        assertEquals(8192, potion.toDamageValue());
+        Potion potion2 = Potion.fromDamage(8192);
+        assertEquals(potion, potion2);
+        assertEquals(0, potion.getLevel());
+    }
+
+    @Test
+    public void awkward() {
+        Potion potion = new Potion(16);
+        assertEquals(16, potion.getNameId());
+        assertFalse(potion.isSplash());
+        assertFalse(potion.hasExtendedDuration());
+        assertNull(potion.getType());
+        assertEquals(16, potion.toDamageValue());
     }
 
     private static final int EXTENDED_BIT = 0x40;