Skip to content

Commit

Permalink
Some fixes to c421d72
Browse files Browse the repository at this point in the history
Checkstyle fixes.
  • Loading branch information
immortius committed Oct 16, 2013
1 parent 55f7888 commit 42870d3
Show file tree
Hide file tree
Showing 23 changed files with 22 additions and 35 deletions.
1 change: 1 addition & 0 deletions config/checkstyle/suppressions.xml
Expand Up @@ -6,4 +6,5 @@
<suppressions>
<suppress checks="." files="org[\\/]terasology[\\/]protobuf[\\/].*"/>
<suppress checks="MethodNameCheck" files="org[\\/]terasology[\\/]engine[\\/]paths[\\/]windows.*"/>
<suppress checks="ParameterAssignment" files="org[\\/]terasology[\\/]math[\\/]TeraMath"/>
</suppressions>
3 changes: 3 additions & 0 deletions src/main/java/org/terasology/engine/GameThread.java
Expand Up @@ -37,6 +37,9 @@ public final class GameThread {
private static Thread gameThread;
private static BlockingDeque<Runnable> pendingRunnables = Queues.newLinkedBlockingDeque();

private GameThread() {
}

/**
* @return Whether the currentThread is the gameThread.
*/
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/org/terasology/math/MatrixUtils.java
Expand Up @@ -30,6 +30,9 @@
*/
public final class MatrixUtils {

private MatrixUtils() {
}

/**
* Copies the given matrix into a newly allocated FloatBuffer.
* The order of the elements is column major (as used by OpenGL).
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/terasology/math/Rect2i.java
Expand Up @@ -17,15 +17,14 @@

import com.google.common.collect.Lists;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

/**
* 2D Rectangle
*/
// TODO: Review and bring into line with Region3i's api
public class Rect2i {
public final class Rect2i {
public static final Rect2i EMPTY = new Rect2i();

// position
Expand Down Expand Up @@ -184,6 +183,7 @@ public String toString() {

/**
* Returns the difference between a and b - that is all parts of a that are not contained by b.
*
* @param a
* @param b
* @return A collection of rectangles that compose the difference of a - b. May be empty if a is completely encompassed by b.
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/terasology/math/Region3i.java
Expand Up @@ -24,7 +24,7 @@
*
* @author Immortius
*/
public class Region3i implements Iterable<Vector3i> {
public final class Region3i implements Iterable<Vector3i> {
public static final Region3i EMPTY = new Region3i();

private final Vector3i min = new Vector3i();
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/terasology/math/TeraMath.java
Expand Up @@ -163,7 +163,7 @@ public static int pow(int base, int exp) {
if (exp <= 0) {
// x^0 and 1/(1^x) for any x are the only cases where an integer could represent a non-zero value
// 0^0 is an indetermination, but Integers provides no means to represent it
if (exp == 0) {
if (exp == 0 || base == 1) {
return 1;
}
return 0;
Expand Down Expand Up @@ -395,7 +395,8 @@ public static int calcChunkPosY(int y) {
* @return The Y-coordinate of the chunk
*/
public static int calcChunkPosY(int y, int chunkPowerY) {
return 0;//(y >> chunkPowerY);
return 0;
//return (y >> chunkPowerY);
}

/**
Expand Down
Expand Up @@ -43,7 +43,6 @@
import java.awt.event.MouseWheelEvent;
import java.awt.event.MouseWheelListener;
import java.awt.image.BufferedImage;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
Expand Down
Expand Up @@ -19,7 +19,6 @@
import org.terasology.asset.Asset;
import org.terasology.rendering.assets.material.Material;
import org.terasology.rendering.assets.mesh.Mesh;
import org.terasology.rendering.nui.Color;
import org.terasology.rendering.nui.HorizontalAlignment;

import java.util.List;
Expand Down
Expand Up @@ -18,14 +18,7 @@
import com.google.common.collect.Maps;
import gnu.trove.map.TIntObjectMap;
import gnu.trove.map.hash.TIntObjectHashMap;
import org.terasology.asset.AssetType;
import org.terasology.asset.AssetUri;
import org.terasology.asset.Assets;
import org.terasology.engine.Terasology;
import org.terasology.engine.TerasologyConstants;
import org.terasology.persistence.ModuleContext;
import org.terasology.rendering.assets.material.Material;
import org.terasology.rendering.assets.material.MaterialData;
import org.terasology.rendering.assets.texture.Texture;

import java.util.Map;
Expand Down
Expand Up @@ -20,9 +20,7 @@
import org.terasology.asset.AssetType;
import org.terasology.asset.AssetUri;
import org.terasology.asset.Assets;
import org.terasology.engine.TerasologyConstants;
import org.terasology.engine.module.Module;
import org.terasology.persistence.ModuleContext;
import org.terasology.rendering.assets.material.Material;
import org.terasology.rendering.assets.material.MaterialData;
import org.terasology.rendering.assets.texture.Texture;
Expand Down
1 change: 0 additions & 1 deletion src/main/java/org/terasology/rendering/cameras/Camera.java
Expand Up @@ -18,7 +18,6 @@
import org.terasology.config.Config;
import org.terasology.engine.CoreRegistry;
import org.terasology.math.MatrixUtils;
import org.terasology.math.TeraMath;

import javax.vecmath.Matrix4f;
import javax.vecmath.Vector3f;
Expand Down
Expand Up @@ -18,7 +18,6 @@
import org.lwjgl.opengl.GL11;
import org.terasology.engine.CoreRegistry;
import org.terasology.math.MatrixUtils;
import org.terasology.math.TeraMath;
import org.terasology.rendering.oculusVr.OculusVrHelper;
import org.terasology.rendering.world.WorldRenderer;

Expand Down
Expand Up @@ -17,7 +17,6 @@

import org.lwjgl.opengl.GL11;
import org.terasology.math.MatrixUtils;
import org.terasology.math.TeraMath;

import static org.lwjgl.opengl.GL11.GL_PROJECTION;
import static org.lwjgl.opengl.GL11.glMatrixMode;
Expand Down
@@ -1,4 +1,3 @@

/*
* Copyright 2013 MovingBlocks
*
Expand Down
Expand Up @@ -24,9 +24,6 @@
import org.terasology.rendering.nui.HorizontalAlignment;
import org.terasology.rendering.nui.LwjglCanvas;
import org.terasology.rendering.nui.ScaleMode;
import org.terasology.rendering.nui.SubRegion;

import javax.vecmath.Vector2f;

/**
* @author Immortius
Expand Down
Expand Up @@ -38,7 +38,6 @@
import org.terasology.logic.location.Location;
import org.terasology.logic.location.LocationComponent;
import org.terasology.math.MatrixUtils;
import org.terasology.math.TeraMath;
import org.terasology.rendering.assets.animation.MeshAnimation;
import org.terasology.rendering.assets.animation.MeshAnimationFrame;
import org.terasology.rendering.assets.material.Material;
Expand Down
2 changes: 0 additions & 2 deletions src/main/java/org/terasology/rendering/nui/Canvas.java
Expand Up @@ -20,8 +20,6 @@
import org.terasology.rendering.assets.font.Font;
import org.terasology.rendering.assets.texture.Texture;

import javax.vecmath.Vector2f;

/**
* Canvas provides primitive drawing operations for use by the UI.
*
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/terasology/rendering/nui/LineBuilder.java
Expand Up @@ -31,7 +31,7 @@ public class LineBuilder {
private final int maxWidth;
private List<String> lines = Lists.newArrayList();

private int currentLineLength = 0;
private int currentLineLength;
private StringBuilder lineBuilder = new StringBuilder();

public LineBuilder(Font font, int maxWidth) {
Expand All @@ -48,7 +48,8 @@ public static List<String> getLines(Font font, String text, int maxWidth) {

public void addText(String text) {
List<String> paragraphs = Arrays.asList(text.split("\\r?\\n"));
for (String remainder : paragraphs) {
for (String paragraph : paragraphs) {
String remainder = paragraph;

This comment has been minimized.

Copy link
@ahoehma

ahoehma Oct 16, 2013

Contributor

Is this necessary ? :-)

This comment has been minimized.

Copy link
@socram8888

socram8888 Oct 16, 2013

Contributor

It looks like it isn't.

This comment has been minimized.

Copy link
@immortius

immortius Oct 16, 2013

Author Member

It is, because checkstyle complains about modifying control variables of loops.

Or to be less slavish to the opinions of machines, you shouldn't be changing the value of paragraph because it changes its meaning which can cause programmer error.

This comment has been minimized.

Copy link
@immortius

immortius Oct 16, 2013

Author Member

At any rate I wrote this code so you shouldn't be answering, socram8888. :p

This comment has been minimized.

Copy link
@socram8888

socram8888 Oct 16, 2013

Contributor

For some reason I'm being notified every time somebody replies in this commit :P

while (!remainder.isEmpty()) {
String[] split = remainder.split(" ", 2);
String word = split[0];
Expand Down
1 change: 0 additions & 1 deletion src/main/java/org/terasology/rendering/nui/ScaleMode.java
Expand Up @@ -16,7 +16,6 @@
package org.terasology.rendering.nui;

import org.terasology.math.Rect2i;
import org.terasology.math.Vector2i;

import javax.vecmath.Vector2f;

Expand Down
Expand Up @@ -34,7 +34,6 @@
import org.terasology.asset.AssetUri;
import org.terasology.engine.CoreRegistry;
import org.terasology.math.MatrixUtils;
import org.terasology.math.TeraMath;
import org.terasology.rendering.ShaderManager;
import org.terasology.rendering.assets.material.Material;
import org.terasology.rendering.assets.material.MaterialData;
Expand Down
Expand Up @@ -153,7 +153,7 @@ public boolean randomBoolean() {
public String randomCharacterString(int length) {
char[] randomChars = new char[length];
for (int i = 0; i < length; i++) {
randomChars[i] = VALID_CHARS[VALID_CHARS.length * TeraMath.fastAbs(randomDouble())];
randomChars[i] = VALID_CHARS[randomInt(VALID_CHARS.length)];

This comment has been minimized.

Copy link
@socram8888

socram8888 Oct 16, 2013

Contributor

randomInt can return a negative value. This line should be changed to:

randomChars[i] = VALID_CHARS[randomIntAbs(VALID_CHARS.length)];

This comment has been minimized.

Copy link
@immortius

immortius via email Oct 16, 2013

Author Member

This comment has been minimized.

Copy link
@immortius

immortius via email Oct 16, 2013

Author Member

This comment has been minimized.

Copy link
@socram8888

socram8888 Oct 16, 2013

Contributor

I think it would be a good idea to make all random methods to return positive numbers only, because they're the most used. And for those wanting negative numbers too, all they need to do is to add a certain bias (ie for ints from -5 to 5, use randomInt(10) - 5)

To remove negative integers, all we need to do is a binary and from the seed and Long.MAX_VALUE, therefore clearing the MSB bit.

This comment has been minimized.

Copy link
@socram8888

socram8888 Oct 16, 2013

Contributor

Or instead of removing negative numbers, rename the functions from "randomInt" to "signedInt" (IMHO having "random" in the method name of a RNG is kinda pointless), and adding others like "unsignedInt", which look more clear.

This comment has been minimized.

Copy link
@immortius

immortius Oct 16, 2013

Author Member

I would just get rid of the negative version - nothing intentionally uses it. Possible nextInt for getAbsInteger.... but would want to check the API of Random first.

The more fundamental issue is the current implementation is biased.

}
return new String(randomChars);
}
Expand Down
Expand Up @@ -18,12 +18,12 @@
import org.terasology.math.TeraMath;
import org.terasology.math.Vector3i;
import org.terasology.world.block.Block;
import org.terasology.world.block.BlockManager;
import org.terasology.world.chunks.Chunk;
import org.terasology.world.chunks.ChunkProvider;

/**
* A base world view implementation sitting on ChunkProvider.
*
* @author Immortius
*/
public abstract class AbstractFullWorldView implements PropagatorWorldView {
Expand Down Expand Up @@ -53,8 +53,9 @@ public byte getValueAt(Vector3i pos) {

/**
* Obtains the relevant value from the given chunk
*
* @param chunk
* @param pos The internal position of the chunk to get the value from
* @param pos The internal position of the chunk to get the value from
* @return The relevant value for this view
*/
protected abstract byte getValueAt(Chunk chunk, Vector3i pos);
Expand All @@ -72,8 +73,9 @@ public void setValueAt(Vector3i pos, byte value) {

/**
* Sets the relevant value for the given chunk
*
* @param chunk
* @param pos The internal position of the chunk to set the value of
* @param pos The internal position of the chunk to set the value of
* @param value The new value
*/
protected abstract void setValueAt(Chunk chunk, Vector3i pos, byte value);
Expand Down
Expand Up @@ -25,7 +25,6 @@
import org.terasology.world.chunks.Chunk;

import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Set;

Expand Down

0 comments on commit 42870d3

Please sign in to comment.