A bunch of minor code quality fixes (#1466)

Signed-off-by: Pablo Herrera <[email protected]>

core/src/main/java/tc/oc/pgm/PGMPlugin.java

@@ -458,10 +458,9 @@ private String format(LogRecord record) {
         cause = xmlErr.getCause();
         message = xmlErr.getMessage();
 
-        if (cause instanceof SyntaxException && xmlErr.getNode() != null) {
+        if (cause instanceof SyntaxException se && xmlErr.getNode() != null) {
           String value = xmlErr.getNode().getValue();
 
-          SyntaxException se = (SyntaxException) cause;
           int start = se.getStartIdx();
           if (start == -1 || start > value.length()) start = value.length();
           int end = se.getEndIdx();
@@ -510,7 +509,7 @@ private String format(
 
     private @Nullable <E> E tryException(Class<E> type, @Nullable Throwable thrown) {
       if (thrown == null) return null;
-      return type.isInstance(thrown) ? (E) thrown : tryException(type, thrown.getCause());
+      return type.isInstance(thrown) ? type.cast(thrown) : tryException(type, thrown.getCause());
     }
 
     @Override

core/src/main/java/tc/oc/pgm/action/ActionParser.java

@@ -138,7 +138,7 @@ private <B> Action<? super B> parseReference(Node node, String id, Class<B> boun
     return action;
   }
 
-  @SuppressWarnings("rawtypes, unchecked")
+  @SuppressWarnings({"rawtypes", "unchecked"})
   private void validate(
       Action<?> action, FeatureValidation<ActionDefinition<?>> validation, Node node)
       throws InvalidXMLException {

core/src/main/java/tc/oc/pgm/action/ActionScopeValidation.java

@@ -11,7 +11,14 @@ public class ActionScopeValidation implements FeatureValidation<ActionDefinition
   private static final Map<Class<?>, ActionScopeValidation> INSTANCES = new HashMap<>();
 
   public static ActionScopeValidation of(Class<?> scope) {
-    return INSTANCES.computeIfAbsent(scope, ActionScopeValidation::new);
+    ActionScopeValidation validation = INSTANCES.get(scope);
+    if (validation != null) return validation;
+    synchronized (INSTANCES) {
+      validation = INSTANCES.get(scope);
+      if (validation != null) return validation;
+      INSTANCES.put(scope, validation = new ActionScopeValidation(scope));
+      return validation;
+    }
   }
 
   private final Class<?> scope;

core/src/main/java/tc/oc/pgm/api/map/includes/MapInclude.java

@@ -14,8 +14,8 @@ public interface MapInclude {
   long getLastModified();
 
   /**
-   * Get a collection of {@link Content} which can be merged into an existing {@link Document}. If
-   * the underlying file has changed, it will re-load the xml.
+   * Get a collection of {@link Content} which can be merged into an existing
+   * {@link org.jdom2.Document}. If the underlying file has changed, it will re-load the xml.
    *
    * @return a collection of {@link Content}
    */

core/src/main/java/tc/oc/pgm/command/CycleCommand.java

@@ -46,7 +46,7 @@ public void cycle(
         throw exception("map.setNext.notDev");
       }
       mapOrder.setNextMap(map);
-      MapOrderCommand.sendSetNextMessage(map, sender, match);
+      MapOrderCommand.sendSetNextMessage(map, sender);
     }
 
     match.needModule(CycleMatchModule.class).startCountdown(duration);

core/src/main/java/tc/oc/pgm/command/MapCommand.java

@@ -88,13 +88,9 @@ public void maps(
     search = search.filter(map -> chosenPhases.contains(map.getPhase()));
 
     if (author != null) {
-      String query = author;
-
-      List<MapInfo> collect = search.collect(Collectors.toList());
-      boolean exactMatch =
-          collect.stream().anyMatch((suggestion) -> matchesAuthor(suggestion, query, true));
-      search = collect.stream()
-          .filter((mapSuggestion -> matchesAuthor(mapSuggestion, query, exactMatch)));
+      List<MapInfo> collect = search.toList();
+      boolean exactMatch = collect.stream().anyMatch(map -> matchesAuthor(map, author, true));
+      search = collect.stream().filter(map -> matchesAuthor(map, author, exactMatch));
     }
 
     Set<MapInfo> maps = search.collect(Collectors.toCollection(TreeSet::new));

core/src/main/java/tc/oc/pgm/command/MapOrderCommand.java

@@ -5,6 +5,7 @@
 import static tc.oc.pgm.api.Permissions.DEV;
 import static tc.oc.pgm.api.map.Phase.DEVELOPMENT;
 import static tc.oc.pgm.command.util.ParserConstants.CURRENT;
+import static tc.oc.pgm.util.player.PlayerComponent.player;
 import static tc.oc.pgm.util.text.TextException.exception;
 
 import net.kyori.adventure.text.Component;
@@ -22,11 +23,9 @@
 import tc.oc.pgm.api.Permissions;
 import tc.oc.pgm.api.map.MapInfo;
 import tc.oc.pgm.api.map.MapOrder;
-import tc.oc.pgm.api.match.Match;
 import tc.oc.pgm.channels.ChatManager;
 import tc.oc.pgm.restart.RestartManager;
 import tc.oc.pgm.util.Audience;
-import tc.oc.pgm.util.UsernameFormatUtils;
 import tc.oc.pgm.util.named.MapNameStyle;
 
 public final class MapOrderCommand {
@@ -51,7 +50,6 @@ public void setNext(
       Audience viewer,
       CommandSender sender,
       MapOrder mapOrder,
-      Match match,
       @Flag(value = "force", aliases = "f") boolean force,
       @Flag(value = "reset", aliases = "r") boolean reset,
       @Argument("map") @Default(CURRENT) @FlagYielding MapInfo map) {
@@ -69,11 +67,8 @@ public void setNext(
       if (mapOrder.getNextMap() != null) {
         Component mapName = mapOrder.getNextMap().getStyledName(MapNameStyle.COLOR);
         mapOrder.setNextMap(null);
-        ChatManager.broadcastAdminMessage(translatable(
-            "map.setNext.revert",
-            NamedTextColor.GRAY,
-            UsernameFormatUtils.formatStaffName(sender, match),
-            mapName));
+        ChatManager.broadcastAdminMessage(
+            translatable("map.setNext.revert", NamedTextColor.GRAY, player(sender), mapName));
       } else {
         viewer.sendWarning(translatable("map.noNextMap"));
       }
@@ -87,15 +82,12 @@ public void setNext(
       viewer.sendWarning(translatable("admin.cancelRestart.restartUnqueued", NamedTextColor.GREEN));
     }
 
-    sendSetNextMessage(map, sender, match);
+    sendSetNextMessage(map, sender);
   }
 
-  public static void sendSetNextMessage(@NotNull MapInfo map, CommandSender sender, Match match) {
+  public static void sendSetNextMessage(@NotNull MapInfo map, CommandSender sender) {
     Component mapName = text(map.getName(), NamedTextColor.GOLD);
-    ChatManager.broadcastAdminMessage(translatable(
-        "map.setNext",
-        NamedTextColor.GRAY,
-        UsernameFormatUtils.formatStaffName(sender, match),
-        mapName));
+    ChatManager.broadcastAdminMessage(
+        translatable("map.setNext", NamedTextColor.GRAY, player(sender), mapName));
   }
 }

core/src/main/java/tc/oc/pgm/command/VotingCommand.java

@@ -6,6 +6,7 @@
 import static net.kyori.adventure.text.event.HoverEvent.showText;
 import static tc.oc.pgm.api.Permissions.DEV;
 import static tc.oc.pgm.api.map.Phase.DEVELOPMENT;
+import static tc.oc.pgm.util.player.PlayerComponent.player;
 import static tc.oc.pgm.util.text.TextException.exception;
 
 import java.util.List;
@@ -23,14 +24,12 @@
 import tc.oc.pgm.api.Permissions;
 import tc.oc.pgm.api.map.MapInfo;
 import tc.oc.pgm.api.map.MapOrder;
-import tc.oc.pgm.api.match.Match;
 import tc.oc.pgm.channels.ChatManager;
 import tc.oc.pgm.rotation.MapPoolManager;
 import tc.oc.pgm.rotation.pools.VotingPool;
 import tc.oc.pgm.rotation.vote.MapVotePicker;
 import tc.oc.pgm.rotation.vote.VotePoolOptions;
 import tc.oc.pgm.util.Audience;
-import tc.oc.pgm.util.UsernameFormatUtils;
 import tc.oc.pgm.util.named.MapNameStyle;
 import tc.oc.pgm.util.text.TextFormatter;
 
@@ -44,7 +43,6 @@ public void addMap(
       Audience viewer,
       CommandSender sender,
       MapOrder mapOrder,
-      Match match,
       @Argument("map") @Greedy MapInfo map) {
     if (PGM.get().getConfiguration().enforceDevPhase()
         && DEVELOPMENT.equals(map.getPhase())
@@ -55,10 +53,7 @@ public void addMap(
     VotePoolOptions vote = getVoteOptions(mapOrder);
 
     Component addMessage = translatable(
-        "vote.add",
-        NamedTextColor.GRAY,
-        UsernameFormatUtils.formatStaffName(sender, match),
-        map.getStyledName(MapNameStyle.COLOR));
+        "vote.add", NamedTextColor.GRAY, player(sender), map.getStyledName(MapNameStyle.COLOR));
 
     if (vote.isMapAdded(map)) {
       viewer.sendWarning(addMessage);
@@ -79,14 +74,13 @@ public void removeMap(
       Audience viewer,
       CommandSender sender,
       MapOrder mapOrder,
-      Match match,
       @Argument("map") @Greedy MapInfo map) {
     VotePoolOptions vote = getVoteOptions(mapOrder);
     if (vote.removeMap(map)) {
       ChatManager.broadcastAdminMessage(translatable(
           "vote.remove",
           NamedTextColor.GRAY,
-          UsernameFormatUtils.formatStaffName(sender, match),
+          player(sender),
           map.getStyledName(MapNameStyle.COLOR)));
     } else {
       viewer.sendWarning(translatable("map.notFound"));
@@ -96,21 +90,18 @@ public void removeMap(
   @Command("mode")
   @CommandDescription("Toggle the voting mode between replace and override")
   @Permission(Permissions.SETNEXT)
-  public void mode(CommandSender sender, MapOrder mapOrder, Match match) {
+  public void mode(CommandSender sender, MapOrder mapOrder) {
     VotePoolOptions vote = getVoteOptions(mapOrder);
     Component voteModeName = translatable(
         vote.toggleMode() ? "vote.mode.replace" : "vote.mode.create", NamedTextColor.LIGHT_PURPLE);
-    ChatManager.broadcastAdminMessage(translatable(
-        "vote.toggle",
-        NamedTextColor.GRAY,
-        UsernameFormatUtils.formatStaffName(sender, match),
-        voteModeName));
+    ChatManager.broadcastAdminMessage(
+        translatable("vote.toggle", NamedTextColor.GRAY, player(sender), voteModeName));
   }
 
   @Command("clear")
   @CommandDescription("Clear all custom map selections from the next vote")
   @Permission(Permissions.SETNEXT)
-  public void clearMaps(Audience viewer, CommandSender sender, Match match, MapOrder mapOrder) {
+  public void clearMaps(Audience viewer, CommandSender sender, MapOrder mapOrder) {
     VotePoolOptions vote = getVoteOptions(mapOrder);
 
     List<Component> maps = vote.getCustomVoteMaps().stream()
@@ -119,7 +110,7 @@ public void clearMaps(Audience viewer, CommandSender sender, Match match, MapOrd
     Component clearedMsg = translatable(
         "vote.remove",
         NamedTextColor.GRAY,
-        UsernameFormatUtils.formatStaffName(sender, match),
+        player(sender),
         TextFormatter.list(maps, NamedTextColor.GRAY));
 
     vote.clearMaps();
@@ -177,18 +168,9 @@ public void listMaps(Audience viewer, MapOrder mapOrder) {
   }
 
   public static VotePoolOptions getVoteOptions(MapOrder mapOrder) {
-    if (mapOrder instanceof MapPoolManager) {
-      MapPoolManager manager = (MapPoolManager) mapOrder;
-      if (manager.getActiveMapPool() instanceof VotingPool) {
-        VotingPool votePool = (VotingPool) manager.getActiveMapPool();
-        if (votePool.getCurrentPoll() != null) {
-          throw exception("vote.modify.disallow");
-        }
-        return manager.getVoteOptions();
-      }
-      throw exception("vote.disabled");
-    }
-
-    throw exception("pool.mapPoolsDisabled");
+    if (!(mapOrder instanceof MapPoolManager manager)) throw exception("pool.mapPoolsDisabled");
+    if (!(manager.getActiveMapPool() instanceof VotingPool pool)) throw exception("vote.disabled");
+    if (pool.getCurrentPoll() != null) throw exception("vote.modify.disallow");
+    return manager.getVoteOptions();
   }
 }

core/src/main/java/tc/oc/pgm/death/DeathMessageBuilder.java

@@ -111,11 +111,7 @@ boolean exists(String prefix) {
 
   /** Return a new key built from the current key with the given tokens appended */
   String append(String... tokens) {
-    String newKey = key;
-    for (String token : tokens) {
-      newKey += '.' + token;
-    }
-    return newKey;
+    return key + (tokens.length == 0 ? "" : '.' + String.join(".", tokens));
   }
 
   /**

core/src/main/java/tc/oc/pgm/destroyable/Destroyable.java

@@ -382,11 +382,6 @@ public DestroyableHealthChange handleBlockChange(
     return changeInfo;
   }
 
-  @Override
-  protected void playTouchEffects(ParticipantState toucher) {
-    // We make our own touch sounds
-  }
-
   /**
    * Test if the given block change is allowed by this Destroyable
    *

core/src/main/java/tc/oc/pgm/doublejump/DoubleJumpMatchModule.java

@@ -31,10 +31,10 @@ public class DoubleJumpMatchModule implements MatchModule, Listener, Tickable {
   private static final Sound ZOMBIE_INFECT =
       parse(Sound::valueOf, "ZOMBIE_INFECT", "ENTITY_ZOMBIE_INFECT");
 
-  private class Jumper {
-    final Player player;
-    final DoubleJumpKit kit;
-    float charge;
+  private static class Jumper {
+    private final Player player;
+    private final DoubleJumpKit kit;
+    private float charge;
 
     private Jumper(Player player, DoubleJumpKit kit) {
       this.player = player;

core/src/main/java/tc/oc/pgm/ffa/FreeForAllMatchModule.java

@@ -53,13 +53,7 @@ public class FreeForAllMatchModule implements MatchModule, Listener, JoinHandler
     ChatColor.DARK_RED
   };
 
-  class NeedMorePlayers implements UnreadyReason {
-    final int players;
-
-    NeedMorePlayers(int players) {
-      this.players = players;
-    }
-
+  record NeedMorePlayers(int players) implements UnreadyReason {
     @Override
     public Component getReason() {
       if (players == 1) {
@@ -73,13 +67,7 @@ public Component getReason() {
     public boolean canForceStart() {
       return true;
     }
-
-    @Override
-    public String toString() {
-      return getClass().getSimpleName() + "{players=" + players + "}";
-    }
   }
-  ;
 
   private final Match match;
   private final FreeForAllOptions options;