Remove some global state from shops

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

@@ -35,6 +35,7 @@ import net.citizensnpcs.Settings.Setting;
 import net.citizensnpcs.api.CitizensAPI;
 import net.citizensnpcs.api.CitizensPlugin;
 import net.citizensnpcs.api.InventoryHelper;
+import net.citizensnpcs.api.LocationLookup;
 import net.citizensnpcs.api.SkullMetaProvider;
 import net.citizensnpcs.api.ai.speech.SpeechFactory;
 import net.citizensnpcs.api.command.CommandManager;
@@ -52,6 +53,7 @@ import net.citizensnpcs.api.scripting.EventRegistrar;
 import net.citizensnpcs.api.scripting.ObjectProvider;
 import net.citizensnpcs.api.scripting.ScriptCompiler;
 import net.citizensnpcs.api.trait.TraitFactory;
+import net.citizensnpcs.api.trait.TraitInfo;
 import net.citizensnpcs.api.util.Messaging;
 import net.citizensnpcs.api.util.NBTStorage;
 import net.citizensnpcs.api.util.Storage;
@@ -98,12 +100,13 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
             NMS.updateInventoryTitle(player, view, newTitle);
         }
     };
+    private LocationLookup locationLookup;
     private CitizensNPCRegistry npcRegistry;
     private ProtocolLibListener protocolListener;
     private boolean saveOnDisable = true;
     private NPCDataStore saves;
     private NPCSelector selector;
-    private Storage shops;
+    private StoredShops shops;
     private final SkullMetaProvider skullMetaProvider = new SkullMetaProvider() {
         @Override
         public String getTexture(SkullMeta meta) {
@@ -216,6 +219,11 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
         return inventoryHelper;
     }
 
+    @Override
+    public LocationLookup getLocationLookup() {
+        return locationLookup;
+    }
+
     @Override
     public NPCRegistry getNamedNPCRegistry(String name) {
         if (name.equals(npcRegistry.getName()))
@@ -333,7 +341,6 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
 
     @Override
     public boolean onCommand(CommandSender sender, org.bukkit.command.Command command, String cmdName, String[] args) {
-        // TODO: use injector?
         Object[] methodArgs = { sender, selector == null ? null : selector.getSelected(sender) };
         return commands.executeSafe(command, args, sender, methodArgs);
     }
@@ -353,6 +360,7 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
         despawnNPCs(saveOnDisable);
         HandlerList.unregisterAll(this);
         npcRegistry = null;
+        locationLookup = null;
         enabled = false;
         saveOnDisable = true;
         NMS.shutdown();
@@ -382,16 +390,22 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
         registerScriptHelpers();
 
         saves = createStorage(getDataFolder());
-        shops = new YamlStorage(new File(getDataFolder(), "shops.yml"));
+        shops = new StoredShops(new YamlStorage(new File(getDataFolder(), "shops.yml")));
-        if (saves == null || !shops.load()) {
+        if (saves == null || !shops.loadFromDisk()) {
             Messaging.severeTr(Messages.FAILED_LOAD_SAVES);
             Bukkit.getPluginManager().disablePlugin(this);
             return;
         }
 
+        locationLookup = new LocationLookup();
+        Bukkit.getScheduler().scheduleSyncRepeatingTask(this, locationLookup, 0, 5);
+
         speechFactory = new CitizensSpeechFactory();
         npcRegistry = new CitizensNPCRegistry(saves, "citizens");
         traitFactory = new CitizensTraitFactory();
+        traitFactory.registerTrait(TraitInfo.create(ShopTrait.class).withSupplier(() -> {
+            return new ShopTrait();
+        }));
         selector = new NPCSelector(this);
 
         Bukkit.getPluginManager().registerEvents(new EventListen(storedRegistries), this);
@@ -461,8 +475,8 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
         saves.reloadFromSource();
         saves.loadInto(npcRegistry);
 
+        shops.loadFromDisk();
         shops.load();
-        ShopTrait.loadShops(shops.getKey(""));
 
         getServer().getPluginManager().callEvent(new CitizensReloadEvent());
     }
@@ -533,15 +547,6 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
                     return Iterables.size(npcRegistry);
                 }
             }));
-            /*
-            TODO: not implemented yet
-            metrics.addCustomChart(new Metrics.MultiLineChart("traits", new Callable<Map<String, Integer>>() {
-                @Override
-                public Map<String, Integer> call() throws Exception {
-                    return traitFactory.getTraitPlot();
-                }
-            }));
-            */
         } catch (Exception e) {
             Messaging.logTr(Messages.METRICS_ERROR_NOTIFICATION, e.getMessage());
         }
@@ -555,14 +560,14 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
         if (saves == null)
             return;
         saves.storeAll(npcRegistry);
-        ShopTrait.saveShops(shops.getKey(""));
+        shops.saveShops();
         if (async) {
             saves.saveToDisk();
             new Thread(() -> {
-                shops.save();
+                shops.saveToDisk();
             }).start();
         } else {
-            shops.save();
+            shops.saveToDisk();
             saves.saveToDiskImmediate();
         }
     }
@@ -571,7 +576,7 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
         @Override
         public void run() {
             saves.loadInto(npcRegistry);
-            ShopTrait.loadShops(shops.getKey(""));
+            shops.load();
 
             Messaging.logTr(Messages.NUM_LOADED_NOTIFICATION, Iterables.size(npcRegistry), "?");
             startMetrics();

main/src/main/java/net/citizensnpcs/StoredShops.java

@@ -0,0 +1,39 @@
+package net.citizensnpcs;
+
+import java.util.Map;
+
+import com.google.common.collect.Maps;
+
+import net.citizensnpcs.api.persistence.Persist;
+import net.citizensnpcs.api.persistence.PersistenceLoader;
+import net.citizensnpcs.api.util.Storage;
+import net.citizensnpcs.api.util.YamlStorage;
+import net.citizensnpcs.trait.ShopTrait.NPCShop;
+
+public class StoredShops {
+    @Persist(value = "global", reify = true)
+    public Map<String, NPCShop> globalShops = Maps.newHashMap();
+    @Persist(value = "npc", reify = true)
+    public Map<String, NPCShop> npcShops = Maps.newHashMap();
+    private final Storage storage;
+
+    public StoredShops(YamlStorage storage) {
+        this.storage = storage;
+    }
+
+    public void load() {
+        PersistenceLoader.load(this, storage.getKey(""));
+    }
+
+    public boolean loadFromDisk() {
+        return storage.load();
+    }
+
+    public void saveShops() {
+        PersistenceLoader.save(this, storage.getKey(""));
+    }
+
+    public void saveToDisk() {
+        storage.save();
+    }
+}

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

@@ -1619,8 +1619,6 @@ public class NPCCommands {
         }
         Messaging.send(sender, "    Traits");
         for (Trait trait : npc.getTraits()) {
-            if (CitizensAPI.getTraitFactory().isInternalTrait(trait))
-                continue;
             String message = "     [[- ]]" + trait.getName();
             Messaging.send(sender, message);
         }

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

@@ -3,14 +3,11 @@ package net.citizensnpcs.npc;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
 
-import net.citizensnpcs.api.CitizensAPI;
 import net.citizensnpcs.api.npc.NPC;
 import net.citizensnpcs.api.trait.Trait;
 import net.citizensnpcs.api.trait.TraitFactory;
@@ -42,17 +39,16 @@ import net.citizensnpcs.trait.OcelotModifiers;
 import net.citizensnpcs.trait.Poses;
 import net.citizensnpcs.trait.Powered;
 import net.citizensnpcs.trait.RabbitType;
+import net.citizensnpcs.trait.RotationTrait;
 import net.citizensnpcs.trait.Saddle;
 import net.citizensnpcs.trait.ScoreboardTrait;
 import net.citizensnpcs.trait.ScriptTrait;
 import net.citizensnpcs.trait.SheepTrait;
-import net.citizensnpcs.trait.ShopTrait;
 import net.citizensnpcs.trait.SitTrait;
 import net.citizensnpcs.trait.SkinLayers;
 import net.citizensnpcs.trait.SkinTrait;
 import net.citizensnpcs.trait.SleepTrait;
 import net.citizensnpcs.trait.SlimeSize;
-import net.citizensnpcs.trait.RotationTrait;
 import net.citizensnpcs.trait.SneakTrait;
 import net.citizensnpcs.trait.VillagerProfession;
 import net.citizensnpcs.trait.WitherTrait;
@@ -102,7 +98,6 @@ public class CitizensTraitFactory implements TraitFactory {
         registerTrait(TraitInfo.create(SkinTrait.class));
         registerTrait(TraitInfo.create(SneakTrait.class));
         registerTrait(TraitInfo.create(SlimeSize.class));
-        registerTrait(TraitInfo.create(ShopTrait.class));
         registerTrait(TraitInfo.create(Spawned.class));
         registerTrait(TraitInfo.create(Speech.class));
         registerTrait(TraitInfo.create(Text.class));
@@ -111,10 +106,6 @@ public class CitizensTraitFactory implements TraitFactory {
         registerTrait(TraitInfo.create(WoolColor.class));
         registerTrait(TraitInfo.create(WolfModifiers.class));
         registerTrait(TraitInfo.create(VillagerProfession.class));
-
-        for (String trait : registered.keySet()) {
-            INTERNAL_TRAITS.add(trait);
-        }
     }
 
     @Override
@@ -164,23 +155,6 @@ public class CitizensTraitFactory implements TraitFactory {
         return info == null ? null : info.getTraitClass();
     }
 
-    public Map<String, Integer> getTraitPlot() {
-        Map<String, Integer> counts = Maps.newHashMap();
-        for (NPC npc : CitizensAPI.getNPCRegistry()) {
-            for (Trait trait : npc.getTraits()) {
-                if (INTERNAL_TRAITS.contains(trait.getName()))
-                    continue;
-                counts.put(trait.getName(), counts.getOrDefault(trait.getName(), 0) + 1);
-            }
-        }
-        return counts;
-    }
-
-    @Override
-    public boolean isInternalTrait(Trait trait) {
-        return INTERNAL_TRAITS.contains(trait.getName());
-    }
-
     @Override
     public void registerTrait(TraitInfo info) {
         Preconditions.checkNotNull(info, "info cannot be null");
@@ -193,6 +167,4 @@ public class CitizensTraitFactory implements TraitFactory {
             defaultTraits.add(info);
         }
     }
-
-    private static final Set<String> INTERNAL_TRAITS = Sets.newHashSet();
 }

main/src/main/java/net/citizensnpcs/trait/ShopTrait.java

@@ -20,6 +20,7 @@ import com.google.common.base.Splitter;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 
+import net.citizensnpcs.StoredShops;
 import net.citizensnpcs.api.CitizensAPI;
 import net.citizensnpcs.api.gui.CitizensInventoryClickEvent;
 import net.citizensnpcs.api.gui.ClickHandler;
@@ -34,11 +35,9 @@ import net.citizensnpcs.api.gui.MenuContext;
 import net.citizensnpcs.api.gui.MenuPattern;
 import net.citizensnpcs.api.gui.MenuSlot;
 import net.citizensnpcs.api.persistence.Persist;
-import net.citizensnpcs.api.persistence.PersistenceLoader;
 import net.citizensnpcs.api.trait.Trait;
 import net.citizensnpcs.api.trait.TraitName;
 import net.citizensnpcs.api.util.Colorizer;
-import net.citizensnpcs.api.util.DataKey;
 import net.citizensnpcs.api.util.Messaging;
 import net.citizensnpcs.api.util.Placeholders;
 import net.citizensnpcs.trait.shop.ItemAction;
@@ -59,31 +58,37 @@ import net.citizensnpcs.util.Util;
 public class ShopTrait extends Trait {
     @Persist
     private String rightClickShop;
+    private StoredShops shops;
 
     public ShopTrait() {
         super("shop");
     }
 
+    public ShopTrait(StoredShops shops) {
+        this();
+        this.shops = shops;
+    }
+
     public void deleteShop(String name) {
-        if (StoredShops.GLOBAL_SHOPS.containsKey(name)) {
+        if (shops.globalShops.containsKey(name)) {
-            StoredShops.GLOBAL_SHOPS.remove(name);
+            shops.globalShops.remove(name);
         } else {
-            StoredShops.NPC_SHOPS.remove(name);
+            shops.npcShops.remove(name);
         }
     }
 
     public NPCShop getDefaultShop() {
-        return StoredShops.NPC_SHOPS.computeIfAbsent(npc.getUniqueId().toString(), (s) -> new NPCShop(s));
+        return shops.npcShops.computeIfAbsent(npc.getUniqueId().toString(), (s) -> new NPCShop(s));
     }
 
     public NPCShop getShop(String name) {
-        return StoredShops.GLOBAL_SHOPS.computeIfAbsent(name, (s) -> new NPCShop(s));
+        return shops.globalShops.computeIfAbsent(name, (s) -> new NPCShop(s));
     }
 
     public void onRightClick(Player player) {
         if (rightClickShop == null)
             return;
-        NPCShop shop = StoredShops.GLOBAL_SHOPS.getOrDefault(rightClickShop, getDefaultShop());
+        NPCShop shop = shops.globalShops.getOrDefault(rightClickShop, getDefaultShop());
         shop.display(player);
     }
 
@@ -372,10 +377,8 @@ public class ShopTrait extends Trait {
                 NPCShopAction oldResult = modified.result.stream().filter(template::manages).findFirst().orElse(null);
                 actionItems.getSlots().get(pos)
                         .setItemStack(Util.editTitle(template.createMenuItem(oldResult), title -> title + " Result"));
-                actionItems.getSlots().get(pos).setClickHandler(event -> 
+                actionItems.getSlots().get(pos).setClickHandler(event -> ctx.getMenu().transition(
-                    ctx.getMenu().transition(template.createEditor(oldResult,
+                        template.createEditor(oldResult, result -> modified.changeResult(template::manages, result))));
-                            result -> modified.changeResult(template::manages, result)))
-                );
 
                 pos++;
             }
@@ -645,22 +648,6 @@ public class ShopTrait extends Trait {
         SELL;
     }
 
-    private static class StoredShops {
-        @Persist(value = "global", reify = true)
-        private static Map<String, NPCShop> GLOBAL_SHOPS = Maps.newHashMap();
-        @Persist(value = "npc", reify = true)
-        private static Map<String, NPCShop> NPC_SHOPS = Maps.newHashMap();
-    }
-
-    public static void loadShops(DataKey root) {
-        SAVED = PersistenceLoader.load(StoredShops.class, root);
-    }
-
-    public static void saveShops(DataKey root) {
-        PersistenceLoader.save(SAVED, root);
-    }
-
-    private static StoredShops SAVED = new StoredShops();
     static {
         NPCShopAction.register(ItemAction.class, "items", new ItemActionGUI());
         NPCShopAction.register(PermissionAction.class, "permissions", new PermissionActionGUI());

main/src/main/java/net/citizensnpcs/trait/shop/ItemAction.java

@@ -31,7 +31,7 @@ import net.citizensnpcs.util.Util;
 
 public class ItemAction extends NPCShopAction {
     @Persist
-    public boolean compareSimilarity = false;
+    public boolean compareSimilarity = true;
     @Persist
     public List<ItemStack> items = Lists.newArrayList();
     @Persist