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

refactor: address IntelliJ QAPlug plugin findings #5149

Merged
merged 35 commits into from
Nov 11, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
aa6ca30
qa: make final fields static
jdrueckert Oct 22, 2023
1a53322
qa: remove unnecessary local before return
jdrueckert Oct 22, 2023
db4f626
qa: avoid instanceof checks in catch clause
jdrueckert Oct 22, 2023
e3d80fb
qa: method/constructor should not throw java.lang.Exception
jdrueckert Oct 22, 2023
f22c1fa
qa: instanceof check covers null check
jdrueckert Oct 22, 2023
9d7204b
qa: avoid toString on String objects
jdrueckert Oct 22, 2023
993b370
qa: remove overrides that only call their super
jdrueckert Oct 22, 2023
315ef35
qa: remove unused private methods
jdrueckert Oct 22, 2023
3c5d507
qa: if statement nesting not necessary
jdrueckert Oct 22, 2023
df3539a
qa: remove unused local variable
jdrueckert Oct 22, 2023
b0ec929
qa: close resources after use
jdrueckert Oct 22, 2023
06c5fcf
qa: use equals() to compare object references
jdrueckert Oct 22, 2023
3428369
Merge branch 'develop' into chore/qaplug-findings
jdrueckert Oct 27, 2023
a3f8fbc
fix: reintroduce override, needed to elevate access modifier
jdrueckert Oct 27, 2023
760a174
fix: exception type in override
jdrueckert Oct 27, 2023
f067ba2
chore: replace invalid javadoc character with unicode representation
jdrueckert Oct 27, 2023
077d488
javadoc: fix broken references
jdrueckert Oct 28, 2023
6a816ed
javadoc: fix disallowed self-closing and empty elements
jdrueckert Oct 28, 2023
a27bd8e
javadoc: fix invalid uses of @param and @return
jdrueckert Oct 28, 2023
c93ad63
javadoc: fix bad use of symbols
jdrueckert Oct 28, 2023
ad27ad0
javadoc: fix disallowed list item tag without surrounding list
jdrueckert Oct 28, 2023
cce1a8b
javadoc: fix erroneous tags
jdrueckert Oct 28, 2023
45e6571
javadoc: fix more references
jdrueckert Oct 28, 2023
35c2bc3
javadoc: fix emphasize tags
jdrueckert Oct 28, 2023
60a41a5
chore: fix missing javadoc errors
jdrueckert Oct 28, 2023
3d13e1f
javadoc: fix @see
jdrueckert Oct 29, 2023
848947e
checkstyle: remove illegal throws
jdrueckert Oct 30, 2023
f585450
checkstyle: remove unused imports
jdrueckert Oct 30, 2023
01e8edd
refactor: replace explicit close with try-with-resources
jdrueckert Oct 30, 2023
b841271
revert: removal of unused param from AbstractFamily constructor
jdrueckert Oct 30, 2023
2aad50f
revert: close zip Filesystem
jdrueckert Oct 30, 2023
49725fa
revert: removing required zip.close
jdrueckert Oct 30, 2023
bf26781
Merge branch 'develop' into chore/qaplug-findings
jdrueckert Oct 31, 2023
971c853
Merge branch 'develop' into chore/qaplug-findings
jdrueckert Nov 6, 2023
08bd7ea
Merge branch 'develop' into chore/qaplug-findings
jdrueckert Nov 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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.
if (!pool.isPresent() && id != NULL_ID && !isExistingEntity(id)) {
logger.error("Entity {} doesn't exist", id);
}
return pool;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,8 @@ public boolean updateTarget(Vector3f pos, Vector3f dir, float maxDist) {
EntityRef newTarget = hitInfo.getEntity();

if (hitInfo.isWorldHit()) {
if (targetBlockPos != null) {
if (targetBlockPos.equals(hitInfo.getBlockPosition())) {
return false;
}
if (targetBlockPos != null && targetBlockPos.equals(hitInfo.getBlockPosition())) {
return false;
}
targetBlockPos = hitInfo.getBlockPosition();
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,8 @@ public void onAttackRequest(AttackButton event, EntityRef entity, CharacterHeldI
@ReceiveEvent(components = LocationComponent.class)
public void onAttackRequest(AttackRequest event, EntityRef character, CharacterComponent characterComponent) {
// if an item is used, make sure this entity is allowed to attack with it
if (event.getItem().exists()) {
if (!character.equals(event.getItem().getOwner())) {
return;
}
if (event.getItem().exists() && !character.equals(event.getItem().getOwner())) {
return;
}

OnItemUseEvent onItemUseEvent = new OnItemUseEvent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,10 @@ public void onMessage(MessageEvent event, EntityRef entity) {
ClientComponent client = entity.getComponent(ClientComponent.class);
if (client.local) {
Message message = event.getFormattedMessage();
if (message.getType() == CoreMessageType.CHAT || message.getType() == CoreMessageType.NOTIFICATION) {

if (message.getType() == CoreMessageType.CHAT || message.getType() == CoreMessageType.NOTIFICATION
&& !nuiManager.isOpen(CHAT_UI) && !nuiManager.isOpen(CONSOLE_UI)) {
// show overlay only if chat and console are hidden
if (!nuiManager.isOpen(CHAT_UI) && !nuiManager.isOpen(CONSOLE_UI)) {
overlay.setVisible(true);
}
Comment on lines -75 to -80
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that I sometimes use constructs like this to put emphasis on the logical control flow, trusting in the compiler to be smart about the details.

I find long if conditions with many parts hard to read and to understand, and one has to put quite a lot thought into breaking it up when changing the control flow.

Therefore, I'm not a huge fan of some these changes in particular, but I'm not going to argue about this. Let's see how it plays out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave it as is for now

overlay.setVisible(true);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,8 @@ public boolean execute(String rawCommand, EntityRef callingClient) {

ClientComponent cc = callingClient.getComponent(ClientComponent.class);

if (cc.local) {
if (!rawCommand.isEmpty() && (localCommandHistory.isEmpty() || !localCommandHistory.getLast().equals(rawCommand))) {
localCommandHistory.add(rawCommand);
}
if (cc.local && !rawCommand.isEmpty() && (localCommandHistory.isEmpty() || !localCommandHistory.getLast().equals(rawCommand))) {
localCommandHistory.add(rawCommand);
}

return execute(new Name(commandName), processedParameters, callingClient);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,10 @@ public void initialise() {
if (nuiManager != null) {
overlay = nuiManager.addOverlay(NotificationOverlay.ASSET_URI, NotificationOverlay.class);
console.subscribe((Message message) -> {
if (!nuiManager.isOpen("engine:console")) {
if (!nuiManager.isOpen("engine:console") && message.getType() != CoreMessageType.CHAT
&& message.getType() != CoreMessageType.NOTIFICATION || !nuiManager.isOpen("engine:chat")) {
// make sure the message isn't already shown in the chat overlay
if (message.getType() != CoreMessageType.CHAT && message.getType() != CoreMessageType.NOTIFICATION
|| !nuiManager.isOpen("engine:chat")) {
overlay.setVisible(true);
}
overlay.setVisible(true);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,13 +288,11 @@ public void update(float delta) {

// If an associated held item entity does *not* exist yet, consider making one if the player has an item
// selected
if (currentHeldItem == EntityRef.NULL) {
if (remotePlayer.hasComponent(CharacterHeldItemComponent.class)) {
CharacterHeldItemComponent characterHeldItemComponent =
remotePlayer.getComponent(CharacterHeldItemComponent.class);
if (characterHeldItemComponent != null && !characterHeldItemComponent.selectedItem.equals(EntityRef.NULL)) {
linkHeldItemLocationForRemotePlayer(remotePlayer.getComponent(CharacterHeldItemComponent.class).selectedItem, remotePlayer);
}
if (currentHeldItem == EntityRef.NULL && remotePlayer.hasComponent(CharacterHeldItemComponent.class)) {
CharacterHeldItemComponent characterHeldItemComponent =
remotePlayer.getComponent(CharacterHeldItemComponent.class);
if (characterHeldItemComponent != null && !characterHeldItemComponent.selectedItem.equals(EntityRef.NULL)) {
linkHeldItemLocationForRemotePlayer(remotePlayer.getComponent(CharacterHeldItemComponent.class).selectedItem, remotePlayer);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,10 @@ public void setComponentAdded(int networkId, Class<? extends Component> componen
}

public void setComponentRemoved(int networkId, Class<? extends Component> component) {
if (netRelevant.contains(networkId) && !netInitial.contains(networkId)) {
if (!addedComponents.remove(networkId, component)) {
removedComponents.put(networkId, component);
if (!dirtyComponents.remove(networkId, component)) {
netDirty.add(networkId);
}
if (netRelevant.contains(networkId) && !netInitial.contains(networkId) && !addedComponents.remove(networkId, component)) {
removedComponents.put(networkId, component);
if (!dirtyComponents.remove(networkId, component)) {
netDirty.add(networkId);
}
}
}
Expand Down Expand Up @@ -342,12 +340,11 @@ public void send(Event event, EntityRef target) {
}
} else {
NetworkComponent networkComponent = target.getComponent(NetworkComponent.class);
if (networkComponent != null) {
if (netRelevant.contains(networkComponent.getNetworkId()) || netInitial.contains(networkComponent.getNetworkId())) {
queuedOutgoingEvents.add(NetData.EventMessage.newBuilder()
.setTargetId(networkComponent.getNetworkId())
.setEvent(eventSerializer.serialize(event)).build());
}
if (networkComponent != null && netRelevant.contains(networkComponent.getNetworkId())
|| netInitial.contains(networkComponent.getNetworkId())) {
queuedOutgoingEvents.add(NetData.EventMessage.newBuilder()
.setTargetId(networkComponent.getNetworkId())
.setEvent(eventSerializer.serialize(event)).build());
}
}
} catch (SerializationException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,15 +291,13 @@ public void shutdown() {
server = null;
nextNetId = 1;
netIdToEntityId.clear();
if (mode != NetworkMode.CLIENT) {
if (this.entityManager != null) {
for (EntityRef entity : entityManager.getEntitiesWith(NetworkComponent.class)) {
NetworkComponent netComp = entity.getComponent(NetworkComponent.class);
netComp.setNetworkId(0);
entity.saveComponent(netComp);
}
this.entityManager.unsubscribe(this);
if (mode != NetworkMode.CLIENT && this.entityManager != null) {
for (EntityRef entity : entityManager.getEntitiesWith(NetworkComponent.class)) {
NetworkComponent netComp = entity.getComponent(NetworkComponent.class);
netComp.setNetworkId(0);
entity.saveComponent(netComp);
}
this.entityManager.unsubscribe(this);
}
mode = NetworkMode.NONE;
entityManager = null;
Expand Down Expand Up @@ -328,24 +326,22 @@ public Client joinLocal(String preferredName, Color color) {

@Override
public void update() {
if (mode != NetworkMode.NONE) {
if (entityManager != null) {
processPendingConnections();
processPendingDisconnects();
long currentTimer = time.getRealTimeInMs();
boolean netTick = false;
if (currentTimer > nextNetworkTick) {
nextNetworkTick += NET_TICK_RATE;
netTick = true;
}
PerformanceMonitor.startActivity("Client update");
for (Client client : clientList) {
client.update(netTick);
}
PerformanceMonitor.endActivity();
if (server != null) {
server.update(netTick);
}
if (mode != NetworkMode.NONE && entityManager != null) {
processPendingConnections();
processPendingDisconnects();
long currentTimer = time.getRealTimeInMs();
boolean netTick = false;
if (currentTimer > nextNetworkTick) {
nextNetworkTick += NET_TICK_RATE;
netTick = true;
}
PerformanceMonitor.startActivity("Client update");
for (Client client : clientList) {
client.update(netTick);
}
PerformanceMonitor.endActivity();
if (server != null) {
server.update(netTick);
}
}
}
Expand Down Expand Up @@ -597,14 +593,10 @@ public void connectToEntitySystem(EngineEntityManager newEntityManager, EventLib
public void onEntityComponentAdded(EntityRef entity, Class<? extends Component> component) {
ComponentMetadata<? extends Component> metadata = componentLibrary.getMetadata(component);
NetworkComponent netComp = entity.getComponent(NetworkComponent.class);
if (netComp != null && netComp.getNetworkId() != NULL_NET_ID) {
if (mode.isServer()) {
if (metadata.isReplicated()) {
for (NetClient client : netClientList) {
logger.debug("Component {} added to {}", component, entity);
client.setComponentAdded(netComp.getNetworkId(), component);
}
}
if (netComp != null && netComp.getNetworkId() != NULL_NET_ID && mode.isServer() && metadata.isReplicated()) {
for (NetClient client : netClientList) {
logger.debug("Component {} added to {}", component, entity);
client.setComponentAdded(netComp.getNetworkId(), component);
}
}
updatedOwnedEntities(entity, component, metadata);
Expand All @@ -614,14 +606,10 @@ public void onEntityComponentAdded(EntityRef entity, Class<? extends Component>
public void onEntityComponentRemoved(EntityRef entity, Class<? extends Component> component) {
ComponentMetadata<? extends Component> metadata = componentLibrary.getMetadata(component);
NetworkComponent netComp = entity.getComponent(NetworkComponent.class);
if (netComp != null && netComp.getNetworkId() != NULL_NET_ID) {
if (mode.isServer()) {
if (metadata.isReplicated()) {
for (NetClient client : netClientList) {
logger.debug("Component {} removed from {}", component, entity);
client.setComponentRemoved(netComp.getNetworkId(), component);
}
}
if (netComp != null && netComp.getNetworkId() != NULL_NET_ID && mode.isServer() && metadata.isReplicated()) {
for (NetClient client : netClientList) {
logger.debug("Component {} removed from {}", component, entity);
client.setComponentRemoved(netComp.getNetworkId(), component);
}
}
if (mode.isAuthority() && metadata.isReferenceOwner()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,10 +366,9 @@ private void updateEntity(NetData.UpdateEntityMessage updateEntity) {
boolean blockEntityBefore = currentEntity.hasComponent(BlockComponent.class);
entitySerializer.deserializeOnto(currentEntity, updateEntity.getEntity());
BlockComponent blockComponent = currentEntity.getComponent(BlockComponent.class);
if (blockComponent != null && !blockEntityBefore) {
if (!blockEntityRegistry.getExistingBlockEntityAt(blockComponent.getPosition()).equals(currentEntity)) {
logger.error("Failed to associated new block entity");
}
if (blockComponent != null && !blockEntityBefore
&& !blockEntityRegistry.getExistingBlockEntityAt(blockComponent.getPosition()).equals(currentEntity)) {
logger.error("Failed to associated new block entity");
}
if (netComp.getNetworkId() != updateEntity.getNetId()) {
logger.error("Network ID lost in update: {}, {} -> {}", currentEntity, updateEntity.getNetId(), netComp.getNetworkId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,8 @@ protected Collection<EntityRef> getEntitiesOfChunk(Chunk chunk) {
continue;
}
Vector3f pos = loc.getWorldPosition(new Vector3f());
if (pos.isFinite()) {
if (aabb.containsPoint(loc.getWorldPosition(new Vector3f()))) {
entitiesToStore.add(entity);
}
if (pos.isFinite() && aabb.containsPoint(loc.getWorldPosition(new Vector3f()))) {
entitiesToStore.add(entity);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,16 @@ public RecordingEventSystemDecorator(EventSystem eventSystem, EventCatcher event

@Override
public void send(EntityRef entity, Event event) {
if (currentThreadIsMain()) {
if (recordAndReplayCurrentStatus.getStatus() == RecordAndReplayStatus.RECORDING) {
eventCatcher.addEvent(new PendingEvent(entity, event));
}
if (currentThreadIsMain() && recordAndReplayCurrentStatus.getStatus() == RecordAndReplayStatus.RECORDING) {
eventCatcher.addEvent(new PendingEvent(entity, event));
}
super.send(entity, event);
}

@Override
public void send(EntityRef entity, Event event, Component component) {
if (currentThreadIsMain()) {
if (recordAndReplayCurrentStatus.getStatus() == RecordAndReplayStatus.RECORDING) {
eventCatcher.addEvent(new PendingEvent(entity, event, component));
}
if (currentThreadIsMain() && recordAndReplayCurrentStatus.getStatus() == RecordAndReplayStatus.RECORDING) {
eventCatcher.addEvent(new PendingEvent(entity, event, component));
}
super.send(entity, event, component);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,31 +43,29 @@ public ResourceUrn redirect(ResourceUrn urn) {

@Override
public Optional<MeshData> getAssetData(ResourceUrn urn) throws IOException {
if (TerasologyConstants.ENGINE_MODULE.equals(urn.getModuleName())) {
if (SCREEN_QUAD_RESOURCE_NAME.equals(urn.getResourceName())) {
StandardMeshData data = new StandardMeshData();
Vector3f posDest = new Vector3f();
Vector2f uvDest = new Vector2f();
data.position.put(posDest.set(-1.0f, -1.0f, 0.0f));
data.position.put(posDest.set(-1.0f, 1.0f, 0.0f));
data.position.put(posDest.set(1.0f, 1.0f, 0.0f));
data.position.put(posDest.set(1.0f, -1.0f, 0.0f));
if (TerasologyConstants.ENGINE_MODULE.equals(urn.getModuleName()) && SCREEN_QUAD_RESOURCE_NAME.equals(urn.getResourceName())) {
StandardMeshData data = new StandardMeshData();
Vector3f posDest = new Vector3f();
Vector2f uvDest = new Vector2f();
data.position.put(posDest.set(-1.0f, -1.0f, 0.0f));
data.position.put(posDest.set(-1.0f, 1.0f, 0.0f));
data.position.put(posDest.set(1.0f, 1.0f, 0.0f));
data.position.put(posDest.set(1.0f, -1.0f, 0.0f));

data.indices.put(0);
data.indices.put(1);
data.indices.put(2);
data.indices.put(0);
data.indices.put(1);
data.indices.put(2);

data.indices.put(0);
data.indices.put(2);
data.indices.put(3);
data.indices.put(0);
data.indices.put(2);
data.indices.put(3);

data.uv0.put(uvDest.set(0.0f, 0.0f));
data.uv0.put(uvDest.set(0.0f, 1.0f));
data.uv0.put(uvDest.set(1.0f, 1.0f));
data.uv0.put(uvDest.set(1.0f, 0.0f));
data.uv0.put(uvDest.set(0.0f, 0.0f));
data.uv0.put(uvDest.set(0.0f, 1.0f));
data.uv0.put(uvDest.set(1.0f, 1.0f));
data.uv0.put(uvDest.set(1.0f, 0.0f));

return Optional.of(data);
}
return Optional.of(data);
}
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,8 @@ public final void postInit(Context context) {

public void tryBufferPairPass() {
BufferPairConnection bufferPairConnection = getInputBufferPairConnection(1);
if (bufferPairConnection != null) {
if (bufferPairConnection.getData() != null) {
addOutputBufferPairConnection(1, bufferPairConnection);
}
if (bufferPairConnection != null && bufferPairConnection.getData() != null) {
addOutputBufferPairConnection(1, bufferPairConnection);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,8 @@ public void onOpened() {
if (depth == SortOrderSystem.DEFAULT_DEPTH) {
setDepthAuto();
}
if (SortOrderSystem.isInitialized()) {
if (!SortOrderSystem.getUsed().contains(depth)) {
SortOrderSystem.getUsed().add(depth);
}
if (SortOrderSystem.isInitialized() && !SortOrderSystem.getUsed().contains(depth)) {
SortOrderSystem.getUsed().add(depth);
}
modifyingList = false;
activateBindEvent = false;
Expand Down
Loading