review feedback pr

introduce local varaible where necessary, otherwise ignore warning for the sake of shorter log message line.
MovingBlocks · Mar 3, 2024 · a85dbe7 · a85dbe7
1 parent 993962b
commit a85dbe7
Show file tree

Hide file tree

Showing 23 changed files with 59 additions and 69 deletions.
diff --git a/engine/src/main/java/org/terasology/engine/audio/openAL/OpenALManager.java b/engine/src/main/java/org/terasology/engine/audio/openAL/OpenALManager.java
@@ -52,6 +52,7 @@ public class OpenALManager implements AudioManager {
 
     private final Map<SoundSource<?>, AudioEndListener> endListeners = Maps.newHashMap();
 
+    @SuppressWarnings("PMD.GuardLogStatement")
     public OpenALManager(AudioConfig config) throws OpenALException {
         logger.info("Initializing OpenAL audio manager");
 
@@ -66,14 +67,13 @@ public OpenALManager(AudioConfig config) throws OpenALException {
 
         logger.atInfo().addArgument(() -> AL10.alGetString(AL10.AL_VERSION)).log("OpenAL {} initialized!");
 
-        logger.atInfo().addArgument(() -> AL10.alGetString(AL10.AL_RENDERER)).addArgument(() -> AL10.alGetString(AL10.AL_VENDOR)).
-                log("Using OpenAL: {} by {}");
-        logger.atInfo().addArgument(() -> ALC10.alcGetString(device, ALC10.ALC_DEVICE_SPECIFIER)).log("Using device: {}");
-        logger.atInfo().addArgument(() -> AL10.alGetString(AL10.AL_EXTENSIONS)).log("Available AL extensions: {}");
-        logger.atInfo().addArgument(() -> ALC10.alcGetString(device, ALC10.ALC_EXTENSIONS)).log("Available ALC extensions: {}");
-        logger.atInfo().addArgument(() -> ALC10.alcGetInteger(device, ALC11.ALC_MONO_SOURCES)).log("Max mono sources: {}");
-        logger.atInfo().addArgument(() -> ALC10.alcGetInteger(device, ALC11.ALC_STEREO_SOURCES)).log("Max stereo sources: {}");
-        logger.atInfo().addArgument(() -> ALC10.alcGetInteger(device, ALC10.ALC_FREQUENCY)).log("Mixer frequency: {}");
+        logger.info("Using OpenAL: {} by {}", AL10.alGetString(AL10.AL_RENDERER), AL10.alGetString(AL10.AL_VENDOR));
+        logger.info("Using device: {}", ALC10.alcGetString(device, ALC10.ALC_DEVICE_SPECIFIER));
+        logger.info("Available AL extensions: {}", AL10.alGetString(AL10.AL_EXTENSIONS));
+        logger.info("Available ALC extensions: {}", ALC10.alcGetString(device, ALC10.ALC_EXTENSIONS));
+        logger.info("Max mono sources: {}", ALC10.alcGetInteger(device, ALC11.ALC_MONO_SOURCES));
+        logger.info("Max stereo sources: {}", ALC10.alcGetInteger(device, ALC11.ALC_STEREO_SOURCES));
+        logger.info("Mixer frequency: {}", ALC10.alcGetInteger(device, ALC10.ALC_FREQUENCY));
 
         AL10.alDistanceModel(AL10.AL_INVERSE_DISTANCE_CLAMPED);
 

diff --git a/engine/src/main/java/org/terasology/engine/config/flexible/AutoConfigTypeHandler.java b/engine/src/main/java/org/terasology/engine/config/flexible/AutoConfigTypeHandler.java
@@ -43,7 +43,7 @@ protected PersistedData serializeNonNull(AutoConfig value, PersistedDataSerializ
                     if (typeHandler.isPresent()) {
                         fields.put(field.getName(), typeHandler.get().serialize(setting.get(), serializer));
                     } else {
-                        logger.atError().addArgument(() -> setting.getValueType()).log("Cannot serialize type [{}]");
+                        logger.error("Cannot serialize type [{}]", setting.getValueType()); //NOPMD
                     }
                 }
             } catch (IllegalAccessException e) {
@@ -65,7 +65,7 @@ public Optional<AutoConfig> deserialize(PersistedData data) {
             for (Map.Entry<String, PersistedData> entry : data.getAsValueMap().entrySet()) {
                 Field settingField = settingFields.get(entry.getKey());
                 if (settingField == null) {
-                    logger.atWarn().addArgument(() -> entry.getKey()).log("Cannot to find setting field with name [{}]");
+                    logger.warn("Cannot to find setting field with name [{}]", entry.getKey()); //NOPMD
                     continue;
                 }
                 try {
@@ -77,11 +77,11 @@ public Optional<AutoConfig> deserialize(PersistedData data) {
                         if (value.isPresent()) {
                             setting.set(value.get());
                         } else {
-                            logger.atError().addArgument(() -> entry.getValue()).addArgument(setting.getValueType()).
-                                    log("Cannot deserialize value [{}] to type [{}]");
+                            logger.error("Cannot deserialize value [{}] to type [{}]", entry.getValue(),
+                                    setting.getValueType()); //NOPMD
                         }
                     } else {
-                        logger.atError().addArgument(() -> setting.getValueType()).log("Cannot deserialize type [{}]");
+                        logger.error("Cannot deserialize type [{}]", setting.getValueType()); //NOPMD
                     }
                 } catch (IllegalAccessException e) {
                     // ignore, AutoConfig.getSettingsFieldsIn return public fields.

diff --git a/engine/src/main/java/org/terasology/engine/config/flexible/constraints/StringConstraint.java b/engine/src/main/java/org/terasology/engine/config/flexible/constraints/StringConstraint.java
@@ -49,13 +49,12 @@ public boolean isSatisfiedBy(String value) {
     }
 
     @Override
+    @SuppressWarnings("PMD.GuardLogStatement")
     public void warnUnsatisfiedBy(String value) {
-        logger.atWarn().
-                addArgument(value).
-                addArgument(() -> predicates.stream()
+        logger.warn("String [{}] does not match the conditions: {}", value,
+                predicates.stream()
                         .filter(p -> !p.test(value))
                         .map(StringConstraint::getDescription)
-                        .collect(Collectors.joining(",", "[", "]"))).
-                log("String [{}] does not match the conditions: {}");
+                        .collect(Collectors.joining(",", "[", "]")));
     }
 }
diff --git a/engine/src/main/java/org/terasology/engine/config/flexible/ui/AutoConfigScreen.java b/engine/src/main/java/org/terasology/engine/config/flexible/ui/AutoConfigScreen.java
@@ -46,7 +46,7 @@ public void initialise() {
             if (widget.isPresent()) {
                 mainContainer.addWidget(widget.get());
             } else {
-                logger.atWarn().addArgument(() -> config.getId()).log("Cannot create widget for config: {}");
+                logger.warn("Cannot create widget for config: {}", config.getId()); //NOPMD
             }
         }
         WidgetUtil.trySubscribe(this, "close", button -> triggerBackAnimation());

diff --git a/engine/src/main/java/org/terasology/engine/config/flexible/ui/AutoConfigWidgetFactory.java b/engine/src/main/java/org/terasology/engine/config/flexible/ui/AutoConfigWidgetFactory.java
@@ -79,7 +79,7 @@ public UIWidget buildWidgetFor(AutoConfig config) {
             Optional<UIWidget> settingWidget = settingWidgetFactory.createWidgetFor(setting);
 
             if (!settingWidget.isPresent()) {
-                logger.atError().addArgument(() -> setting.getHumanReadableName()).log("Couldn't find a widget for the Setting [{}]");
+                logger.error("Couldn't find a widget for the Setting [{}]", setting.getHumanReadableName()); //NOPMD
                 continue;
             }
 

diff --git a/engine/src/main/java/org/terasology/engine/core/ComponentSystemManager.java b/engine/src/main/java/org/terasology/engine/core/ComponentSystemManager.java
@@ -68,7 +68,7 @@ public void loadSystems(ModuleEnvironment environment, NetworkMode netMode) {
         ListMultimap<Name, Class<?>> systemsByModule = ArrayListMultimap.create();
         for (Class<?> type : environment.getTypesAnnotatedWith(RegisterSystem.class)) {
             if (!ComponentSystem.class.isAssignableFrom(type)) {
-                logger.atError().addArgument(type.getSimpleName()).log("Cannot load {}, must be a subclass of ComponentSystem");
+                logger.error("Cannot load {}, must be a subclass of ComponentSystem", type.getSimpleName()); //NOPMD
                 continue;
             }
             Name moduleId = environment.getModuleProviding(type);

diff --git a/engine/src/main/java/org/terasology/engine/core/PathManager.java b/engine/src/main/java/org/terasology/engine/core/PathManager.java
@@ -90,7 +90,7 @@ private static Path findInstallPath() {
         // Use the current directory as a fallback.
         Path currentDirectory = Paths.get("").toAbsolutePath();
         installationSearchPaths.add(currentDirectory);
-        LOGGER.atInfo().addArgument(currentDirectory).log("PathManager: Working directory is {}");
+        LOGGER.info("PathManager: Working directory is {}", currentDirectory);
 
         for (Path startPath : installationSearchPaths) {
             Path installationPath = findNativesHome(startPath, 5);

diff --git a/engine/src/main/java/org/terasology/engine/core/TerasologyEngine.java b/engine/src/main/java/org/terasology/engine/core/TerasologyEngine.java
@@ -265,21 +265,18 @@ private void verifyInitialisation() {
     /**
      * Logs software, environment and hardware information.
      */
+    @SuppressWarnings("PMD.GuardLogStatement")
     private void logEnvironmentInfo() {
-        logger.atInfo().addArgument(() -> TerasologyVersion.getInstance()).log("{}");
-        logger.atInfo().addArgument(() -> PathManager.getInstance().getHomePath()).log("Home path: {}");
-        logger.atInfo().addArgument(() -> PathManager.getInstance().getInstallPath()).log("Install path: {}");
-        logger.atInfo().addArgument(() -> System.getProperty("java.version")).addArgument(System.getProperty("java.home")).
-                log("Java: {} in {}");
-        logger.atInfo().addArgument(() -> System.getProperty("java.vm.name")).addArgument(() -> System.getProperty("java.vm" + ".version")).
-                log("Java VM: {}, version: {}");
-        logger.atInfo().
-                addArgument(() -> System.getProperty("os.name")).
-                addArgument(() -> System.getProperty("os.arch")).
-                addArgument(() -> System.getProperty("os.version")).
-                log("OS: {}, arch: {}, version: {}");
-        logger.atInfo().addArgument(() -> Runtime.getRuntime().maxMemory() / ONE_MEBIBYTE).log("Max. Memory: {} MiB");
-        logger.atInfo().addArgument(() -> Runtime.getRuntime().availableProcessors()).log("Processors: {}");
+        logger.info("{}", TerasologyVersion.getInstance());
+        logger.info("Home path: {}", PathManager.getInstance().getHomePath());
+        logger.info("Install path: {}", PathManager.getInstance().getInstallPath());
+        logger.info("Java: {} in {}", System.getProperty("java.version"), System.getProperty("java.home"));
+        logger.info("Java VM: {}, version: {}", System.getProperty("java.vm.name"), System.getProperty("java.vm" +
+                ".version"));
+        logger.info("OS: {}, arch: {}, version: {}", System.getProperty("os.name"), System.getProperty("os.arch"),
+                System.getProperty("os.version"));
+        logger.info("Max. Memory: {} MiB", Runtime.getRuntime().maxMemory() / ONE_MEBIBYTE);
+        logger.info("Processors: {}", Runtime.getRuntime().availableProcessors());
         if (NonNativeJVMDetector.JVM_ARCH_IS_NONNATIVE) {
             logger.warn("Running on a 32-bit JVM on a 64-bit system. This may limit performance.");
         }
@@ -547,7 +544,7 @@ public void cleanup() {
             try {
                 subsystem.preShutdown();
             } catch (RuntimeException e) {
-                logger.atError().addArgument(() -> subsystem.getName()).addArgument(e).log("Error preparing to shutdown {} subsystem");
+                logger.error("Error preparing to shutdown {} subsystem", subsystem.getName(), e); //NOPMD
             }
         }
 
@@ -557,7 +554,7 @@ public void cleanup() {
             try {
                 subsystem.shutdown();
             } catch (RuntimeException e) {
-                logger.atError().addArgument(() -> subsystem.getName()).addArgument(e).log("Error shutting down {} subsystem");
+                logger.error("Error shutting down {} subsystem", subsystem.getName(), e); //NOPMD
             }
         }
     }

diff --git a/engine/src/main/java/org/terasology/engine/core/modes/StateLoading.java b/engine/src/main/java/org/terasology/engine/core/modes/StateLoading.java
@@ -234,7 +234,7 @@ private void popStep() {
         current = null;
         if (!loadProcesses.isEmpty()) {
             current = loadProcesses.remove();
-            logger.atDebug().addArgument(() -> current.getMessage()).log("{}");
+            logger.debug("{}", current.getMessage()); //NOPMD
             current.begin();
         }
     }

diff --git a/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseWorld.java b/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseWorld.java
@@ -93,7 +93,7 @@ public boolean step() {
             worldInfo.setSeed(random.nextString(16));
         }
 
-        logger.atInfo().addArgument(() -> worldInfo.getSeed()).log("World seed: \"{}\"");
+        logger.info("World seed: \"{}\"", worldInfo.getSeed()); //NOPMD
 
         // TODO: Separate WorldRenderer from world handling in general
         WorldGeneratorManager worldGeneratorManager = context.get(WorldGeneratorManager.class);
@@ -104,8 +104,8 @@ public boolean step() {
             worldGenerator.setWorldSeed(worldInfo.getSeed());
             context.put(WorldGenerator.class, worldGenerator);
         } catch (UnresolvedWorldGeneratorException e) {
-            logger.atError().addArgument(() -> worldInfo.getWorldGenerator()).addArgument(() -> worldGeneratorManager.getWorldGenerators()).
-                    log("Unable to load world generator {}. Available world generators: {}");
+            logger.error("Unable to load world generator {}. Available world generators: {}",
+                    worldInfo.getWorldGenerator(), worldGeneratorManager.getWorldGenerators()); //NOPMD
             context.get(GameEngine.class).changeState(new StateMainMenu("Failed to resolve world generator."));
             return true; // We need to return true, otherwise the loading state will just call us again immediately
         }

diff --git a/engine/src/main/java/org/terasology/engine/core/module/ModuleInstaller.java b/engine/src/main/java/org/terasology/engine/core/module/ModuleInstaller.java
@@ -48,7 +48,7 @@ public List<Module> call() throws Exception {
                 Module module = moduleManager.registerArchiveModule(filePath);
                 newInstalledModules.add(module);
             } catch (IOException e) {
-                logger.atWarn().addArgument(() -> filePath.getFileName()).addArgument(e).log("Could not load module {}");
+                logger.warn("Could not load module {}", filePath.getFileName(), e); //NOPMD
             }
         }
         logger.info("Finished loading the downloaded modules");

diff --git a/engine/src/main/java/org/terasology/engine/core/module/ModuleListDownloader.java b/engine/src/main/java/org/terasology/engine/core/module/ModuleListDownloader.java
@@ -59,13 +59,13 @@ public ModuleRegistry call() throws IOException {
                 String json = gson.toJson(jObject);
 
                 ModuleMetadata meta = metaReader.read(new StringReader(json));
-                logger.atDebug().addArgument(() -> meta.getId()).addArgument(() -> meta.getVersion()).log("Read module {} - {}");
+                logger.debug("Read module {} - {}", meta.getId(), meta.getVersion()); //NOPMD
                 modules.add(new Module(meta, new EmptyFileSource(), Collections.emptyList(), new Reflections(),
                         (c) -> false));
             }
 
             int count = modules.size();
-            logger.atInfo().addArgument(() -> count).addArgument(() -> (count == 1) ? "entry" : "entries").log("Retrieved {} {}");
+            logger.info("Retrieved {} {}", count, (count == 1) ? "entry" : "entries"); //NOPMD
         }
         return modules;
     }

diff --git a/engine/src/main/java/org/terasology/engine/core/module/ModuleManager.java b/engine/src/main/java/org/terasology/engine/core/module/ModuleManager.java
@@ -173,10 +173,9 @@ private void loadModulesFromClassPath() {
                     continue;
                 }
                 if (registry.add(module)) {
-                    logger.atInfo().addArgument(() -> module.getId()).addArgument(() -> path).log("Loaded {} from {}");
+                    logger.info("Loaded {} from {}", module.getId(), path); //NOPMD
                 } else {
-                    logger.atInfo().addArgument(() -> module.getId()).addArgument(() -> path).
-                            log("Module {} from {} was a duplicate; not registering this copy.");
+                    logger.info("Module {} from {} was a duplicate; not registering this copy.", module.getId(), path); //NOPMD
                 }
             }
         }

diff --git a/engine/src/main/java/org/terasology/engine/core/subsystem/lwjgl/GLFWErrorCallback.java b/engine/src/main/java/org/terasology/engine/core/subsystem/lwjgl/GLFWErrorCallback.java
@@ -14,8 +14,8 @@ public class GLFWErrorCallback implements GLFWErrorCallbackI {
     private static final Logger logger = LoggerFactory.getLogger("GLFW");
 
     @Override
+    @SuppressWarnings("PMD.GuardLogStatement")
     public void invoke(int error, long description) {
-        logger.atError().addArgument(() -> error).addArgument(() -> MemoryUtil.memASCII(description)).
-                log("Received error. Code: {}, Description: {}");
+        logger.error("Received error. Code: {}, Description: {}", error, MemoryUtil.memASCII(description));
     }
 }
diff --git a/...e/src/main/java/org/terasology/engine/entitySystem/entity/internal/PojoEntityManager.java b/...e/src/main/java/org/terasology/engine/entitySystem/entity/internal/PojoEntityManager.java
@@ -526,8 +526,7 @@ public <T extends Component> T addComponent(long entityId, T component) {
         if (!oldComponent.isPresent()) {
             notifyComponentAdded(getEntity(entityId), component.getClass());
         } else {
-            logger.atError().addArgument(() -> component.getClass()).addArgument(() -> entityId).
-                    log("Adding a component ({}) over an existing component for entity {}");
+            logger.error("Adding a component ({}) over an existing component for entity {}", component.getClass(), entityId); //NOPMD
             notifyComponentChanged(getEntity(entityId), component.getClass());
         }
 
@@ -581,8 +580,7 @@ public void saveComponent(long entityId, Component component) {
                 .map(pool -> pool.getComponentStore().put(entityId, component));
 
         if (!oldComponent.isPresent()) {
-            logger.atError().addArgument(() -> component.getClass()).addArgument(() -> entityId).
-                    log("Saving a component ({}) that doesn't belong to this entity {}");
+            logger.error("Saving a component ({}) that doesn't belong to this entity {}", component.getClass(), entityId); //NOPMD
         }
         if (eventSystem != null) {
             EntityRef entityRef = getEntity(entityId);

diff --git a/engine/src/main/java/org/terasology/engine/entitySystem/event/internal/EventSystemImpl.java b/engine/src/main/java/org/terasology/engine/entitySystem/event/internal/EventSystemImpl.java
@@ -137,8 +137,8 @@ public void registerEventHandler(ComponentSystem handler) {
                 List<Class<? extends Component>> componentParams = Lists.newArrayList();
                 for (int i = 2; i < types.length; ++i) {
                     if (!Component.class.isAssignableFrom(types[i])) {
-                        logger.atError().addArgument(() -> method.getName()).addArgument(types[i]).
-                                log("Invalid event handler method: {} - {} is not a component class");
+                        logger.error("Invalid event handler method: {} - {} is not a component class",
+                                method.getName(), types[i]); //NOPMD
                         return;
                     }
                     requiredComponents.add((Class<? extends Component>) types[i]);

diff --git a/engine/src/main/java/org/terasology/engine/entitySystem/metadata/ComponentLibrary.java b/engine/src/main/java/org/terasology/engine/entitySystem/metadata/ComponentLibrary.java
@@ -46,8 +46,7 @@ public <T> ComponentLibrary createCopyUsingCopyStrategy(Class<T> type, CopyStrat
         try {
             info = new ComponentMetadata<>(uri, type, factory, copyStrategies);
         } catch (NoSuchMethodException e) {
-            logger.atError().addArgument(() -> type.getSimpleName()).addArgument(() -> e).
-                    log("Unable to register class {}: Default Constructor Required");
+            logger.error("Unable to register class {}: Default Constructor Required", type.getSimpleName(), e); //NOPMD
             return null;
         } catch (NoClassDefFoundError e) {
             // log what class was not found so that diagnosis is easier

diff --git a/engine/src/main/java/org/terasology/engine/game/GameManifest.java b/engine/src/main/java/org/terasology/engine/game/GameManifest.java
@@ -188,14 +188,13 @@ public void addModule(Name id, Version version) {
     public String mainWorldDisplayName(WorldGeneratorManager manager) {
         var world = getWorldInfo(TerasologyConstants.MAIN_WORLD);
         if (world == null) {
-            logger.atWarn().addArgument(() -> this).log("{} has no MAIN_WORLD");
+            logger.warn("{} has no MAIN_WORLD", this);
             return "ERROR: No main world";
         }
         SimpleUri generatorUri = world.getWorldGenerator();
         var generator = manager.getWorldGeneratorInfo(generatorUri);
         if (generator == null) {
-            logger.atWarn().addArgument(() -> this).addArgument(() -> manager).addArgument(() -> generatorUri).
-                    log("{}: {} has no generator for {}");
+            logger.warn("{}: {} has no generator for {}", this, manager, generatorUri);
             return "ERROR: No generator found for " + generatorUri;
         }
         return generator.getDisplayName();

diff --git a/engine/src/main/java/org/terasology/engine/logic/characters/CharacterSystem.java b/engine/src/main/java/org/terasology/engine/logic/characters/CharacterSystem.java
@@ -155,7 +155,7 @@ public String getDamageTypeName(Prefab damageType) {
             return damageType.getComponent(DisplayNameComponent.class).name;
         } else {
             String damageTypeName = damageType.getName();
-            logger.atInfo().addArgument(damageTypeName).log("{} is missing a readable DisplayName");
+            logger.info("{} is missing a readable DisplayName", damageTypeName);
             //damageType.getName() returns a ResourceUrn String such as "engine:directDamage"
             //The following calls change the damage type to be more readable
             //For instance, "engine:directDamage" becomes "Direct Damage"