Refactor and improve WorldGroup.

src/main/java/com/onarandombox/multiverseinventories/AbstractWorldGroupManager.java

@@ -54,7 +54,7 @@ public List<WorldGroup> getGroupsForWorld(String worldName) {
         worldName = worldName.toLowerCase();
         List<WorldGroup> worldGroups = new ArrayList<>();
         for (WorldGroup worldGroup : getGroupNames().values()) {
-            if (worldGroup.containsWorld(worldName)) {
+            if (worldGroup.getWorlds().contains(worldName)) {
                 worldGroups.add(worldGroup);
             }
         }
@@ -139,22 +139,24 @@ public void createDefaultGroup() {
         World defaultWorld = Bukkit.getWorlds().get(0);
         World defaultNether = Bukkit.getWorld(defaultWorld.getName() + "_nether");
         World defaultEnd = Bukkit.getWorld(defaultWorld.getName() + "_the_end");
+
         WorldGroup worldGroup = new WorldGroup(plugin, DEFAULT_GROUP_NAME);
-        worldGroup.getShares().mergeShares(Sharables.allOf());
-        worldGroup.addWorld(defaultWorld);
-        StringBuilder worlds = new StringBuilder().append(defaultWorld.getName());
-        if (defaultNether != null) {
-            worldGroup.addWorld(defaultNether);
-            worlds.append(", ").append(defaultNether.getName());
-        }
-        if (defaultEnd != null) {
-            worldGroup.addWorld(defaultEnd);
-            worlds.append(", ").append(defaultEnd.getName());
-        }
-        updateGroup(worldGroup);
+
+        worldGroup.modify(group -> {
+            group.getShares().mergeShares(Sharables.allOf());
+            group.getWorlds().add(defaultWorld);
+            if (defaultNether != null) {
+                group.getWorlds().add(defaultNether);
+            }
+            if (defaultEnd != null) {
+                group.getWorlds().add(defaultEnd);
+            }
+        });
+
         plugin.getMVIConfig().setFirstRun(false);
         plugin.getMVIConfig().save();
-        Logging.info("Created a default group for you containing all of your default worlds: " + worlds.toString());
+        Logging.info("Created a default group for you containing all of your default worlds: %s",
+                worldGroup.getWorlds().toString());
     }
 
     /**
@@ -163,11 +165,11 @@ public void createDefaultGroup() {
     @Override
     public WorldGroup getDefaultGroup() {
         WorldGroup group = getGroupNames().get(DEFAULT_GROUP_NAME);
-        if (group == null) {
-            group = newEmptyGroup(DEFAULT_GROUP_NAME);
-            group.getShares().setSharing(Sharables.allOf(), true);
-            updateGroup(group);
+        if (group != null) {
+            return group;
         }
+        group = newEmptyGroup(DEFAULT_GROUP_NAME);
+        group.modify(worldGroup -> worldGroup.getShares().setSharing(Sharables.allOf(), true));
         return group;
     }
 

src/main/java/com/onarandombox/multiverseinventories/InventoriesListener.java

@@ -247,8 +247,8 @@ public void playerTeleport(PlayerTeleportEvent event) {
         for (WorldGroup fromGroup : fromGroups) {
             playerProfile = fromGroup.getGroupProfileContainer().getPlayerData(event.getPlayer());
 
-            if (fromGroup.containsWorld(toWorldName)) {
-                if (!fromGroup.isSharing(Sharables.LAST_LOCATION)) {
+            if (fromGroup.getWorlds().contains(toWorldName)) {
+                if (!fromGroup.getShares().isSharing(Sharables.LAST_LOCATION)) {
                     playerProfile.set(Sharables.LAST_LOCATION, event.getFrom());
                 }
             } else {
@@ -428,8 +428,8 @@ public void entityPortal(EntityPortalEvent event) {
         List<WorldGroup> fromGroups = inventories.getGroupManager().getGroupsForWorld(fromWorld.getName());
         List<WorldGroup> toGroups = inventories.getGroupManager().getGroupsForWorld(toWorld.getName());
         // We only care about the groups that have the inventory sharable
-        fromGroups = fromGroups.stream().filter(it -> it.isSharing(Sharables.INVENTORY)).collect(Collectors.toList());
-        toGroups = toGroups.stream().filter(it -> it.isSharing(Sharables.INVENTORY)).collect(Collectors.toList());
+        fromGroups = fromGroups.stream().filter(it -> it.getShares().isSharing(Sharables.INVENTORY)).collect(Collectors.toList());
+        toGroups = toGroups.stream().filter(it -> it.getShares().isSharing(Sharables.INVENTORY)).collect(Collectors.toList());
         for (WorldGroup fromGroup : fromGroups) {
             if (toGroups.contains(fromGroup)) {
                 Logging.finest("Allowing item or inventory holding %s to go from world %s to world %s", entity,

src/main/java/com/onarandombox/multiverseinventories/WorldChangeShareHandler.java

@@ -142,7 +142,7 @@ private boolean isPlayerBypassingChange(WorldGroup worldGroup) {
         }
 
         private boolean isFromWorldNotInToWorldGroup(WorldGroup worldGroup) {
-            return !worldGroup.containsWorld(fromWorld);
+            return !worldGroup.getWorlds().contains(fromWorld);
         }
 
         private void addReadProfileForWorldGroup(WorldGroup worldGroup) {
@@ -211,7 +211,7 @@ boolean isEligibleForWrite() {
         }
 
         private boolean groupDoesNotContainWorld(String world) {
-            return !worldGroup.containsWorld(world);
+            return !worldGroup.getWorlds().contains(world);
         }
 
         private boolean isNotSharingAll() {

src/main/java/com/onarandombox/multiverseinventories/WorldGroup.java

@@ -1,20 +1,20 @@
 package com.onarandombox.multiverseinventories;
 
+import com.onarandombox.multiverseinventories.profile.container.ProfileContainer;
 import com.onarandombox.multiverseinventories.share.Sharable;
 import com.onarandombox.multiverseinventories.share.Sharables;
 import com.onarandombox.multiverseinventories.share.Shares;
-import com.onarandombox.multiverseinventories.profile.container.ProfileContainer;
+import com.onarandombox.multiverseinventories.util.WorldsSet;
 import org.bukkit.World;
 import org.bukkit.event.EventPriority;
 
-import java.util.HashSet;
-import java.util.Set;
+import java.util.function.Consumer;
 
 public final class WorldGroup {
 
     private final MultiverseInventories plugin;
     private final String name;
-    private final HashSet<String> worlds = new HashSet<>();
+    private final WorldsSet worlds = new WorldsSet();
     private final Shares shares = Sharables.noneOf();
 
     private String spawnWorld = null;
@@ -25,6 +25,23 @@ public final class WorldGroup {
         this.name = name;
     }
 
+    /**
+     * Run changes to the World Group, then save it to data source for the changes to be permanent.
+     *
+     * @param modifications Consume any changes to the {@link WorldGroup}.
+     */
+    public void modify(Consumer<WorldGroup> modifications) {
+        modifications.accept(this);
+        this.save();
+    }
+
+    /**
+     * Save World Group to data source for the changes to be permanent.
+     */
+    public void save() {
+        this.plugin.getGroupManager().updateGroup(this);
+    }
+
     /**
      * Get the name of this World Group.
      *
@@ -38,7 +55,10 @@ public String getName() {
      * Adds a world to this world group and updates it in the Config.
      *
      * @param worldName The name of the world to add.
+     *
+     * @deprecated Use getWorlds().add(String).
      */
+    @Deprecated
     public void addWorld(String worldName) {
         this.addWorld(worldName, true);
     }
@@ -48,7 +68,10 @@ public void addWorld(String worldName) {
      *
      * @param worldName    The name of the world to add.
      * @param updateConfig True to update this group in the config.
+     *
+     * @deprecated Use getWorlds().add(String).
      */
+    @Deprecated
     public void addWorld(String worldName, boolean updateConfig) {
         this.getWorlds().add(worldName.toLowerCase());
         if (updateConfig) {
@@ -60,7 +83,10 @@ public void addWorld(String worldName, boolean updateConfig) {
      * Convenience method to add a {@link org.bukkit.World} to this World Group.
      *
      * @param world The world to add.
+     *
+     * @deprecated Use getWorlds().add(World).
      */
+    @Deprecated
     public void addWorld(World world) {
         this.addWorld(world.getName());
     }
@@ -69,7 +95,10 @@ public void addWorld(World world) {
      * Removes a world from this world group and updates the group in the Config.
      *
      * @param worldName The name of the world to remove.
+     *
+     * @deprecated Use getWorlds().remove(String).
      */
+    @Deprecated
     public void removeWorld(String worldName) {
         this.removeWorld(worldName, true);
     }
@@ -79,7 +108,10 @@ public void removeWorld(String worldName) {
      *
      * @param worldName    The name of the world to remove.
      * @param updateConfig True to update this group in the config.
+     *
+     * @deprecated Use getWorlds().remove(String).
      */
+    @Deprecated
     public void removeWorld(String worldName, boolean updateConfig) {
         this.getWorlds().remove(worldName.toLowerCase());
         if (updateConfig) {
@@ -91,35 +123,46 @@ public void removeWorld(String worldName, boolean updateConfig) {
      * Convenience method to remove a {@link org.bukkit.World} from this World Group.
      *
      * @param world The world to remove.
+     *
+     * @deprecated Use getWorlds().remove(World).
      */
+    @Deprecated
     public void removeWorld(World world) {
         this.removeWorld(world.getName());
     }
 
     /**
-     * Retrieves all of the worlds in this World Group.
+     * Retrieves all of the worlds in this World Group. Any changes made are not permanently saved to data source.
+     * <br>
+     * To permanently save changes, either do {@link #save()} after this method or call this method in
+     * Consumer of {@link #modify(Consumer)}.
      *
      * @return The worlds of this World Group.
      */
-    public Set<String> getWorlds() {
+    public WorldsSet getWorlds() {
         return this.worlds;
     }
 
     /**
-     * Checks if this group is sharing sharable.  This will check both shares and negative shares of the group.
+     * Checks if this group is sharing sharable. This will check both shares and negative shares of the group.
      * This is the preferred method for checking if a group shares something as shares may contain ALL shares while
      * ones indicated in negative shares means those aren't actually shared.
      *
      * @param sharable Sharable to check if sharing.
      * @return true is the sharable is shared for this group.
+     *
+     * @deprecated Use getShares().isSharing(Sharable).
      */
+    @Deprecated
     public boolean isSharing(Sharable sharable) {
         return getShares().isSharing(sharable);
     }
 
     /**
-     * Retrieves the shares for this World Group.  Any changes to this group must be subsequently saved to the data
-     * source for the changes to be permanent.
+     * Retrieves the shares for this World Group. Any changes made are not permanently saved to data source.
+     * <br>
+     * To permanently save changes, either do {@link #save()} after this method or call this method in
+     * Consumer of {@link #modify(Consumer)}.
      *
      * @return The shares for this World Group.
      */
@@ -130,12 +173,17 @@ public Shares getShares() {
     /**
      * @param worldName Name of world to check for.
      * @return True if specified world is part of this group.
+     *
+     * @deprecated Use getWorlds().contains(String).
      */
+    @Deprecated
     public boolean containsWorld(String worldName) {
         return this.getWorlds().contains(worldName.toLowerCase());
     }
 
     /**
+     * Gets spawn world of this World Group.
+     *
      * @return The name of the world that will be used as the spawn for this group.
      *         Or null if no world was specified as the group spawn world.
      */
@@ -144,20 +192,32 @@ public String getSpawnWorld() {
     }
 
     /**
+     * Sets spawn world of this World Group. Changes are not permanently saved to data source.
+     * <br>
+     * To permanently save changes, either do {@link #save()} after this method or call this method in
+     * Consumer of {@link #modify(Consumer)}.
+     *
      * @param worldName The name of the world to set this groups spawn to.
      */
     public void setSpawnWorld(String worldName) {
         this.spawnWorld = worldName.toLowerCase();
     }
 
     /**
+     * Gets event priority to be used during player respawn.
+     *
      * @return The priority for the respawn event that this spawn will act on.
      */
     public EventPriority getSpawnPriority() {
         return this.spawnPriority;
     }
 
     /**
+     * Sets event priority to be used during player respawn. Changes are not permanently saved to data source.
+     * <br>
+     * To permanently save changes, either do {@link #save()} after this method or call this method in
+     * Consumer of {@link #modify(Consumer)}.
+     *
      * @param priority The priority that will be used for respawning the player at
      *                 this group's spawn location if there is one set.
      */
@@ -175,7 +235,7 @@ public boolean isDefault() {
     }
 
     /**
-     * Returns the profile container for this group.
+     * Gets the profile container for this group.
      *
      * @return the profile container for this group.
      */
@@ -185,19 +245,13 @@ public ProfileContainer getGroupProfileContainer() {
 
     @Override
     public String toString() {
-        StringBuilder builder = new StringBuilder();
-        builder.append(this.getName()).append(": {Worlds: [");
-        String[] worldsString = this.getWorlds().toArray(new String[this.getWorlds().size()]);
-        for (int i = 0; i < worldsString.length; i++) {
-            if (i != 0) {
-                builder.append(", ");
-            }
-            builder.append(worldsString[i]);
-        }
-        builder.append("], Shares: [").append(this.getShares().toString()).append("]");
+        StringBuilder builder = new StringBuilder()
+                .append(this.getName()).append(": {Worlds: [")
+                .append(this.getWorlds().toString())
+                .append("], Shares: [").append(this.getShares().toString()).append("]");
         if (this.getSpawnWorld() != null) {
-            builder.append(", Spawn World: ").append(this.getSpawnWorld());
-            builder.append(", Spawn Priority: ").append(this.getSpawnPriority().toString());
+            builder.append(", Spawn World: ").append(this.getSpawnWorld())
+                    .append(", Spawn Priority: ").append(this.getSpawnPriority().toString());
         }
         builder.append("}");
         return builder.toString();

src/main/java/com/onarandombox/multiverseinventories/YamlWorldGroupManager.java

@@ -136,7 +136,7 @@ private WorldGroup deserializeGroup(final String name, final Map<String, Object>
                             Logging.fine("Error with a world listed in group: " + name);
                             continue;
                         }
-                        profile.addWorld(worldNameObj.toString(), false);
+                        profile.getWorlds().add(worldNameObj.toString());
                         World world = Bukkit.getWorld(worldNameObj.toString());
                         if (world == null) {
                             if (builder.length() != 0) {
@@ -220,6 +220,7 @@ public void updateGroup(final WorldGroup worldGroup) {
         super.updateGroup(worldGroup);
         updateWorldGroup(worldGroup);
         save();
+        Logging.finest("Updated World Group '%s'.", worldGroup.getName());
     }
 
     @Override

src/main/java/com/onarandombox/multiverseinventories/command/AddSharesCommand.java

@@ -71,9 +71,12 @@ public void runCommand(CommandSender sender, List<String> args) {
             this.messager.normal(Message.ERROR_NO_GROUP, sender, args.get(1));
             return;
         }
-        worldGroup.getShares().mergeShares(newShares);
-        worldGroup.getShares().removeAll(negativeShares);
-        this.plugin.getGroupManager().updateGroup(worldGroup);
+
+        worldGroup.modify(group -> {
+            group.getShares().mergeShares(newShares);
+            group.getShares().removeAll(negativeShares);
+        });
+
         this.plugin.getMVIConfig().save();
         this.messager.normal(Message.NOW_SHARING, sender, worldGroup.getName(),
                 worldGroup.getShares().toString());

src/main/java/com/onarandombox/multiverseinventories/command/AddWorldCommand.java

@@ -42,13 +42,14 @@ public void runCommand(CommandSender sender, List<String> args) {
             this.messager.normal(Message.ERROR_NO_GROUP, sender, args.get(1));
             return;
         }
-        if (worldGroup.containsWorld(world.getName())) {
+        if (worldGroup.getWorlds().contains(world.getName())) {
             this.messager.normal(Message.WORLD_ALREADY_EXISTS, sender, world.getName(),
                     worldGroup.getName());
             return;
         }
-        worldGroup.addWorld(world);
-        this.plugin.getGroupManager().updateGroup(worldGroup);
+
+        worldGroup.modify(group -> group.getWorlds().add(world));
+
         this.plugin.getMVIConfig().save();
         this.messager.normal(Message.WORLD_ADDED, sender, world.getName(),
                 worldGroup.getName());