Some work to solve #70

api/src/main/java/net/jitse/npclib/internal/NPCBase.java

@@ -16,6 +16,7 @@ import net.jitse.npclib.api.skin.Skin;
 import net.jitse.npclib.api.state.NPCSlot;
 import net.jitse.npclib.api.state.NPCState;
 import net.jitse.npclib.hologram.Hologram;
+import net.jitse.npclib.utilities.MathUtil;
 import net.labymod.utilities.LMCUtils;
 import org.bukkit.Bukkit;
 import org.bukkit.Location;
@@ -102,7 +103,7 @@ public abstract class NPCBase implements NPC, NPCPacketHandler {
     }
 
     public void disableFOV() {
-        this.cosFOV = 0; // Or equals Math.cos(1/2 * Math.PI).
+        this.cosFOV = 0;
     }
 
     public void setFOV(double fov) {
@@ -154,6 +155,26 @@ public abstract class NPCBase implements NPC, NPCPacketHandler {
         hasTeamRegistered.remove(player.getUniqueId());
     }
 
+    public boolean inRangeOf(Player player) {
+        if (!player.getWorld().equals(location.getWorld())) {
+            // No need to continue our checks, they are in different worlds.
+            return false;
+        }
+
+        // If Bukkit doesn't track the NPC entity anymore, bypass the hiding distance variable.
+        // This will cause issues otherwise (e.g. custom skin disappearing).
+        double hideDistance = instance.getAutoHideDistance();
+        double distanceSquared = player.getLocation().distanceSquared(location);
+        double bukkitRange = Bukkit.getViewDistance() << 4;
+
+        return distanceSquared <= MathUtil.square(hideDistance) && distanceSquared <= MathUtil.square(bukkitRange);
+    }
+
+    public boolean inViewOf(Player player) {
+        Vector dir = location.toVector().subtract(player.getEyeLocation().toVector()).normalize();
+        return dir.dot(player.getLocation().getDirection()) >= cosFOV;
+    }
+
     @Override
     public void show(Player player) {
         show(player, false);
@@ -166,46 +187,33 @@ public abstract class NPCBase implements NPC, NPCPacketHandler {
             return;
         }
 
-        if (!canSeeNPC(player)) {
+        if (isShown(player)) {
-            if (!auto) {
+            throw new IllegalArgumentException("NPC is already shown to player");
-                shown.add(player.getUniqueId());
-            }
-
-            autoHidden.add(player.getUniqueId());
-            return;
         }
 
         if (auto) {
             sendShowPackets(player);
             sendMetadataPacket(player);
             sendEquipmentPackets(player);
+
+            // NPC is auto-shown now, we can remove the UUID from the set.
+            autoHidden.remove(player.getUniqueId());
         } else {
-            if (isShown(player)) {
+            // Adding the UUID to the set.
-                throw new IllegalStateException("Cannot call show method twice.");
-            }
-
-            if (shown.contains(player.getUniqueId())) {
-                return;
-            }
-
             shown.add(player.getUniqueId());
 
-            if (player.getWorld().equals(location.getWorld()) && player.getLocation().distance(location)
+            if (inRangeOf(player) && inViewOf(player)) {
-                    <= instance.getAutoHideDistance()) {
+                // The player can see the NPC and is in range, send the packets.
                 sendShowPackets(player);
                 sendMetadataPacket(player);
                 sendEquipmentPackets(player);
             } else {
+                // We'll wait until we can show the NPC to the player via auto-show.
                 autoHidden.add(player.getUniqueId());
             }
         }
     }
 
-    private boolean canSeeNPC(Player player) {
-        Vector dir = location.toVector().subtract(player.getEyeLocation().toVector()).normalize();
-        return dir.dot(player.getLocation().getDirection()) >= cosFOV;
-    }
-
     @Override
     public void hide(Player player) {
         hide(player, false);
@@ -218,19 +226,28 @@ public abstract class NPCBase implements NPC, NPCPacketHandler {
             return;
         }
 
-        if (auto) {
-            sendHidePackets(player);
-        } else {
         if (!shown.contains(player.getUniqueId())) {
-                throw new IllegalStateException("Cannot call hide method without calling NPC#show.");
+            throw new IllegalArgumentException("NPC cannot be hidden from player before calling NPC#show first");
         }
 
+        if (auto) {
+            if (autoHidden.contains(player.getUniqueId())) {
+                throw new IllegalStateException("NPC cannot be auto-hidden twice");
+            }
+
+            sendHidePackets(player);
+
+            // NPC is auto-hidden now, we will add the UUID to the set.
+            autoHidden.add(player.getUniqueId());
+        } else {
+            // Removing the UUID from the set.
             shown.remove(player.getUniqueId());
 
-            if (player.getWorld().equals(location.getWorld()) && player.getLocation().distance(location)
+            if (inRangeOf(player)) {
-                    <= instance.getAutoHideDistance()) {
+                // The player is in range of the NPC, send the packets.
                 sendHidePackets(player);
             } else {
+                // We don't have to send any packets, just don't let it auto-show again by removing the UUID from the set.
                 autoHidden.remove(player.getUniqueId());
             }
         }

api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java

@@ -7,7 +7,6 @@ package net.jitse.npclib.listeners;
 import net.jitse.npclib.NPCLib;
 import net.jitse.npclib.internal.NPCBase;
 import net.jitse.npclib.internal.NPCManager;
-import org.bukkit.Bukkit;
 import org.bukkit.Location;
 import org.bukkit.World;
 import org.bukkit.entity.Player;
@@ -49,14 +48,11 @@ public class PlayerListener implements Listener {
         // Need to auto hide the NPCs from the player, or else the system will think they can see the NPC on respawn.
         Player player = event.getEntity();
         for (NPCBase npc : NPCManager.getAllNPCs()) {
-            if (npc.getWorld().equals(player.getWorld())) {
+            if (npc.isShown(player) && npc.getWorld().equals(player.getWorld())) {
-                if (!npc.getAutoHidden().contains(player.getUniqueId())) {
-                    npc.getAutoHidden().add(player.getUniqueId());
                 npc.hide(player, true);
             }
         }
     }
-    }
 
     @EventHandler
     public void onPlayerRespawn(PlayerRespawnEvent event) {
@@ -83,28 +79,24 @@ public class PlayerListener implements Listener {
         Player player = event.getPlayer();
         World from = event.getFrom();
 
-        // The PlayerTeleportEvent is call, and will handle visibility in the new world.
+        // The PlayerTeleportEvent is called, and will handle visibility in the new world.
         for (NPCBase npc : NPCManager.getAllNPCs()) {
-            if (npc.getWorld().equals(from)) {
+            if (npc.isShown(player) && npc.getWorld().equals(from)) {
-                if (!npc.getAutoHidden().contains(player.getUniqueId())) {
-                    npc.getAutoHidden().add(player.getUniqueId());
                 npc.hide(player, true);
             }
         }
     }
-    }
 
     @EventHandler
     public void onPlayerMove(PlayerMoveEvent event) {
         Location from = event.getFrom();
         Location to = event.getTo();
-        // 11/4/20: Added pitch and yaw to the if statement. If the change in this is 10 or more degrees, check the movement.
+        // Only check movement when the player moves from one block to another. The event is called often
-        if (to == null || (Math.abs(from.getPitch() - to.getPitch()) <= 10
+        // as it is also called when the pitch or yaw change. This is worth it from a performance view.
-                || Math.abs(from.getYaw() - to.getYaw()) <= 10
+        if (to == null || from.getBlockX() != to.getBlockX()
-                || from.getBlockX() != to.getBlockX()
                 || from.getBlockY() != to.getBlockY()
-                || from.getBlockZ() != to.getBlockZ()))
+                || from.getBlockZ() != to.getBlockZ())
-            handleMove(event.getPlayer()); // Verify the player changed which block they are on. Since PlayerMoveEvent is one of the most called events, this is worth it.
+            handleMove(event.getPlayer());
     }
 
     @EventHandler
@@ -113,41 +105,18 @@ public class PlayerListener implements Listener {
     }
 
     private void handleMove(Player player) {
-        Location location = player.getLocation();
         for (NPCBase npc : NPCManager.getAllNPCs()) {
             if (!npc.getShown().contains(player.getUniqueId())) {
                 continue; // NPC was never supposed to be shown to the player.
             }
 
-            if (!npc.getWorld().equals(location.getWorld())) {
+            if (!npc.isShown(player) && npc.inRangeOf(player) && npc.inViewOf(player)) {
-                continue; // NPC is not in the same world.
+                // The player is in range and can see the NPC, auto-show it.
-            }
-
-            // If Bukkit doesn't track the NPC entity anymore, bypass the hiding distance variable.
-            // This will cause issues otherwise (e.g. custom skin disappearing).
-            double hideDistance = instance.getAutoHideDistance();
-            double distanceSquared = location.distanceSquared(npc.getLocation());
-
-            int tempRange = Bukkit.getViewDistance() << 4;
-            boolean inRange = distanceSquared <= square(hideDistance) && distanceSquared <= square(tempRange);
-            if (npc.getAutoHidden().contains(player.getUniqueId())) {
-                // Check if the player and NPC are within the range to sendShowPackets it again.
-                if (inRange) {
-                    npc.getAutoHidden().remove(player.getUniqueId());
                 npc.show(player, true);
-                }
+            } else if (npc.isShown(player) && !npc.inRangeOf(player)) {
-            } else {
+                // The player is not in range of the NPC anymore, auto-hide it.
-                // Check if the player and NPC are out of range to sendHidePackets it.
-                if (!inRange) {
-                    npc.getAutoHidden().add(player.getUniqueId());
                 npc.hide(player, true);
             }
         }
     }
 }
-
-    // Avoiding Math.pow due to how resource intensive it is.
-    private double square(double val) {
-        return val * val;
-    }
-}

api/src/main/java/net/jitse/npclib/utilities/MathUtil.java

@@ -0,0 +1,9 @@
+package net.jitse.npclib.utilities;
+
+public class MathUtil {
+
+    // Avoiding Math.pow due to how resource intensive it is.
+    public static double square(double val) {
+        return val * val;
+    }
+}