Improvements to arena joining

- Competitions with players are prioritized now when running /<arena> join without specifying a map
- Added option to config to allow for /<arena> join without a map to randomly select a map
- Added a config updater to automatically include new config options to old configs whenever they are added

plugin/src/main/java/org/battleplugins/arena/BattleArena.java

@@ -79,7 +79,9 @@ public class BattleArena extends JavaPlugin implements LoggerHolder {
 
     private Path arenasPath;
 
-    private boolean debugMode;
+    // Set to true before config is loaded in the event that the config
+    // fails to load and the additional debug information is required
+    private boolean debugMode = true;
 
     @Override
     public void onLoad() {

plugin/src/main/java/org/battleplugins/arena/BattleArenaConfig.java

@@ -2,13 +2,17 @@
 
 import org.battleplugins.arena.competition.event.EventOptions;
 import org.battleplugins.arena.config.ArenaOption;
+import org.battleplugins.arena.config.Updater;
+import org.battleplugins.arena.config.updater.ConfigUpdater;
+import org.battleplugins.arena.config.updater.UpdaterStep;
 
 import java.util.List;
 import java.util.Map;
 
 /**
  * Represents the BattleArena configuration.
  */
+@Updater(BattleArenaConfig.Updater.class)
 public class BattleArenaConfig {
 
     @ArenaOption(name = "config-version", description = "The version of the config.", required = true)
@@ -23,6 +27,9 @@ public class BattleArenaConfig {
     @ArenaOption(name = "max-dynamic-maps", description = "The maximum number of dynamic maps an Arena can have allocated at once.", required = true)
     private int maxDynamicMaps;
 
+    @ArenaOption(name = "randomized-arena-join", description = "Whether players should be randomly placed in an Arena when joining without specifying a map.", required = true)
+    private boolean randomizedArenaJoin;
+
     @ArenaOption(name = "disabled-modules", description = "Modules that are disabled by default.")
     private List<String> disabledModules;
 
@@ -48,6 +55,10 @@ public int getMaxDynamicMaps() {
         return this.maxDynamicMaps;
     }
 
+    public boolean isRandomizedArenaJoin() {
+        return this.randomizedArenaJoin;
+    }
+
     public List<String> getDisabledModules() {
         return this.disabledModules == null ? List.of() : List.copyOf(this.disabledModules);
     }
@@ -59,4 +70,21 @@ public Map<String, List<EventOptions>> getEvents() {
     public boolean isDebugMode() {
         return this.debugMode;
     }
+
+    public static class Updater implements ConfigUpdater<BattleArenaConfig> {
+
+        @Override
+        public Map<String, UpdaterStep<BattleArenaConfig>> buildUpdaters() {
+            return Map.of(
+                    "3.1", (config, instance) -> {
+                        config.set("randomized-arena-join", false);
+                        config.setComments("randomized-arena-join", List.of(
+                                "Whether joining an arena using /<arena> join without specifying a map should",
+                                "randomly pick an arena, rather than joining the most convenient one. Competitions",
+                                "with players waiting will always be prioritized though, even with this setting",
+                                "enabled."
+                        ));
+                    });
+        }
+    }
 }

plugin/src/main/java/org/battleplugins/arena/command/ArenaCommandExecutor.java

@@ -10,6 +10,7 @@
 import org.battleplugins.arena.competition.PlayerRole;
 import org.battleplugins.arena.competition.map.CompetitionMap;
 import org.battleplugins.arena.competition.map.LiveCompetitionMap;
+import org.battleplugins.arena.competition.map.MapType;
 import org.battleplugins.arena.competition.phase.CompetitionPhase;
 import org.battleplugins.arena.competition.phase.CompetitionPhaseType;
 import org.battleplugins.arena.competition.phase.PhaseManager;
@@ -26,6 +27,7 @@
 import org.bukkit.Bukkit;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
+import org.jetbrains.annotations.Nullable;
 
 import java.util.HashMap;
 import java.util.List;
@@ -34,6 +36,19 @@
 import java.util.Set;
 
 public class ArenaCommandExecutor extends BaseCommandExecutor {
+    private static final CompetitionMap RANDOM_MAP_MARKER = new CompetitionMap() {
+
+        @Override
+        public String getName() {
+            return "marker";
+        }
+
+        @Override
+        public MapType getType() {
+            return MapType.STATIC;
+        }
+    };
+
     protected final Arena arena;
 
     public ArenaCommandExecutor(Arena arena) {
@@ -48,13 +63,7 @@ public ArenaCommandExecutor(String parentCommand, Arena arena) {
 
     @ArenaCommand(commands = { "join", "j" }, description = "Join an arena.", permissionNode = "join")
     public void join(Player player) {
-        List<LiveCompetitionMap> maps = this.arena.getPlugin().getMaps(this.arena);
-        if (maps.isEmpty()) {
-            Messages.NO_OPEN_ARENAS.send(player);
-            return;
-        }
-
-        this.join(player, maps.iterator().next());
+        this.join(player, RANDOM_MAP_MARKER);
     }
 
     @ArenaCommand(commands = { "join", "j" }, description = "Join an arena.", permissionNode = "join.map")
@@ -69,7 +78,7 @@ public void join(Player player, @Argument(name = "map") CompetitionMap map) {
             return;
         }
 
-        List<Competition<?>> competitions = this.arena.getPlugin().getCompetitions(this.arena, map.getName());
+        List<Competition<?>> competitions = map == RANDOM_MAP_MARKER ? this.arena.getPlugin().getCompetitions(this.arena) : this.arena.getPlugin().getCompetitions(this.arena, map.getName());
         this.arena.getPlugin().findJoinableCompetition(competitions, player, PlayerRole.PLAYING).whenCompleteAsync((result, e) -> {
             if (e != null) {
                 Messages.ARENA_ERROR.send(player, e.getMessage());
@@ -83,6 +92,11 @@ public void join(Player player, @Argument(name = "map") CompetitionMap map) {
 
                 Messages.ARENA_JOINED.send(player, competition.getMap().getName());
             } else {
+                if (map == RANDOM_MAP_MARKER) {
+                    Messages.NO_OPEN_ARENAS.send(player);
+                    return;
+                }
+
                 // Try and create a dynamic competition if possible
                 this.arena.getPlugin()
                         .getOrCreateCompetition(this.arena, player, PlayerRole.PLAYING, map.getName())

plugin/src/main/java/org/battleplugins/arena/competition/CompetitionManager.java

@@ -120,12 +120,21 @@ public CompletableFuture<CompetitionResult> findJoinableCompetition(List<Competi
         return this.findJoinableCompetition(competitions, player, role, null);
     }
 
-    public CompletableFuture<CompetitionResult> findJoinableCompetition(List<Competition<?>> competitions, Player player, PlayerRole role, @Nullable JoinResult lastResult) {
+    private CompletableFuture<CompetitionResult> findJoinableCompetition(List<Competition<?>> competitions, Player player, PlayerRole role, @Nullable JoinResult lastResult) {
         if (competitions.isEmpty()) {
             return CompletableFuture.completedFuture(new CompetitionResult(null, lastResult == null ? JoinResult.NOT_JOINABLE : lastResult));
         }
 
-        Competition<?> competition = competitions.get(0);
+        if (this.plugin.getMainConfig().isRandomizedArenaJoin()) {
+            competitions = new ArrayList<>(competitions);
+            Collections.shuffle(competitions);
+        }
+
+        // Select the competition with the most number of players
+        Competition<?> competition = competitions.stream()
+                .max(Comparator.comparingInt(Competition::getAlivePlayerCount))
+                .orElse(null);
+
         CompletableFuture<JoinResult> result = competition.canJoin(player, role);
         JoinResult joinResult = result.join();
         if (joinResult == JoinResult.SUCCESS) {

plugin/src/main/java/org/battleplugins/arena/config/ArenaConfigParser.java

@@ -1,18 +1,25 @@
 package org.battleplugins.arena.config;
 
 import org.apache.commons.lang3.reflect.FieldUtils;
+import org.battleplugins.arena.BattleArena;
 import org.battleplugins.arena.config.context.ContextProvider;
+import org.battleplugins.arena.config.updater.ConfigUpdater;
+import org.battleplugins.arena.config.updater.UpdaterStep;
+import org.battleplugins.arena.util.Version;
 import org.bukkit.configuration.ConfigurationSection;
 import org.bukkit.configuration.MemoryConfiguration;
+import org.bukkit.configuration.file.FileConfiguration;
 import org.jetbrains.annotations.Nullable;
 
+import java.io.IOException;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
@@ -44,6 +51,8 @@ public static <T> T newInstance(Class<T> type, ConfigurationSection configuratio
 
     public static <T> T newInstance(@Nullable Path sourceFile, Class<T> type, ConfigurationSection configuration, @Nullable Object scope, @Nullable Object id) throws ParseException {
         T instance = newClassInstance(sourceFile, type);
+        updateConfig(type, configuration, instance, sourceFile);
+
         try {
             populateFields(sourceFile, instance, configuration, scope, id);
             if (instance instanceof PostProcessable postProcessable) {
@@ -530,6 +539,67 @@ private static List<ConfigurationSection> toMemorySections(List<?> list) {
         return sections;
     }
 
+    @SuppressWarnings("unchecked")
+    private static <T> void updateConfig(Class<T> type, ConfigurationSection configuration, T instance, @Nullable Path sourceFile) throws ParseException {
+        // Check if the config has an updater
+        if (!type.isAnnotationPresent(Updater.class)) {
+            return;
+        }
+
+        Updater updater = type.getDeclaredAnnotation(Updater.class);
+        try {
+            // Build the updaters and update the config if applicable
+            ConfigUpdater<T> configUpdater = (ConfigUpdater<T>) updater.value().getConstructor().newInstance();
+            Map<String, UpdaterStep<T>> updaters = configUpdater.buildUpdaters();
+            List<UpdaterEntry<T>> entries = updaters.entrySet().stream()
+                    .map(entry -> new UpdaterEntry<>(Version.of(entry.getKey()), entry.getValue()))
+                    .sorted(Comparator.comparing(UpdaterEntry::version))
+                    .toList();
+
+            // No entries - no need to update
+            if (entries.isEmpty()) {
+                return;
+            }
+
+            Version configVersion = Version.of(configuration.getString("config-version"));
+            Version latestVersion = entries.get(entries.size() - 1).version();
+
+            // If the config version is equal to (or greater?) than the latest version, we don't need to update
+            if (!configVersion.isGreaterThanOrEqualTo(latestVersion)) {
+                // Otherwise, we need to update
+                boolean updatersRan = false;
+                for (UpdaterEntry<T> entry : entries) {
+                    if (configVersion.isLessThan(entry.version())) {
+                        entry.step().update(configuration, instance);
+
+                        // Set the new config version
+                        configuration.set("config-version", entry.version().toString());
+
+                        BattleArena.getInstance().info("Updated config {} to version {}", type.getSimpleName(), entry.version());
+                        updatersRan = true;
+                    }
+                }
+
+                if (updatersRan) {
+                    // Save config
+                    if (configuration instanceof FileConfiguration fileConfig) {
+                        try {
+                            fileConfig.save(sourceFile.toFile());
+                        } catch (IOException e) {
+                            throw new ParseException("Failed to save configuration file " + sourceFile + " after processing updates", e)
+                                    .cause(ParseException.Cause.INTERNAL_ERROR)
+                                    .sourceFile(sourceFile);
+                        }
+                    }
+                }
+            }
+        } catch (InstantiationException | IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
+            throw new ParseException("Failed to instantiate updater for class " + type.getName(), e)
+                    .cause(ParseException.Cause.INTERNAL_ERROR)
+                    .sourceFile(sourceFile);
+        }
+    }
+
     private static ConfigurationSection toMemorySection(Map<String, Object> map) {
         MemoryConfiguration memoryConfig = new MemoryConfiguration();
         memoryConfig.addDefaults(map);
@@ -539,4 +609,6 @@ private static ConfigurationSection toMemorySection(Map<String, Object> map) {
     public interface Parser<T> {
         T parse(Object object) throws ParseException;
     }
+
+    private record UpdaterEntry<T>(Version version, UpdaterStep<T> step) {}
 }

plugin/src/main/java/org/battleplugins/arena/config/Updater.java

@@ -0,0 +1,25 @@
+package org.battleplugins.arena.config;
+
+import org.battleplugins.arena.config.updater.ConfigUpdater;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * An annotation used over a configurable class to specify
+ * which updater should be used to update the configuration
+ * across versions.
+ */
+@Target(ElementType.TYPE)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Updater {
+
+    /**
+     * The class of the updater.
+     *
+     * @return the class of the updater
+     */
+    Class<? extends ConfigUpdater<?>> value();
+}

plugin/src/main/java/org/battleplugins/arena/config/updater/ConfigUpdater.java

@@ -0,0 +1,20 @@
+package org.battleplugins.arena.config.updater;
+
+import java.util.Map;
+
+/**
+ * An updater for updating a config file across versions.
+ */
+public interface ConfigUpdater<T> {
+
+    /**
+     * Builds the updater for the config file.
+     * <p>
+     * The key of the map will be the version to update
+     * to, and the value will be the updater step to update
+     * the config file.
+     *
+     * @return the updater for the config file
+     */
+    Map<String, UpdaterStep<T>> buildUpdaters();
+}

plugin/src/main/java/org/battleplugins/arena/config/updater/UpdaterStep.java

@@ -0,0 +1,17 @@
+package org.battleplugins.arena.config.updater;
+
+import org.bukkit.configuration.ConfigurationSection;
+
+/**
+ * An updater step for updating a config file across versions.
+ */
+public interface UpdaterStep<T> {
+
+    /**
+     * Updates the config file to the specified version.
+     *
+     * @param config the config
+     * @param instance the config instance
+     */
+    void update(ConfigurationSection config, T instance);
+}

plugin/src/main/java/org/battleplugins/arena/util/Version.java

@@ -28,7 +28,7 @@ public class Version implements Comparable<Version> {
 
     private Version(String version) {
         this.version = version;
-        this.tester = () -> Bukkit.getPluginManager().isPluginEnabled("BattleArena");
+        this.tester = () -> true;
     }
 
     private Version(Plugin plugin) {
@@ -319,4 +319,4 @@ public static Version of(Plugin plugin) {
     public static Version getServerVersion() {
         return SERVER_VERSION;
     }
-}
+}

plugin/src/main/resources/config.yml

@@ -5,7 +5,7 @@
 # Support: https://discord.gg/tMVPVJf
 # GitHub: https://github.com/BattlePlugins/BattleArena
 # -----------------
-config-version: 3.0 # The config version, do not change!
+config-version: 3.1 # The config version, do not change!
 
 # Whether player inventories should be backed up when joining competitions.
 backup-inventories: true
@@ -17,6 +17,12 @@ max-backups: 5
 # Set to -1 to disable this limit.
 max-dynamic-maps: 5
 
+# Whether joining an arena using /<arena> join without specifying a map should
+# randomly pick an arena, rather than joining the most convenient one. Competitions
+# with players waiting will always be prioritized though, even with this setting
+# enabled.
+randomized-arena-join: false
+
 # Modules that are disabled by default. BattleArena comes pre-installed with
 # multiple modules that can be disabled below if their behavior is not desired
 disabled-modules: []