Made entity listener use the world purger's logic

This should fix #872 and most other spawning issues.

src/main/java/com/onarandombox/MultiverseCore/api/WorldPurger.java

@@ -3,14 +3,14 @@ package com.onarandombox.MultiverseCore.api;
 import java.util.List;
 
 import org.bukkit.command.CommandSender;
+import org.bukkit.entity.Entity;
 
 /**
  * Used to remove animals from worlds that don't belong there.
  */
 public interface WorldPurger {
-
     /**
-     * Synchronizes the given world with it's settings.
+     * Synchronizes the given worlds with their settings.
      *
      * @param worlds A list of {@link MultiverseWorld}
      */
@@ -46,4 +46,23 @@ public interface WorldPurger {
     void purgeWorld(MultiverseWorld mvworld, List<String> thingsToKill, boolean negateAnimals,
             boolean negateMonsters, CommandSender sender);
 
+    /**
+     * Determines whether the specified creature should be killed.
+     *
+     * @param e The creature.
+     * @param thingsToKill A {@link List} of animals/monsters to be killed.
+     * @param negateAnimals Whether the monsters in the list should be negated.
+     * @param negateMonsters Whether the animals in the list should be negated.
+     * @return {@code true} if the creature should be killed, otherwise {@code false}.
+     */
+    boolean shouldWeKillThisCreature(Entity e, List<String> thingsToKill, boolean negateAnimals, boolean negateMonsters);
+
+    /**
+     * Determines whether the specified creature should be killed and automatically reads the params from a world object.
+     *
+     * @param w The world.
+     * @param e The creature.
+     * @return {@code true} if the creature should be killed, otherwise {@code false}.
+     */
+    boolean shouldWeKillThisCreature(MultiverseWorld w, Entity e);
 }

src/main/java/com/onarandombox/MultiverseCore/listeners/MVEntityListener.java

@@ -11,13 +11,8 @@ import com.onarandombox.MultiverseCore.MultiverseCore;
 import com.onarandombox.MultiverseCore.api.MVWorldManager;
 import com.onarandombox.MultiverseCore.api.MultiverseWorld;
 import org.bukkit.World;
-import org.bukkit.entity.Animals;
 import org.bukkit.entity.EntityType;
-import org.bukkit.entity.Ghast;
-import org.bukkit.entity.Monster;
 import org.bukkit.entity.Player;
-import org.bukkit.entity.Slime;
-import org.bukkit.entity.Squid;
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.Listener;
 import org.bukkit.event.entity.CreatureSpawnEvent;
@@ -26,14 +21,12 @@ import org.bukkit.event.entity.EntityRegainHealthEvent;
 import org.bukkit.event.entity.EntityRegainHealthEvent.RegainReason;
 import org.bukkit.event.entity.FoodLevelChangeEvent;
 
-import java.util.List;
 import java.util.logging.Level;
 
 /**
  * Multiverse's Entity {@link Listener}.
  */
 public class MVEntityListener implements Listener {
-
     private MultiverseCore plugin;
     private MVWorldManager worldManager;
 
@@ -87,7 +80,8 @@ public class MVEntityListener implements Listener {
     public void creatureSpawn(CreatureSpawnEvent event) {
         // Check to see if the Creature is spawned by a plugin, we don't want to prevent this behaviour.
         // TODO: Allow the egg thing to be a config param. Doubt this will be per world; seems silly.
-        if (event.getSpawnReason() == SpawnReason.CUSTOM || event.getSpawnReason() == SpawnReason.SPAWNER_EGG) {
+        if (event.getSpawnReason() == SpawnReason.CUSTOM || event.getSpawnReason() == SpawnReason.SPAWNER_EGG
+                || event.getSpawnReason() == SpawnReason.BREEDING) {
             return;
         }
 
@@ -100,8 +94,6 @@ public class MVEntityListener implements Listener {
             return;
 
         EntityType type = event.getEntityType();
-        MultiverseWorld mvworld = this.worldManager.getMVWorld(world.getName());
-
         /**
          * Handle people with non-standard animals: ie a patched craftbukkit.
          */
@@ -110,43 +102,8 @@ public class MVEntityListener implements Listener {
             return;
         }
 
-        /**
+        MultiverseWorld mvworld = this.worldManager.getMVWorld(world.getName());
-         * Animal Handling
+        event.setCancelled(this.plugin.getMVWorldManager().getTheWorldPurger().shouldWeKillThisCreature(mvworld, event.getEntity()));
-         */
-        if (!event.isCancelled() && (event.getEntity() instanceof Animals || event.getEntity() instanceof Squid)) {
-            event.setCancelled(shouldWeKillThisCreature(mvworld.getAnimalList(), mvworld.canAnimalsSpawn(), type.getName().toUpperCase()));
-        }
-        /**
-         * Monster Handling
-         */
-        if (!event.isCancelled() && (event.getEntity() instanceof Monster || event.getEntity() instanceof Ghast || event.getEntity() instanceof Slime)) {
-            event.setCancelled(shouldWeKillThisCreature(mvworld.getMonsterList(), mvworld.canMonstersSpawn(), type.getName().toUpperCase()));
-        }
-    }
-
-    private static boolean shouldWeKillThisCreature(List<String> creatureList, boolean allowCreatureSpawning, String creature) {
-        if (creatureList.isEmpty() && allowCreatureSpawning) {
-            // 1. There are no exceptions and animals are allowed. Save it.
-            return false;
-        } else if (creatureList.isEmpty()) {
-            // 2. There are no exceptions and animals are NOT allowed. Kill it.
-            return true;
-        } else if (creatureList.contains(creature.toUpperCase()) && allowCreatureSpawning) {
-            // 3. There ARE exceptions and animals ARE allowed. Kill it.
-            return true;
-        } else if (!creatureList.contains(creature.toUpperCase()) && allowCreatureSpawning) {
-            // 4. There ARE exceptions and animals ARE NOT allowed. SAVE it.
-            return false;
-        } else if (creatureList.contains(creature.toUpperCase()) && !allowCreatureSpawning) {
-            // 5. No animals are allowed to be spawned, BUT this one can stay...
-            return false;
-        } else if (!creatureList.contains(creature.toUpperCase()) && !allowCreatureSpawning) {
-            // 6. Animals are NOT allowed to spawn, and this creature is not in the save list... KILL IT
-            return true;
-        } else {
-            // This code should NEVER execute. I just left the verbose conditions in right now.
-            throw new UnsupportedOperationException();
-        }
     }
 
 }

src/main/java/com/onarandombox/MultiverseCore/utils/SimpleWorldPurger.java

@@ -62,6 +62,16 @@ public class SimpleWorldPurger implements WorldPurger {
         purgeWorld(world, allMobs, !world.canAnimalsSpawn(), !world.canMonstersSpawn());
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public boolean shouldWeKillThisCreature(MultiverseWorld world, Entity e) {
+        ArrayList<String> allMobs = new ArrayList<String>(world.getAnimalList());
+        allMobs.addAll(world.getMonsterList());
+        return this.shouldWeKillThisCreature(e, allMobs, !world.canAnimalsSpawn(), !world.canMonstersSpawn());
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -80,49 +90,9 @@ public class SimpleWorldPurger implements WorldPurger {
         boolean specifiedAnimals = thingsToKill.contains("ANIMALS") || specifiedAll;
         boolean specifiedMonsters = thingsToKill.contains("MONSTERS") || specifiedAll;
         for (Entity e : world.getEntities()) {
-            boolean negate = false;
+            if (killDecision(e, thingsToKill, negateAnimals, negateMonsters, specifiedAnimals, specifiedMonsters)) {
-            boolean specified = false;
-            if (e instanceof Squid || e instanceof Animals) {
-                // it's an animal
-                if (specifiedAnimals && !negateAnimals) {
-                    this.plugin.log(Level.FINEST, "Removing an entity because I was told to remove all animals: " + e);
-                    e.remove();
-                    entitiesKilled++;
-                    continue;
-                }
-                if (specifiedAnimals)
-                    specified = true;
-                negate = negateAnimals;
-            } else if (e instanceof Monster || e instanceof Ghast || e instanceof Slime) {
-                // it's a monster
-                if (specifiedMonsters && !negateMonsters) {
-                    this.plugin.log(Level.FINEST, "Removing an entity because I was told to remove all monsters: " + e);
-                    e.remove();
-                    entitiesKilled++;
-                    continue;
-                }
-                if (specifiedMonsters)
-                    specified = true;
-                negate = negateMonsters;
-            }
-            for (String s : thingsToKill) {
-                EntityType type = EntityType.fromName(s);
-                if (type != null && type.equals(e.getType())) {
-                    specified = true;
-                    if (!negate) {
-                        this.plugin.log(Level.FINEST, "Removing an entity because it WAS specified and we are NOT negating: " + e);
-                        e.remove();
-                        entitiesKilled++;
-                        continue;
-                    }
-                    break;
-                }
-            }
-            if (!specified && negate) {
-                this.plugin.log(Level.FINEST, "Removing an entity because it was NOT specified and we ARE negating: " + e);
                 e.remove();
                 entitiesKilled++;
-                continue;
             }
         }
         if (sender != null) {
@@ -130,6 +100,59 @@ public class SimpleWorldPurger implements WorldPurger {
         }
     }
 
+    private boolean killDecision(Entity e, List<String> thingsToKill, boolean negateAnimals,
+            boolean negateMonsters, boolean specifiedAnimals, boolean specifiedMonsters) {
+        boolean negate = false;
+        boolean specified = false;
+        if (e instanceof Squid || e instanceof Animals) {
+            // it's an animal
+            if (specifiedAnimals && !negateAnimals) {
+                this.plugin.log(Level.FINEST, "Removing an entity because I was told to remove all animals: " + e);
+                return true;
+            }
+            if (specifiedAnimals)
+                specified = true;
+            negate = negateAnimals;
+        } else if (e instanceof Monster || e instanceof Ghast || e instanceof Slime) {
+            // it's a monster
+            if (specifiedMonsters && !negateMonsters) {
+                this.plugin.log(Level.FINEST, "Removing an entity because I was told to remove all monsters: " + e);
+                return true;
+            }
+            if (specifiedMonsters)
+                specified = true;
+            negate = negateMonsters;
+        }
+        for (String s : thingsToKill) {
+            EntityType type = EntityType.fromName(s);
+            if (type != null && type.equals(e.getType())) {
+                specified = true;
+                if (!negate) {
+                    this.plugin.log(Level.FINEST, "Removing an entity because it WAS specified and we are NOT negating: " + e);
+                    return true;
+                }
+                break;
+            }
+        }
+        if (!specified && negate) {
+            this.plugin.log(Level.FINEST, "Removing an entity because it was NOT specified and we ARE negating: " + e);
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public boolean shouldWeKillThisCreature(Entity e, List<String> thingsToKill, boolean negateAnimals, boolean negateMonsters) {
+        boolean specifiedAll = thingsToKill.contains("ALL");
+        boolean specifiedAnimals = thingsToKill.contains("ANIMALS") || specifiedAll;
+        boolean specifiedMonsters = thingsToKill.contains("MONSTERS") || specifiedAll;
+        return this.killDecision(e, thingsToKill, negateAnimals, negateMonsters, specifiedAnimals, specifiedMonsters);
+    }
+
     /**
      * {@inheritDoc}
      */

src/test/java/com/onarandombox/MultiverseCore/test/TestEntitySpawnRules.java

@@ -0,0 +1,158 @@
+package com.onarandombox.MultiverseCore.test;
+
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import org.bukkit.World;
+import org.bukkit.entity.Entity;
+import org.bukkit.entity.EntityType;
+import org.bukkit.entity.LivingEntity;
+import org.bukkit.entity.Sheep;
+import org.bukkit.entity.Zombie;
+import org.bukkit.event.entity.CreatureSpawnEvent;
+import org.bukkit.event.entity.CreatureSpawnEvent.SpawnReason;
+import org.bukkit.plugin.PluginDescriptionFile;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import com.onarandombox.MultiverseCore.MultiverseCore;
+import com.onarandombox.MultiverseCore.api.MVWorldManager;
+import com.onarandombox.MultiverseCore.api.MultiverseWorld;
+import com.onarandombox.MultiverseCore.listeners.MVEntityListener;
+import com.onarandombox.MultiverseCore.test.utils.TestInstanceCreator;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ MultiverseCore.class, PluginDescriptionFile.class })
+public class TestEntitySpawnRules {
+    TestInstanceCreator creator;
+    MultiverseCore core;
+    MVEntityListener listener;
+
+    MultiverseWorld mvWorld;
+    World cbworld;
+
+    Sheep sheep;
+    Zombie zombie;
+
+    CreatureSpawnEvent sheepEvent;
+    CreatureSpawnEvent zombieEvent;
+
+    @Before
+    public void setUp() throws Exception {
+        creator = new TestInstanceCreator();
+        assertTrue(creator.setUp());
+        core = creator.getCore();
+        listener = core.getEntityListener();
+
+        mvWorld = mock(MultiverseWorld.class);
+        cbworld = mock(World.class);
+        when(mvWorld.getCBWorld()).thenReturn(cbworld);
+
+        MVWorldManager worldman = mock(MVWorldManager.class);
+        when(worldman.isMVWorld(anyString())).thenReturn(true);
+        when(worldman.getMVWorld(anyString())).thenReturn(mvWorld);
+        Field worldmanfield = MVEntityListener.class.getDeclaredField("worldManager");
+        worldmanfield.setAccessible(true);
+        worldmanfield.set(listener, worldman);
+
+        core.getMVConfig().setGlobalDebug(3);
+    }
+
+    @After
+    public void tearDown() throws Exception {
+        creator.tearDown();
+    }
+
+    private static CreatureSpawnEvent mockSpawnEvent(LivingEntity e, SpawnReason reason) {
+        CreatureSpawnEvent event = mock(CreatureSpawnEvent.class);
+        when(event.getEntity()).thenReturn(e);
+        EntityType type = e.getType();
+        when(event.getEntityType()).thenReturn(type);
+        when(event.getSpawnReason()).thenReturn(reason);
+        return event;
+    }
+
+    private void spawnAll(SpawnReason reason) {
+        sheepEvent = mockSpawnEvent(sheep, reason);
+        zombieEvent = mockSpawnEvent(zombie, reason);
+        listener.creatureSpawn(sheepEvent);
+        listener.creatureSpawn(zombieEvent);
+    }
+
+    private void spawnAllNatural() {
+        spawnAll(SpawnReason.NATURAL);
+    }
+
+    private void adjustSettings(boolean animalSpawn, boolean monsterSpawn,
+            List<String> animalExceptions, List<String> monsterExceptions) {
+        when(this.mvWorld.canAnimalsSpawn()).thenReturn(animalSpawn);
+        when(this.mvWorld.canMonstersSpawn()).thenReturn(monsterSpawn);
+        when(this.mvWorld.getAnimalList()).thenReturn(animalExceptions);
+        when(this.mvWorld.getMonsterList()).thenReturn(monsterExceptions);
+    }
+
+    @Test
+    public void test() {
+        // test 1: no spawning at all allowed
+        adjustSettings(false, false, Collections.EMPTY_LIST, Collections.EMPTY_LIST);
+        createAnimals();
+        spawnAllNatural();
+        verify(sheepEvent).setCancelled(true);
+        verify(zombieEvent).setCancelled(true);
+
+        // test 2: only monsters
+        adjustSettings(false, true, Collections.EMPTY_LIST, Collections.EMPTY_LIST);
+        createAnimals();
+        spawnAllNatural();
+        verify(sheepEvent).setCancelled(true);
+        verify(zombieEvent).setCancelled(false);
+
+        // test 3: all spawning allowed
+        adjustSettings(true, true, Collections.EMPTY_LIST, Collections.EMPTY_LIST);
+        createAnimals();
+        spawnAllNatural();
+        verify(sheepEvent).setCancelled(false);
+        verify(zombieEvent).setCancelled(false);
+
+        // test 4: no spawning with zombie exception
+        adjustSettings(false, false, Collections.EMPTY_LIST, Arrays.asList("ZOMBIE"));
+        createAnimals();
+        spawnAllNatural();
+        verify(sheepEvent).setCancelled(true);
+        verify(zombieEvent).setCancelled(false);
+
+        // test 5: all spawning with sheep exception
+        adjustSettings(true, true, Arrays.asList("SHEEP"), Collections.EMPTY_LIST);
+        createAnimals();
+        spawnAllNatural();
+        verify(sheepEvent).setCancelled(true);
+        verify(zombieEvent).setCancelled(false);
+
+        // test 6: eggs
+        adjustSettings(false, false, Collections.EMPTY_LIST, Collections.EMPTY_LIST);
+        createAnimals();
+        spawnAll(SpawnReason.SPAWNER_EGG);
+        verify(sheepEvent, never()).setCancelled(anyBoolean());
+        verify(zombieEvent, never()).setCancelled(anyBoolean());
+    }
+
+    private void createAnimals() {
+        sheep = mock(Sheep.class);
+        when(sheep.getType()).thenReturn(EntityType.SHEEP);
+        when(sheep.getWorld()).thenReturn(this.cbworld);
+        zombie = mock(Zombie.class);
+        when(zombie.getType()).thenReturn(EntityType.ZOMBIE);
+        when(zombie.getWorld()).thenReturn(this.cbworld);
+
+        when(cbworld.getEntities()).thenReturn(Arrays.asList((Entity) sheep, (Entity) zombie));
+    }
+}