Building a Breakout game from scratch

Question

I'm a beginner, currently taking the Stanford's free CS106A course which teaches Java. The second assignment has you build Breakout from scratch using the ACM programs and graphics packages.

Since it's an online course and I don't have anyone who can review my code, I thought I might ask you what newbie mistakes I'm making. I'm sure there are a lot.

I had some trouble figuring out where to stick some of the behavior. I have a Ball class within which I wanted to put the collision detection and reaction, but ended up having to put it in the main program, because ACM wouldn't let me check the canvas around the ball from within the ball.

I also could manage to get the main program's instance from anywhere, only its class, which wasn't what I needed. Any ideas on how to do this? Other instances were problems, too. For instance, is there any way for me to get the Paddle's instance within the Ball without passing the paddle into the constructor? Is there any way to call it?

I wasn't sure about where to use private/public and static. I feel as though a lot of my uses were workarounds.

How would you have built this differently? What huge mistakes should I stop making? Is it organized awfully?

Main program:

import acm.graphics.*; 
import acm.program.*; 
import acm.util.*; 
import java.applet.*; 
import java.awt.*; 
import java.awt.event.*; 

public class Breakout extends GraphicsProgram { 

public void init() { 
    setBricks();
    add(paddle);
    add(ball,WIDTH/2,500);
    lifeBar.setColor(Color.WHITE);
    add(lifeBar,20,60);
    addMouseListeners();
    setBackground(Color.BLACK);
}

public void run() { 
    while(gameOver != true) {
        pause(20);
        checkCollision(ball);
        ball.move();

        if (prize != null) {
            prize.drop();
            if (prize.leftScreen()) remove(prize);
        }
    }
    addMouseListeners();
} 

private void setBricks() {
    for (int row = 0; row < NBRICK_ROWS; row++) {
        Color rowColor = checkColor(row);   
        for (int col = 0; col < NBRICKS_PER_ROW; col++) {
            add(new Brick(col,row,rowColor));
        }
    }
}

public void mouseClicked(MouseEvent e) {
    removeBrick(getElementAt(e.getX(),e.getY()));
}

public void mouseMoved(MouseEvent e) {
    paddle.followMouse(e.getX());
}


private Color checkColor(int row) {
    switch (row) {
        case 0:
        case 1: return Color.RED;
        case 2:
        case 3: return Color.ORANGE;
        case 4:
        case 5: return Color.YELLOW;
        case 6:
        case 7: return Color.GREEN;
        case 8:
        case 9: return Color.CYAN;
    }
    return Color.CYAN;
}


private void removeBrick(GObject brick) {
    /* add test to see if removing brick is possible */
    if (brick != null && brick instanceof Brick) {
        remove(brick);
        if (((Brick) brick).isSpecial()) callPrize(brick.getX(),brick.getY());
        if (Brick.decreaseBricks()) callGameOver(true);
    }
}

private void removeLife() {
    lives--;
    lifeBar.setLabel("Lives: " + lives);
    if (lives <= 0) callGameOver(false);
}

public void callPrize(double x, double y) {
    prize = new Prize(x,y,Color.MAGENTA, paddle);
    add(prize);
}

public void callGameOver(boolean won) {
    GRect gameOverScreen = new GRect(0,0,WIDTH,HEIGHT);
    gameOverScreen.setFilled(true);
    gameOverScreen.sendToFront();
    GLabel gameOverLabel = new GLabel("");
    if (won == true) {
        gameOverScreen.setColor(Color.WHITE);
        gameOverLabel.setColor(Color.BLUE);
        gameOverLabel.setLabel("WOO YOU WON!!!");
        if (lives == 3) gameOverLabel.setLabel("HOLY SHIT PERFECT SCORE!!!");
    } else {
        gameOverScreen.setColor(Color.RED);
        gameOverLabel.setColor(Color.WHITE);
        gameOverLabel.setLabel("HAH WHAT A LOSER");
    }
    add(gameOverScreen);
    add(gameOverLabel,WIDTH/2-gameOverLabel.getWidth()/2,HEIGHT/2+gameOverLabel.getAscent()/2);

    gameOver = true;
}

private void checkCollision(Ball ball) {
        double r = ball.getRadius();
        ball.move();
        double x = ball.getX();
        double y = ball.getY(); 
        ball.moveBack();
        GObject collider = null;
        int dir = 0;

        // Check walls + keep within bounds (keep from getting stuck)
        if (x-r <= 0) {
            ball.setLocation(r,y);
            ball.changeDirection(1);
        } else if (x+r >= WIDTH) {
            ball.setLocation(WIDTH-r,y);
            ball.changeDirection(1);
        }

        if (y-r <= 0) {
            ball.setLocation(x,r);
            ball.changeDirection(2);
        // less life
        } else if (y+r >= HEIGHT) {
            removeLife();
            ball.restart();
        }

        //Check elements

        //Check right
        if (getElementAt(x+r,y) != null && getElementAt(x+r,y) != ball && !(getElementAt(x+r,y) instanceof GLabel) && !(getElementAt(x+r,y) instanceof Prize)) {
            ball.changeDirection(1);
            collider = getElementAt(x+r,y);
        } 
        //Check left
        if (getElementAt(x-r,y) != null && getElementAt(x-r,y) != ball && !(getElementAt(x-r,y) instanceof GLabel) && !(getElementAt(x-r,y) instanceof Prize)) {
            ball.changeDirection(1);
            collider = getElementAt(x-r,y);
        } 
        //Check bottom
        if (getElementAt(x,y+r) != null && getElementAt(x,y+r) != ball && !(getElementAt(x,y+r) instanceof GLabel) && !(getElementAt(x,y+r) instanceof Prize)) {
            ball.changeDirection(2);
            collider = getElementAt(x,y+r);
        } 
        //Check top
        if (getElementAt(x,y-r) != null && getElementAt(x,y-r) != ball && !(getElementAt(x,y-r) instanceof GLabel) && !(getElementAt(x,y-r) instanceof Prize)) {
            ball.changeDirection(2);
            collider = getElementAt(x,y-r); 
        } 

        ball.changeDirection(dir);
        if (collider instanceof Brick) removeBrick(collider);
        if (collider == paddle) {
            ball.setDirection(paddle.collide(x,r));
        }
}


/* Init things */
private Paddle paddle = new Paddle();       
private Ball ball = new Ball(BALL_RADIUS);

/** Width and height of application window in pixels */ 
    public static final int APPLICATION_WIDTH = 400; 
    public static final int APPLICATION_HEIGHT = 600; 

 /** Dimensions of game board (usually the same) */ 
    public static final int WIDTH = APPLICATION_WIDTH; 
    public static final int HEIGHT = APPLICATION_HEIGHT; 

 /** Offset of the paddle up from the bottom */ 
    public static final int PADDLE_Y_OFFSET = 30; 

 /** Number of bricks per row */ 
    public static final int NBRICKS_PER_ROW = 10;

 /** Number of rows of bricks */ 
    private static final int NBRICK_ROWS = 10;

 /** Radius of the ball in pixels */ 
    private static final int BALL_RADIUS = 10;

 /** Offset of the top brick row from the top */ 
    public static final int BRICK_Y_OFFSET = 70;

 /** Number of turns */ 
    private static final int NTURNS = 3;


/** Points */
    private int lives = NTURNS;  

/** Game is Over */
    private boolean gameOver = false;  

/* Prize on Board */
    Prize prize = null;


    private GLabel lifeBar = new GLabel("Lives: " + lives);
 }

Ball Class

import acm.graphics.*; 
import acm.util.*; 
import java.awt.*; 

public class Ball extends GCompound {

public Ball(double radius) {
    r = radius;
    GOval ball = new GOval(-r,-r,r*2,r*2);
    ball.setFilled(true);
    ball.setColor(new Color(200,200,200));


    GOval light = new GOval(-r+3,-r+3,r*2-6,r-2);

    light.setFilled(true);
    light.setColor(new Color(255,255,255,150));
    add(ball);
    add(light);

    direction = rgen.nextDouble(220,320);
    direction = 290;
}

public void move() {
    this.movePolar(velocity,direction);
}

public void moveBack() {
    this.movePolar(velocity, (direction+180)%360);
}

public void restart() {
    this.setVisible(false);
    pause(100);
    this.setVisible(true);
    pause(100);
    this.setVisible(false);
    pause(100);
    this.setLocation(Breakout.WIDTH/2,Breakout.HEIGHT/2);
    this.setVisible(true);
    direction = rgen.nextDouble(220,320);
}

public double getRadius() {
    return r;
}

public void changeDirection(int xory) {
    if (xory == 1) direction -= (direction-270)*2;
    if (xory == 2) direction -= (direction-180)*2;
}

public double getDirection() {
    return direction;
}

public static void setVelocity(double v) {
    velocity = v;
}

public void setDirection(double d) {
    direction = d;
}

private double r;
private double direction;


private static final double STARTING_VELOCITY = 5.0;
private static double velocity = STARTING_VELOCITY;

private RandomGenerator rgen = RandomGenerator.getInstance();
}

Brick Class

import acm.graphics.*; 
import acm.util.*;  
import java.awt.*; 

public class Brick extends GCompound {

public Brick(int colNum, int rowNum, Color color) {

    if (rgen.nextBoolean(0.01)) {
        color = Color.MAGENTA;
        special = true;
    }
    GRect fill = new GRect(0,0,BRICK_WIDTH, BRICK_HEIGHT);
    fill.setFilled(true);
    fill.setColor(color);
    GRect light = new GRect(0,0,BRICK_WIDTH,BRICK_HEIGHT/2);
    light.setFilled(true);
    light.setColor(new Color(255,255,255,120+10*rowNum));
    if (special == true) light.setColor(new Color(255,255,255,120));



    add(fill);
    add(light);

    // Find and set location
    double x = START_X + colNum * (BRICK_WIDTH + BRICK_SEP);
    double y = START_Y + rowNum * (BRICK_HEIGHT + BRICK_SEP);
    setLocation(x,y);

    // Add to bricks remaining;
    bricksRemaining++;
}

public static int getBricksRemaining() {
    return bricksRemaining;
}

/* decreases bricks remaining and checks for end */
public static boolean decreaseBricks() {
    bricksRemaining--;
    if (bricksRemaining < 75) Ball.setVelocity(7); //Set the velocity higher if
    if (bricksRemaining < 50) Ball.setVelocity(9); 
    if (bricksRemaining < 25) Ball.setVelocity(11); 
    return (bricksRemaining > 0) ? false : true;
}

public boolean isSpecial() {
    return special;
}

 /** Separation between bricks */ 
    public static final int BRICK_SEP = 4;

 /** Width of a brick */ 
    public static final int BRICK_WIDTH = 
    (Breakout.WIDTH - (Breakout.NBRICKS_PER_ROW - 1) * BRICK_SEP) / Breakout.NBRICKS_PER_ROW; 

 /** Height of a brick */ 
    public static final int BRICK_HEIGHT = 12;


/** Starting point X and Y values for first brick */
    private static final double START_X = (Breakout.WIDTH - ((Breakout.NBRICKS_PER_ROW * BRICK_WIDTH) + (Breakout.NBRICKS_PER_ROW - 1) * BRICK_SEP)) / 2 ;      
    private static final int START_Y = Breakout.BRICK_Y_OFFSET;


    private boolean special = false;

    private static int bricksRemaining = 0;

    private RandomGenerator rgen = RandomGenerator.getInstance();
}

Paddle Class

import acm.graphics.*; 
import java.awt.*; 

public class Paddle extends GCompound {

public Paddle() {
    fill = new GRect(0,0, PADDLE_STARTING_WIDTH, PADDLE_HEIGHT);
    fill.setFilled(true);
    fill.setColor(Color.GRAY);
    light = new GRect(0,PADDLE_HEIGHT/2, PADDLE_STARTING_WIDTH, PADDLE_HEIGHT/2);
    light.setFilled(true);
    light.setColor(new Color(0,0,0,50));
    add(fill);
    add(light);
    this.setLocation((Breakout.WIDTH - PADDLE_STARTING_WIDTH) / 2, PADDLE_Y);
}

public void followMouse(double mouseX) {
    if (mouseX < paddleWidth/2) {
        this.setLocation(0, PADDLE_Y);
    } else if (mouseX > Breakout.WIDTH - paddleWidth/2) {
        this.setLocation(Breakout.WIDTH - paddleWidth,PADDLE_Y);
    } else {
        this.move(mouseX-paddleWidth/2 - this.getX(), 0);
    }
}

public double collide(double ballX, double r) {
    double paddleX = this.getX();
    setColor(Color.RED);
    double hitSpot = ballX - paddleX;
    double maxPaddle = paddleWidth + r;
    double minPaddle = -r;
    double paddleRange = maxPaddle - minPaddle;
    double minAngle = 160;
    double maxAngle = 20;
    double angleRange = maxAngle - minAngle;
    double newDirection = ((hitSpot * angleRange) / paddleRange) + minAngle;


    return newDirection;
}

private void widen() {
    paddleWidth += 30;
    this.move(-15, 0);
    fill.setSize(paddleWidth, PADDLE_HEIGHT);
    light.setSize(paddleWidth, PADDLE_HEIGHT / 2);
}

public void checkForPrize(Prize prize, double x, double y) {
    if ((y >= PADDLE_Y) && (y <= PADDLE_Y + PADDLE_HEIGHT) && x > this.getX() && x < this.getX() + paddleWidth) {
        prize.pickUp();
        this.widen();
    }
}


 /** Dimensions of the paddle */ 
private static final int PADDLE_STARTING_WIDTH = 60; 
private static final int PADDLE_HEIGHT = 15;

private GRect fill;
private GRect light;
private int paddleWidth = PADDLE_STARTING_WIDTH;
 /** Dimensions of the paddle */ 
public static final int PADDLE_Y = Breakout.HEIGHT - Breakout.PADDLE_Y_OFFSET;
}

Prize Class

import acm.graphics.*; 
import java.awt.*; 


public class Prize extends GOval {

public Prize(double x, double y, Color color, Paddle pad) {
    super(x + Brick.BRICK_HEIGHT/2, y + Brick.BRICK_WIDTH/2, PRIZE_RADIUS * 2, PRIZE_RADIUS * 2);
    setFilled(true);
    setColor(color);

    paddle = pad;
}

public void drop() {
    this.move(0, PRIZE_SPEED);
    paddle.checkForPrize(this,this.getX()+PRIZE_RADIUS,this.getY()+PRIZE_RADIUS*2);
}

public boolean leftScreen() {
    if (this.getY() - PRIZE_RADIUS > Breakout.HEIGHT || pickedUp == true) {
        return true;
    } else {
        return false;
    }   
}

public void pickUp() {
    pickedUp = true;
}

private boolean pickedUp = false;

private Paddle paddle;

public static final int PRIZE_RADIUS = 5;
public static final int PRIZE_SPEED = 10;
}

Answer 1

I haven't tried to get into the high level design of your game but here are a few details you might want to fix.

• if (won == true) can be written if (won)

• You can use the ternary operator to use setLabel only once : gameOverLabel.setLabel((lives == 3)?"HOLY SHIT PERFECT SCORE!!!":"WOO YOU WON!!!");

• I guess the code to check elements can be factorised.

• Using instanceof is usually a bad thing.

• You probably want to store your magic numbers in constant variables to make things easier to understand.

• The way you initialise direction in the Ball constructor is weird (and the way you change direction is pretty awkward as well)

• There's probably a problem in the decreaseBricks() method : you might want to do the test in the other order and separated by else and you can simply return (bricksRemaining > 0) (maybe returning bricksRemaining would work but I just can't remember how it works in Java)

• The leftScreen() method could be a return <your big expression>;.

Answer 2

General

Your code would be more readable without the redundant "this."s.

Main program

private void checkCollision(Ball ball) {
        double r = ball.getRadius();
//// If you moved the ball BEFORE checkCollision, you wouldn't need
//// to move it here, and move it back only to move it again, AFTER.
        ball.move();
        double x = ball.getX();
        double y = ball.getY(); 

//// There's no point in finishing this function after removeLife,
//// so test for it up-front and get it out of the way.

        // less life
        if (y+r >= HEIGHT) {
            removeLife();
            ball.restart();
            return;
        }

        ball.moveBack();
        GObject collider = null;
        int dir = 0;

        // Check walls + keep within bounds (keep from getting stuck)
        if (x-r <= 0) {
            ball.setLocation(r,y);
            ball.changeDirection(1);
        } else if (x+r >= WIDTH) {
            ball.setLocation(WIDTH-r,y);
            ball.changeDirection(1);
        }

        if (y-r <= 0) {
            ball.setLocation(x,r);
            ball.changeDirection(2);
        }

        //Check elements

This "Check elements" part could be much faster if you established "zones" (y value ranges) where you would expect to collide with a brick or a paddle and only test for collisions with these things when the ball is in their zones.

Calling getElementAt ONCE for each given argument pair and reusing the result is faster to execute and easier to read.

You are not handling the case of hitting two bricks at once that are side-by-side, top-to-bottom, or touching corners. You only process the second collision detected. But if you fix this, don't forget that the SAME brick might also match in multiple directions.

You only have to test for bricks in the 1 or 2 direction(s) that the ball is currently heading one each of +/-x (unless direction is 90) and +/-y.

Since the bricks aren't moving, it might be easier to code your own function that tests which bricks you are colliding with, based on where you originally placed them. It would help, then, to have your bricks in a 2D array. Then you could deal with the paddle separately and forget about the GLabels.

Either zoning or special casing the brick collisions relieves you of the need to check for (collider instanceof Brick) which is good to avoid.

You don't need to check for the paddle when moving upward.

Ball class

Use consts or enums instead of 1 or 2 for xory here and in calls to here.

public void changeDirection(int xory) {
    if (xory == 1) direction -= (direction-270)*2;
    if (xory == 2) direction -= (direction-180)*2;
}

As already commented, these are strange direction expressions. They are technically correct for x-axis reflection and y-axis reflection, but a very roundabout way of expressing:

direction = ((xory==1) ?  180 : 0) - direction;

Brick Class

NEVER CALLED:

public static int getBricksRemaining() {
    return bricksRemaining;
}

bricksRemaining, decreaseBricks, et. al. should probably be handled by the main Breakout class, so that Brick does not need to depend on Ball.

Paddle class

//// These should be static consts.
    double minAngle = 160;
    double maxAngle = 20;
    double angleRange = maxAngle - minAngle;



public void checkForPrize(Prize prize, double x, double y) {
    if ((y >= PADDLE_Y) && (y <= PADDLE_Y + PADDLE_HEIGHT) && x > getX() && x < getX() + paddleWidth) {
        prize.pickUp();
//// Moving the next statement into Prize::pickUp makes the purpose of the Prize 
//// more self-evident within its class and would allow different prizes to have
//// different effects.
        widen();
    }
}

Answer 3

Split changeDirection in the Ball class into two methods, and simplify the mathematics in each of them. Something like

public void bounceVertical(){
    direction = 540 - direction;
}

public void bounceHorizontal(){
    direction = 360 - direction;
}

plus all the good suggestions made by the other respondents.

Answer 4

There is a fair amount of redundant code. In addition to things already pointed out, you can add at the very least

    direction = rgen.nextDouble(220,320);
    direction = 290;

where the first line is redundant, and is the only thing which calls rgen, so that can be removed too.

public void run() { 
    while(gameOver != true) {
        pause(20);
        checkCollision(ball);
        ball.move();

        if (prize != null) {
            prize.drop();
            if (prize.leftScreen()) remove(prize);
        }
    }
    addMouseListeners();
} 

Why add mouse listeners after the game is over?

As a corollary to Josay's comment about if (won == true), you can rewrite while(gameOver != true) as while(!gameOver), which is more idiomatic.

A point specifically on game implementation: your timing loop is slightly off. You're going to get a bit under 50fps, but without much consistency. If you want a smooth game you should time how long things take and then take that into account. Using nanosecond timing for accuracy,

while (!gameOver) {
    long start = System.nanoTime();

    ... do physics and rendering

    long end = System.nanoTime();
    long delay = (1000000000L / DESIRED_FPS) - (end - start);
    if (delay > 0) Thread.sleep(delay / 1000000, (int)(delay % 1000000));
}