Improve group conflict checking api and apply to all group modify commands

src/main/java/org/mvplugins/multiverse/inventories/MultiverseInventories.java

@@ -261,7 +261,8 @@ public void reloadConfig() {
 
                 inventoriesConfig.get().setFirstRun(false);
             }
-            worldGroupManager.get().checkForConflicts(null);
+            worldGroupManager.get().checkForConflicts()
+                    .sendConflictIssue(commandManagerProvider.get().getConsoleCommandIssuer());
         }, 1L);
     }
 }

src/main/java/org/mvplugins/multiverse/inventories/commands/AddDisabledSharesCommand.java

@@ -49,5 +49,6 @@ void onAddDisabledSharesCommand(
         issuer.sendInfo(MVInvi18n.DISABLEDSHARES_NOWSHARING,
                 replace("{group}").with(group.getName()),
                 replace("{shares}").with(group.getDisabledShares().toStringList()));
+        worldGroupManager.checkForConflicts().sendConflictIssue(issuer);
     }
 }

src/main/java/org/mvplugins/multiverse/inventories/commands/AddSharesCommand.java

@@ -54,5 +54,6 @@ void onAddSharesCommand(
                 replace("{group}").with(group.getName()),
                 replace("{shares}").with(group.getShares().toStringList()),
                 replace("{negativeshares}").with(negativeshares.toStringList()));
+        worldGroupManager.checkForConflicts().sendConflictIssue(issuer);
     }
 }

src/main/java/org/mvplugins/multiverse/inventories/commands/AddWorldsCommand.java

@@ -61,5 +61,6 @@ void onAddWorldCommand(
         issuer.sendInfo(MVInvi18n.ADDWORLD_WORLDADDED,
                 replace("{group}").with(group.getName()),
                 replace("{world}").with(worldNamesString));
+        worldGroupManager.checkForConflicts().sendConflictIssue(issuer);
     }
 }

src/main/java/org/mvplugins/multiverse/inventories/commands/CheckGroupConflictsCommand.java

@@ -0,0 +1,35 @@
+package org.mvplugins.multiverse.inventories.commands;
+
+import org.jvnet.hk2.annotations.Service;
+import org.mvplugins.multiverse.core.command.MVCommandIssuer;
+import org.mvplugins.multiverse.external.acf.commands.annotation.CommandPermission;
+import org.mvplugins.multiverse.external.acf.commands.annotation.Description;
+import org.mvplugins.multiverse.external.acf.commands.annotation.Subcommand;
+import org.mvplugins.multiverse.external.jakarta.inject.Inject;
+import org.mvplugins.multiverse.external.jetbrains.annotations.NotNull;
+import org.mvplugins.multiverse.inventories.profile.group.GroupingConflictResult;
+import org.mvplugins.multiverse.inventories.profile.group.WorldGroupManager;
+import org.mvplugins.multiverse.inventories.util.MVInvi18n;
+
+@Service
+final class CheckGroupConflictsCommand extends InventoriesCommand {
+    private final WorldGroupManager worldGroupManager;
+
+    @Inject
+    CheckGroupConflictsCommand(@NotNull WorldGroupManager worldGroupManager) {
+        this.worldGroupManager = worldGroupManager;
+    }
+
+    @Subcommand("check-group-conflicts")
+    @CommandPermission("multiverse.inventories.checkgroupconflict")
+    @Description("Check for conflicts in World Groups.")
+    void onCommand(@NotNull MVCommandIssuer issuer) {
+        issuer.sendInfo(MVInvi18n.CONFLICT_CHECKING);
+        GroupingConflictResult groupingConflictResult = worldGroupManager.checkForConflicts();
+        if (groupingConflictResult.hasConflict()) {
+            groupingConflictResult.sendConflictIssue(issuer);
+        } else {
+            issuer.sendInfo(MVInvi18n.CONFLICT_NOTFOUND);
+        }
+    }
+}

src/main/java/org/mvplugins/multiverse/inventories/commands/CreateGroupCommand.java

@@ -62,5 +62,6 @@ void onCreateGroupCommand(
         issuer.sendInfo(MVInvi18n.GROUP_CREATIONCOMPLETE, replace("{group}").with(groupName));
         issuer.sendInfo(MVInvi18n.INFO_GROUP_INFO, replace("{worlds}").with(worldGroup.getConfigWorlds()));
         issuer.sendInfo(MVInvi18n.INFO_GROUP_INFOSHARES, replace("{shares}").with(worldGroup.getShares()));
+        worldGroupManager.checkForConflicts().sendConflictIssue(issuer);
     }
 }

src/main/java/org/mvplugins/multiverse/inventories/commands/DeleteGroupCommand.java

@@ -49,6 +49,7 @@ void onDeleteGroupCommand(
         commandQueueManager.addToQueue(CommandQueuePayload.issuer(issuer)
                 .prompt(Message.of(MVInvi18n.DELETEGROUP_CONFIRMPROMPT, replace("{group}").with(group.getName())))
                 .action(() -> doDeleteGroup(issuer, group)));
+        worldGroupManager.checkForConflicts().sendConflictIssue(issuer);
     }
 
     private void doDeleteGroup(MVCommandIssuer issuer, WorldGroup group) {

src/main/java/org/mvplugins/multiverse/inventories/commands/RemoveDisabledSharesCommand.java

@@ -49,5 +49,6 @@ void onRemoveSharesCommand(
         issuer.sendInfo(MVInvi18n.DISABLEDSHARES_NOWSHARING,
                 replace("{group}").with(group.getName()),
                 replace("{shares}").with(group.getDisabledShares().toStringList()));
+        worldGroupManager.checkForConflicts().sendConflictIssue(issuer);
     }
 }

src/main/java/org/mvplugins/multiverse/inventories/commands/RemoveSharesCommand.java

@@ -52,5 +52,6 @@ void onRemoveSharesCommand(
                 replace("{group}").with(group.getName()),
                 replace("{shares}").with(group.getShares().toStringList()),
                 replace("{negativeshares}").with(negativeshares.toStringList()));
+        worldGroupManager.checkForConflicts().sendConflictIssue(issuer);
     }
 }

src/main/java/org/mvplugins/multiverse/inventories/commands/RemoveWorldsCommand.java

@@ -58,5 +58,6 @@ void onRemoveWorldCommand(
         issuer.sendInfo(MVInvi18n.REMOVEWORLD_WORLDREMOVED,
                 replace("{group}").with(group.getName()),
                 replace("{world}").with(worldNames));
+        worldGroupManager.checkForConflicts().sendConflictIssue(issuer);
     }
 }

src/main/java/org/mvplugins/multiverse/inventories/commands/prompts/GroupSharesPrompt.java

@@ -63,7 +63,7 @@ public Prompt acceptInput(@NotNull final ConversationContext conversationContext
             issuer.sendInfo(MVInvi18n.INFO_GROUP, replace("{group}").with(group.getName()));
             issuer.sendInfo(MVInvi18n.INFO_GROUP_INFO, replace("{worlds}").with(group.getConfigWorlds()));
             issuer.sendInfo(MVInvi18n.INFO_GROUP_INFOSHARES, replace("{shares}").with(group.getShares()));
-            worldGroupManager.checkForConflicts(issuer);
+            worldGroupManager.checkForConflicts().sendConflictIssue(issuer);
             return nextPrompt;
         }
 

src/main/java/org/mvplugins/multiverse/inventories/profile/group/AbstractWorldGroupManager.java

@@ -13,6 +13,8 @@
 import org.mvplugins.multiverse.core.world.LoadedMultiverseWorld;
 import org.mvplugins.multiverse.core.world.WorldManager;
 import org.mvplugins.multiverse.external.jetbrains.annotations.NotNull;
+import org.mvplugins.multiverse.external.vavr.Tuple;
+import org.mvplugins.multiverse.external.vavr.Tuple2;
 import org.mvplugins.multiverse.inventories.MultiverseInventories;
 import org.mvplugins.multiverse.inventories.config.InventoriesConfig;
 import org.mvplugins.multiverse.inventories.profile.container.ProfileContainerStoreProvider;
@@ -24,9 +26,11 @@
 
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import static org.mvplugins.multiverse.core.locale.message.MessageReplacement.replace;
 
@@ -185,41 +189,57 @@ public WorldGroup getDefaultGroup() {
      * {@inheritDoc}
      */
     @Override
-    public List<GroupingConflict> checkGroups() {
+    public GroupingConflictResult checkForConflicts() {
         List<GroupingConflict> conflicts = new ArrayList<>();
-        Map<WorldGroup, WorldGroup> previousConflicts = new HashMap<>();
+        Set<Tuple2<WorldGroup, WorldGroup>> checkedPairs = new HashSet<>();
         for (WorldGroup checkingGroup : getGroupNames().values()) {
             for (String worldName : checkingGroup.getApplicableWorlds()) {
                 for (WorldGroup worldGroup : getGroupsForWorld(worldName)) {
-                    if (checkingGroup.equals(worldGroup)) {
-                        continue;
-                    }
-                    if (previousConflicts.containsKey(checkingGroup)) {
-                        if (previousConflicts.get(checkingGroup).equals(worldGroup)) {
-                            continue;
-                        }
-                    }
-                    if (previousConflicts.containsKey(worldGroup)) {
-                        if (previousConflicts.get(worldGroup).equals(checkingGroup)) {
-                            continue;
-                        }
-                    }
-                    previousConflicts.put(checkingGroup, worldGroup);
-                    Shares conflictingShares = worldGroup.getShares()
-                            .compare(checkingGroup.getShares());
-                    if (!conflictingShares.isEmpty()) {
-                        if (checkingGroup.getApplicableWorlds().containsAll(worldGroup.getApplicableWorlds())
-                                || worldGroup.getApplicableWorlds().containsAll(checkingGroup.getApplicableWorlds())) {
-                            continue;
-                        }
-                        conflicts.add(new GroupingConflict(checkingGroup, worldGroup,
-                                Sharables.fromShares(conflictingShares)));
-                    }
+                    checkConflict(checkingGroup, worldGroup, checkedPairs, conflicts);
                 }
             }
         }
+        return new GroupingConflictResult(conflicts);
+    }
 
-        return conflicts;
+    private void checkConflict(WorldGroup checkingGroup,
+                               WorldGroup worldGroup,
+                               Set<Tuple2<WorldGroup, WorldGroup>> checkedPairs,
+                               List<GroupingConflict> conflicts) {
+        if (checkingGroup.equals(worldGroup)) {
+            // Don't check against itself.
+            return;
+        }
+        if (checkedPairs.contains(new Tuple2<>(checkingGroup, worldGroup))) {
+            // Already checked this pair.
+            return;
+        }
+        if (checkedPairs.contains(new Tuple2<>(worldGroup, checkingGroup))) {
+            // Already checked this pair in the opposite order.
+            return;
+        }
+        Logging.finer("Checking conflict between %s and %s", checkingGroup.getName(), worldGroup.getName());
+        checkedPairs.add(new Tuple2<>(checkingGroup, worldGroup));
+        Shares conflictingShares = worldGroup.getApplicableShares().compare(checkingGroup.getApplicableShares());
+        if (conflictingShares.isEmpty()) {
+            // No overlapping shares.
+            return;
+        }
+        if (checkingGroup.getApplicableWorlds().containsAll(worldGroup.getApplicableWorlds())
+                || worldGroup.getApplicableWorlds().containsAll(checkingGroup.getApplicableWorlds())) {
+            // If one group contains all the worlds of the other, we don't consider it a conflict.
+            return;
+        }
+        Logging.finer("Conflict found for %s and %s", checkingGroup.getName(), worldGroup.getName());
+        conflicts.add(new GroupingConflict(checkingGroup, worldGroup, Sharables.fromShares(conflictingShares)));
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public List<GroupingConflict> checkGroups() {
+        return checkForConflicts().getConflicts();
     }
 
     /**
@@ -230,18 +250,10 @@ public void checkForConflicts(MVCommandIssuer issuer) {
         if (issuer == null) {
             issuer = commandManager.getCommandIssuer(Bukkit.getConsoleSender());
         }
-
         issuer.sendInfo(MVInvi18n.CONFLICT_CHECKING);
-        List<GroupingConflict> conflicts = checkGroups();
-        for (GroupingConflict conflict : conflicts) {
-            issuer.sendInfo(MVInvi18n.CONFLICT_RESULTS,
-                    replace("{group1}").with(conflict.getFirstGroup().getName()),
-                    replace("{group2}").with(conflict.getSecondGroup().getName()),
-                    replace("{shares}").with(conflict.getConflictingShares().toString()),
-                    replace("{worlds}").with(conflict.getWorldsString()));
-        }
-        if (!conflicts.isEmpty()) {
-            issuer.sendInfo(MVInvi18n.CONFLICT_FOUND);
+        GroupingConflictResult groupingConflictResult = checkForConflicts();
+        if (groupingConflictResult.hasConflict()) {
+            groupingConflictResult.sendConflictIssue(issuer);
         } else {
             issuer.sendInfo(MVInvi18n.CONFLICT_NOTFOUND);
         }

src/main/java/org/mvplugins/multiverse/inventories/profile/group/GroupingConflict.java

@@ -14,6 +14,11 @@ public final class GroupingConflict {
     private final WorldGroup groupTwo;
     private final Shares conflictingShares;
 
+    /**
+     * @deprecated This shouldn't have been public. Please never be instantiated directly.
+     *             Instead, you should get the result from {@link WorldGroupManager#checkForConflicts()}.
+     */
+    @Deprecated
     public GroupingConflict(WorldGroup groupOne, WorldGroup groupTwo, Shares conflictingShares) {
         this.groupOne = groupOne;
         this.groupTwo = groupTwo;
@@ -58,14 +63,6 @@ public List<String> getConflictingWorlds() {
      * @return The worlds the two groups share as a single string.
      */
     public String getWorldsString() {
-        StringBuilder builder = new StringBuilder();
-        for (String world : this.getConflictingWorlds()) {
-            if (!builder.toString().isEmpty()) {
-                builder.append(", ");
-            }
-            builder.append(world);
-        }
-        return builder.toString();
+        return String.join(", ", this.getConflictingWorlds());
     }
 }
-

src/main/java/org/mvplugins/multiverse/inventories/profile/group/GroupingConflictResult.java

@@ -0,0 +1,60 @@
+package org.mvplugins.multiverse.inventories.profile.group;
+
+import org.jetbrains.annotations.ApiStatus;
+import org.mvplugins.multiverse.core.command.MVCommandIssuer;
+import org.mvplugins.multiverse.inventories.util.MVInvi18n;
+
+import java.util.List;
+
+import static org.mvplugins.multiverse.core.locale.message.MessageReplacement.replace;
+
+/**
+ * Represents the result of checking for grouping conflicts.
+ * This class encapsulates a list of conflicts found between world groups.
+ */
+@ApiStatus.AvailableSince("5.2")
+public final class GroupingConflictResult {
+    private final List<GroupingConflict> conflicts;
+
+    GroupingConflictResult(List<GroupingConflict> conflicts) {
+        this.conflicts = conflicts;
+    }
+
+    /**
+     * Checks if there are any conflicts found.
+     *
+     * @return true if there are conflicts, false otherwise.
+     */
+    @ApiStatus.AvailableSince("5.2")
+    public boolean hasConflict() {
+        return !conflicts.isEmpty();
+    }
+
+    /**
+     * Sends message to the issuer detailing the conflicts found.
+     *
+     * @param issuer    the sender of the message
+     */
+    @ApiStatus.AvailableSince("5.2")
+    public void sendConflictIssue(MVCommandIssuer issuer) {
+        if (conflicts.isEmpty()) {
+            return;
+        }
+        conflicts.forEach(conflict -> issuer.sendInfo(MVInvi18n.CONFLICT_RESULTS,
+                replace("{group1}").with(conflict.getFirstGroup().getName()),
+                replace("{group2}").with(conflict.getSecondGroup().getName()),
+                replace("{shares}").with(conflict.getConflictingShares().toString()),
+                replace("{worlds}").with(conflict.getWorldsString())));
+        issuer.sendInfo(MVInvi18n.CONFLICT_FOUND);
+    }
+
+    /**
+     * Gets the list of conflicts found.
+     *
+     * @return a list of {@link GroupingConflict} objects representing the conflicts.
+     */
+    @ApiStatus.AvailableSince("5.2")
+    public List<GroupingConflict> getConflicts() {
+        return conflicts;
+    }
+}

src/main/java/org/mvplugins/multiverse/inventories/profile/group/WorldGroupManager.java

@@ -97,18 +97,36 @@ public sealed interface WorldGroupManager permits AbstractWorldGroupManager {
      */
     WorldGroup getDefaultGroup();
 
+    /**
+     * Checks for conflicts between groups and returns a result object containing the conflicts.
+     *
+     * @return A result object containing the conflicts found (if any).
+     *
+     * @since 5.2
+     */
+    @ApiStatus.AvailableSince("5.2")
+    GroupingConflictResult checkForConflicts();
+
     /**
      * Checks all the world groups to see if there are any potential issues.
      *
      * @return A list of all the potential conflicts.
+     *
+     * @deprecated Use {@link #checkForConflicts()} instead.
      */
+    @Deprecated(since = "5.2", forRemoval = true)
+    @ApiStatus.ScheduledForRemoval(inVersion = "6.0")
     List<GroupingConflict> checkGroups();
 
     /**
      * Runs a check for conflicts between groups and displays them to issuer or console.
      *
      * @param issuer The issuer to relay information to. If null, info only displayed in console.
+     *
+     * @deprecated Use {@link #checkForConflicts()} instead.
      */
+    @Deprecated(since = "5.2", forRemoval = true)
+    @ApiStatus.ScheduledForRemoval(inVersion = "6.0")
     void checkForConflicts(MVCommandIssuer issuer);
 
     /**

src/main/resources/multiverse-inventories_en.properties

@@ -18,15 +18,15 @@ mv-inventories.generic.off=OFF
 # Errors
 mv-inventories.error.configload=Encountered an error while loading the configuration file.  Disabling...
 mv-inventories.error.dataload=Encountered an error while loading the data file.  Disabling...
-mv-inventories.error.nogroup=&6There is no group with the name: &f{group}
-mv-inventories.error.noworld=&6There is no world with the name: &f{world}
-mv-inventories.error.noworldprofile=&6There is no world profile for the world: &f{world}
+mv-inventories.error.nogroup=&cThere is no group with the name: &f{group}
+mv-inventories.error.noworld=&cThere is no world with the name: &f{world}
+mv-inventories.error.noworldprofile=&cThere is no world profile for the world: &f{world}
 mv-inventories.error.nosharesspecified=&cYou did not specify any valid shares!
 
 # Conflicts
-mv-inventories.conflict.results=Conflict found for groups: ''{group1}'' and ''{group2}'' because they both share: ''{shares}'' for the world(s): ''{worlds}''
+mv-inventories.conflict.results=&cConflict found for groups: '&6{group1}&c' and '&6{group2}&c'! Both groups contains world(s) '&3{worlds}&c' that share(s): '&3{shares}&c'.
 mv-inventories.conflict.checking=Checking for conflicts in groups...
-mv-inventories.conflict.found=Conflicts have been found... If these are not resolved, you may experience problems with your data.
+mv-inventories.conflict.found=&cConflicts have been found... If these are not resolved, you may experience problems with your data.
 mv-inventories.conflict.notfound=No group conflicts found!
 
 # Commands

src/test/java/org/mvplugins/multiverse/inventories/InjectionTest.kt

@@ -15,7 +15,7 @@ class InjectionTest : TestWithMockBukkit() {
 
     @Test
     fun `InventoriesCommand are available as a service`() {
-        assertEquals(31, serviceLocator.getAllActiveServices(InventoriesCommand::class.java).size)
+        assertEquals(32, serviceLocator.getAllActiveServices(InventoriesCommand::class.java).size)
     }
 
     @Test