Merge pull request #496 from JCThePants/skins2-1

fix skin reload on citizens reload

src/main/java/net/citizensnpcs/Citizens.java

@@ -55,6 +55,8 @@ import net.citizensnpcs.npc.CitizensTraitFactory;
 import net.citizensnpcs.npc.NPCSelector;
 import net.citizensnpcs.npc.ai.speech.Chat;
 import net.citizensnpcs.npc.ai.speech.CitizensSpeechFactory;
+import net.citizensnpcs.npc.profile.ProfileFetcher;
+import net.citizensnpcs.npc.skin.Skin;
 import net.citizensnpcs.util.Messages;
 import net.citizensnpcs.util.NMS;
 import net.citizensnpcs.util.StringHelper;
@@ -340,6 +342,8 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
         Editor.leaveAll();
         config.reload();
         despawnNPCs();
+        ProfileFetcher.reset();
+        Skin.clearCache();
         saves.loadInto(npcRegistry);
 
         getServer().getPluginManager().callEvent(new CitizensReloadEvent());

src/main/java/net/citizensnpcs/EventListen.java

@@ -18,6 +18,7 @@ import com.mojang.authlib.properties.Property;
 import net.citizensnpcs.Settings.Setting;
 import net.citizensnpcs.api.CitizensAPI;
 import net.citizensnpcs.api.event.CitizensDeserialiseMetaEvent;
+import net.citizensnpcs.api.event.CitizensReloadEvent;
 import net.citizensnpcs.api.event.CitizensSerialiseMetaEvent;
 import net.citizensnpcs.api.event.CommandSenderCreateNPCEvent;
 import net.citizensnpcs.api.event.DespawnReason;
@@ -451,6 +452,18 @@ public class EventListen implements Listener {
         recalculatePlayer(event.getPlayer(), 10, false);
     }
 
+    @EventHandler(priority = EventPriority.MONITOR)
+    public void onCitizensReload(CitizensReloadEvent event) {
+        skinUpdateTrackers.clear();
+        for (Player player : Bukkit.getOnlinePlayers()) {
+
+            if (player.hasMetadata("NPC"))
+                continue;
+
+            skinUpdateTrackers.put(player.getUniqueId(), new SkinUpdateTracker(player));
+        }
+    }
+
     public void recalculatePlayer(final Player player, long delay, final boolean isInitial) {
 
         if (player.hasMetadata("NPC"))
@@ -589,8 +602,9 @@ public class EventListen implements Listener {
     private class SkinUpdateTracker {
         float initialYaw;
         final Location location = new Location(null, 0, 0, 0);
-        boolean hasMoved;
         int rotationCount;
+        float upperBound;
+        float lowerBound;
 
         SkinUpdateTracker(Player player) {
             reset(player);
@@ -598,22 +612,13 @@ public class EventListen implements Listener {
 
         boolean shouldUpdate(Player player) {
 
-            // check if this is the first time the player has moved
-            if (!hasMoved) {
-                hasMoved = true;
-                reset(player);
-                return true;
-            }
-
             Location currentLoc = player.getLocation(YAW_LOCATION);
-            float currentYaw = currentLoc.getYaw();
 
             if (rotationCount < 2) {
-
+                float yaw = NMS.clampYaw(currentLoc.getYaw());
-                float rotationDegrees = Setting.NPC_SKIN_ROTATION_UPDATE_DEGREES.asFloat();
+                boolean hasRotated = upperBound < lowerBound
-
+                        ? yaw > upperBound && yaw < lowerBound
-                boolean hasRotated =
+                        : yaw > upperBound || yaw < lowerBound;
-                        Math.abs(NMS.clampYaw(currentYaw - this.initialYaw)) < rotationDegrees;
 
                 // update the first 2 times the player rotates. helps load skins around player
                 // after the player logs/teleports.
@@ -639,7 +644,10 @@ public class EventListen implements Listener {
         // current location and yaw.
         void reset(Player player) {
             player.getLocation(location);
-            this.initialYaw = location.getYaw();
+            this.initialYaw = NMS.clampYaw(location.getYaw());
+            float rotationDegrees = Setting.NPC_SKIN_ROTATION_UPDATE_DEGREES.asFloat();
+            this.upperBound = NMS.clampYaw(this.initialYaw + rotationDegrees);
+            this.lowerBound = NMS.clampYaw(this.initialYaw - rotationDegrees);
         }
     }
 

src/main/java/net/citizensnpcs/Settings.java

@@ -88,6 +88,7 @@ public class Settings {
         KEEP_CHUNKS_LOADED("npc.chunks.always-keep-loaded", false),
         LOCALE("general.translation.locale", ""),
         MAX_NPC_LIMIT_CHECKS("npc.limits.max-permission-checks", 100),
+        MAX_NPC_SKIN_RETRIES("npc.skins.max-retries", -1),
         MAX_PACKET_ENTRIES("npc.limits.max-packet-entries", 15),
         MAX_SPEED("npc.limits.max-speed", 100),
         MAX_TEXT_RANGE("npc.chat.options.max-text-range", 500),
@@ -95,8 +96,9 @@ public class Settings {
         NEW_PATHFINDER_OPENS_DOORS("npc.pathfinding.new-finder-open-doors", false),
         NPC_ATTACK_DISTANCE("npc.pathfinding.attack-range", 1.75 * 1.75),
         NPC_COST("economy.npc.cost", 100D),
-        NPC_SKIN_UPDATE("npc.skins.update", false),
+        NPC_SKIN_USE_LATEST("npc.skins.use-latest", true),
         NPC_SKIN_VIEW_DISTANCE("npc.skins.view-distance", 100D),
+        NPC_SKIN_RETRY_DELAY("npc.skins.retry-delay", 120),
         NPC_SKIN_ROTATION_UPDATE_DEGREES("npc.skins.rotation-update-degrees", 90f),
         PACKET_UPDATE_DELAY("npc.packets.update-delay", 30),
         QUICK_SELECT("npc.selection.quick-select", false),

src/main/java/net/citizensnpcs/commands/NPCCommands.java

@@ -1307,7 +1307,7 @@ public class NPCCommands {
                 throw new CommandException();
             npc.data().setPersistent(NPC.PLAYER_SKIN_UUID_METADATA, args.getString(1));
             if (args.hasFlag('p')) {
-                npc.data().setPersistent(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_METADATA, "cache");
+                npc.data().setPersistent(NPC.PLAYER_SKIN_USE_LATEST, false);
             }
             skinName = args.getString(1);
         }

src/main/java/net/citizensnpcs/npc/CitizensNPC.java

@@ -192,7 +192,7 @@ public class CitizensNPC extends AbstractNPC {
         SkinnableEntity skinnable = NMS.getSkinnable(getEntity());
         if (skinnable != null) {
             final double viewDistance = Settings.Setting.NPC_SKIN_VIEW_DISTANCE.asDouble();
-            skinnable.getSkinTracker().updateNearbyViewers(viewDistance);
+            //skinnable.getSkinTracker().updateNearbyViewers(viewDistance);
             Bukkit.getScheduler().scheduleSyncDelayedTask(CitizensAPI.getPlugin(), new Runnable() {
                 @Override
                 public void run() {

src/main/java/net/citizensnpcs/npc/profile/ProfileFetchThread.java

@@ -57,11 +57,15 @@ class ProfileFetchThread implements Runnable {
                 requested.put(name, request);
                 return;
             }
+            else if (request.getResult() == ProfileFetchResult.TOO_MANY_REQUESTS) {
+                queue.add(request);
+            }
         }
 
         if (handler != null) {
 
-            if (request.getResult() == ProfileFetchResult.PENDING) {
+            if (request.getResult() == ProfileFetchResult.PENDING ||
+                    request.getResult() == ProfileFetchResult.TOO_MANY_REQUESTS) {
                 addHandler(request, handler);
             } else {
                 sendResult(handler, request);

src/main/java/net/citizensnpcs/npc/profile/ProfileFetcher.java

@@ -1,10 +1,10 @@
 package net.citizensnpcs.npc.profile;
 
 import java.util.Collection;
-
 import javax.annotation.Nullable;
 
 import org.bukkit.Bukkit;
+import org.bukkit.scheduler.BukkitTask;
 
 import com.google.common.base.Preconditions;
 import com.mojang.authlib.Agent;
@@ -109,12 +109,18 @@ public class ProfileFetcher {
         Preconditions.checkNotNull(name);
 
         if (PROFILE_THREAD == null) {
-            PROFILE_THREAD = new ProfileFetchThread();
+            initThread();
-            Bukkit.getScheduler().runTaskTimerAsynchronously(CitizensAPI.getPlugin(), PROFILE_THREAD, 11, 20);
         }
         PROFILE_THREAD.fetch(name, handler);
     }
 
+    /**
+     * Clear all queued and cached requests.
+     */
+    public static void reset() {
+        initThread();
+    }
+
     @Nullable
     private static ProfileRequest findRequest(String name, Collection<ProfileRequest> requests) {
         name = name.toLowerCase();
@@ -149,5 +155,15 @@ public class ProfileFetcher {
                 || (cause != null && cause.contains("too many requests"));
     }
 
+    private static void initThread() {
+        if (THREAD_TASK != null)
+            THREAD_TASK.cancel();
+
+        PROFILE_THREAD = new ProfileFetchThread();
+        THREAD_TASK = Bukkit.getScheduler().runTaskTimerAsynchronously(
+                CitizensAPI.getPlugin(), PROFILE_THREAD, 21, 20);
+    }
+
     private static ProfileFetchThread PROFILE_THREAD;
+    private static BukkitTask THREAD_TASK;
 }

src/main/java/net/citizensnpcs/npc/skin/Skin.java

@@ -1,6 +1,8 @@
 package net.citizensnpcs.npc.skin;
 
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 import java.util.WeakHashMap;
@@ -12,11 +14,15 @@ import com.google.common.collect.Iterables;
 import com.mojang.authlib.GameProfile;
 import com.mojang.authlib.properties.Property;
 
+import org.bukkit.Bukkit;
+import org.bukkit.scheduler.BukkitTask;
+
 import net.citizensnpcs.Settings.Setting;
+import net.citizensnpcs.api.CitizensAPI;
 import net.citizensnpcs.api.event.DespawnReason;
 import net.citizensnpcs.api.npc.NPC;
+import net.citizensnpcs.api.util.Messaging;
 import net.citizensnpcs.npc.profile.ProfileFetchHandler;
-import net.citizensnpcs.npc.profile.ProfileFetchResult;
 import net.citizensnpcs.npc.profile.ProfileFetcher;
 import net.citizensnpcs.npc.profile.ProfileRequest;
 
@@ -24,11 +30,14 @@ import net.citizensnpcs.npc.profile.ProfileRequest;
  * Stores data for a single skin.
  */
 public class Skin {
+    private boolean hasFetched;
     private volatile boolean isValid = true;
     private final Map<SkinnableEntity, Void> pending = new WeakHashMap<SkinnableEntity, Void>(15);
+    private BukkitTask retryTask;
     private volatile Property skinData;
     private volatile UUID skinId;
     private final String skinName;
+    private int fetchRetries = -1;
 
     /**
      * Constructor.
@@ -46,21 +55,7 @@ public class Skin {
             CACHE.put(this.skinName, this);
         }
 
-        ProfileFetcher.fetch(this.skinName, new ProfileFetchHandler() {
+        fetch();
-            @Override
-            public void onResult(ProfileRequest request) {
-
-                if (request.getResult() == ProfileFetchResult.NOT_FOUND) {
-                    isValid = false;
-                    return;
-                }
-
-                if (request.getResult() == ProfileFetchResult.SUCCESS) {
-                    GameProfile profile = request.getProfile();
-                    setData(profile);
-                }
-            }
-        });
     }
 
     /**
@@ -74,40 +69,39 @@ public class Skin {
      * @param entity
      *            The skinnable entity.
      *
-     * @return True if the skin data was available and applied, false if the data is being retrieved.
+     * @return True if skin was applied, false if the data is being retrieved.
      */
     public boolean apply(SkinnableEntity entity) {
         Preconditions.checkNotNull(entity);
 
         NPC npc = entity.getNPC();
 
-        if (!hasSkinData()) {
+        // Use npc cached skin if available.
-            // Use npc cached skin if available.
+        // If npc requires latest skin, cache is used for faster
-            // If npc requires latest skin, cache is used for faster
+        // availability until the latest skin can be loaded.
-            // availability until the latest skin can be loaded.
+        String cachedName = npc.data().get(CACHED_SKIN_UUID_NAME_METADATA);
-            String cachedName = npc.data().get(CACHED_SKIN_UUID_NAME_METADATA);
+        String texture = npc.data().get(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_METADATA, "cache");
-            if (this.skinName.equals(cachedName)) {
+        if (this.skinName.equals(cachedName) && !texture.equals("cache")) {
-                skinData = new Property(this.skinName,
+            Property localData = new Property("textures", texture,
-                        npc.data().<String> get(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_METADATA),
+                    npc.data().<String> get(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_SIGN_METADATA));
-                        npc.data().<String> get(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_SIGN_METADATA));
+            setNPCTexture(entity, localData);
 
-                skinId = UUID.fromString(npc.data().<String> get(CACHED_SKIN_UUID_METADATA));
+            // check if NPC prefers to use cached skin over the latest skin.
-                setNPCSkinData(entity, skinName, skinId, skinData);
+            if (!entity.getNPC().data().get(NPC.PLAYER_SKIN_USE_LATEST,
-
+                    Setting.NPC_SKIN_USE_LATEST.asBoolean())) {
-                // check if NPC prefers to use cached skin over the latest skin.
+                // cache preferred
-                if (!entity.getNPC().data().get("update-skin", Setting.NPC_SKIN_UPDATE.asBoolean())) {
+                return true;
-                    // cache preferred
-                    return true;
-                }
-
-                if (!Setting.NPC_SKIN_UPDATE.asBoolean()) {
-                    // cache preferred
-                    return true;
-                }
             }
+        }
 
-            pending.put(entity, null);
+        if (!hasSkinData()) {
-            return false;
+            if (hasFetched) {
+                return true;
+            }
+            else {
+                pending.put(entity, null);
+                return false;
+            }
         }
 
         setNPCSkinData(entity, skinName, skinId, skinData);
@@ -180,9 +174,57 @@ public class Skin {
         skinId = profile.getId();
         skinData = Iterables.getFirst(profile.getProperties().get("textures"), null);
 
-        for (SkinnableEntity entity : pending.keySet()) {
+        List<SkinnableEntity> entities = new ArrayList<SkinnableEntity>(pending.keySet());
+        for (SkinnableEntity entity : entities) {
             applyAndRespawn(entity);
         }
+        pending.clear();
+    }
+
+    private void fetch() {
+
+        final int maxRetries = Setting.MAX_NPC_SKIN_RETRIES.asInt();
+        if (maxRetries > -1 && fetchRetries >= maxRetries) {
+            if (Messaging.isDebugging()) {
+                Messaging.debug("Reached max skin fetch retries for '" + skinName + "'");
+            }
+            return;
+        }
+
+        ProfileFetcher.fetch(this.skinName, new ProfileFetchHandler() {
+            @Override
+            public void onResult(ProfileRequest request) {
+
+                hasFetched = true;
+
+                switch (request.getResult()) {
+                    case NOT_FOUND:
+                        isValid = false;
+                        break;
+                    case TOO_MANY_REQUESTS:
+                        if (maxRetries == 0) {
+                            break;
+                        }
+                        fetchRetries++;
+                        long delay = Setting.NPC_SKIN_RETRY_DELAY.asLong();
+                        retryTask = Bukkit.getScheduler().runTaskLater(CitizensAPI.getPlugin(), new Runnable() {
+                            @Override
+                            public void run() {
+                                fetch();
+                            }
+                        }, delay);
+
+                        if (Messaging.isDebugging()) {
+                            Messaging.debug("Retrying skin fetch for '" + skinName + "' in " + delay + "ticks.");
+                        }
+                        break;
+                    case SUCCESS:
+                        GameProfile profile = request.getProfile();
+                        setData(profile);
+                        break;
+                }
+            }
+        });
     }
 
     /**
@@ -212,6 +254,21 @@ public class Skin {
         return skin;
     }
 
+    /**
+     * Clear all cached skins.
+     */
+    public static void clearCache() {
+        synchronized (CACHE) {
+            for (Skin skin : CACHE.values()) {
+                skin.pending.clear();
+                if (skin.retryTask != null) {
+                    skin.retryTask.cancel();
+                }
+            }
+            CACHE.clear();
+        }
+    }
+
     private static void setNPCSkinData(SkinnableEntity entity, String skinName, UUID skinId, Property skinProperty) {
         NPC npc = entity.getNPC();
 
@@ -221,16 +278,19 @@ public class Skin {
         if (skinProperty.getValue() != null) {
             npc.data().setPersistent(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_METADATA, skinProperty.getValue());
             npc.data().setPersistent(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_SIGN_METADATA, skinProperty.getSignature());
-
+            setNPCTexture(entity, skinProperty);
-            GameProfile profile = entity.getProfile();
-            profile.getProperties().removeAll("textures"); // ensure client does not crash due to duplicate properties.
-            profile.getProperties().put("textures", skinProperty);
         } else {
             npc.data().remove(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_METADATA);
             npc.data().remove(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_SIGN_METADATA);
         }
     }
 
+    private static void setNPCTexture(SkinnableEntity entity, Property skinProperty) {
+        GameProfile profile = entity.getProfile();
+        profile.getProperties().removeAll("textures"); // ensure client does not crash due to duplicate properties.
+        profile.getProperties().put("textures", skinProperty);
+    }
+
     private static final Map<String, Skin> CACHE = new HashMap<String, Skin>(20);
     public static final String CACHED_SKIN_UUID_METADATA = "cached-skin-uuid";
     public static final String CACHED_SKIN_UUID_NAME_METADATA = "cached-skin-uuid-name";