Rewrite EntityTargetEvent handling.

This commit is a complete rewrite of the target event handling logic in
the ArenaListener class.

Instead of the complex, inconsistent code structure with too many line
breaks between control flow branches, we just have a thin logic wrapper
that delegates the event handling to smaller, more focused functions
that handle arena pets, arena monsters, and foreign entities on their
own.

A couple of auxiliary methods are introduced to try to limit the amount
of warnings produced by checking set membership with `contains()` when
the entity/target is an Entity and the collection is a sub type.

Fixes #572

changelog.md

@@ -12,6 +12,7 @@ These changes will (most likely) be included in the next version.
 
 ## [Unreleased]
 - It is no longer necessary to have recurrent waves for an arena to work. MobArena automatically creates a "catch all" recurrent wave in case the arena session reaches a wave number that isn't covered by any other wave definitions.
+- Entities outside of the arena can no longer target players, pets, or monsters inside of the arena.
 
 ## [0.104] - 2019-08-08
 - Extended and upgraded potions are now supported in the item syntax by prepending `long_` or `strong_` to the data portion of a potion item (e.g. `potion:strong_instant_heal:1` will yield a Potion of Healing II). Check the wiki for details.

src/main/java/com/garbagemule/MobArena/ArenaListener.java

@@ -828,35 +828,73 @@ public class ArenaListener
         Entity entity = event.getEntity();
         Entity target = event.getTarget();
 
-        if (arena.hasPet(entity)) {
-            // Pets should never attack players
-            if (target instanceof Player) {
-                event.setCancelled(true);
-            }
+        if (isArenaPet(entity)) {
+            onPetTarget(event, target);
+        } else if (isArenaMonster(entity)) {
+            onMonsterTarget(event, entity, target);
+        } else {
+            onForeignTarget(event, target);
+        }
+    }
+
+    private void onPetTarget(EntityTargetEvent event, Entity target) {
+        // If the target is null, do nothing
+        if (target == null) {
+            return;
         }
 
-        else if (monsters.getMonsters().contains(entity)) {
-            // If the target is null, we probably forgot or the target died
-            if (target == null) {
-                event.setTarget(MAUtils.getClosestPlayer(plugin, entity, arena));
-            }
-
-            // Pets are untargetable
-            else if (arena.hasPet(target)) {
-                event.setCancelled(true);
-            }
-
-            // So are non-arena players
-            else if (target instanceof Player && !arena.inArena((Player) target)) {
-                event.setCancelled(true);
-            }
-
-            // And other mobs unless infighting is enabled
-            else if (monsters.getMonsters().contains(target) && !monsterInfight) {
-                event.setCancelled(true);
-            }
+        // Pets should only attack monsters in the arena, nothing else
+        if (!isArenaMonster(target)) {
+            event.setCancelled(true);
         }
     }
+
+    private void onMonsterTarget(EntityTargetEvent event, Entity monster, Entity target) {
+        // Null means we lost our target or the target died, so find a new one
+        if (target == null) {
+            event.setTarget(MAUtils.getClosestPlayer(plugin, monster, arena));
+            return;
+        }
+
+        // If monster infighting is on and target is monster, let it happen
+        if (monsterInfight && isArenaMonster(target)) {
+            return;
+        }
+
+        // At this point, if the target is _not_ an arena player, we can just
+        // cancel, because this check also covers the target being a pet, an
+        // arena monster when infighting is off (due to the check above), or
+        // any foreign entity.
+        if (!isArenaPlayer(target)) {
+            event.setCancelled(true);
+        }
+    }
+
+    private void onForeignTarget(EntityTargetEvent event, Entity target) {
+        // If null, just bail
+        if (target == null) {
+            return;
+        }
+
+        // Foreign entities can't target arena pets, -players, or -monsters.
+        if (isArenaPet(target) || isArenaPlayer(target) || isArenaMonster(target)) {
+            event.setCancelled(true);
+        }
+    }
+
+    @SuppressWarnings("SuspiciousMethodCalls")
+    private boolean isArenaPlayer(Entity entity) {
+        return arena.getPlayersInArena().contains(entity);
+    }
+
+    @SuppressWarnings("SuspiciousMethodCalls")
+    private boolean isArenaMonster(Entity entity) {
+        return monsters.getMonsters().contains(entity);
+    }
+
+    private boolean isArenaPet(Entity entity) {
+        return arena.hasPet(entity);
+    }
     
     public void onEntityTeleport(EntityTeleportEvent event) {
         if (monsters.hasPet(event.getEntity()) && region.contains(event.getTo())) {