5
\$\begingroup\$

I've made a simple snake game on which I want to expand with some properties and settings to allow the user to change the speed increments of the snake and the game field size. Before expanding on it though I'd love some feedback on the current code.

Main:

public class Main extends Application {

@Override
public void start(Stage primaryStage) throws Exception {
    FXMLLoader loader = new FXMLLoader(getClass().getResource("/be/vincent_nagy/snake/view/snake.fxml"));

    SnakeController controller = new SnakeController();
    loader.setController(controller);

    Parent root = loader.load();
    Scene scene = new Scene(root);
    primaryStage.setTitle("SnakeController");
    primaryStage.setScene(scene);

    controller.setScene(scene);

    primaryStage.show();
}

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

Controller:

public class SnakeController {
@FXML
private Button startButton;
@FXML
private Canvas canvas;
@FXML
private Label textLabel;

private Snake snake;
private Scene scene;
private Game game;

public void initialize() {

    startButton.setOnAction(event -> {
        canvas.getGraphicsContext2D().clearRect(0,0,canvas.getWidth(), canvas.getHeight());
        setTextLabel("");
        game = null;
        snake = new Snake(canvas.getGraphicsContext2D());
    });


}

public void setScene(Scene scene) {
    this.scene = scene;
    scene.addEventFilter(KeyEvent.KEY_PRESSED, this::buttonPressed);
}

public void setTextLabel(String text){
    this.textLabel.setText(text);
}

private void buttonPressed(KeyEvent event) {
    switch (event.getCode()) {
        case RIGHT:
        case NUMPAD6:
            if(snake.getDirection() != Direction.LEFT)
                snake.setDirection(RIGHT);
            break;
        case LEFT:
        case NUMPAD4:
            if(snake.getDirection() != Direction.RIGHT)
                snake.setDirection(LEFT);
            break;
        case UP:
        case NUMPAD8:
            if(snake.getDirection() != Direction.DOWN)
                snake.setDirection(UP);
            break;
        case DOWN:
        case NUMPAD2:
            if(snake.getDirection() != Direction.UP)
                snake.setDirection(DOWN);
            break;
    }
    if(game == null){
        System.out.println("Creating a new game");
        game = new Game(this, snake, canvas.getGraphicsContext2D());
    }


    event.consume();
}
}

Game:

public class Game implements Runnable{
private SnakeController controller;
private Thread gameThread;
private Snake snake;
private SnakePart food;
private Random rng = new Random();
private GraphicsContext gfx;
private int tickSpeed = 500;
private boolean isRunning;
private static final int SQUARE_SIZE = 20;
private static final int FIELD_SIZE = 300;

public Game(SnakeController controller, Snake snake, GraphicsContext gfx) {
    this.controller = controller;
    this.snake = snake;
    this.gfx = gfx;
    startGameLoop();
    spawnFood();
}

private void spawnFood() {
    do {
        System.out.println("Attempting to spawn food");
        int x = rng.nextInt(FIELD_SIZE / SQUARE_SIZE);
        int y = rng.nextInt(FIELD_SIZE / SQUARE_SIZE);
        food = new SnakePart(x, y);
    }while(isSpotTaken(food));
    food.drawPart(gfx);
}

private void startGameLoop(){
    if(gameThread == null){
        System.out.println("Creating new gamethread");
        gameThread = new Thread(this);
        gameThread.setDaemon(true);
        gameThread.start();
        isRunning = true;
    }
}

@Override
public void run() {
    synchronized (this) {
        System.out.println("Start running the gameloop");
        if(snake != null){
            Direction direction = snake.getDirection();
            while (isRunning) {
                if(direction != null){
                    doTick();
                }
                try {
                    wait(tickSpeed);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                    isRunning = false;
                }

            }
        }
        System.out.println("End running the gameloop");
    }
}

private void doTick(){
    System.out.println("Validate next position");
    //Copy the first part
    SnakePart newPos = new SnakePart(snake.getParts().get(0));
    //Move it to the new position
    newPos.move(snake.getDirection());

    if(isNextPosValid(newPos)){
        System.out.println("Move in run() to " + snake.getDirection());
        snake.move(newPos);
    } else {
        if(food.getX() == newPos.getX() && food.getY() == newPos.getY()){
            System.out.println("Add part");
            snake.add(newPos,0);
            tickSpeed *= 0.9;
            spawnFood();
        } else {
            isRunning = false;
            Platform.runLater(() -> controller.setTextLabel("Game over!"));
        }
    }
}

private boolean isNextPosValid(SnakePart newSpot) {
    boolean isValid;

    //Check if the new spot is part of the snake
    isValid = !(isSpotTaken(newSpot));

    //Check if the new spot is a food
    if(food.getX() == newSpot.getX() && food.getY() == newSpot.getY()){
        isValid = false;
    }

    return isValid;
}


private boolean isSpotTaken(SnakePart newPos) {
    return snake.getParts().stream().anyMatch(snakePart -> snakePart.getX() == newPos.getX() && snakePart.getY() == newPos.getY());
}
}

Snake:

public class Snake {
private static final int FIRST = 0;

private boolean isRunning;
private int partCount;
private List<SnakePart> parts;
private Direction direction;
private GraphicsContext gfx;

public Snake(GraphicsContext gfx) {
    this.gfx = gfx;
    partCount = 1;
    parts = new ArrayList<>(partCount);
    parts.add(0, new SnakePart(7, 7));

    parts.get(0).drawPart(gfx);
}


void setRunning(boolean running) {
    isRunning = running;
}

void move(SnakePart newPosition) {
    //Remove last part
    parts.get(partCount - 1).clearPart(gfx);
    parts.remove(partCount - 1);
    //Place new part at the start
    parts.add(0, newPosition);
    //Draw the new first part
    //parts.get(FIRST).drawPart(gfx);
    repaint();
}

private void repaint() {
    parts.forEach(p -> p.drawPart(gfx));
}

void add(SnakePart newPart, int index){
    parts.add(index,newPart);
    partCount++;
}



public Direction getDirection() {
    return (direction == null) ? Direction.RIGHT : direction;
}

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

public List<SnakePart> getParts() {
    return parts;
}
}

SnakePart:

public class SnakePart {
private static final int SQUARE_SIZE = 20;
private static final int FIELD_SIZE = 300;

private int x;
private int y;

public SnakePart(int xOnGrid, int yOnGrid) {
    x = xOnGrid * SQUARE_SIZE;
    y = yOnGrid * SQUARE_SIZE;
}

public SnakePart(SnakePart part) {
    this.x = part.x;
    this.y = part.y;
}

public void drawPart(GraphicsContext gfx){
    gfx.fillRect(x,y,SQUARE_SIZE,SQUARE_SIZE);
}

public void clearPart(GraphicsContext gfx) {
    gfx.clearRect(x,y,SQUARE_SIZE,SQUARE_SIZE);
}

public void move(Direction d) {
    switch(d){
        case RIGHT:
            x = (x + SQUARE_SIZE + FIELD_SIZE) % FIELD_SIZE;
            break;
        case LEFT:
            x =( x - SQUARE_SIZE + FIELD_SIZE) % FIELD_SIZE;
            break;
        case UP:
            y = ( y - SQUARE_SIZE + FIELD_SIZE) % FIELD_SIZE;
            break;
        case DOWN:
            y = ( y + SQUARE_SIZE + FIELD_SIZE) % FIELD_SIZE;;
            break;
    }
}

public int getX() {
    return x;
}

public int getY() {
    return y;
}
}

Snakegame in action Snakegame in action 2

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

It looks good to me. I only have minor reviews.

  1. All constants should be declared in a final variable.

    FXMLLoader loader = new;
    FXMLLoader(getClass().getResource(
        "/be/vincent_nagy/snake/view/snake.fxml"));
    

    Should be:

    private final String VIEW_DIRECTORY = "/be/vincent_nagy/snake/view/";
    private final String SNAKE_FXML = "snake.fxml";
    FXMLLoader loader = new FXMLLoader(getClass().getResource(VIEW_DIRECTORY + SNAKE_FXML));
    
  2. Methods and classes should have a documentary comment for readability.

  3. Avoid using one-letter variables or a non-descriptive name as it will lead to confusion, especially when your program grows larger.

    private GraphicsContext gfx;
    

    Should be:

    private GraphicsContext graphicsContext;
    
\$\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.