Logger Improvements (#1399)

* Null checks & using getOptions

* Removed useless variable, and made HashMap's final

* This list & Map should not be modified.
Plus small changes

* Small things

* Fix #1398

* Apply suggestions from code review

* Remove Dead call

* Handle options being null

* Remove redundant field explainers

Those were there for when they were intermediary fields, they now have a proper name

src/main/java/carpet/logging/HUDController.java

@@ -29,7 +29,7 @@ import java.util.function.Consumer;
 
 public class HUDController
 {
-    private static List<Consumer<MinecraftServer>> HUDListeners = new ArrayList<>();
+    private static final List<Consumer<MinecraftServer>> HUDListeners = new ArrayList<>();
 
     /**
      * Adds listener to be called when HUD is updated for logging information
@@ -40,7 +40,7 @@ public class HUDController
         HUDListeners.add(listener);
     }
 
-    public static Map<ServerPlayer, List<Component>> player_huds = new HashMap<>();
+    public static final Map<ServerPlayer, List<Component>> player_huds = new HashMap<>();
 //keyed with player names so unlogged players don't hold the reference
     public static final Map<String, Component> scarpet_headers = new HashMap<>();
 
@@ -91,19 +91,12 @@ public class HUDController
 
         if (LoggerRegistry.__mobcaps)
             LoggerRegistry.getLogger("mobcaps").log((option, player) -> {
-                ResourceKey<Level> dim = player.level.dimension(); //getDimType
-                switch (option)
-                {
-                    case "overworld":
-                        dim = Level.OVERWORLD; // OW
-                        break;
-                    case "nether":
-                        dim = Level.NETHER; // nether
-                        break;
-                    case "end":
-                        dim = Level.END; // end
-                        break;
-                }
+                ResourceKey<Level> dim = switch (option) {
+                    case "overworld" -> Level.OVERWORLD;
+                    case "nether" -> Level.NETHER;
+                    case "end" -> Level.END;
+                    default -> player.level.dimension();
+                };
                 return new Component[]{SpawnReporter.printMobcapsForDimension(server.getLevel(dim), false).get(0)};
             });
 
@@ -111,7 +104,7 @@ public class HUDController
             LoggerRegistry.getLogger("counter").log((option)->send_counter_info(server, option));
 
         if (LoggerRegistry.__packets)
-            LoggerRegistry.getLogger("packets").log(()-> packetCounter());
+            LoggerRegistry.getLogger("packets").log(HUDController::packetCounter);
 
         // extensions have time to pitch in.
         HUDListeners.forEach(l -> l.accept(server));

src/main/java/carpet/logging/Logger.java

@@ -62,7 +62,7 @@ public class Logger
         this.acceleratorField = acceleratorField;
         this.logName = logName;
         this.default_option = def;
-        this.options = options;
+        this.options = options == null ? new String[0] : options;
         this.strictOptions = strictOptions;
         if (acceleratorField == null)
             CarpetSettings.LOG.error("[CM] Logger "+getLogName()+" is missing a specified accelerator");
@@ -74,10 +74,6 @@ public class Logger
     }
     public String [] getOptions()
     {
-        if (options == null)
-        {
-            return new String[0];
-        }
         return options;
     }
     public String getLogName()
@@ -219,9 +215,13 @@ public class Logger
         else if(firstTime)
         {
             Set<String> loggingOptions = new HashSet<>(Arrays.asList(CarpetSettings.defaultLoggers.split(",")));
-            if (loggingOptions.contains(getLogName()))
-            {
-                LoggerRegistry.subscribePlayer(playerName, getLogName(), getDefault());
+            String logName = getLogName();
+            for (String str : loggingOptions) {
+                String[] vars = str.split(" ", 2);
+                if (vars[0].equals(logName)) {
+                    LoggerRegistry.subscribePlayer(playerName, getLogName(), vars.length == 1 ? getDefault() : vars[1]);
+                    break;
+                }
             }
         }
         LoggerRegistry.setAccess(this);
@@ -241,15 +241,15 @@ public class Logger
 
     public String getAcceptedOption(String arg)
     {
-        if (options != null && Arrays.asList(options).contains(arg)) return arg;
+        if (Arrays.asList(this.getOptions()).contains(arg)) return arg;
         return null;
     }
 
     public boolean isOptionValid(String option) {
         if (strictOptions)
         {
-            return Arrays.asList(this.options).contains(option);
+            return Arrays.asList(this.getOptions()).contains(option);
         }
-        return true;
+        return option != null;
     }
 }

src/main/java/carpet/logging/LoggerRegistry.java

@@ -14,9 +14,9 @@ import net.minecraft.world.item.DyeColor;
 public class LoggerRegistry
 {
     // Map from logger names to loggers.
-    private static Map<String, Logger> loggerRegistry = new HashMap<>();
+    private static final Map<String, Logger> loggerRegistry = new HashMap<>();
     // Map from player names to the set of names of the logs that player is subscribed to.
-    private static Map<String, Map<String, String>> playerSubscriptions = new HashMap<>();
+    private static final Map<String, Map<String, String>> playerSubscriptions = new HashMap<>();
     //statics to quickly asses if its worth even to call each one
     public static boolean __tnt;
     public static boolean __projectiles;
@@ -116,7 +116,6 @@ public class LoggerRegistry
 
     protected static void setAccess(Logger logger)
     {
-        String name = logger.getLogName();
         boolean value = logger.hasOnlineSubscribers();
         try
         {
@@ -137,7 +136,7 @@ public class LoggerRegistry
         setAccess(logger);
     }
 
-    private static Set<String> seenPlayers = new HashSet<>();
+    private final static Set<String> seenPlayers = new HashSet<>();
 
     public static void stopLoggers()
     {

src/main/java/carpet/logging/logHelpers/ExplosionLogHelper.java

@@ -23,7 +23,7 @@ public class ExplosionLogHelper
     public final Entity entity;
     private final float power;
     private boolean affectBlocks = false;
-    private Object2IntMap<EntityChangedStatusWithCount> impactedEntities = new Object2IntOpenHashMap<>();
+    private final Object2IntMap<EntityChangedStatusWithCount> impactedEntities = new Object2IntOpenHashMap<>();
 
     private static long lastGametime = 0;
     private static int explosionCountInCurretGT = 0;

src/main/java/carpet/logging/logHelpers/PacketCounter.java

@@ -4,5 +4,5 @@ public class PacketCounter
 {
     public static long totalOut=0;
     public static long totalIn=0;
-    public static void reset() {totalIn = 0l; totalOut = 0L; }
+    public static void reset() {totalIn = 0L; totalOut = 0L; }
 }

src/main/java/carpet/logging/logHelpers/TNTLogHelper.java

@@ -33,24 +33,19 @@ public class TNTLogHelper
             lastGametime = gametime;
         }
         tntCount++;
-        LoggerRegistry.getLogger("tnt").log( (option) -> {
-            switch (option)
-            {
-                case "brief":
-                    return new Component[]{Messenger.c(
-                            "l P ",Messenger.dblt("l",primedX,primedY,primedZ),
-                            "w  ",Messenger.dblt("l", primedAngle.x, primedAngle.y, primedAngle.z),
-                            "r  E ",Messenger.dblt("r",x, y, z))};
-                case "full":
-                    return new Component[]{Messenger.c(
-                            "r #" + tntCount,
-                            "m @" + gametime,
-                            "g : ",
-                            "l P ",Messenger.dblf("l",primedX,primedY,primedZ),
-                            "w  ",Messenger.dblf("l", primedAngle.x, primedAngle.y, primedAngle.z),
-                            "r  E ",Messenger.dblf("r",x, y, z))};
-            }
-            return null;
+        LoggerRegistry.getLogger("tnt").log( (option) -> switch (option) {
+            case "brief" -> new Component[]{Messenger.c(
+                    "l P ", Messenger.dblt("l", primedX, primedY, primedZ),
+                    "w  ", Messenger.dblt("l", primedAngle.x, primedAngle.y, primedAngle.z),
+                    "r  E ", Messenger.dblt("r", x, y, z))};
+            case "full" -> new Component[]{Messenger.c(
+                    "r #" + tntCount,
+                    "m @" + gametime,
+                    "g : ",
+                    "l P ", Messenger.dblf("l", primedX, primedY, primedZ),
+                    "w  ", Messenger.dblf("l", primedAngle.x, primedAngle.y, primedAngle.z),
+                    "r  E ", Messenger.dblf("r", x, y, z))};
+            default -> null;
         });
     }
 }

src/main/java/carpet/logging/logHelpers/TrajectoryLogHelper.java

@@ -17,10 +17,10 @@ public class TrajectoryLogHelper
     private static final int MAX_TICKS_PER_LINE = 20;
 
     private boolean doLog;
-    private Logger logger;
+    private final Logger logger;
 
-    private ArrayList<Vec3> positions = new ArrayList<>();
-    private ArrayList<Vec3> motions = new ArrayList<>();
+    private final ArrayList<Vec3> positions = new ArrayList<>();
+    private final ArrayList<Vec3> motions = new ArrayList<>();
 
     public TrajectoryLogHelper(String logName)
     {
@@ -40,37 +40,32 @@ public class TrajectoryLogHelper
         if (!doLog) return;
         logger.log( (option) -> {
             List<Component> comp = new ArrayList<>();
-            switch (option)
-            {
-                case "brief":
+            switch (option) {
+                case "brief" -> {
                     comp.add(Messenger.s(""));
                     List<String> line = new ArrayList<>();
-
-                    for (int i = 0; i < positions.size(); i++)
-                    {
+                    for (int i = 0; i < positions.size(); i++) {
                         Vec3 pos = positions.get(i);
                         Vec3 mot = motions.get(i);
                         line.add("w  x");
                         line.add(String.format("^w Tick: %d\nx: %f\ny: %f\nz: %f\n------------\nmx: %f\nmy: %f\nmz: %f",
                                 i, pos.x, pos.y, pos.z, mot.x, mot.y, mot.z));
-                        if ((((i+1) % MAX_TICKS_PER_LINE)==0) || i == positions.size()-1)
-                        {
+                        if ((((i + 1) % MAX_TICKS_PER_LINE) == 0) || i == positions.size() - 1) {
                             comp.add(Messenger.c(line.toArray(new Object[0])));
                             line.clear();
                         }
                     }
-                    break;
-                case "full":
+                }
+                case "full" -> {
                     comp.add(Messenger.c("w ---------"));
-                    for (int i = 0; i < positions.size(); i++)
-                    {
+                    for (int i = 0; i < positions.size(); i++) {
                         Vec3 pos = positions.get(i);
                         Vec3 mot = motions.get(i);
                         comp.add(Messenger.c(
-                                String.format("w tick: %3d pos",i),Messenger.dblt("w",pos.x, pos.y, pos.z),
-                                "w   mot",Messenger.dblt("w",mot.x, mot.y, mot.z)));
+                                String.format("w tick: %3d pos", i), Messenger.dblt("w", pos.x, pos.y, pos.z),
+                                "w   mot", Messenger.dblt("w", mot.x, mot.y, mot.z)));
                     }
-                    break;
+                }
             }
             return comp.toArray(new Component[0]);
         });