From 37984f847bbab7330aedf2efc811566e8d81cf00 Mon Sep 17 00:00:00 2001 From: JCThePants Date: Sat, 29 Aug 2015 07:20:09 -0700 Subject: [PATCH] fix skin reload on citizens reload reset skin and profile fetcher cache on citizens reload fix -p flag in skin command not implemented use latest skin by default fix Property created for cached skins uses incorrect name fix rotation based skin update in EventListen.SkinUpdateTracker remove redundant skin update on first move in EventListen.SkinUpdateTracker re-implement skin fetch retry; re-add settings (since reloading can cause too many requests) --- src/main/java/net/citizensnpcs/Citizens.java | 4 + .../java/net/citizensnpcs/EventListen.java | 38 +++-- src/main/java/net/citizensnpcs/Settings.java | 4 +- .../citizensnpcs/commands/NPCCommands.java | 2 +- .../net/citizensnpcs/npc/CitizensNPC.java | 2 +- .../npc/profile/ProfileFetchThread.java | 6 +- .../npc/profile/ProfileFetcher.java | 22 ++- .../java/net/citizensnpcs/npc/skin/Skin.java | 152 ++++++++++++------ 8 files changed, 162 insertions(+), 68 deletions(-) diff --git a/src/main/java/net/citizensnpcs/Citizens.java b/src/main/java/net/citizensnpcs/Citizens.java index d49183930..26b9b5422 100644 --- a/src/main/java/net/citizensnpcs/Citizens.java +++ b/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()); diff --git a/src/main/java/net/citizensnpcs/EventListen.java b/src/main/java/net/citizensnpcs/EventListen.java index 1eb609b73..372460782 100644 --- a/src/main/java/net/citizensnpcs/EventListen.java +++ b/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 rotationDegrees = Setting.NPC_SKIN_ROTATION_UPDATE_DEGREES.asFloat(); - - boolean hasRotated = - Math.abs(NMS.clampYaw(currentYaw - this.initialYaw)) < rotationDegrees; + float yaw = NMS.clampYaw(currentLoc.getYaw()); + boolean hasRotated = upperBound < lowerBound + ? yaw > upperBound && yaw < lowerBound + : yaw > upperBound || yaw < lowerBound; // 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); } } diff --git a/src/main/java/net/citizensnpcs/Settings.java b/src/main/java/net/citizensnpcs/Settings.java index 6b3fc35e9..b602ae53c 100644 --- a/src/main/java/net/citizensnpcs/Settings.java +++ b/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), diff --git a/src/main/java/net/citizensnpcs/commands/NPCCommands.java b/src/main/java/net/citizensnpcs/commands/NPCCommands.java index d74e4d4a9..2ddfb38e6 100644 --- a/src/main/java/net/citizensnpcs/commands/NPCCommands.java +++ b/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); } diff --git a/src/main/java/net/citizensnpcs/npc/CitizensNPC.java b/src/main/java/net/citizensnpcs/npc/CitizensNPC.java index 4f6c29eb7..1506e394b 100644 --- a/src/main/java/net/citizensnpcs/npc/CitizensNPC.java +++ b/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() { diff --git a/src/main/java/net/citizensnpcs/npc/profile/ProfileFetchThread.java b/src/main/java/net/citizensnpcs/npc/profile/ProfileFetchThread.java index b69d83f77..f23500121 100644 --- a/src/main/java/net/citizensnpcs/npc/profile/ProfileFetchThread.java +++ b/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); diff --git a/src/main/java/net/citizensnpcs/npc/profile/ProfileFetcher.java b/src/main/java/net/citizensnpcs/npc/profile/ProfileFetcher.java index 61b7f7389..4310e4aae 100644 --- a/src/main/java/net/citizensnpcs/npc/profile/ProfileFetcher.java +++ b/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(); - Bukkit.getScheduler().runTaskTimerAsynchronously(CitizensAPI.getPlugin(), PROFILE_THREAD, 11, 20); + initThread(); } PROFILE_THREAD.fetch(name, handler); } + /** + * Clear all queued and cached requests. + */ + public static void reset() { + initThread(); + } + @Nullable private static ProfileRequest findRequest(String name, Collection 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; } diff --git a/src/main/java/net/citizensnpcs/npc/skin/Skin.java b/src/main/java/net/citizensnpcs/npc/skin/Skin.java index 394ec3ffa..85af82ca2 100644 --- a/src/main/java/net/citizensnpcs/npc/skin/Skin.java +++ b/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 pending = new WeakHashMap(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() { - @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); - } - } - }); + fetch(); } /** @@ -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. - // If npc requires latest skin, cache is used for faster - // availability until the latest skin can be loaded. - String cachedName = npc.data().get(CACHED_SKIN_UUID_NAME_METADATA); - if (this.skinName.equals(cachedName)) { - skinData = new Property(this.skinName, - npc.data(). get(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_METADATA), - npc.data(). get(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_SIGN_METADATA)); + // Use npc cached skin if available. + // If npc requires latest skin, cache is used for faster + // availability until the latest skin can be loaded. + 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) && !texture.equals("cache")) { + Property localData = new Property("textures", texture, + npc.data(). get(NPC.PLAYER_SKIN_TEXTURE_PROPERTIES_SIGN_METADATA)); + setNPCTexture(entity, localData); - skinId = UUID.fromString(npc.data(). get(CACHED_SKIN_UUID_METADATA)); - setNPCSkinData(entity, skinName, skinId, skinData); - - // check if NPC prefers to use cached skin over the latest skin. - if (!entity.getNPC().data().get("update-skin", Setting.NPC_SKIN_UPDATE.asBoolean())) { - // cache preferred - return true; - } - - if (!Setting.NPC_SKIN_UPDATE.asBoolean()) { - // cache preferred - return true; - } + // check if NPC prefers to use cached skin over the latest skin. + if (!entity.getNPC().data().get(NPC.PLAYER_SKIN_USE_LATEST, + Setting.NPC_SKIN_USE_LATEST.asBoolean())) { + // cache preferred + return true; } + } - pending.put(entity, null); - return false; + if (!hasSkinData()) { + 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 entities = new ArrayList(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()); - - GameProfile profile = entity.getProfile(); - profile.getProperties().removeAll("textures"); // ensure client does not crash due to duplicate properties. - profile.getProperties().put("textures", skinProperty); + setNPCTexture(entity, 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 CACHE = new HashMap(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";