Cleanup setOwners/setMembers, only edit WorldGuard region if different

- Improve code layout for changing owners and members of WorldGuard regions
- Only edit flags and other WorldGuard region settings if they are different than the current option, this prevents useless saves in WorldGuard (which shows a message in console)

.gitignore

@@ -33,4 +33,3 @@ local.properties
 dependency-reduced-pom.xml
 
 desktop.ini
-.*

AreaShop/src/main/java/me/wiefferink/areashop/features/WorldGuardRegionFlagsFeature.java

@@ -7,12 +7,15 @@ import com.sk89q.worldguard.protection.flags.RegionGroupFlag;
 import com.sk89q.worldguard.protection.regions.ProtectedRegion;
 import me.wiefferink.areashop.AreaShop;
 import me.wiefferink.areashop.events.notify.UpdateRegionEvent;
+import me.wiefferink.areashop.interfaces.RegionAccessSet;
 import me.wiefferink.areashop.regions.GeneralRegion;
 import me.wiefferink.interactivemessenger.processing.Message;
 import org.bukkit.configuration.ConfigurationSection;
 import org.bukkit.event.EventHandler;
 
+import java.util.Objects;
 import java.util.Set;
+import java.util.UUID;
 
 public class WorldGuardRegionFlagsFeature extends RegionFeature {
 
@@ -83,15 +86,17 @@ public class WorldGuardRegionFlagsFeature extends RegionFeature {
 				value = translateBukkitToWorldGuardColors(value);
 			}
 			if(flagName.equalsIgnoreCase("members")) {
-				plugin.getWorldGuardHandler().setMembers(worldguardRegion, value);
+				plugin.getWorldGuardHandler().setMembers(worldguardRegion, parseAccessSet(value));
 				//AreaShop.debug("  Flag " + flagName + " set: " + members.toUserFriendlyString());
 			} else if(flagName.equalsIgnoreCase("owners")) {
-				plugin.getWorldGuardHandler().setOwners(worldguardRegion, value);
+				plugin.getWorldGuardHandler().setOwners(worldguardRegion, parseAccessSet(value));
 				//AreaShop.debug("  Flag " + flagName + " set: " + owners.toUserFriendlyString());
 			} else if(flagName.equalsIgnoreCase("priority")) {
 				try {
 					int priority = Integer.parseInt(value);
-					worldguardRegion.setPriority(priority);
+					if(worldguardRegion.getPriority() != priority) {
+						worldguardRegion.setPriority(priority);
+					}
 					//AreaShop.debug("  Flag " + flagName + " set: " + value);
 				} catch(NumberFormatException e) {
 					AreaShop.warn("The value of flag " + flagName + " is not a number");
@@ -103,11 +108,13 @@ public class WorldGuardRegionFlagsFeature extends RegionFeature {
 				}
 				ProtectedRegion parentRegion = worldGuard.getRegionManager(region.getWorld()).getRegion(value);
 				if(parentRegion != null) {
-					try {
+					if(!parentRegion.equals(worldguardRegion.getParent())) {
-						worldguardRegion.setParent(parentRegion);
+						try {
-						//AreaShop.debug("  Flag " + flagName + " set: " + value);
+							worldguardRegion.setParent(parentRegion);
-					} catch(ProtectedRegion.CircularInheritanceException e) {
+							//AreaShop.debug("  Flag " + flagName + " set: " + value);
-						AreaShop.warn("The parent set in the config is not correct (circular inheritance)");
+						} catch(ProtectedRegion.CircularInheritanceException e) {
+							AreaShop.warn("The parent set in the config is not correct (circular inheritance)");
+						}
 					}
 				} else {
 					AreaShop.warn("The parent set in the config is not correct (region does not exist)");
@@ -172,10 +179,44 @@ public class WorldGuardRegionFlagsFeature extends RegionFeature {
 			}
 		}
 		// Indicate that the regions needs to be saved
+		// TODO do we still need this? maybe only for old WorldGuard?
 		plugin.getFileManager().saveIsRequiredForRegionWorld(region.getWorldName());
 		return result;
 	}
 
+	/**
+	 * Build an RegionAccessSet from an input that specifies player names, player uuids and groups.
+	 * @param input Input string defining the access set
+	 * @return RegionAccessSet containing the entities parsed from the input
+	 */
+	public RegionAccessSet parseAccessSet(String input) {
+		RegionAccessSet result = new RegionAccessSet();
+
+		String[] inputParts = input.split(", ");
+		for(String access : inputParts) {
+			if(access != null && !access.isEmpty()) {
+				// Check for groups
+				if(access.startsWith("g:")) {
+					if(access.length() > 2) {
+						result.getGroupNames().add(access.substring(2));
+					}
+				} else if(access.startsWith("n:")) {
+					if(access.length() > 2) {
+						result.getPlayerNames().add(access.substring(2));
+					}
+				} else {
+					try {
+						result.getPlayerUniqueIds().add(UUID.fromString(access));
+					} catch(IllegalArgumentException e) {
+						AreaShop.warn("Tried using '" + access + "' as uuid for a region member/owner, is your flagProfiles section correct?");
+					}
+				}
+			}
+		}
+
+		return result;
+	}
+
 	/**
 	 * Set a WorldGuard region flag.
 	 * @param region The WorldGuard region to set
@@ -185,7 +226,12 @@ public class WorldGuardRegionFlagsFeature extends RegionFeature {
 	 * @throws InvalidFlagFormat When the value of the flag is wrong
 	 */
 	private <V> void setFlag(ProtectedRegion region, Flag<V> flag, String value) throws InvalidFlagFormat {
-		region.setFlag(flag, plugin.getWorldGuardHandler().parseFlagInput(flag, value));
+		V current = region.getFlag(flag);
+		V next = plugin.getWorldGuardHandler().parseFlagInput(flag, value);
+
+		if(!Objects.equals(current, next)) {
+			region.setFlag(flag, next);
+		}
 	}
 
 	/**

AreaShop/src/main/java/me/wiefferink/areashop/tools/GithubUpdateChecker.java

@@ -0,0 +1,4 @@
+package me.wiefferink.areashop.tools;
+
+public class GithubUpdateChecker {
+}

AreaShop/src/main/resources/default.yml

@@ -5,7 +5,7 @@
 # ╚════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════╝
 # ┌────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
 # │ GENERAL: Settings that apply to all regions.                                                                               │
-# └────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘  
+# └────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘
 general:
   # The y location within the region to start searching for safe teleport locations (x and z are in the middle of the region).
   # Possible values: 'bottom', 'middle', 'top' or a number indicating an exact y coordinate.
@@ -235,4 +235,4 @@ buy:
   moneyBack: 100
   # Automatically sell the region after the specified number of minutes between the last login time of the buyer and the current time
   # Use times like '1 day' etc, or 'disabled' for never.
-  inactiveTimeUntilSell: 'disabled'
+  inactiveTimeUntilSell: 'disabled'

Interfaces/src/main/java/me/wiefferink/areashop/interfaces/RegionAccessSet.java

@@ -0,0 +1,44 @@
+package me.wiefferink.areashop.interfaces;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.UUID;
+
+public class RegionAccessSet {
+	private Set<String> playerNames;
+	private Set<UUID> playerUniqueIds;
+	private Set<String> groupNames;
+
+	/**
+	 * Constructor, creates an empty set.
+	 */
+	public RegionAccessSet() {
+		playerNames = new HashSet<>();
+		playerUniqueIds = new HashSet<>();
+		groupNames = new HashSet<>();
+	}
+
+	/**
+	 * Get the players that have been added by name.
+	 * @return Set with players that have been added by name
+	 */
+	public Set<String> getPlayerNames() {
+		return playerNames;
+	}
+
+	/**
+	 * Get the players that have been added by uuid.
+	 * @return Set with players that have been added by uuid
+	 */
+	public Set<UUID> getPlayerUniqueIds() {
+		return playerUniqueIds;
+	}
+
+	/**
+	 * Get the groups.
+	 * @return Set with groups added to this RegionAccessSet
+	 */
+	public Set<String> getGroupNames() {
+		return groupNames;
+	}
+}

Interfaces/src/main/java/me/wiefferink/areashop/interfaces/WorldGuardInterface.java

@@ -17,20 +17,6 @@ public abstract class WorldGuardInterface {
 		this.pluginInterface = pluginInterface;
 	}
 
-	/**
-	 * Parse an owner(s) string and set the players as owner of the WorldGuard region (set by UUID or name depending on implementation).
-	 * @param region The WorldGuard region to set the owners of
-	 * @param input  The owner(s) string to parse and set
-	 */
-	public abstract void setOwners(ProtectedRegion region, String input);
-
-	/**
-	 * Parse a member(s) string and set the players as member of the WorldGuard region (set by UUID or name depending on implementation).
-	 * @param region The WorldGuard region to set the members of
-	 * @param input  The member(s) string to parse and set
-	 */
-	public abstract void setMembers(ProtectedRegion region, String input);
-
 	/**
 	 * Get a set of ProtectedRegion's that are present on a certain location.
 	 * @param location The location to check
@@ -38,6 +24,21 @@ public abstract class WorldGuardInterface {
 	 */
 	public abstract Set<ProtectedRegion> getApplicableRegionsSet(Location location);
 
+	// DefaultDomain changed from abstract to non-abstract in version 6, so we need to handle that differently
+	/**
+	 * Parse an owner(s) string and set the players as owner of the WorldGuard region (set by UUID or name depending on implementation).
+	 * @param region The WorldGuard region to set the owners of
+	 * @param regionAccessSet  The owner(s) string to set
+	 */
+	public abstract void setOwners(ProtectedRegion region, RegionAccessSet regionAccessSet);
+
+	/**
+	 * Parse a member(s) string and set the players as member of the WorldGuard region (set by UUID or name depending on implementation).
+	 * @param region The WorldGuard region to set the members of
+	 * @param regionAccessSet  The member(s) string to set
+	 */
+	public abstract void setMembers(ProtectedRegion region, RegionAccessSet regionAccessSet);
+
 	/**
 	 * Check if a player is a member of the WorldGuard region.
 	 * @param region The region to check
@@ -54,6 +55,7 @@ public abstract class WorldGuardInterface {
 	 */
 	public abstract boolean containsOwner(ProtectedRegion region, UUID player);
 
+	// New flag system was introcuded in version 6.1.3, requiring different flag parsing
 	/**
 	 * Get a flag from the name of a flag.
 	 * @param flagName The name of the flag to get

WorldGuard 5/src/main/java/me/wiefferink/areashop/handlers/WorldGuardHandler5.java

@@ -10,6 +10,7 @@ import com.sk89q.worldguard.protection.flags.RegionGroup;
 import com.sk89q.worldguard.protection.flags.RegionGroupFlag;
 import com.sk89q.worldguard.protection.regions.ProtectedRegion;
 import me.wiefferink.areashop.interfaces.AreaShopInterface;
+import me.wiefferink.areashop.interfaces.RegionAccessSet;
 import me.wiefferink.areashop.interfaces.WorldGuardInterface;
 import org.bukkit.Bukkit;
 import org.bukkit.Location;
@@ -25,82 +26,6 @@ public class WorldGuardHandler5 extends WorldGuardInterface {
 		super(pluginInterface);
 	}
 
-	@Override
-	public void setOwners(ProtectedRegion region, String input) {
-		// Split the string and parse all values
-		String[] names = input.split(", ");
-		DefaultDomain owners = region.getOwners();
-		owners.removeAll();
-		for(String owner : names) {
-			if(owner != null && !owner.isEmpty()) {
-				// Check for groups
-				if(owner.startsWith("g:")) {
-					if(owner.length() > 2) {
-						owners.addGroup(owner.substring(2));
-					}
-				} else if(owner.startsWith("n:")) {
-					if(owner.length() > 2) {
-						owners.addPlayer(owner.substring(2));
-					}
-				} else {
-					UUID uuid;
-					try {
-						uuid = UUID.fromString(owner);
-					} catch(IllegalArgumentException e) {
-						System.out.println("Tried using '" + owner + "' as uuid for a region owner, is your flagProfiles section correct?");
-						uuid = null;
-					}
-					if(uuid != null) {
-						OfflinePlayer offlinePlayer = Bukkit.getOfflinePlayer(uuid);
-						if(offlinePlayer != null && offlinePlayer.getName() != null) {
-							owners.addPlayer(offlinePlayer.getName());
-						}
-					}
-				}
-			}
-		}
-		region.setOwners(owners);
-		//System.out.println("  Flag " + flagName + " set: " + owners.toUserFriendlyString());		
-	}
-
-	@Override
-	public void setMembers(ProtectedRegion region, String input) {
-		// Split the string and parse all values
-		String[] names = input.split(", ");
-		DefaultDomain members = region.getMembers();
-		members.removeAll();
-		for(String member : names) {
-			if(member != null && !member.isEmpty()) {
-				// Check for groups
-				if(member.startsWith("g:")) {
-					if(member.length() > 2) {
-						members.addGroup(member.substring(2));
-					}
-				} else if(member.startsWith("n:")) {
-					if(member.length() > 2) {
-						members.addPlayer(member.substring(2));
-					}
-				} else {
-					UUID uuid;
-					try {
-						uuid = UUID.fromString(member);
-					} catch(IllegalArgumentException e) {
-						System.out.println("Tried using '" + member + "' as uuid for a region member, is your flagProfiles section correct?");
-						uuid = null;
-					}
-					if(uuid != null) {
-						OfflinePlayer offlinePlayer = Bukkit.getOfflinePlayer(uuid);
-						if(offlinePlayer != null && offlinePlayer.getName() != null) {
-							members.addPlayer(offlinePlayer.getName());
-						}
-					}
-				}
-			}
-		}
-		region.setMembers(members);
-		//System.out.println("  Flag " + flagName + " set: " + members.toUserFriendlyString());		
-	}
-
 	@Override
 	public Set<ProtectedRegion> getApplicableRegionsSet(Location location) {
 		Set<ProtectedRegion> result = new HashSet<>();
@@ -113,6 +38,49 @@ public class WorldGuardHandler5 extends WorldGuardInterface {
 		return result;
 	}
 
+	@Override
+	public void setOwners(ProtectedRegion region, RegionAccessSet regionAccessSet) {
+		DefaultDomain defaultDomain = buildDomain(regionAccessSet);
+		if(!region.getOwners().toUserFriendlyString().equals(defaultDomain.toUserFriendlyString())) {
+			region.setOwners(defaultDomain);
+		}
+	}
+
+	@Override
+	public void setMembers(ProtectedRegion region, RegionAccessSet regionAccessSet) {
+		DefaultDomain defaultDomain = buildDomain(regionAccessSet);
+		if(!region.getMembers().toUserFriendlyString().equals(defaultDomain.toUserFriendlyString())) {
+			region.setMembers(defaultDomain);
+		}
+	}
+
+	/**
+	 * Build a DefaultDomain from a RegionAccessSet.
+	 * @param regionAccessSet RegionAccessSet to read
+	 * @return DefaultDomain containing the entities from the RegionAccessSet
+	 */
+	private DefaultDomain buildDomain(RegionAccessSet regionAccessSet) {
+		DefaultDomain owners = new DefaultDomain();
+
+		for(String playerName : regionAccessSet.getPlayerNames()) {
+			owners.addPlayer(playerName);
+		}
+
+		// Add by name since UUIDs were not yet supported
+		for(UUID uuid : regionAccessSet.getPlayerUniqueIds()) {
+			OfflinePlayer offlinePlayer = Bukkit.getOfflinePlayer(uuid);
+			if(offlinePlayer != null && offlinePlayer.getName() != null) {
+				owners.addPlayer(offlinePlayer.getName());
+			}
+		}
+
+		for(String group : regionAccessSet.getGroupNames()) {
+			owners.addGroup(group);
+		}
+
+		return owners;
+	}
+
 	@Override
 	public boolean containsMember(ProtectedRegion region, UUID player) {
 		if(player == null) {
@@ -147,4 +115,4 @@ public class WorldGuardHandler5 extends WorldGuardInterface {
 	public RegionGroup parseFlagGroupInput(RegionGroupFlag flag, String input) throws InvalidFlagFormat {
 		return flag.parseInput(WorldGuardPlugin.inst(), null, input);
 	}
-}
+}

WorldGuard 6/src/main/java/me/wiefferink/areashop/handlers/WorldGuardHandler6.java

@@ -10,6 +10,7 @@ import com.sk89q.worldguard.protection.flags.RegionGroup;
 import com.sk89q.worldguard.protection.flags.RegionGroupFlag;
 import com.sk89q.worldguard.protection.regions.ProtectedRegion;
 import me.wiefferink.areashop.interfaces.AreaShopInterface;
+import me.wiefferink.areashop.interfaces.RegionAccessSet;
 import me.wiefferink.areashop.interfaces.WorldGuardInterface;
 import org.bukkit.Location;
 
@@ -23,78 +24,6 @@ public class WorldGuardHandler6 extends WorldGuardInterface {
 		super(pluginInterface);
 	}
 
-	@Override
-	public void setOwners(ProtectedRegion region, String input) {
-		// Split the string and parse all values
-		String[] names = input.split(", ");
-		DefaultDomain owners = region.getOwners();
-		owners.removeAll();
-		for(String owner : names) {
-			if(owner != null && !owner.isEmpty()) {
-				// Check for groups
-				if(owner.startsWith("g:")) {
-					if(owner.length() > 2) {
-						owners.addGroup(owner.substring(2));
-					}
-				} else if(owner.startsWith("n:")) {
-					if(owner.length() > 2) {
-						owners.addPlayer(owner.substring(2));
-					}
-				} else {
-					UUID uuid;
-					try {
-						uuid = UUID.fromString(owner);
-					} catch(IllegalArgumentException e) {
-						// Don't like this but cannot access main plugin class from this module...
-						System.out.println("[AreaShop] Tried using '" + owner + "' as uuid for a region owner, is your flagProfiles section correct?");
-						uuid = null;
-					}
-					if(uuid != null) {
-						owners.addPlayer(uuid);
-					}
-				}
-			}
-		}
-		region.setOwners(owners);
-		//System.out.println("  Flag " + flagName + " set: " + owners.toUserFriendlyString());		
-	}
-
-	@Override
-	public void setMembers(ProtectedRegion region, String input) {
-		// Split the string and parse all values
-		String[] names = input.split(", ");
-		DefaultDomain members = region.getMembers();
-		members.removeAll();
-		for(String member : names) {
-			if(member != null && !member.isEmpty()) {
-				// Check for groups
-				if(member.startsWith("g:")) {
-					if(member.length() > 2) {
-						members.addGroup(member.substring(2));
-					}
-				} else if(member.startsWith("n:")) {
-					if(member.length() > 2) {
-						members.addPlayer(member.substring(2));
-					}
-				} else {
-					UUID uuid;
-					try {
-						uuid = UUID.fromString(member);
-					} catch(IllegalArgumentException e) {
-						// Don't like this but cannot access main plugin class from this module...
-						System.out.println("[AreaShop] Tried using '" + member + "' as uuid for a region member, is your flagProfiles section correct?");
-						uuid = null;
-					}
-					if(uuid != null) {
-						members.addPlayer(uuid);
-					}
-				}
-			}
-		}
-		region.setMembers(members);
-		//System.out.println("  Flag " + flagName + " set: " + members.toUserFriendlyString());
-	}
-
 	@Override
 	public Set<ProtectedRegion> getApplicableRegionsSet(Location location) {
 		Set<ProtectedRegion> result = new HashSet<>();
@@ -107,6 +36,45 @@ public class WorldGuardHandler6 extends WorldGuardInterface {
 		return result;
 	}
 
+	@Override
+	public void setOwners(ProtectedRegion region, RegionAccessSet regionAccessSet) {
+		DefaultDomain defaultDomain = buildDomain(regionAccessSet);
+		if(!region.getOwners().toUserFriendlyString().equals(defaultDomain.toUserFriendlyString())) {
+			region.setOwners(defaultDomain);
+		}
+	}
+
+	@Override
+	public void setMembers(ProtectedRegion region, RegionAccessSet regionAccessSet) {
+		DefaultDomain defaultDomain = buildDomain(regionAccessSet);
+		if(!region.getMembers().toUserFriendlyString().equals(defaultDomain.toUserFriendlyString())) {
+			region.setMembers(defaultDomain);
+		}
+	}
+
+	/**
+	 * Build a DefaultDomain from a RegionAccessSet.
+	 * @param regionAccessSet RegionAccessSet to read
+	 * @return DefaultDomain containing the entities from the RegionAccessSet
+	 */
+	private DefaultDomain buildDomain(RegionAccessSet regionAccessSet) {
+		DefaultDomain owners = new DefaultDomain();
+
+		for(String playerName : regionAccessSet.getPlayerNames()) {
+			owners.addPlayer(playerName);
+		}
+
+		for(UUID uuid : regionAccessSet.getPlayerUniqueIds()) {
+			owners.addPlayer(uuid);
+		}
+
+		for(String group : regionAccessSet.getGroupNames()) {
+			owners.addGroup(group);
+		}
+
+		return owners;
+	}
+
 	@Override
 	public boolean containsMember(ProtectedRegion region, UUID player) {
 		return region.getMembers().contains(player);
@@ -131,4 +99,4 @@ public class WorldGuardHandler6 extends WorldGuardInterface {
 	public RegionGroup parseFlagGroupInput(RegionGroupFlag flag, String input) throws InvalidFlagFormat {
 		return flag.parseInput(WorldGuardPlugin.inst(), null, input);
 	}
-}
+}