5
\$\begingroup\$

I have made a Snake clone using Java 14 and JavaFX 14. My game uses an anonymous instance of the AnimationTimer class as the game loop. The basic UI for the start screen uses FXML, but all of the UI elements in the actual game Scene were added programmatically.

The game board is stored as both a GridPane and as a 2D array of Square objects. Each Square extends javafx.scene.control.Label. The GridPane is used to display the game to the user, and the 2D array is used internally to handle the game logic. Every instance of Square in addition to being a Label, also has added instance variables whose getters and setters are used in conjunction with the GameLogic class. An instance of the GameLogic class is created by the GUI class, which handles the UI.

The basic idea of the program is that each Square stores the direction that the snake body part on that Square should move in when the next frame loads. The head of Snake assigns these directions. The direction that the snake head assigns to the next Square is based on which arrow key the user has most recently pressed. The head of the snake is also used to determine the game over conditions based on whether it has hit the edge of the board or another snake body part. The tail of the snake can either leave its former Square empty or not depending on whether the head has "eaten" the apple. That's how the snake gets longer when an apple is eaten. The snake is defined as the Squares on the board that are also contained in a particular List<Square>. The head is the Square in the List<Square> located at index 0. The tail is located at index size() - 1.

Thus, the structure of my program can be summarized as follows: At the top level is a GUI class which contains an instance of the GameLogic class which includes a 2D array of Square objects. The GUI class is called by a start screen which is controlled by a Main class and an FXML file called start.fxml.

I am going to outline the five files of this program. All but one, start.fxml, are .java files. Feel free to look at them all together, or just review an individual file. The main files in this game are GameLogic.java and GUI.java, which control the internal logic of the game and the user interface, respectively.

First, the start screen: Main.java

import javafx.application.Application;
import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.fxml.FXMLLoader;
import javafx.scene.Node;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.stage.Stage;

import java.io.IOException;

public class Main extends Application {
    @Override
    public void start(Stage stage) throws IOException {
        // Stage set up
      
        // Add title
        stage.setTitle("Snake");

        // Create root from FXML file
        Parent root = FXMLLoader.load(getClass().getResource("start.fxml"));

        // Create a Scene using that root, and set that as the Scene for the Stage
        stage.setScene(new Scene(root));

        // Show the Stage to the user
        stage.show(); 
    }

    @FXML
    private void startButtonClicked(ActionEvent actionEvent) {
        // This method starts the game when the user clicks on the Button
        
        // First we get the Button that the user clicked
        Node source = (Node) (actionEvent.getSource());

        // We use that button to get the Stage
        Stage stage = (Stage) (source.getScene().getWindow());

        // We get the game Scene from GUI.java, and set that as the Scene for the Stage
        stage.setScene(GUI.getGameScene()); 
    }

    public static void main(String[] args) {
        launch(args); // launch the program
    }
}

Most of this is just JavaFX boilerplate code. This class is both the point of entry for the program, and the controller for start.fxml.

Which brings us to: start.fxml

<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.scene.control.*?>
<?import javafx.scene.layout.*?>
<?import javafx.scene.text.*?>

<VBox fx:controller="Main" alignment="CENTER" maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity" prefHeight="111.0" prefWidth="296.0" spacing="20.0" style="-fx-background-color: rgb(30, 30, 30);" xmlns="http://javafx.com/javafx/10.0.2-internal" xmlns:fx="http://javafx.com/fxml/1">
   <Label alignment="CENTER" text="Welcome to Snake" textAlignment="CENTER" textFill="WHITE">
      <font>
         <Font name="Century Gothic" size="20.0" />
      </font>
   </Label>
   <Button alignment="CENTER" mnemonicParsing="false" onAction="#startButtonClicked" style="-fx-background-color: transparent; -fx-border-color: white;" text="Start" textAlignment="CENTER" textFill="WHITE">
      <font>
         <Font name="Century Gothic" size="15.0" />
      </font>
   </Button>
</VBox>

I wasn't able to add comments to the code because I do not know how to write XML. This code was written with the JavaFX SceneBuilder.


Now for the game itself. I'm going to work bottom up, posting Square.java, then GameLogic.java and lastly GUI.java. But first, I need to point out that I am using the following enum throughout the program.

Direction.java

public enum Direction {
    UP, DOWN, RIGHT, LEFT
}

Square.java

import javafx.scene.control.Label;

public class Square extends Label {
    
    // Stores the Square's location in the 2D array
    private final int row;
    private final int column;
    
    // The board has a checkerboard patter, some some Squares are white and some are black
    private final boolean white;

    // The user controls the snake and attempts to get to a special square which is an apple. This boolean determines if this is that Square
    private boolean apple;
    
    // This is the direction that the particular snake body part should travel in if it is on this square
    private Direction direction;
    
    /*The rest of the methods are the standard constructor, getters, and setters*/
    
    public Square(int row, int column, boolean white) {
        super();
        this.row = row;
        this.column = column;
        this.white = white;
        apple = false;
        direction = null;
        setMaxHeight(15);
        setMaxWidth(15);
        setMinWidth(15);
        setMinHeight(15);
    }

    public int getRow() {
        return row;
    }

    public int getColumn() {
        return column;
    }

    public boolean isWhite() {
        return white;
    }

    public boolean isApple() {
        return apple;
    }

    public void setApple(boolean apple) {
        this.apple = apple;
    }

    public Direction getDirection() {
        return direction;
    }

    public void setDirection(Direction direction) {
        this.direction = direction;
    }
}

The GameLogic class contains both a 2D array of Square objects, and a special List of Square objects which identifies those Squares that the snake is currently on.

GameLogic.java

import java.util.List;
import java.util.Random;

public class GameLogic {

    // The game board
    private final Square[][] board;

    // The particular Squares on the game board that the snake is on
    // The Square at index 0 is always the head
    // The Square at index snake.size() - 1 is always the tail
    private final List<Square> snake;

    // Standard constructor
    public GameLogic(Square[][] board, List<Square> snake) {
        this.board = board;
        this.snake = snake;
    }

    // Change the direction that the head of the snake should move in
    public void changeDirection(Direction direction) {
        Square head = snake.get(0);
        if ((head.getDirection() == Direction.UP && direction == Direction.DOWN) ||
                (head.getDirection() == Direction.DOWN && direction == Direction.UP) ||
                (head.getDirection() == Direction.RIGHT && direction == Direction.LEFT) ||
                (head.getDirection() == Direction.LEFT && direction == Direction.RIGHT)) return;

        head.setDirection(direction);
    }

    // This method increments the game by performing the next move
    public boolean nextMove() {
        
        // Identify the row and column of the head
        int row = snake.get(0).getRow();
        int column = snake.get(0).getColumn();
        
        // Create a variable that each square on the snake should replace itself with when the snake moves
        Square nextSquare = null;
        
        // Has the snake eaten an apple this move? Assume no at first
        boolean ateTheApple = false;

        // Determine which direction the snake should move in
        // I will only add comments to the first case, since they all function in the exact same way
        switch (snake.get(0).getDirection()) {
            case UP:
                
                // If the snake is trying to move off the board, or if the place it is moving to is on its body, game over
                if (row == 0 || snake.contains(board[row - 1][column])) return false;
                
                // Otherwise, we can now instantiate nextSquare
                nextSquare = board[row - 1][column];
                
                // Thee head is the only body part that passes its direction to nextSquare
                nextSquare.setDirection(snake.get(0).getDirection());
                
                // Set nextSquare to be the head
                snake.set(0, nextSquare);
                break;
            case DOWN:
                if (row == board.length - 1 || snake.contains(board[row + 1][column])) return false;
                nextSquare = board[row + 1][column];
                nextSquare.setDirection(snake.get(0).getDirection());
                snake.set(0, nextSquare);
                break;
            case RIGHT:
                if (column == board[0].length - 1 || snake.contains(board[row][column + 1])) return false;
                nextSquare = board[row][column + 1];
                nextSquare.setDirection(snake.get(0).getDirection());
                snake.set(0, nextSquare);
                break;
            case LEFT:
                if (column == 0 || snake.contains(board[row][column - 1])) return false;
                nextSquare = board[row][column - 1];
                nextSquare.setDirection(snake.get(0).getDirection());
                snake.set(0, nextSquare);
                break;
        }

        // If the nextSquare variable is an apple
        if (nextSquare.isApple()) {
            
            // We don't want this Square to be an apple in the next frame, as the snake's head is currently on it
            nextSquare.setApple(false);
            
            // We have eaten the apple
            ateTheApple = true;
        }

        // Loop through the rest of the body parts except for the tail
        for (int i = 1; i < snake.size() - 1; i++) {
            switch (snake.get(i).getDirection()) {
                case UP:
                    nextSquare = board[snake.get(i).getRow() - 1][snake.get(i).getColumn()];
                    break;
                case DOWN:
                    nextSquare = board[snake.get(i).getRow() + 1][snake.get(i).getColumn()];
                    break;
                case RIGHT:
                    nextSquare = board[snake.get(i).getRow()][snake.get(i).getColumn() + 1];
                    break;
                case LEFT:
                    nextSquare = board[snake.get(i).getRow()][snake.get(i).getColumn() - 1];
                    break;
            }
            
            // Move the body part to nextSquare
            snake.set(i, nextSquare);
        }

        // Identify the tail
        Square tail = snake.get(snake.size() - 1);
        
        switch (tail.getDirection()) {
            case UP:
                nextSquare = board[tail.getRow() - 1][tail.getColumn()];
                break;
            case DOWN:
                nextSquare = board[tail.getRow() + 1][tail.getColumn()];
                break;
            case RIGHT:
                nextSquare = board[tail.getRow()][tail.getColumn() + 1];
                break;
            case LEFT:
                nextSquare = board[tail.getRow()][tail.getColumn() - 1];
                break;
        }
        
        // Move the tail
        snake.set(snake.size() - 1, nextSquare);

        // If we ate the apple
        if (ateTheApple) {
            
            // Add the former tail right back to increase the length of the tail 
            snake.add(tail);
            
            // Find a random spot to place the new apple
            Random random = new Random();
            int r, c;
            while (true) {
                r = random.nextInt(board.length);
                c = random.nextInt(board[0].length);
                if (!snake.contains(board[r][c])) {
                    board[r][c].setApple(true);
                    break;
                }
            }
        }

        // Were done. The move worked, so we return true
        return true;
    }

    // Given the current state of the new board, repaint all the Squares
    public void paintBoard() {
        for (Square[] row : board) {
            for (Square square : row) {
                if (square == null) {
                    System.out.println("Square is null");
                    return;
                }
                if (snake.contains(square)) {
                    square.setStyle("-fx-background-color: green;");
                    continue;
                }
                if (square.isApple()) {
                    square.setStyle("-fx-background-color: red;");
                    continue;
                }
                square.setStyle("-fx-background-color: " + (square.isWhite()? "rgb(200, 200, 200)" : "rgb(50, 50, 50)") + ";");
            }
        }
    }
}

Finally, an instance of the GameLogic class is created by the GUI class which displays the game to the user

GUI.java

import javafx.animation.AnimationTimer;
import javafx.scene.Scene;
import javafx.scene.layout.GridPane;

import java.util.ArrayList;
import java.util.List;
import java.util.Random;

public class GUI {
    public static Scene getGameScene() {
        
        // This GridPane stores the board
        GridPane grid = new GridPane();
        
        // This 2D array also stores the board
        Square[][] board = new Square[30][30];
        
        // This identifies which Squares are on the snake
        List<Square> snake = new ArrayList<>();

        // Loop through the board and initialize the Squares
        int count = 0, i, j;
        for (i = 0; i < board.length; i++) {
            for (j = 0; j < board[0].length; j++) {
                board[i][j] = new Square(i, j, (i + count) % 2 == 0);
                count++;
                grid.add(board[i][j], j, i);
                
                // If the Square is add the starting location, place a snake body part there
                // and set its direction to be RIGHT by default
                if (i == 10 && j >= 10 && j <= 12) {
                    board[i][j].setDirection(Direction.RIGHT);
                    snake.add(0, board[i][j]);
                }
            }
        }

        // Place the apple somewhere random
        Random random = new Random();
        int r, c;
        while (true) {
            r = random.nextInt(30);
            c = random.nextInt(30);
            if (!snake.contains(board[r][c])) {
                board[r][c].setApple(true);
                break;
            }
        }

        // Create an instance of GameLogic. Pass it the board and the list of snake body parts
        GameLogic snakeGame = new GameLogic(board, snake);
        
        // Paint the initial board
        snakeGame.paintBoard();
        
        // Create a scene and add the GridPane to it
        Scene scene = new Scene(grid);

        // Store the user inputs
        List<String> input = new ArrayList<>();
        
        // Get the inputs to store from the scene
        scene.setOnKeyPressed(keyEvent -> {
            String code = keyEvent.getCode().toString();
            if (input.size() == 0) {
                input.add(code);
            }
        });
        scene.setOnKeyReleased(keyEvent -> {
            String code = keyEvent.getCode().toString();
            input.remove(code);
        });

        // Start time for animation timer
        final long[] lastTime = {System.nanoTime()};

        // The game loop
        new AnimationTimer() {
            @Override
            public void handle(long currentTime) {
                
                // If the user has requested a change of direction, do it now
                if (input.size() != 0) {
                    snakeGame.changeDirection(Direction.valueOf(input.get(0)));
                }
                
                // Calculate how much time has elapsed since the last frame
                double elapsedTime = (currentTime - lastTime[0]) / 1000000000.0;
                
                // If it is time to launch a new frame, do it
                if (elapsedTime >= 0.2) {
                    
                    // Reset the time
                    lastTime[0] = System.nanoTime();
                    
                    // Attempt the move
                    boolean move = snakeGame.nextMove();
                    
                    // Repaint the board
                    snakeGame.paintBoard();
                    
                    // If the user got out, end the game
                    if (!move) {
                        grid.setDisable(true);
                        stop();
                    }
                }
            }
        }.start(); // Start the game loop
        
        // Finally, return this Scene to to the stage in Main.java
        return scene;
    }
}

That's it. I'm relatively new to JavaFX so I don't really know how game development with it is supposed to work. I used this article as my starting point.

The start screen:

Start screen

The game in progress:

The Game Screen

\$\endgroup\$
4
+50
\$\begingroup\$

TL;DR summary: Use less comments, use meaningful comments, use Deque, DRY and generalise the repetitive code (with some smarts), out-of-border checking, make Square abstract & paint in it's subclass, repaint only when something changes.


Firstly, I am totally new to this StackExchange, so sorry if I misunderstood my review task or I cross any borders - I honestly don't want to be mean to you, just to point out style errors or things that can be improved in your design or implementation.

I have not run your solution at all, it looks functional and I believe you it works. But I have read all the code.


Review

  1. You have good direction of control - you are calling the core functionality from the UI. However, it could be brilliant if you could get rid of the dependency on JavaFX totally. The GameLogic should be UI-agnostic, it should be an independent module you could reuse from say a console UI. You are on a very good way here - the only thing you do with JavaFX in GameLogic is paintBoard(). And good job injecting Square[][]! The color of the tile, or Square as you call it, should be the responsibility of the Square class itself. A color is not logically the responsibilty of the game logic. The GameLogic can call a method of the Square to change it's state and it is the responsibility of the Square to manifest the changed state by changing it's color.

And the Square can easily do this task itself, you have provided it with fields (defining the state) white, apple. So the default color can be given by white, and then in the setApple() you can change the color if needed (hurray encapsualtion with setter methods!).

The only other state which has another color is when the snake is on the square.

  • You could introduce another field marking this state (also update in setter).

  • Another solution which comes to my mind is to consider Square as a place where something can stand on or cover the square. This would be a good solution if you want to extend the possible entities which can be in your world (you could have poisoned apples, walls, holes...). I would implement this by introducing a new interface, f.e. Placeable with some method to draw the UI & the Square would have a field Placeable placedObject or something similar. This way you do not need to add more fields to the Square for each item and each item has its own UI responsibility.

The next step in making the core game logic independent is to make Square not extend Label, make it abstract. Extract the painting logic (which calls setStyle) into an abstract method and implement it in a subclass JavaFxSquare which can extend Label. You will probably call the paint() method in the setters or on demand.

  1. Why does Square extend Label in the first place? It does not contain any text. I vaguely remember that I had a problem with javafx.scene.shape.Rectangle putting it into a GridPane - is this the reason? Anyway: don't extend Label, probably extending Region is enough.

  2. Rename white => isWhite and apple => hasApple. Usually boolean variable names are adjectives or start with is or has

  3. The field white can be calculated inside the constructor of Square. One could say it is his responsibility, but if you want it to be configurable, it can stay a constructor parameter.

  4. You have waaaaaaaaaaaaaaay too many comments. I am not sure whether you have commented each line just for the review or you actually have so many comments. The problem is these comments have no meaning most of the time:

// Create a scene and add the GridPane to it
stage.setScene(new Scene(root));

// Store the user inputs
List<String> input = new ArrayList<>();

// Create an instance of GameLogic. Pass it the board and the list of snake body parts
GameLogic snakeGame = new GameLogic(board, snake);

// If the nextSquare variable is an apple
        if (nextSquare.isApple()) {

A lot of code you have commented is self-explanatory and do not need comments. Some well-named variable is way better. So many comments are distracting when reading the code, because after a while I was just ignoring the comments - and this way I can miss something important! And comments should be only for the important stuff - something unusual, some bugfix which is not apparent why the implementation is that way... If you need to comment a block of code, you should probably extract it to a well-named method.

  1. Rename i, j to row and col.

  2. count variable has no sense, it is identical to j (i.e. col)

  3. What happens if you eat 2 apples quickly? can it happen?

  4. Good job with detecting if the new apple position is not inside the snake already! However:

    • DRY (don't repeat yourself): it should be on 1 place, probably inside GameLogic (and call it in constructor)
    • creating a new Random() always is not a good idea, it can produce the same values if initialised with a short interval. You should initialise it once in your constructor.
  5. LinkedList is the perfect implementation for your Snake. Manipulating the "head" and "tail" should be enough for you, so you could use the Deque interface. You should replace your method calls:

    • snake.get(0) => snake.getFirst()
    • snake.set(0, x) => snake.addFrist(x)
    • snake.get(snake.size() - 1) => snake.getLast()
    • snake.set(snake.size() - 1, nextSquare) => snake.addLast(nextSquare)

Why are you actually moving all the squares? It is enough to add a new head and remove the tail if it haven't eaten an apple. The other parts of the Snake stay untouched.

  1. Change input.size() == 0 => input.isEmpty().

Why is input a list? How does your game work if you hold one arrow and then press another arrow without releasing the first? The snake does not change it's direction, does it? Is it the expected behavior? If you want to store only the most recent key pressed, it would be enough to not use a List.

What happens if you press a non-arrow key?

Instead of using a String you can also store the KeyCode (maybe later you will want to enable WASD also, so you can have a method which maps it to Direction).

- I am not sure how the threads on key pressed work, but maybe you need to `synchronise` the assignment and read of `input`
  1. You have a hidden logic when you test
head.getDirection() == Direction.UP && direction == Direction.DOWN

How would you name it? I'd say you are checking whether the directions are opposite. I suggest you add an opposite field to your Direction like so:

    public enum Direction {
        UP, DOWN, RIGHT, LEFT;

        private Direction opposite;

        static {
            UP.opposite = DOWN;
            DOWN.opposite = UP;
            RIGHT.opposite = LEFT;
            LEFT.opposite = RIGHT;
        }

        Direction getOpposite() {
            return opposite;
        }
    }

Sadly, it is a bit complicated because of Illegal forward reference.

This way you can change your 4 (!) conditions to this:

head.getDirection() == direction.opposite()
  1. You yourself have commented:

"since they all function in the exact same way"

Again: DRY! You should generalise the following code. Most of it is identical, except for the index calculation and border checking.

  1. The index calculation depends on the Direction you take. There is a pattern in which you move by 1 in the x-y axis. You can solve the index calculation by adding 1 if you move in the direction of the axis, subtracting 1 if you move in the opposite direction, or adding 0 if you stay on that axis. So:
public enum Direction {
    UP(-1, 0),
    DOWN(1, 0),
    RIGHT(0, 1),
    LEFT(0, -1);

    private int rowChange;
    private int colChange;

    Direction(int rowChange, int colChange) {
        this.rowChange = rowChange;
        this.colChange = colChange;
    }

    int getRowChange() {
        return rowChange;
    }

    int getColChange() {
        return colChange;
    }
}

So the resulting code is:

nextSquare = board[row + direction.getRowChange()][column + direction.getColChange()];
  1. Border checking is easy if you check the nextSquare: does it have a row or col < 0 or >= size ?
  1. The changeDirection() comments nothing about ignoring opposite direction - THAT should be commented, it's an interesting edge case.

  2. nextMove() comment has nothing saying about the meaning of return value. The name of the method neither does help. The return type should be well documented in this case, it is not apparent - JavaDoc @return is just for this!

  3. It could be considered for nextMove() to be void and throw a GameOverException (what a cool name!). It's not necessary, just a possibility. Having a boolean returned in this case is even better, because philosophically it is the expected behaviour to hit a wall or eat your tail. However, see point 16.

  4. what's this about? why should it be null?

if (square == null) {
    System.out.println("Square is null");
    return;
}
  1. Is repainting of the whole board needed? repaint only what has changed, with larger grids, it can very quickly lag.

If you implement reactive change inside Square upon setting apple for example, this is not an issue anymore.

  1. In your UI class, the size could be parameterizable. For example the user could input it. Keep it in mind and use a variable for the size, not hardcoded int literals.

  2. Calculate the middle of the board for the initial position of the snake. Alternatively, you could generate it randomly. The direction could also be random.


I hope all of this helps :D I think you could do most of the points separately, so don't be intimidated by the high amount. I am very much looking forward to your next steps and development! Don't hesitate to write in case of any questions.


Future extensions

You could think about your solution to be flexible and extensible in the future. You could implement these in the future, or prepare your solution to be extended once. Some ideas:

  1. Configurable board size
  2. Performance improvement - multi-threading
  3. Other kinds of objects on the board, like walls, golden apples, laxatives, energy drink
    • think of leveraging the game aspects f.e. length of snake, score, speed of game, game over
  4. Another UI support - console or some other GUI
  5. Multiplayer? :D
  6. Tracking the score, keeping a highscore
\$\endgroup\$
3
  • \$\begingroup\$ Thanks, you've given me a lot to think about. This is the first game I've made with graphics (I only learned how to program this year), so I'm sort of figuring it out as I go. I made Square extend Label for precisely the reason you mentioned; it just didn't work with Rectangles for some reason. The comments were added in a hurry for the purpose of this review. I don't actually use comments in real life(I should probably change that). It occurs to me that they probably did more harm then good. The check for (square == null) was a remnant of the debugging process that I forgot to remove. \$\endgroup\$
    – qwerty
    Aug 11 '20 at 1:01
  • 1
    \$\begingroup\$ Congrats, you are advancing very well! I think your design is good, it was a good decision to use your classes and enum.Only you need to pay attention to the dependency directions - from UI to controller to business logic to entities (see Clean Architecture). This helps extensibility - you could use another UI. Implementation-wise you should look out for repetitive code and try to simplify it. Regarding the comments - most of the time they say WHY you do something, not WHAT you do (the variable and method names should be self-explanatory). Also good idea to add JavaDocs to public methods \$\endgroup\$
    – Hawk
    Aug 11 '20 at 9:35
  • \$\begingroup\$ Hello Hawk, you have a great answer, but I would object to 2. Performance improvement - multi-threading when talking about JavaFX. With the emphasis on multi-threading. Though Threads can be used in JavaFX, using something from the Animation API, is the best choice here. \$\endgroup\$
    – Sedrick
    Jan 19 '21 at 5:46

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.