3
\$\begingroup\$

Initial note: You do not need to have knowledge of LWJGL or OpenGL to review this code. The only parts of the code that require LWJGL knowledge are the Game class, and the rest, although it may contain some LWJGL/OpenGL/Slick-util code, it is rather self explanatory for anybody who's worked in Java before.

So I've built this simple game in Java using LWJGL. It's a relatively simple platformer, and I would like some opinions on my code so that I can correct it. Feel free to leave any constructive criticism or thoughts that you may have. Any and all opinions are appreciated. This is my first game, so it's bound to be a bit rough. This is also still early in development. I'll post all the classes that I've written code for so far.

Game class:

import static org.lwjgl.opengl.GL11.*;

import org.lwjgl.LWJGLException;
import org.lwjgl.opengl.Display;
import org.lwjgl.opengl.DisplayMode;
import org.newdawn.slick.opengl.Texture;

import com.hasherr.platformer.entity.Player;

public class Game { // starts game & manages display/init OpenGL

    public Game() {
        initDisplay(800, 500, "Komo"); // Temporary game name - KOMO
    }

    // initiate necessary classes
    Player player = new Player(); // user-representation
    InputHandler inputHandler = new InputHandler(player);
    SpriteHandler spriteHandler = new SpriteHandler();
    TextureHandler textureHandler = new TextureHandler();

    private void initDisplay(int width, int height, String title) {
        try {
            Display.setDisplayMode(new DisplayMode(width, height));
            Display.setTitle(title);
            Display.create();
        } catch (LWJGLException e) {
            e.printStackTrace();
            System.exit(0);
        }
        grabAllTextures();
        while (!Display.isCloseRequested()) {
            Display.update();
            Display.sync(60); // 60 FPS
            inputHandler.handleInput();
            initGL(width, height);
            spriteHandler.drawSprite(playerTexture, player.xPos, player.yPos);
        }

        Display.destroy();
    }

    private void initGL(int width, int height) {
        glClear(GL_COLOR_BUFFER_BIT); // clear screen for efficient rendering
        glMatrixMode(GL_PROJECTION);
        glLoadIdentity();
        glOrtho(0, width, 0, height, 1, -1); // set orthographic view
        glMatrixMode(GL_MODELVIEW);
        glEnable(GL_TEXTURE_2D); // enable 2d rendering for textures & sprites
    }

    // Textures (only one so far)
    Texture playerTexture;

    private void grabAllTextures() { // basically TextHandler in one method.
        playerTexture = textureHandler.grabTexture("Png", "scary_monster"); // on tex so far
    }

    public static void main(String[] args) { // start yer engines
        new Game(); // start the show, ready go
    }

}

InputHandler Class:

import org.lwjgl.input.Keyboard;
import org.lwjgl.input.Mouse;

import com.hasherr.platformer.entity.Player;

public class InputHandler { // handles all input

    Player player;

    public InputHandler(Player targetToHandle) {
        player = targetToHandle;
    }

    public void handleInput() {
        // keyboard input
        if (!Keyboard.isKeyDown(Keyboard.KEY_D) && !Keyboard.isKeyDown(        // no 
                Keyboard.KEY_A) && !Keyboard.isKeyDown(Keyboard.KEY_SPACE)) { // input
            player.moveSpeed = 10.0f;
        }
        if (Keyboard.isKeyDown(Keyboard.KEY_D)) {
            player.moveRight(); // move right
        } 
        if (Keyboard.isKeyDown(Keyboard.KEY_A)) {
            player.moveLeft(); // move left
        } 
        if (Keyboard.isKeyDown(Keyboard.KEY_SPACE)) {
            // jump
        }

        // mouse input
        if (Mouse.isButtonDown(0)) {

        }
    }
}

SpriteHandler class:

import org.newdawn.slick.opengl.Texture;

import static org.lwjgl.opengl.GL11.*;

public class SpriteHandler { // draws all textures onto quads & handles importing of textures

    public void drawSprite(Texture texture, float x, float y) {

    float width = texture.getImageWidth();
    float height = texture.getImageHeight();

    texture.bind(); // wrap it before you tap it. (bind it)
    glBegin(GL_QUADS);
        glTexCoord2f(0, 1);
        glVertex2f(x, y);

        glTexCoord2f(1, 1);
        glVertex2f(x + width, y);

        glTexCoord2f(1, 0);
        glVertex2f(x + width, y + height);

        glTexCoord2f(0, 0);
        glVertex2f(x, y + height);
    glEnd();

    }

}

TextureHandler class:

import java.io.IOException;

import org.newdawn.slick.opengl.Texture;
import org.newdawn.slick.opengl.TextureLoader;
import org.newdawn.slick.util.ResourceLoader;

public class TextureHandler {

    Texture texture;

    public Texture grabTexture(String ext, String name) {
        try {
            texture = TextureLoader.getTexture(
                    ext.toUpperCase(), ResourceLoader.getResourceAsStream(
                            "/res/textures/" + name + "." + ext.toLowerCase()));
        } catch (IOException e) {
            e.printStackTrace();
        }

        return texture;
    }
}

And last but not least, the Player class:

public class Player { // controls Player & player physics

    // velocity & physics fields
    private float angle = 30.0f;
    private float acceleration = 3.0f;

    private float scaleX = (float) Math.cos(angle);
    private float scaleY = (float) Math.sin(angle);

    public float moveSpeed = 16.0f; // initial value, obv. set to move
    private final static int MAX_MOVE_SPEED = 150;

    private float xVelocity = scaleX * moveSpeed;
    private float yVelocity = scaleY * moveSpeed;

    // position fields
    public float xPos = 0; // init
    public float yPos = 0; // init 

    private void printPos() {
        System.out.println("Movespeed: " + moveSpeed + ", X-Velocity: "
            + xVelocity);
    }

    private void handleAcceleration(float force) {
        if (moveSpeed < MAX_MOVE_SPEED && moveSpeed > -MAX_MOVE_SPEED) {
            moveSpeed += force;
            xVelocity = scaleX * moveSpeed;
        } else if (moveSpeed >= MAX_MOVE_SPEED) {
            moveSpeed = MAX_MOVE_SPEED;
            xVelocity = scaleX * moveSpeed;
        } else if (moveSpeed <= -MAX_MOVE_SPEED) {
            moveSpeed = -MAX_MOVE_SPEED;
            xVelocity = scaleX * moveSpeed;
        }

        xPos += xVelocity;
    }

    public void moveRight() {
        handleAcceleration(acceleration);
        printPos();
    }

    public void moveLeft() {
        handleAcceleration(-acceleration);
        printPos();
    }

    public void jump() {
        // jump (spacebar)
    }

    public void shoot() {
        // shoot (mouse)
    }
}

Like I said, these are only the classes that I've coded so far. All my code is open-source, so if you want to view the whole repository, you can view it here. Let me know if you guys need anything else, as I'll be glad to help anybody who is willing to help me.

\$\endgroup\$
3
\$\begingroup\$

General:

Dont use } else { or } catch(...) { because it breaks the brace rule and it's not that readable.

Do code a Vector class because it groups your coordinates together so the code gets more easily readable and managable.


SpriteHandler Class:

 glBegin(GL_QUADS);
    glTexCoord2f(0, 1);
    glVertex2f(x, y);

Don't format the code this way because you don't open a new brace level.


TextureHandler Class:

    } catch (IOException e) {
        e.printStackTrace();
    }

don't eat important error/fault information, propagate it further downwards with a exception or a null result which is checked.

Don't call the class Handler because a Handler handles player input or a fault.


Player Class:

    if (moveSpeed < MAX_MOVE_SPEED && moveSpeed > -MAX_MOVE_SPEED) {
        moveSpeed += force;
        xVelocity = scaleX * moveSpeed;
    } else if (moveSpeed >= MAX_MOVE_SPEED) {
        moveSpeed = MAX_MOVE_SPEED;
        xVelocity = scaleX * moveSpeed;
    } else if (moveSpeed <= -MAX_MOVE_SPEED) {
        moveSpeed = -MAX_MOVE_SPEED;
        xVelocity = scaleX * moveSpeed;
    }

one word: redudandency

You should try to avoid it as best as possible, and on top of that there is no else part, so a better code is

    if (abs(moveSpeed) < MAX_MOVE_SPEED) {
        moveSpeed += force;
    }
    else {
        moveSpeed = clamp(moveSpeed, -MAX_MOVE_SPEED, MAX_MOVE_SPEED);
    }

    xVelocity = scaleX * moveSpeed;
\$\endgroup\$
3
\$\begingroup\$

You have a Texture texture; field in your TextureHandler class that seems to only be used in one method, and should therefore be a local variable.

public class TextureHandler {
    public Texture grabTexture(String ext, String name) {
        try {
            texture = TextureLoader.getTexture(
                    ext.toUpperCase(), ResourceLoader.getResourceAsStream(
                            "/res/textures/" + name + "." + ext.toLowerCase()));
            return texture;
        } catch (IOException e) {
            e.printStackTrace();
            return null;
        }
    }
}

I also think that you should make better use of the private keyword. There are several fields in your classes that don't need the "default" visibility but should rather be marked private.

Besides this, I agree with Quonux's answer (except the first part about } else { or } catch(...) { which you are doing totally fine).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.