Skip to content

Commit

Permalink
refactor: address IntelliJ QAPlug plugin findings (#5149)
Browse files Browse the repository at this point in the history
* make final fields static
* remove unnecessary local before return
* remove instanceof checks in catch clause
* avoid throwing java.lang.Exception
* remove null checks covered by instanceof check
* remove toString on String objects
* remove overrides that only call their super
* remove unused private methods
* remove unnecessary if statement nesting
* remove unused local variables
* use try-with-resources to close resources after use
* use equals() to compare object references
* fix broken javadoc references
* fix disallowed self-closing and empty javadoc elements
* fix bad use of symbols in javadoc
* fix disallowed list item tag in javadoc without surrounding list
* fix erroneous javadoc tags
* fix emphasize tags in javadoc
* remove illegal throws
* remove unused imports

Related:
Terasology/CoreRendering#75
Terasology/Furnishings#17
Terasology/Health#105
Terasology/Inventory#51
Terasology/Behaviors#114
Terasology/CoreWorlds#45
Terasology/FlexiblePathfinding#32

Adds to #3859
  • Loading branch information
jdrueckert authored Nov 11, 2023
1 parent 8545be0 commit 1cef7c1
Show file tree
Hide file tree
Showing 128 changed files with 323 additions and 714 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,14 @@ class Environment {
Environment(Name... moduleNames) {
try {
reset(Sets.newHashSet(moduleNames));
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
if (e instanceof RuntimeException) {
throw (RuntimeException) e;
} else {
throw new RuntimeException(e);
}
throw new RuntimeException(e);
}
}

protected void reset(Set<Name> moduleNames) throws Exception {
protected void reset(Set<Name> moduleNames) throws IOException {
this.context = new ContextImpl();
RecordAndReplayCurrentStatus recordAndReplayCurrentStatus = new RecordAndReplayCurrentStatus();
context.put(RecordAndReplayCurrentStatus.class, recordAndReplayCurrentStatus);
Expand Down Expand Up @@ -99,7 +97,7 @@ protected void setupPathManager() throws IOException {
PathManager.getInstance();
}

protected void setupModuleManager(Set<Name> moduleNames) throws Exception {
protected void setupModuleManager(Set<Name> moduleNames) {
// empty
}

Expand Down Expand Up @@ -160,9 +158,9 @@ protected void setupCelestialSystem() {
/**
* Cleans up all resources (similar to AutoCloseable)
*
* @throws Exception if something goes wrong
* @throws RuntimeException if something goes wrong
*/
public void close() throws Exception {
public void close() {
CoreRegistry.setContext(null);
context = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ protected void setupConfig() {
}

@Override
protected void setupModuleManager(Set<Name> moduleNames) throws Exception {
protected void setupModuleManager(Set<Name> moduleNames) throws RuntimeException {
TypeRegistry.WHITELISTED_CLASSES = ExternalApiWhitelist.CLASSES.stream().map(Class::getName).collect(Collectors.toSet());

ModuleManager moduleManager = ModuleManagerFactory.create();
Expand Down Expand Up @@ -320,7 +320,7 @@ protected void loadPrefabs() {
}

@Override
public void close() throws Exception {
public void close() throws RuntimeException {
// it would be nice, if elements in the context implemented (Auto)Closeable

// The StorageManager creates a thread pool (through TaskMaster)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.terasology.reflection.ModuleTypeRegistry;
import org.terasology.reflection.TypeRegistry;

import java.io.IOException;
import java.nio.file.Path;
import java.util.stream.Collectors;

Expand All @@ -21,7 +22,7 @@ public abstract class ModuleEnvironmentTest {
protected ModuleTypeRegistry typeRegistry;

@BeforeEach
public void before(@TempDir Path tempHome) throws Exception {
public void before(@TempDir Path tempHome) throws IOException {
PathManager.getInstance().useOverrideHomePath(tempHome);

moduleManager = ModuleManagerFactory.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public final class ModuleManagerFactory {
private ModuleManagerFactory() {
}

public static ModuleManager create() throws Exception {
public static ModuleManager create() {
// Loading screens, among other things, break when NUI classes are not added to engine.
ModuleManager moduleManager = new ModuleManager("", ImmutableList.of(UIWidget.class));
Module unittestModule = moduleManager.registerPackageModule("org.terasology.unittest");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public long skip(long n) throws IOException {
/**
* Initalizes the vorbis stream. Reads the stream until info and comment are read.
*/
private void initVorbis() throws Exception {
private void initVorbis() throws IOException {
// Now we can read pages
syncState.init();

Expand All @@ -283,7 +283,7 @@ private void initVorbis() throws Exception {
return; //break;
}
// error case. Must not be Vorbis data
throw new Exception("Input does not appear to be an Ogg bitstream.");
throw new IOException("Input does not appear to be an Ogg bitstream.");
}

// Get the serial number and set up the rest of decode.
Expand All @@ -302,15 +302,15 @@ private void initVorbis() throws Exception {
comment.init();
if (streamState.pagein(page) < 0) {
// error; stream version mismatch perhaps
throw new Exception("Error reading first page of Ogg bitstream data.");
throw new IOException("Error reading first page of Ogg bitstream data.");
}
if (streamState.packetout(packet) != 1) {
// no page? must not be vorbis
throw new Exception("Error reading initial header packet.");
throw new IOException("Error reading initial header packet.");
}
if (info.synthesis_headerin(comment, packet) < 0) {
// error case; not a vorbis header
throw new Exception("This Ogg bitstream does not contain Vorbis audio data.");
throw new IOException("This Ogg bitstream does not contain Vorbis audio data.");
}

// At this point, we're sure we're Vorbis. We've set up the logical
Expand Down Expand Up @@ -346,7 +346,7 @@ private void initVorbis() throws Exception {
if (result == -1) {
// Uh oh; data at some point was corrupted or missing!
// We can't tolerate that in a header. Die.
throw new Exception("Corrupt secondary header. Exiting.");
throw new IOException("Corrupt secondary header. Exiting.");
}
info.synthesis_headerin(comment, packet);
i++;
Expand All @@ -364,7 +364,7 @@ private void initVorbis() throws Exception {
bytes = 0;
}
if (bytes == 0 && i < 2) {
throw new Exception("End of file before finding all Vorbis headers!");
throw new IOException("End of file before finding all Vorbis headers!");
}
syncState.wrote(bytes);
}
Expand All @@ -386,7 +386,7 @@ private void initVorbis() throws Exception {
*/
private int decodePacket() {
// check the endianes of the computer.
final boolean bigEndian = ByteOrder.nativeOrder() == ByteOrder.BIG_ENDIAN;
final boolean bigEndian = ByteOrder.nativeOrder().equals(ByteOrder.BIG_ENDIAN);

if (block.synthesis(packet) == 0) {
dspState.synthesis_blockin(block);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@ public BaseSoundPool() {

public SOURCE getLockedSource() {
for (SOURCE source : soundSources.keySet()) {
if (!isActive(source)) {
if (lock(source)) {
return source;
}
if (!isActive(source) && lock(source)) {
return source;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ private float findClosestIndex(Color color) {

private Color findClosestColor(float findex) {
int index = DoubleMath.roundToInt(findex * (double) (colors.size() - 1), RoundingMode.HALF_UP);
Color color = colors.get(index);
return color;
return colors.get(index);
}
}
10 changes: 5 additions & 5 deletions engine/src/main/java/org/terasology/engine/core/GameThread.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ private GameThread() {
* @return Whether the currentThread is the gameThread.
*/
public static boolean isCurrentThread() {
return Thread.currentThread() == gameThread;
return Thread.currentThread().equals(gameThread);
}

/**
Expand All @@ -41,7 +41,7 @@ public static boolean isCurrentThread() {
* @param process
*/
public static void asynch(Runnable process) {
if (Thread.currentThread() != gameThread) {
if (!Thread.currentThread().equals(gameThread)) {
pendingRunnables.push(process);
} else {
process.run();
Expand All @@ -56,7 +56,7 @@ public static void asynch(Runnable process) {
* @param process
*/
public static void synch(Runnable process) throws InterruptedException {
if (Thread.currentThread() != gameThread) {
if (!Thread.currentThread().equals(gameThread)) {
BlockingProcess blockingProcess = new BlockingProcess(process);
pendingRunnables.push(blockingProcess);
blockingProcess.waitForCompletion();
Expand All @@ -69,7 +69,7 @@ public static void synch(Runnable process) throws InterruptedException {
* Runs all pending processes submitted from other threads
*/
public static void processWaitingProcesses() {
if (Thread.currentThread() == gameThread) {
if (Thread.currentThread().equals(gameThread)) {
List<Runnable> processes = Lists.newArrayList();
pendingRunnables.drainTo(processes);
processes.forEach(Runnable::run);
Expand All @@ -80,7 +80,7 @@ public static void processWaitingProcesses() {
* Removes all pending processes without running them
*/
public static void clearWaitingProcesses() {
if (gameThread == Thread.currentThread()) {
if (gameThread.equals(Thread.currentThread())) {
pendingRunnables.clear();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public AbstractEventSystemDecorator(EventSystem eventSystem) {
}

public boolean currentThreadIsMain() {
return mainThread == Thread.currentThread();
return mainThread.equals(Thread.currentThread());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

import com.google.common.base.MoreObjects;
import org.terasology.engine.audio.AudioManager;
import org.terasology.engine.config.Config;
import org.terasology.engine.config.TelemetryConfig;
import org.terasology.engine.core.GameEngine;
import org.terasology.engine.core.LoggingContext;
import org.terasology.engine.core.modes.loadProcesses.RegisterInputSystem;
Expand All @@ -17,11 +15,7 @@
import org.terasology.engine.rendering.nui.NUIManager;
import org.terasology.engine.rendering.nui.editor.systems.NUIEditorSystem;
import org.terasology.engine.rendering.nui.editor.systems.NUISkinEditorSystem;
import org.terasology.engine.rendering.nui.layers.mainMenu.LaunchPopup;
import org.terasology.engine.rendering.nui.layers.mainMenu.MessagePopup;
import org.terasology.engine.telemetry.TelemetryScreen;
import org.terasology.engine.telemetry.TelemetryUtils;
import org.terasology.engine.telemetry.logstash.TelemetryLogstashAppender;
import org.terasology.engine.utilities.Assets;

/**
Expand Down Expand Up @@ -97,35 +91,6 @@ public void init(GameEngine gameEngine) {
// pushLaunchPopup();
}

private void pushLaunchPopup() {
Config config = context.get(Config.class);
TelemetryConfig telemetryConfig = config.getTelemetryConfig();
TranslationSystem translationSystem = context.get(TranslationSystem.class);
TelemetryLogstashAppender appender = TelemetryUtils.fetchTelemetryLogstashAppender();
if (!telemetryConfig.isLaunchPopupDisabled()) {
String telemetryTitle = translationSystem.translate("${engine:menu#telemetry-launch-popup-title}");
String telemetryMessage = translationSystem.translate("${engine:menu#telemetry-launch-popup-text}");
LaunchPopup telemetryConfirmPopup = nuiManager.pushScreen(LaunchPopup.ASSET_URI, LaunchPopup.class);
telemetryConfirmPopup.setMessage(telemetryTitle, telemetryMessage);
telemetryConfirmPopup.setYesHandler(() -> {
telemetryConfig.setTelemetryAndErrorReportingEnable(true);

// Enable error reporting
appender.start();
});
telemetryConfirmPopup.setNoHandler(() -> {
telemetryConfig.setTelemetryAndErrorReportingEnable(false);

// Disable error reporting
appender.stop();
});
telemetryConfirmPopup.setOptionButtonText(translationSystem.translate("${engine:menu#telemetry-button}"));
telemetryConfirmPopup.setOptionHandler(() -> {
nuiManager.pushScreen(TelemetryScreen.ASSET_URI, TelemetryScreen.class);
});
}
}

@Override
public void dispose(boolean shuttingDown) {
stopBackgroundMusic();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import java.util.Date;
/**
* A set of module extensions for remote modules.
* NOTE: this is copy&paste from meta-server.
* NOTE: this is copy&amp;paste from meta-server.
*/
public enum RemoteModuleExtension {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package org.terasology.engine.core.subsystem.config;

import org.terasology.engine.config.BindsConfig;
import org.terasology.engine.context.Context;
import org.terasology.engine.core.SimpleUri;
import org.terasology.engine.input.BindAxisEvent;
import org.terasology.engine.input.BindButtonEvent;
Expand Down Expand Up @@ -35,14 +34,14 @@
public interface BindsManager {

/**
* The actual binds config. This reflects the current status from {@link #updateDefaultBinds(Context)}
* The actual binds config. This reflects the current status from {@link BindsManager#updateConfigWithDefaultBinds()}.
* and all further modifications like modifications in the user input settings.
* @return

Check warning on line 39 in engine/src/main/java/org/terasology/engine/core/subsystem/config/BindsManager.java

View check run for this annotation

Terasology Jenkins.io / JavaDoc

JavaDoc @return

NORMAL: no description for @return
*/
BindsConfig getBindsConfig();

/**
* The default bindings. This reflects the current status from {@link #updateDefaultBinds(Context)}.
* The default bindings. This reflects the current status from {@link BindsManager#updateConfigWithDefaultBinds()}.
* @return Returns the default bindings. Changes to this config modify the actual instance
* but become invalid the next time {@link #updateConfigWithDefaultBinds()} is called.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ private BindableAxis registerBindAxis(String id, BindAxisEvent event, BindableBu
}

private BindableAxis registerRealBindAxis(String id, BindAxisEvent event) {
BindableRealAxis axis = new BindableRealAxis(id.toString(), event);
BindableRealAxis axis = new BindableRealAxis(id, event);
axisBinds.add(axis);
axisLookup.put(id, axis);
return axis;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,9 @@ public boolean addPrefab(String prefabName) {
}
}
/**
* Adds all of the components from a prefab to this builder
* Adds all components from a prefab to this builder
*
* @param prefab the prefab to add
* @return whether the prefab was successfully added
*/
public void addPrefab(Prefab prefab) {
if (prefab != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ public interface EntityPool {
* All entities containing every one of the given components.
* <p>
* Implementation note:
* <a href="http://www.angelikalanger.com/GenericsFAQ/FAQSections/ProgrammingIdioms.html#Topic5"
* title="Designing Generic Methods">Java generic types are a mess</a>, especially where
* varargs are involved. We can't use {@link SafeVarargs @SafeVarargs} on any interface methods
* Java generic types are a mess (see <a href="http://www.angelikalanger.com/GenericsFAQ/FAQSections/ProgrammingIdioms.html#Topic5">
* Designing Generic Methods</a>), especially where varargs are involved.
* We can't use {@link SafeVarargs @SafeVarargs} on any interface methods
* because there is no way to know the <em>implementations</em> of the interface are safe.
* <p>
* In this case, in practice, there are few (if any) callers that pass more than two values.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,8 @@ public boolean isAlwaysRelevant() {

@Override
public void setAlwaysRelevant(boolean alwaysRelevant) {
if (exists()) {
if (alwaysRelevant != isAlwaysRelevant()) {
setScope(alwaysRelevant ? GLOBAL : CHUNK);
}
if (exists() && alwaysRelevant != isAlwaysRelevant()) {
setScope(alwaysRelevant ? GLOBAL : CHUNK);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,13 +605,9 @@ public void saveComponent(long entityId, Component component) {

public Optional<EngineEntityPool> getPool(long id) {
Optional<EngineEntityPool> pool = Optional.ofNullable(poolMap.get(id));
if (!pool.isPresent()) {
if (id != NULL_ID) {
// TODO: Entity pools assignment is not needed as of now, can be enabled later on when necessary.
if (!isExistingEntity(id)) {
logger.error("Entity {} doesn't exist", id);
}
}
// TODO: Entity pools assignment is not needed as of now, can be enabled later on when necessary.

Check warning on line 608 in engine/src/main/java/org/terasology/engine/entitySystem/entity/internal/PojoEntityManager.java

View check run for this annotation

Terasology Jenkins.io / Open Tasks Scanner

TODO

NORMAL: Entity pools assignment is not needed as of now, can be enabled later on when necessary.
if (!pool.isPresent() && id != NULL_ID && !isExistingEntity(id)) {
logger.error("Entity {} doesn't exist", id);
}
return pool;
}
Expand Down Expand Up @@ -707,16 +703,6 @@ protected void notifyComponentChanged(EntityRef changedEntity, Class<? extends C
}
}

/**
* This method gets called when the entity gets reactivated. e.g. after storage an entity needs to be reactivated.
*/
private void notifyReactivation(EntityRef entity, Collection<Component> components) {
for (EntityChangeSubscriber subscriber : subscribers) {
subscriber.onReactivation(entity, components);
}
}


/**
* This method gets called before an entity gets deactivated (e.g. for storage).
*/
Expand Down
Loading

0 comments on commit 1cef7c1

Please sign in to comment.