Changes to the maximum shops limit.

* A value of '0' for the 'max-shops-per-player' setting no longer indicates no limit, but can be used to disable the creation and hiring of player shops. 'No limit' is indicated by a value of '-1' now. Any previous limit of '0' is automatically migrated.
* The permissions specified inside the config get cached and checked in decreasing order now. We abort checking permissions if they cannot further increase the player's current shops limit. An effect of this is that it is only possible to increase the default limit, not decrease it.
* Added permission node 'shopkeeper.maxshops.unlimited' (default: op), which disables the max shops limit for a player.
* Message: Changed the 'too-many-shops' message to be more general.

CHANGELOG.md

@@ -15,6 +15,13 @@ Date format: (YYYY-MM-DD)
   * It is now possible to create admin shops via command even when targeting a container. However, the admin shop type has to be explicitly specified as command argument.
   * When a player shop type is selected, we send appropriate feedback messages depending on whether player shop creation via command is enabled, whether a container is targeted, and whether it is a supported type of container.
   * When not specifying a shop object type, we pick the first shop object type that can be used by the player. This is consistent between the creation of player and admin shops now.
+* Changes to the maximum shops limit:
+  * Config: A value of '0' for the 'max-shops-per-player' setting no longer indicates no limit, but can be used to disable the creation and hiring of player shops. 'No limit' is indicated by a value of '-1' now. Any previous limit of '0' is automatically migrated.
+  * The permissions specified inside the config get cached and checked in decreasing order now. We abort checking permissions if they cannot further increase the player's current shops limit. An effect of this is that it is only possible to increase the default limit, not decrease it.
+  * Added permission node 'shopkeeper.maxshops.unlimited' (default: op), which disables the max shops limit for a player.
+
+API:  
+* PlayerCreatePlayerShopkeeperEvent and PlayerShopkeeperHireEvent: The meaning of the max shops limit has changed. A value of 0 or less no longer indicates 'no limit'.
 
 Migration notes:  
 * The folder structure has changed:
@@ -35,6 +42,7 @@ Messages:
 * Removed 'no-player-shop-type-selected'.
 * Moved 'editor-title' and 'for-hire-title' from config into messages.
 * Changed the default color of 'villager-editor-title' to be less bright.
+* Changed the 'too-many-shops' message to be more general.
 
 You will have to manually update your custom language files to adapt for these changes.
 

src/api/java/com/nisovin/shopkeepers/api/ShopkeepersPlugin.java

@@ -55,6 +55,7 @@ public interface ShopkeepersPlugin extends Plugin {
 	public static final String EDIT_VILLAGERS_PERMISSION = "shopkeeper.edit-villagers";
 	public static final String EDIT_WANDERING_TRADERS_PERMISSION = "shopkeeper.edit-wandering-traders";
 	public static final String BYPASS_PERMISSION = "shopkeeper.bypass";
+	public static final String MAXSHOPS_UNLIMITED_PERMISSION = "shopkeeper.maxshops.unlimited";
 
 	public static final String ADMIN_PERMISSION = "shopkeeper.admin";
 	public static final String PLAYER_SELL_PERMISSION = "shopkeeper.player.sell";

src/api/java/com/nisovin/shopkeepers/api/events/PlayerCreatePlayerShopkeeperEvent.java

@@ -24,6 +24,8 @@ public class PlayerCreatePlayerShopkeeperEvent extends PlayerCreateShopkeeperEve
 
 	/**
 	 * Gets the maximum number of shops the owner of the new shopkeeper can have.
+	 * <p>
+	 * {@link Integer#MAX_VALUE} indicates no limit.
 	 * 
 	 * @return the owner's max shops limit
 	 */
@@ -36,11 +38,16 @@ public class PlayerCreatePlayerShopkeeperEvent extends PlayerCreateShopkeeperEve
 	 * <p>
 	 * The new max shops limit only affects this specific shopkeeper creation. If the player already has more shops than
 	 * this, the shop will not be created.
+	 * <p>
+	 * {@link Integer#MAX_VALUE} indicates no limit.
 	 * 
 	 * @param maxShopsLimit
 	 *            the owner's max shops limit to use for this shopkeeper creation
 	 */
 	public void setMaxShopsLimit(int maxShopsLimit) {
+		if (maxShopsLimit < 0) {
+			throw new IllegalArgumentException("maxShopsLimit cannot be negative!");
+		}
 		this.maxShopsLimit = maxShopsLimit;
 	}
 

src/api/java/com/nisovin/shopkeepers/api/events/PlayerShopkeeperHireEvent.java

@@ -62,6 +62,8 @@ public class PlayerShopkeeperHireEvent extends ShopkeeperEvent implements Cancel
 
 	/**
 	 * Gets the maximum number of shops the hiring player can have.
+	 * <p>
+	 * {@link Integer#MAX_VALUE} indicates no limit.
 	 * 
 	 * @return the hiring player's max shops limit
 	 */
@@ -74,11 +76,16 @@ public class PlayerShopkeeperHireEvent extends ShopkeeperEvent implements Cancel
 	 * <p>
 	 * The new max shops limit only affects this specific shopkeeper hire. If the player already has more shops than
 	 * this, the shop will not be hired.
+	 * <p>
+	 * {@link Integer#MAX_VALUE} indicates no limit.
 	 * 
 	 * @param maxShopsLimit
 	 *            the hiring player's max shops limit to use for this hire
 	 */
 	public void setMaxShopsLimit(int maxShopsLimit) {
+		if (maxShopsLimit < 0) {
+			throw new IllegalArgumentException("maxShopsLimit cannot be negative!");
+		}
 		this.maxShopsLimit = maxShopsLimit;
 	}
 

src/main/java/com/nisovin/shopkeepers/Messages.java

@@ -140,7 +140,7 @@ public class Messages {
 	public static Text containerNotPlaced = Text.parse("&7You must select a container you have recently placed!");
 	public static Text containerAlreadyInUse = Text.parse("&7Another shopkeeper is already using the selected container!");
 	public static Text noContainerAccess = Text.parse("&7You cannot access the selected container!");
-	public static Text tooManyShops = Text.parse("&7You have too many shops!");
+	public static Text tooManyShops = Text.parse("&7You have already reached the limit of how many shops you can own!");
 	public static Text noPlayerShopsViaCommand = Text.parse("&7Player shops can only be created via the shop creation item!");
 	public static Text shopCreateFail = Text.parse("&7You cannot create a shopkeeper there.");
 

src/main/java/com/nisovin/shopkeepers/Settings.java

@@ -9,6 +9,7 @@ import java.lang.reflect.Modifier;
 import java.lang.reflect.ParameterizedType;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
@@ -24,8 +25,10 @@ import org.bukkit.entity.EntityType;
 import org.bukkit.entity.Player;
 import org.bukkit.inventory.ItemStack;
 
+import com.nisovin.shopkeepers.api.ShopkeepersPlugin;
 import com.nisovin.shopkeepers.config.ConfigLoadException;
 import com.nisovin.shopkeepers.config.migration.ConfigMigrations;
+import com.nisovin.shopkeepers.util.ConversionUtils;
 import com.nisovin.shopkeepers.util.ItemData;
 import com.nisovin.shopkeepers.util.ItemUtils;
 import com.nisovin.shopkeepers.util.Log;
@@ -144,7 +147,7 @@ public class Settings {
 
 	public static boolean requireContainerRecentlyPlaced = true;
 	public static int maxContainerDistance = 15;
-	public static int maxShopsPerPlayer = 0;
+	public static int maxShopsPerPlayer = -1;
 	public static String maxShopsPermOptions = "10,15,25";
 
 	public static boolean protectContainers = true;
@@ -451,6 +454,23 @@ public class Settings {
 
 	// Item utilities:
 
+	public static class MaxShopsPermission implements Comparable<MaxShopsPermission> {
+
+		// Integer.MAX_VALUE indicates no limit.
+		public final int maxShops;
+		public final String permission;
+
+		private MaxShopsPermission(int maxShops, String permission) {
+			this.maxShops = maxShops;
+			this.permission = permission;
+		}
+
+		@Override
+		public int compareTo(MaxShopsPermission other) {
+			return Integer.compare(this.maxShops, other.maxShops);
+		}
+	}
+
 	// Stores derived settings which get setup after loading the config.
 	public static class DerivedSettings {
 
@@ -467,6 +487,9 @@ public class Settings {
 
 		public static Pattern shopNamePattern = Pattern.compile("^[A-Za-z0-9 ]{3,32}$");
 
+		// Sorted in descending order:
+		public static final List<MaxShopsPermission> maxShopsPermissions = new ArrayList<>();
+
 		// Gets called after the config has been loaded:
 		private static void setup() {
 			// Ignore display name (which is used for specifying the new shopkeeper name):
@@ -482,6 +505,7 @@ public class Settings {
 			deleteVillagerButtonItem = new ItemData(ItemUtils.setItemStackNameAndLore(deleteItem.createItemStack(), Messages.buttonDeleteVillager, Messages.buttonDeleteVillagerLore));
 			villagerInventoryButtonItem = new ItemData(ItemUtils.setItemStackNameAndLore(containerItem.createItemStack(), Messages.buttonVillagerInventory, Messages.buttonVillagerInventoryLore));
 
+			// Shop name pattern:
 			try {
 				shopNamePattern = Pattern.compile("^" + Settings.nameRegex + "$");
 			} catch (PatternSyntaxException e) {
@@ -489,6 +513,23 @@ public class Settings {
 				Settings.nameRegex = "[A-Za-z0-9 ]{3,32}";
 				shopNamePattern = Pattern.compile("^" + Settings.nameRegex + "$");
 			}
+
+			// Maximum shops permissions:
+			maxShopsPermissions.clear();
+			// Add permission for an unlimited number of shops:
+			maxShopsPermissions.add(new MaxShopsPermission(Integer.MAX_VALUE, ShopkeepersPlugin.MAXSHOPS_UNLIMITED_PERMISSION));
+			String[] maxShopsPermOptions = Settings.maxShopsPermOptions.replace(" ", "").split(",");
+			for (String permOption : maxShopsPermOptions) {
+				// Validate:
+				Integer maxShops = ConversionUtils.parseInt(permOption);
+				if (maxShops == null || maxShops <= 0) {
+					Log.warning("Config: Ignoring invalid entry in 'max-shops-perm-options': " + permOption);
+					continue;
+				}
+				String permission = "shopkeeper.maxshops." + permOption;
+				maxShopsPermissions.add(new MaxShopsPermission(maxShops, permission));
+			}
+			Collections.sort(maxShopsPermissions, Collections.reverseOrder()); // Descending order
 		}
 	}
 
@@ -583,12 +624,20 @@ public class Settings {
 
 	//
 
-	public static int getMaxShops(Player player) {
-		int maxShops = Settings.maxShopsPerPlayer;
-		String[] maxShopsPermOptions = Settings.maxShopsPermOptions.replace(" ", "").split(",");
-		for (String perm : maxShopsPermOptions) {
-			if (PermissionUtils.hasPermission(player, "shopkeeper.maxshops." + perm)) {
-				maxShops = Integer.parseInt(perm);
+	// Integer.MAX_VALUE indicates no limit.
+	public static int getMaxShopsLimit(Player player) {
+		if (Settings.maxShopsPerPlayer == -1) {
+			return Integer.MAX_VALUE; // No limit by default
+		}
+		int maxShops = Settings.maxShopsPerPlayer; // Default
+		for (MaxShopsPermission entry : DerivedSettings.maxShopsPermissions) {
+			// Note: The max shops permission entries are sorted in descending order.
+			if (entry.maxShops <= maxShops) {
+				break;
+			}
+			if (PermissionUtils.hasPermission(player, entry.permission)) {
+				maxShops = entry.maxShops;
+				break;
 			}
 		}
 		return maxShops;

src/main/java/com/nisovin/shopkeepers/config/migration/ConfigMigration4.java

@@ -12,12 +12,18 @@ public class ConfigMigration4 implements ConfigMigration {
 	@Override
 	public void apply(Configuration config) {
 		// Migrate language 'en' to 'en-default':
-		String key = "language";
+		migrateValue(config, "language", "en", "en-default");
+
+		// Migrate max shops limit from 0 to -1 (indicating no limit by default):
+		migrateValue(config, "max-shops-per-player", 0, -1);
+	}
+
+	private static void migrateValue(Configuration config, String key, Object expectedOldValue, Object newValue) {
 		if (config.isSet(key)) {
 			Object oldValue = config.get(key);
-			if (oldValue.equals("en")) {
-				Log.info("  Migrating setting '" + key + "' from value 'en' to new value 'en-default'.");
-				config.set(key, "en-default");
+			if (expectedOldValue.equals(oldValue)) {
+				Log.info("  Migrating setting '" + key + "' from value '" + oldValue + "' to new value '" + newValue + "'.");
+				config.set(key, newValue);
 			}
 		}
 	}

src/main/java/com/nisovin/shopkeepers/shopkeeper/player/AbstractPlayerShopType.java

@@ -67,7 +67,7 @@ public abstract class AbstractPlayerShopType<T extends AbstractPlayerShopkeeper>
 			return false;
 		}
 
-		// Check worldguard:
+		// Check WorldGuard:
 		if (Settings.enableWorldGuardRestrictions) {
 			if (!WorldGuardHandler.isShopAllowed(creator, spawnLocation)) {
 				TextUtils.sendMessage(creator, Messages.shopCreateFail);
@@ -75,7 +75,7 @@ public abstract class AbstractPlayerShopType<T extends AbstractPlayerShopkeeper>
 			}
 		}
 
-		// Check towny:
+		// Check Towny:
 		if (Settings.enableTownyRestrictions) {
 			if (!TownyHandler.isCommercialArea(spawnLocation)) {
 				TextUtils.sendMessage(creator, Messages.shopCreateFail);
@@ -83,7 +83,7 @@ public abstract class AbstractPlayerShopType<T extends AbstractPlayerShopkeeper>
 			}
 		}
 
-		int maxShopsLimit = Settings.getMaxShops(creator);
+		int maxShopsLimit = Settings.getMaxShopsLimit(creator);
 		// Call event:
 		PlayerCreatePlayerShopkeeperEvent createEvent = new PlayerCreatePlayerShopkeeperEvent(shopCreationData, maxShopsLimit);
 		Bukkit.getPluginManager().callEvent(createEvent);
@@ -95,7 +95,7 @@ public abstract class AbstractPlayerShopType<T extends AbstractPlayerShopkeeper>
 		}
 
 		// Count owned shops:
-		if (maxShopsLimit > 0) {
+		if (maxShopsLimit != Integer.MAX_VALUE) {
 			int count = SKShopkeepersPlugin.getInstance().getShopkeeperRegistry().getPlayerShopkeepersByOwner(creator.getUniqueId()).size();
 			if (count >= maxShopsLimit) {
 				TextUtils.sendMessage(creator, Messages.tooManyShops);

src/main/java/com/nisovin/shopkeepers/shopkeeper/player/PlayerShopHiringHandler.java

@@ -103,8 +103,8 @@ public class PlayerShopHiringHandler extends HiringHandler {
 			}
 
 			// Call event:
-			int maxShops = Settings.getMaxShops(player);
-			PlayerShopkeeperHireEvent hireEvent = new PlayerShopkeeperHireEvent(shopkeeper, player, newPlayerInventoryContents, maxShops);
+			int maxShopsLimit = Settings.getMaxShopsLimit(player);
+			PlayerShopkeeperHireEvent hireEvent = new PlayerShopkeeperHireEvent(shopkeeper, player, newPlayerInventoryContents, maxShopsLimit);
 			Bukkit.getPluginManager().callEvent(hireEvent);
 			if (hireEvent.isCancelled()) {
 				Log.debug("PlayerShopkeeperHireEvent was cancelled!");
@@ -114,10 +114,10 @@ public class PlayerShopHiringHandler extends HiringHandler {
 			}
 
 			// Check max shops limit:
-			maxShops = hireEvent.getMaxShopsLimit();
-			if (maxShops > 0) {
+			maxShopsLimit = hireEvent.getMaxShopsLimit();
+			if (maxShopsLimit != Integer.MAX_VALUE) {
 				int count = SKShopkeepersPlugin.getInstance().getShopkeeperRegistry().getPlayerShopkeepersByOwner(player.getUniqueId()).size();
-				if (count >= maxShops) {
+				if (count >= maxShopsLimit) {
 					TextUtils.sendMessage(player, Messages.tooManyShops);
 					this.getUISession(player).abortDelayed();
 					return;

src/main/resources/config.yml

@@ -144,12 +144,14 @@ require-container-recently-placed: true
 # The maximum distance a player shopkeeper can be placed from its backing
 # container. This cannot be set to a value greater than 50.
 max-container-distance: 15
-# The default maximum number of shops a player can have. Set to 0 to allow any
-# number of shops.
-max-shops-per-player: 0
-# A list of permission nodes that can be used to explicitly set the maximum
-# number of shops a specific player or group of players can have. Use the
-# shopkeeper.maxshops.<count> permission node pattern to use this feature.
+# The default maximum number of shops a player can have. Set to -1 to disable
+# this limit.
+max-shops-per-player: -1
+# A list of permission nodes that can be used to set the maximum number of
+# shops a specific player or group of players can have. Use the permission node
+# pattern 'shopkeeper.maxshops.<count>' to use this feature. The permission
+# node 'shopkeeper.maxshops.unlimited' indicates no limit. These permission
+# nodes can only be used to increase the default limit, not decrease it.
 max-shops-perm-options: 5,15,25
 
 # Whether to protect player shop containers from being accessed or broken. It

src/main/resources/lang/language-de.yml

@@ -180,7 +180,7 @@ container-too-far-away: "&7Der Behälter des Shops ist zu weit entfernt!"
 container-not-placed: "&7Du musst einen Behälter auswählen, den du kürzlich platziert hast!"
 container-already-in-use: "&7Ein anderer Shop benutzt bereits den ausgewählten Behälter!"
 no-container-access: "&7Du hast keinen Zugriff auf den ausgewählten Behälter!"
-too-many-shops: "&7Du hast bereits zu viele Shops!"
+too-many-shops: "&7Du hast bereits das Limit an Shops erreicht, die du besitzen kannst!"
 no-player-shops-via-command: "&7Spieler-Shops können nur mit dem Shop-Erstellungs Item erzeugt werden!"
 shop-create-fail: "&7Du kannst dort keinen Shop erstellen."
 

src/main/resources/lang/language-en-default.yml

@@ -181,7 +181,7 @@ container-too-far-away: "&7The shopkeeper's container is too far away!"
 container-not-placed: "&7You must select a container you have recently placed!"
 container-already-in-use: "&7Another shopkeeper is already using the selected container!"
 no-container-access: "&7You cannot access the selected container!"
-too-many-shops: "&7You have too many shops!"
+too-many-shops: "&7You have already reached the limit of how many shops you can own!"
 no-player-shops-via-command: "&7Player shops can only be created via the shop creation item!"
 shop-create-fail: "&7You cannot create a shopkeeper there."
 

src/main/resources/plugin.yml

@@ -99,6 +99,9 @@ permissions:
     shopkeeper.bypass:
         description: Bypass player shop restrictions (access player shops of others)
         default: op
+    shopkeeper.maxshops.unlimited:
+        description: Allows the creation of an unlimited number of shops
+        default: op
     # Dynamic max shops permissions: The default max shops limit is configured in the config.
     # shopkeeper.maxshops.<count>
     #   description: The maximum number of shops a player can have, only values setup in the config can be used
@@ -175,6 +178,7 @@ permissions:
             shopkeeper.trade: true
             shopkeeper.hire: true
             shopkeeper.bypass: true
+            shopkeeper.maxshops.unlimited: true
             shopkeeper.admin: true
             shopkeeper.player: true
             shopkeeper.sign: true