Fix non-english locales, multiplex shop inventories for better trader UI experience

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

@@ -514,9 +514,6 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
 
     private void setupTranslator() {
         Locale locale = Locale.getDefault();
-        if (!locale.getLanguage().equals("en")) {
-            Messaging.logTr(Messages.CONTRIBUTE_TO_TRANSLATION_PROMPT);
-        }
         String setting = Setting.LOCALE.asString();
         if (!setting.isEmpty()) {
             String[] parts = setting.split("[\\._]");
@@ -535,6 +532,9 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
             }
         }
         Translator.setInstance(new File(getDataFolder(), "lang"), locale);
+        if (!locale.getLanguage().equals("en")) {
+            Messaging.logTr(Messages.CONTRIBUTE_TO_TRANSLATION_PROMPT);
+        }
     }
 
     private void startMetrics() {

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

@@ -65,7 +65,6 @@ public class AdminCommands {
             aliases = { "citizens" },
             usage = "save (-a)",
             desc = "",
-            help = Messages.COMMAND_SAVE_HELP,
             modifiers = { "save" },
             min = 1,
             max = 1,

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

@@ -194,7 +194,6 @@ public class NPCCommands {
             aliases = { "npc" },
             usage = "age [age] (-l(ock))",
             desc = "",
-            help = "citizens.commands.npc.age.help",
             flags = "l",
             modifiers = { "age" },
             min = 1,
@@ -463,7 +462,6 @@ public class NPCCommands {
             aliases = { "npc" },
             usage = "command|cmd (add [command] | remove [id|all] | permissions [permissions] | sequential | cycle | random | clearerror [type] (name|uuid) | errormsg [type] [msg] | persistsequence [true|false] | cost [cost] (id) | expcost [cost] (id) | itemcost (id)) (-s(hift)) (-l[eft]/-r[ight]) (-p[layer] -o[p]), --cooldown --gcooldown [seconds] --delay [ticks] --permissions [perms] --n [max # of uses]",
             desc = "",
-            help = "citizens.commands.npc.command.help",
             modifiers = { "command", "cmd" },
             min = 1,
             flags = "lrpos",
@@ -1265,7 +1263,6 @@ public class NPCCommands {
             aliases = { "npc" },
             usage = "horse|donkey|mule (--color color) (--type type) (--style style) (-cb)",
             desc = "",
-            help = "Use the -c flag to make the NPC have a chest, or the -b flag to stop them from having a chest.",
             modifiers = { "horse", "donkey", "mule" },
             min = 1,
             max = 1,

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

@@ -101,19 +101,19 @@ public class CommandTrait extends Trait {
             return Transaction.success();
         if (cost > 0 && !player.hasPermission("citizens.npc.command.ignoreerrors.cost")) {
             action = new MoneyAction(cost);
-            if (!action.take(player, 1).isPossible()) {
+            if (!action.take(player, null, 1).isPossible()) {
                 sendErrorMessage(player, CommandTraitError.MISSING_MONEY, null, cost);
             }
         }
         if (experienceCost > 0 && !player.hasPermission("citizens.npc.command.ignoreerrors.expcost")) {
             action = new ExperienceAction(experienceCost);
-            if (!action.take(player, 1).isPossible()) {
+            if (!action.take(player, null, 1).isPossible()) {
                 sendErrorMessage(player, CommandTraitError.MISSING_EXPERIENCE, null, experienceCost);
             }
         }
         if (itemRequirements.size() > 0 && !player.hasPermission("citizens.npc.command.ignoreerrors.itemcost")) {
             action = new ItemAction(itemRequirements);
-            if (!action.take(player, 1).isPossible()) {
+            if (!action.take(player, null, 1).isPossible()) {
                 ItemStack stack = itemRequirements.get(0);
                 sendErrorMessage(player, CommandTraitError.MISSING_ITEM, null, Util.prettyEnum(stack.getType()),
                         stack.getAmount());
@@ -121,26 +121,26 @@ public class CommandTrait extends Trait {
         }
         if (command.cost != -1 && !player.hasPermission("citizens.npc.command.ignoreerrors.cost")) {
             action = new MoneyAction(command.cost);
-            if (!action.take(player, 1).isPossible()) {
+            if (!action.take(player, null, 1).isPossible()) {
                 sendErrorMessage(player, CommandTraitError.MISSING_MONEY, null, command.cost);
             }
         }
         if (command.experienceCost != -1 && !player.hasPermission("citizens.npc.command.ignoreerrors.expcost")) {
             action = new ExperienceAction(command.experienceCost);
-            if (!action.take(player, 1).isPossible()) {
+            if (!action.take(player, null, 1).isPossible()) {
                 sendErrorMessage(player, CommandTraitError.MISSING_EXPERIENCE, null, command.experienceCost);
             }
         }
         if (command.itemCost != null && command.itemCost.size() > 0
                 && !player.hasPermission("citizens.npc.command.ignoreerrors.itemcost")) {
             action = new ItemAction(command.itemCost);
-            if (!action.take(player, 1).isPossible()) {
+            if (!action.take(player, null, 1).isPossible()) {
                 ItemStack stack = command.itemCost.get(0);
                 sendErrorMessage(player, CommandTraitError.MISSING_ITEM, null, Util.prettyEnum(stack.getType()),
                         stack.getAmount());
             }
         }
-        return action == null ? Transaction.success() : action.take(player, 1);
+        return action == null ? Transaction.success() : action.take(player, null, 1);
     }
 
     public void clear() {

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

@@ -71,6 +71,7 @@ import net.citizensnpcs.trait.shop.NPCShopAction.GUI;
 import net.citizensnpcs.trait.shop.NPCShopAction.Transaction;
 import net.citizensnpcs.trait.shop.PermissionAction;
 import net.citizensnpcs.trait.shop.PermissionAction.PermissionActionGUI;
+import net.citizensnpcs.util.InventoryMultiplexer;
 import net.citizensnpcs.util.Util;
 
 /**
@@ -105,9 +106,11 @@ public class ShopTrait extends Trait {
     }
 
     public void onRightClick(Player player) {
-        if (rightClickShop == null || rightClickShop.isEmpty()
+        if (rightClickShop == null || rightClickShop.isEmpty())
-                || !Setting.SHOP_GLOBAL_VIEW_PERMISSION.asString().isEmpty()
+            return;
-                        && !player.hasPermission(Setting.SHOP_GLOBAL_VIEW_PERMISSION.asString()))
+
+        String globalViewPermission = Setting.SHOP_GLOBAL_VIEW_PERMISSION.asString();
+        if (!globalViewPermission.isEmpty() && !player.hasPermission(globalViewPermission))
             return;
 
         NPCShop shop = shops.globalShops.getOrDefault(rightClickShop, getDefaultShop());
@@ -432,9 +435,9 @@ public class ShopTrait extends Trait {
             }
         }
 
-        public void onClick(NPCShop shop, Player player, boolean shiftClick, boolean secondClick) {
+        public void onClick(NPCShop shop, Player player, ItemStack[] inventory, boolean shiftClick,
-            if (purchases.containsKey(player.getUniqueId()) && timesPurchasable > 0
+                boolean secondClick) {
-                    && purchases.get(player.getUniqueId()) == timesPurchasable) {
+            if (timesPurchasable > 0 && purchases.getOrDefault(player.getUniqueId(), 0) == timesPurchasable) {
                 if (alreadyPurchasedMessage != null) {
                     Messaging.sendColorless(player, placeholders(alreadyPurchasedMessage, player));
                 }
@@ -447,7 +450,7 @@ public class ShopTrait extends Trait {
             int max = Integer.MAX_VALUE;
             if (maxRepeatsOnShiftClick && shiftClick) {
                 for (NPCShopAction action : cost) {
-                    int r = action.getMaxRepeats(player);
+                    int r = action.getMaxRepeats(player, inventory);
                     if (r != -1) {
                         max = Math.min(max, r);
                     }
@@ -456,14 +459,14 @@ public class ShopTrait extends Trait {
                     return;
             }
             int repeats = max == Integer.MAX_VALUE ? 1 : max;
-            List<Transaction> take = apply(cost, action -> action.take(player, repeats));
+            List<Transaction> take = apply(cost, action -> action.take(player, inventory, repeats));
             if (take == null) {
                 if (costMessage != null) {
                     Messaging.sendColorless(player, placeholders(costMessage, player));
                 }
                 return;
             }
-            if (apply(result, action -> action.grant(player, repeats)) == null) {
+            if (apply(result, action -> action.grant(player, inventory, repeats)) == null) {
                 take.forEach(Transaction::rollback);
                 return;
             }
@@ -471,9 +474,7 @@ public class ShopTrait extends Trait {
                 Messaging.sendColorless(player, placeholders(resultMessage, player));
             }
             if (timesPurchasable > 0) {
-                int timesPurchasedAlready = purchases.get(player.getUniqueId()) == null ? 0
+                purchases.put(player.getUniqueId(), purchases.getOrDefault(player.getUniqueId(), 0) + 1);
-                        : purchases.get(player.getUniqueId());
-                purchases.put(player.getUniqueId(), ++timesPurchasedAlready);
             }
         }
 
@@ -835,7 +836,11 @@ public class ShopTrait extends Trait {
                 ctx.getSlot(i).setItemStack(item.getDisplayItem(player));
                 ctx.getSlot(i).setClickHandler(evt -> {
                     evt.setCancelled(true);
-                    item.onClick(shop, (Player) evt.getWhoClicked(), evt.isShiftClick(), lastClickedItem == item);
+                    InventoryMultiplexer multiplexer = new InventoryMultiplexer(
+                            ((Player) evt.getWhoClicked()).getInventory());
+                    item.onClick(shop, (Player) evt.getWhoClicked(), multiplexer.getInventory(), evt.isShiftClick(),
+                            lastClickedItem == item);
+                    multiplexer.save();
                     lastClickedItem = item;
                 });
             }
@@ -921,15 +926,15 @@ public class ShopTrait extends Trait {
             evt.setCancelled(true);
             if (evt.getSlotType() != SlotType.RESULT || !evt.getAction().name().contains("PICKUP"))
                 return;
-            // TODO: work around crafting slot limitations in minecraft
+            Inventory syntheticInventory = Bukkit.createInventory(null, 9);
-            player.getInventory().addItem(evt.getClickedInventory().getItem(0));
+            syntheticInventory.setItem(0, evt.getClickedInventory().getItem(0));
-            evt.getClickedInventory().setItem(0, null);
+            syntheticInventory.setItem(1, evt.getClickedInventory().getItem(1));
-            if (evt.getClickedInventory().getItem(1) != null) {
+            InventoryMultiplexer multiplexer = new InventoryMultiplexer(player.getInventory(), syntheticInventory);
-                player.getInventory().addItem(evt.getClickedInventory().getItem(1));
+            trades.get(selectedTrade).onClick(shop, player, multiplexer.getInventory(), evt.getClick().isShiftClick(),
-                evt.getClickedInventory().setItem(1, null);
-            }
-            trades.get(selectedTrade).onClick(shop, player, evt.getClick().isShiftClick(),
                     lastClickedTrade == selectedTrade);
+            multiplexer.save();
+            evt.getClickedInventory().setItem(0, syntheticInventory.getItem(0));
+            evt.getClickedInventory().setItem(1, syntheticInventory.getItem(1));
             lastClickedTrade = selectedTrade;
         }
 

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

@@ -48,12 +48,12 @@ public class CommandAction extends NPCShopAction {
     }
 
     @Override
-    public int getMaxRepeats(Entity entity) {
+    public int getMaxRepeats(Entity entity, ItemStack[] inventory) {
         return -1;
     }
 
     @Override
-    public Transaction grant(Entity entity, int repeats) {
+    public Transaction grant(Entity entity, ItemStack[] inventory, int repeats) {
         if (!(entity instanceof Player))
             return Transaction.fail();
         Player player = (Player) entity;
@@ -68,7 +68,7 @@ public class CommandAction extends NPCShopAction {
     }
 
     @Override
-    public Transaction take(Entity entity, int repeats) {
+    public Transaction take(Entity entity, ItemStack[] inventory, int repeats) {
         if (!(entity instanceof Player))
             return Transaction.fail();
         Player player = (Player) entity;

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

@@ -29,7 +29,7 @@ public class ExperienceAction extends NPCShopAction {
     }
 
     @Override
-    public int getMaxRepeats(Entity entity) {
+    public int getMaxRepeats(Entity entity, ItemStack[] inventory) {
         if (!(entity instanceof Player))
             return 0;
 
@@ -37,7 +37,7 @@ public class ExperienceAction extends NPCShopAction {
     }
 
     @Override
-    public Transaction grant(Entity entity, int repeats) {
+    public Transaction grant(Entity entity, ItemStack[] inventory, int repeats) {
         if (!(entity instanceof Player))
             return Transaction.fail();
 
@@ -51,7 +51,7 @@ public class ExperienceAction extends NPCShopAction {
     }
 
     @Override
-    public Transaction take(Entity entity, int repeats) {
+    public Transaction take(Entity entity, ItemStack[] inventory, int repeats) {
         if (!(entity instanceof Player))
             return Transaction.fail();
 

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

@@ -51,9 +51,8 @@ public class ItemAction extends NPCShopAction {
         this.items = items;
     }
 
-    private boolean containsItems(Inventory source, int repeats, boolean modify) {
+    private boolean containsItems(ItemStack[] contents, int repeats, boolean modify) {
         List<Integer> req = items.stream().map(i -> i.getAmount() * repeats).collect(Collectors.toList());
-        ItemStack[] contents = source.getContents();
         for (int i = 0; i < contents.length; i++) {
             ItemStack toMatch = contents[i];
             if (toMatch == null || toMatch.getType() == Material.AIR || tooDamaged(toMatch))
@@ -78,9 +77,9 @@ public class ItemAction extends NPCShopAction {
                 }
                 if (modify) {
                     if (toMatch == null) {
-                        source.clear(i);
+                        contents[i] = null;
                     } else {
-                        source.setItem(i, toMatch.clone());
+                        contents[i] = toMatch.clone();
                     }
                 }
                 req.set(j, remaining - taken);
@@ -106,7 +105,7 @@ public class ItemAction extends NPCShopAction {
     }
 
     @Override
-    public int getMaxRepeats(Entity entity) {
+    public int getMaxRepeats(Entity entity, ItemStack[] inventory) {
         if (!(entity instanceof InventoryHolder))
             return 0;
 
@@ -138,7 +137,7 @@ public class ItemAction extends NPCShopAction {
     }
 
     @Override
-    public Transaction grant(Entity entity, int repeats) {
+    public Transaction grant(Entity entity, ItemStack[] inventory, int repeats) {
         if (!(entity instanceof InventoryHolder))
             return Transaction.fail();
         Inventory source = ((InventoryHolder) entity).getInventory();
@@ -201,15 +200,15 @@ public class ItemAction extends NPCShopAction {
     }
 
     @Override
-    public Transaction take(Entity entity, int repeats) {
+    public Transaction take(Entity entity, ItemStack[] inventory, int repeats) {
         if (!(entity instanceof InventoryHolder))
             return Transaction.fail();
 
-        Inventory source = ((InventoryHolder) entity).getInventory();
+        return Transaction.create(() -> containsItems(inventory, repeats, false), () -> {
-        return Transaction.create(() -> containsItems(source, repeats, false), () -> {
+            containsItems(inventory, repeats, true);
-            containsItems(source, repeats, true);
         }, () -> {
-            source.addItem(items.stream().map(ItemStack::clone).toArray(ItemStack[]::new));
+            ((InventoryHolder) entity).getInventory()
+                    .addItem(items.stream().map(ItemStack::clone).toArray(ItemStack[]::new));
         });
     }
 

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

@@ -32,7 +32,7 @@ public class MoneyAction extends NPCShopAction {
     }
 
     @Override
-    public int getMaxRepeats(Entity entity) {
+    public int getMaxRepeats(Entity entity, ItemStack[] inventory) {
         if (!(entity instanceof Player))
             return 0;
 
@@ -41,7 +41,7 @@ public class MoneyAction extends NPCShopAction {
     }
 
     @Override
-    public Transaction grant(Entity entity, int repeats) {
+    public Transaction grant(Entity entity, ItemStack[] inventory, int repeats) {
         if (!(entity instanceof Player))
             return Transaction.fail();
 
@@ -57,7 +57,7 @@ public class MoneyAction extends NPCShopAction {
     }
 
     @Override
-    public Transaction take(Entity entity, int repeats) {
+    public Transaction take(Entity entity, ItemStack[] inventory, int repeats) {
         if (!(entity instanceof Player))
             return Transaction.fail();
 

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

@@ -25,11 +25,11 @@ public abstract class NPCShopAction implements Cloneable {
 
     public abstract String describe();
 
-    public abstract int getMaxRepeats(Entity entity);
+    public abstract int getMaxRepeats(Entity entity, ItemStack[] inventory);
 
-    public abstract Transaction grant(Entity entity, int repeats);
+    public abstract Transaction grant(Entity entity, ItemStack[] inventory, int repeats);
 
-    public abstract Transaction take(Entity entity, int repeats);
+    public abstract Transaction take(Entity entity, ItemStack[] inventory, int repeats);
 
     public static interface GUI {
         public InventoryMenuPage createEditor(NPCShopAction previous, Consumer<NPCShopAction> callback);

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

@@ -46,12 +46,12 @@ public class PermissionAction extends NPCShopAction {
     }
 
     @Override
-    public int getMaxRepeats(Entity entity) {
+    public int getMaxRepeats(Entity entity, ItemStack[] inventory) {
         return -1;
     }
 
     @Override
-    public Transaction grant(Entity entity, int repeats) {
+    public Transaction grant(Entity entity, ItemStack[] inventory, int repeats) {
         if (!(entity instanceof Player))
             return Transaction.fail();
         Player player = (Player) entity;
@@ -68,7 +68,7 @@ public class PermissionAction extends NPCShopAction {
     }
 
     @Override
-    public Transaction take(Entity entity, int repeats) {
+    public Transaction take(Entity entity, ItemStack[] inventory, int repeats) {
         if (!(entity instanceof Player))
             return Transaction.fail();
         Player player = (Player) entity;

main/src/main/java/net/citizensnpcs/util/InventoryMultiplexer.java

@@ -0,0 +1,43 @@
+package net.citizensnpcs.util;
+
+import java.util.Collection;
+
+import org.bukkit.inventory.Inventory;
+import org.bukkit.inventory.ItemStack;
+
+import com.google.common.collect.ImmutableList;
+
+public class InventoryMultiplexer {
+    private final ItemStack[] inventory;
+    private final Collection<Inventory> sources;
+
+    public InventoryMultiplexer(Collection<Inventory> sources) {
+        this.sources = sources;
+        int size = sources.stream().mapToInt(Inventory::getSize).sum();
+        this.inventory = new ItemStack[size];
+        int i = 0;
+        for (Inventory sourceInventory : sources) {
+            ItemStack[] source = sourceInventory.getContents();
+            System.arraycopy(source, 0, inventory, i, source.length);
+            i += source.length;
+        }
+    }
+
+    public InventoryMultiplexer(Inventory... inventories) {
+        this(ImmutableList.copyOf(inventories));
+    }
+
+    public ItemStack[] getInventory() {
+        return inventory;
+    }
+
+    public void save() {
+        int i = 0;
+        for (Inventory source : sources) {
+            ItemStack[] result = new ItemStack[source.getSize()];
+            System.arraycopy(inventory, i, result, 0, result.length);
+            source.setContents(result);
+            i += result.length;
+        }
+    }
+}

main/src/main/java/net/citizensnpcs/util/Messages.java

@@ -72,7 +72,6 @@ public class Messages {
     public static final String COMMAND_PAGE_MISSING = "citizens.commands.page-missing";
     public static final String COMMAND_REMOVED = "citizens.commands.npc.command.command-removed";
     public static final String COMMAND_RIGHT_HAND_HEADER = "citizens.commands.npc.command.right-hand-header";
-    public static final String COMMAND_SAVE_HELP = "citizens.commands.citizens.save.help";
     public static final String COMMAND_TEMPORARY_PERMISSIONS_SET = "citizens.commands.npc.command.temporary-permissions-set";
     public static final String COMMAND_TRIGGER_ADDED = "citizens.editors.waypoints.triggers.command.added";
     public static final String COMMAND_TRIGGER_PROMPT = "citizens.editors.waypoints.triggers.command.prompt";

v1_19_R3/src/main/java/net/citizensnpcs/nms/v1_19_R3/entity/nonliving/InteractionController.java

@@ -110,10 +110,9 @@ public class InteractionController extends MobEntityController {
 
         @Override
         public void tick() {
+            super.tick();
             if (npc != null) {
                 npc.update();
-            } else {
-                super.tick();
             }
         }
 

v1_20_R3/src/main/java/net/citizensnpcs/nms/v1_20_R3/entity/nonliving/InteractionController.java

@@ -109,10 +109,9 @@ public class InteractionController extends MobEntityController {
 
         @Override
         public void tick() {
+            super.tick();
             if (npc != null) {
                 npc.update();
-            } else {
-                super.tick();
             }
         }