Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qa: address pmd findings #5274

Merged
merged 12 commits into from
Oct 4, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
/**
* Configures the underlying logback logging framework.
*/
@SuppressWarnings("PMD.SystemPrintln")
public final class LoggingContext {

/**
Expand Down Expand Up @@ -74,7 +75,7 @@ public static void initialize(Path logFileFolder) {
try {
deleteLogFiles(logFileFolder, Duration.ofDays(5).getSeconds());
} catch (IOException e) {
e.printStackTrace();
e.printStackTrace(); //NOPMD
}

// Unfortunately, setting context-based variables works only after initialization
Expand Down
10 changes: 5 additions & 5 deletions engine/src/main/java/org/terasology/engine/core/PathManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ private static Path findInstallPath() {
}
}

System.err.printf(
"Native library installation directory not found. %n" +
"Things will almost certainly crash as a result, %n" +
"unless something else installed everything to java.library.path. %n" +
"Searched: %s%n", installationSearchPaths
LOGGER.error(
"Native library installation directory not found. /n" +
"Things will almost certainly crash as a result, /n" +
"unless something else installed everything to java.library.path. /n" +
"Searched: {}/n", installationSearchPaths
);
return currentDirectory;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private Map<URI, Path> getDownloadUrls(Iterable<Module> modules) {
try {
uri = RemoteModuleExtension.getDownloadUrl(metadata).toURI();
} catch (URISyntaxException e) {
e.printStackTrace();
logger.error("Couldn't get download URL: ", e);
}
String fileName = String.format("%s-%s.jar", id, version);
Path folder = PathManager.getInstance().getHomeModPath().normalize();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
// SPDX-License-Identifier: Apache-2.0
package org.terasology.engine.core.module.rendering;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.terasology.context.annotation.API;
import org.terasology.engine.context.Context;
import org.terasology.engine.rendering.dag.ModuleRendering;
import org.terasology.gestalt.module.ModuleEnvironment;
import org.terasology.context.annotation.API;
import org.terasology.gestalt.naming.Name;

import javax.annotation.Nullable;
Expand All @@ -20,6 +22,7 @@

@API
public class RenderingModuleRegistry {
private static final Logger LOGGER = LoggerFactory.getLogger(RenderingModuleRegistry.class);

private List<ModuleRendering> orderedModuleRenderingInstances = new ArrayList<>();

Expand Down Expand Up @@ -126,7 +129,7 @@ private Set<ModuleRendering> fetchRenderingModules(ModuleEnvironment moduleEnvir
Constructor<?> constructor = renderingClass.getConstructor(Context.class);
moduleRenderingInstance = (ModuleRendering) constructor.newInstance(context);
} catch (InstantiationException | IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
e.printStackTrace();
LOGGER.error("Couldn't get constructor: ", e);
}
}
if (moduleRenderingInstance != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,24 @@ public class DebugCallback implements org.lwjgl.opengl.GLDebugMessageCallbackI {
@Override
public void invoke(int source, int type, int id, int severity, int length, long message, long userParam) {
String logFormat = "[{}] [{}] [{}] {}";
Object[] args = new Object[]{
"0x" + Integer.toHexString(id).toUpperCase(Locale.ROOT),
getSourceString(source),
getTypeString(type),
MemoryUtil.memASCII(message).trim()
};
String idString = "0x" + Integer.toHexString(id).toUpperCase(Locale.ROOT);
String sourceString = getSourceString(source);
String typeString = getTypeString(type);
String messageString = MemoryUtil.memASCII(message).trim();

switch (severity) {
case GL_DEBUG_SEVERITY_HIGH:
logger.error(logFormat, args);
logger.error(logFormat, idString, sourceString, typeString, messageString);
break;
case GL_DEBUG_SEVERITY_MEDIUM:
logger.warn(logFormat, args);
logger.warn(logFormat, idString, sourceString, typeString, messageString);
break;
case GL_DEBUG_SEVERITY_LOW:
logger.debug(logFormat, args);
logger.debug(logFormat, idString, sourceString, typeString, messageString);
break;
default:
case GL_DEBUG_SEVERITY_NOTIFICATION:
logger.trace(logFormat, args);
logger.trace(logFormat, idString, sourceString, typeString, messageString);
break;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,15 @@ private Component getNext() {
if (componentsToAdd.containsKey(result.getClass())) {
throw new IllegalStateException("Requested to add component that was already defined for this entity");
}
if (componentsToRemove.contains(result.getClass())) {
continue;
if (!componentsToRemove.contains(result.getClass())) {
return result;
}
return result;
}
while (addedIterator.hasNext()) {
final Component result = addedIterator.next();
if (componentsToRemove.contains(result.getClass())) {
continue;
if (!componentsToRemove.contains(result.getClass())) {
return result;
}
return result;
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* @see Group
*/
@API
@SuppressWarnings("PMD.AvoidPrintStackTrace")
public class GroupBuilder {

private GroupData groupData;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ public String playerEyeHeight(@Sender EntityRef client, @CommandParam("eye-heigh
}
return "";
} catch (NullPointerException e) {
e.printStackTrace();
logger.error("Couldn't set player eye height: e");
return "";
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,9 @@ protected Vector3f findSpawnPosition(World world, Vector2i pos, int searchRadius
// nothing above sea level found
for (Vector2ic test : spiral) {
Optional<Float> val = getWorld.apply(test);
if (!val.isPresent()) {
continue;
if (val.isPresent()) {
return new Vector3f(test.x(), TeraMath.floorToInt(val.get()), test.y());
}
return new Vector3f(test.x(), TeraMath.floorToInt(val.get()), test.y());
}

throw new IllegalStateException("No spawn location found");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ private void saveLists() {
whitelistWriter.close();
}
} catch (IOException e) {
e.printStackTrace();
logger.error("Couldn't save lists: ", e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ private void applyCommonDataDelta(EntityData.Prefab delta, PrefabData result) {
}

private void deserializeCommonData(EntityData.Prefab prefabData, PrefabData result) {
result.setPersisted((prefabData.hasPersisted()) ? prefabData.getPersisted() : true);
result.setAlwaysRelevant(prefabData.hasAlwaysRelevant() ? prefabData.getAlwaysRelevant() : false);
result.setPersisted(!prefabData.hasPersisted() || prefabData.getPersisted());
result.setAlwaysRelevant(prefabData.hasAlwaysRelevant() && prefabData.getAlwaysRelevant());
if (prefabData.hasParentName()) {
Prefab parent = Assets.get(prefabData.getParentName(), Prefab.class).orElse(null);
result.setParent(parent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ public class BulletConvexHullShape extends BulletCollisionShape implements Conve

public BulletConvexHullShape(List<Vector3f> vertices) {
FloatBuffer buffer = BufferUtils.createFloatBuffer(vertices.size() * 3);
for (int i = 0; i < vertices.size(); i++) {
Vector3f vertex = vertices.get(i);
for (Vector3f vertex : vertices) {
buffer.put(vertex.x);
buffer.put(vertex.y);
buffer.put(vertex.z);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ private StandardMeshData processData(List<Vector3f> rawVertices, List<Vector3f>
List<Vector2f> rawTexCoords, List<Vector3i[]> rawIndices) throws IOException {
int numIndices = 0;
int numVerts = 0;
for (int x = 0; x < rawIndices.size(); x++) {
numIndices += (rawIndices.get(x).length - 2) * 3;
numVerts += rawIndices.get(x).length;
for (Vector3i[] rawIndex : rawIndices) {
numIndices += (rawIndex.length - 2) * 3;
numVerts += rawIndex.length;
}

StandardMeshData result = new StandardMeshData();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.IntStream;


public class SkeletalMeshData implements AssetData {
Expand Down Expand Up @@ -170,12 +172,10 @@
}

private void calculateNormals() {
// TODO: Better algorithm (take into account triangle size and angles

Check warning on line 175 in engine/src/main/java/org/terasology/engine/rendering/assets/skeletalmesh/SkeletalMeshData.java

View check run for this annotation

Terasology Jenkins.io / Open Tasks Scanner

TODO

NORMAL: Better algorithm (take into account triangle size and angles
List<Vector3f> vertices = getBindPoseVertexPositions();
List<Vector3f> normals = Lists.newArrayListWithCapacity(vertices.size());
for (int i = 0; i < vertices.size(); ++i) {
normals.add(new Vector3f());
}
List<Vector3f> normals = IntStream.range(0, vertices.size()).mapToObj(i -> new Vector3f()).collect(Collectors.toCollection(() ->
Lists.newArrayListWithCapacity(vertices.size())));
Vector3f v1 = new Vector3f();
Vector3f v2 = new Vector3f();
Vector3f norm = new Vector3f();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.nio.ByteBuffer;
import java.util.List;
import java.util.function.Predicate;
import java.util.stream.IntStream;

import static org.terasology.gestalt.assets.module.ModuleAssetScanner.OVERRIDE_FOLDER;

Expand Down Expand Up @@ -79,34 +80,34 @@ public static TextureData convertToTextureData(final BufferedImage image, Textur

// Convert AWT image to proper internal format
if (image.getType() == BufferedImage.TYPE_3BYTE_BGR) {
for (int i = 0; i < data.length; i += stride) {
IntStream.iterate(0, i -> i < data.length, i -> i + stride).forEach(i -> {
buf.put(data, i + 2, 1); // R
buf.put(data, i + 1, 1); // G
buf.put(data, i + 0, 1); // B
buf.put((byte) 255); // A
}
});
} else if (image.getType() == BufferedImage.TYPE_4BYTE_ABGR) {
for (int i = 0; i < data.length; i += stride) {
IntStream.iterate(0, i -> i < data.length, i -> i + stride).forEach(i -> {
buf.put(data, i + 3, 1); // R
buf.put(data, i + 2, 1); // G
buf.put(data, i + 1, 1); // B
buf.put(data, i + 0, 1); // A
}
});
} else if (image.getType() == BufferedImage.TYPE_INT_ARGB) {
for (int i = 0; i < data.length; i += stride) {
IntStream.iterate(0, i -> i < data.length, i -> i + stride).forEach(i -> {
buf.put(data, i + 1, 1); // R
buf.put(data, i + 2, 1); // G
buf.put(data, i + 3, 1); // B
buf.put(data, i + 0, 1); // A
}
});
} else if (image.getType() == BufferedImage.TYPE_BYTE_INDEXED) {
final ColorModel cm = image.getColorModel();
for (int i = 0; i < data.length; i += stride) {
IntStream.iterate(0, i -> i < data.length, i -> i + stride).forEach(i -> {
buf.put((byte) cm.getRed(data[i])); // R
buf.put((byte) cm.getGreen(data[i])); // G
buf.put((byte) cm.getBlue(data[i])); // B
buf.put((byte) cm.getAlpha(data[i])); // A
}
});
} else {
throw new IOException("Unsupported AWT format: " + image.getType());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.terasology.engine.config.Config;
import org.terasology.engine.context.Context;
import org.terasology.engine.core.GameEngine;
Expand Down Expand Up @@ -51,10 +53,10 @@
import org.terasology.gestalt.entitysystem.component.Component;
import org.terasology.gestalt.module.Module;
import org.terasology.gestalt.module.ModuleEnvironment;
import org.terasology.gestalt.module.exceptions.UnresolvedDependencyException;
import org.terasology.gestalt.module.dependencyresolution.DependencyInfo;
import org.terasology.gestalt.module.dependencyresolution.DependencyResolver;
import org.terasology.gestalt.module.dependencyresolution.ResolutionResult;
import org.terasology.gestalt.module.exceptions.UnresolvedDependencyException;
import org.terasology.gestalt.naming.Name;
import org.terasology.math.TeraMath;
import org.terasology.nui.WidgetUtil;
Expand Down Expand Up @@ -96,6 +98,8 @@
public class UniverseSetupScreen extends CoreScreenLayer implements UISliderOnChangeTriggeredListener, PropertyChangeListener {
public static final ResourceUrn ASSET_URI = new ResourceUrn("engine:universeSetupScreen");

private static final Logger LOGGER = LoggerFactory.getLogger(UniverseSetupScreen.class);

@In
private WorldGeneratorManager worldGeneratorManager;

Expand Down Expand Up @@ -203,12 +207,10 @@

@Override
public void set(WorldGeneratorInfo worldGeneratorInfo) {
if (worldGeneratorInfo != null) {
if (context.get(UniverseWrapper.class).getWorldGenerator() == null
|| !worldGeneratorInfo.getUri().equals(context.get(UniverseWrapper.class).getWorldGenerator().getUri())) {
config.getWorldGeneration().setDefaultGenerator(worldGeneratorInfo.getUri());
addNewWorld(worldGeneratorInfo);
}
if (worldGeneratorInfo != null && (context.get(UniverseWrapper.class).getWorldGenerator() == null
|| !worldGeneratorInfo.getUri().equals(context.get(UniverseWrapper.class).getWorldGenerator().getUri()))) {
config.getWorldGeneration().setDefaultGenerator(worldGeneratorInfo.getUri());
addNewWorld(worldGeneratorInfo);
}
}
});
Expand Down Expand Up @@ -356,10 +358,10 @@
} catch (UnresolvedWorldGeneratorException e) {
getManager().pushScreen(MessagePopup.ASSET_URI, MessagePopup.class).setMessage("Selected world generator cannot be resolved!",
"Please report this issue on Discord/GitHub and select a different world generator!");
e.printStackTrace();
LOGGER.error("Selected world generator cannot be resolved: ", e);
} catch (UnresolvedDependencyException e) {
//TODO: this will likely fail at game creation time later-on due to lack of world generator - don't just ignore this

Check warning on line 363 in engine/src/main/java/org/terasology/engine/rendering/nui/layers/mainMenu/UniverseSetupScreen.java

View check run for this annotation

Terasology Jenkins.io / Open Tasks Scanner

TODO

NORMAL: this will likely fail at game creation time later-on due to lack of world generator - don't just ignore this
e.printStackTrace();
LOGGER.error("Selected world generator cannot be resolved: ", e);
}

configureProperties();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ public interface OpenGLMeshBase {

default boolean updateState(VBOContext state) {
GL30.glBindBuffer(GL30.GL_ARRAY_BUFFER, state.vbo);
for (int x = 0; x < state.entries.length; x++) {
if (state.entries[x].version != state.entries[x].resource.getVersion()) {
if (state.entries[x].size != state.entries[x].resource.inSize()) {
for (VBOContext.VBOSubBuffer subBuffer : state.entries) {
if (subBuffer.version != subBuffer.resource.getVersion()) {
if (subBuffer.size != subBuffer.resource.inSize()) {
return false;
}
VertexResource resource = state.entries[x].resource;
int offset = state.entries[x].offset;
VertexResource resource = subBuffer.resource;
int offset = subBuffer.offset;
resource.writeBuffer((buffer) -> {
GL30.glBufferSubData(GL30.GL_ARRAY_BUFFER, offset, buffer);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import java.util.concurrent.atomic.AtomicLong;

public class WorldTimeImpl extends BaseComponentSystem implements WorldTime, UpdateSubscriberSystem {

private static final float WORLD_TIME_MULTIPLIER = 48f;

private AtomicLong worldTime = new AtomicLong(0);
Expand Down Expand Up @@ -68,17 +67,14 @@

if (startTick != endTick) {
long tick = endTime - endTime % TICK_EVENT_RATE;
getWorldEntity().send(new WorldTimeEvent(tick));
for (EntityRef e : entityManager.getEntitiesWith(WorldComponent.class)) {
// this may not be called if there's no entity with World Component (yet)
// TODO: consider catching / logging this case (someplace better than here)

Check warning on line 72 in engine/src/main/java/org/terasology/engine/world/time/WorldTimeImpl.java

View check run for this annotation

Terasology Jenkins.io / Open Tasks Scanner

TODO

NORMAL: consider catching / logging this case (someplace better than here)
e.send(new WorldTimeEvent(tick));
}
}

// TODO: consider sending a DailyTick (independent from solar events such as midnight)

Check warning on line 77 in engine/src/main/java/org/terasology/engine/world/time/WorldTimeImpl.java

View check run for this annotation

Terasology Jenkins.io / Open Tasks Scanner

TODO

NORMAL: consider sending a DailyTick (independent from solar events such as midnight)
}
}

private EntityRef getWorldEntity() {
for (EntityRef entity : entityManager.getEntitiesWith(WorldComponent.class)) {
return entity;
}
return EntityRef.NULL;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ private void populateSubsystems(TerasologyEngineBuilder builder) {
builder.add(new HibernationSubsystem());
}

@SuppressWarnings({"PMD.SystemPrintln", "PMD.AvoidPrintStackTrace"})
private void reportException(Throwable throwable) {
Path logPath = LoggingContext.getLoggingPath();

Expand Down
Loading