Eclipse Game Lags Too Much

Question

I am making a game and tutorials for how to make it on youtube. Here is the link to the channel. I explain the first part of what I have and why I have it because I know that is helpful for filling you in.

Link to part 1(Then watch the rest of parts. @Chris, this is helpful for solving the problem so don't flag the post).

https://www.youtube.com/watch?v=IRn_ZGhJZ94

I noticed as I was testing out my code for part 4. before recording, the game lagged HORRIBLY. I have alot of code, and any help is appreciated.

Game class:

import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.ArrayList;

import javax.swing.ImageIcon;
import javax.swing.JPanel;
import javax.swing.Timer;

@SuppressWarnings("serial")
public class Game extends JPanel implements ActionListener{

Timer mainTimer;

Paddle paddle;
Ball ball;
int blockCount = 16;
static ArrayList<Block> blocks = new ArrayList<Block>();

public Game() {

    setFocusable(true);

    paddle = new Paddle(250, 300);
    addKeyListener(new KeyAdapt(paddle));

    ball = new Ball(275, 280);

    mainTimer = new Timer(10, this);
    mainTimer.start();
}
public void paint(Graphics g) {
    super.paint(g);
    Graphics2D g2d = (Graphics2D) g;

    ImageIcon ic = new ImageIcon("C:/Users/Elliot/Desktop/Eclipse Game/background.png");
    g2d.drawImage(ic.getImage(), 0, 0, null);


    paddle.draw(g2d);
    ball.draw(g2d);
    for(int i = 0; i < blockCount; i++) {
        Block b = blocks.get(i);
        b.draw(g2d);
    }

}
@Override
public void actionPerformed(ActionEvent arg0) {
    paddle.update();
    ball.update();

    for(int i = 0; i < blocks.size(); i++) {
        Block b = blocks.get(i);
        b.update();
    }


    repaint();

    startGame();
}

public void addBlock(Block b) {
    blocks.add(b);
}

public static void removeBlock(Block b) {
    blocks.remove(b);
}

public static ArrayList<Block> getBlockList() {
    return blocks;
}

public void startGame() {

    for(int i = 0; i < blockCount; i++) {
        addBlock(new Block(i*60 + 7, 20));
        addBlock(new Block(i*60 + 7, 0));
    }

}
}

Main class(The frame part):

import javax.swing.JFrame;

public class Main {
    public static void main(String[] args) {
        JFrame frame = new JFrame("Game");
        frame.setSize(500, 400);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.add(new Game());
        frame.setResizable(false);
        frame.setVisible(true);
    }
}

Key Adapt class:

import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;

public class KeyAdapt extends KeyAdapter{

    Paddle p;

    public KeyAdapt(Paddle paddle) {
        p = paddle;
    }

    public void keyPressed(KeyEvent e) {
        p.keyPressed(e);
    }

    public void keyReleased(KeyEvent e) {
        p.keyReleased(e);
    }
}

Paddle class:

import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.Rectangle;
import java.awt.event.KeyEvent;

import javax.swing.ImageIcon;

public class Paddle {

    int velX;
    int speed = 3;
    static int x1, y1;
    public Paddle(int x1, int y1) {
        this.x1 = x1;
        this.y1 = y1;
    }

    public void update() {
        x1+=velX;
        checkCollisions();
    }

    public void draw(Graphics2D g2d) {
        g2d.drawImage(getPaddleImg(), x1, y1, null);
    }

    public static Image getPaddleImg() {
        ImageIcon ic = new ImageIcon("C:/Users/Elliot/Desktop/Eclipse Game/paddle.png");
        return ic.getImage();
    }

    public void keyPressed(KeyEvent e) {
        int key = e.getKeyCode();

        if(key==KeyEvent.VK_D) {
            velX = speed;
        } else if(key==KeyEvent.VK_A){
            velX = -speed;
        }
    }

    public void keyReleased(KeyEvent e) {
int key = e.getKeyCode();

        if(key==KeyEvent.VK_D) {
            velX = 0;
        } else if(key==KeyEvent.VK_A){
            velX = 0;
        }
    }

    public void checkCollisions() {
        if(getBounds().getX() + getBounds().getWidth() >= 500) {
            x1 = 440;
        } else if(getBounds().getX() <= 0) {
            x1 = 0;
        }
    }
    public static Rectangle getBounds() {
        return new Rectangle(x1, y1 - 1, getPaddleImg().getWidth(null), getPaddleImg().getHeight(null));
    }

}

Ball class:

import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.Rectangle;

import javax.swing.ImageIcon;
import javax.swing.JOptionPane;

public class Ball {

    int velX;
    int velY;
    int speed = 3;
    int x, y;
    public Ball(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public void update() {
        x+=velX;
        y+=velY;
        checkCollisions();
    }

    public void draw(Graphics2D g2d) {
        g2d.drawImage(getBallImg(), x, y, null);
    }

    public Image getBallImg() {
        ImageIcon ic = new ImageIcon("C:/Users/Elliot/Desktop/Eclipse Game/ball.png");
        return ic.getImage();
    }

    public void checkCollisions() {

        for(int i = 0; i < Game.getBlockList().size(); i++) {
            Block b = Game.getBlockList().get(i);
            if(getBounds().intersects(b.getBounds()) && velX!=-speed) {
                velY=speed;
                velX =- speed;
                Game.removeBlock(b);
            }
            else if(getBounds().intersects(b.getBounds())) {
                velY=speed;
                velX = speed;
                Game.removeBlock(b);
            }
        }
        if(getBounds().intersects(Paddle.getBounds())) {
            velY = -speed;
        } else if (getBounds().getY() <= 0 && velX!=speed) {
            velY = speed;
            velX =- speed;
        }else if (getBounds().getY() <= 0 && velX!=-speed) {
            velY = speed;
            velX = speed;
        } else if(getBounds().getY() >= 400) {
            JOptionPane.showMessageDialog(null, "You Lost!  :( ");
            System.exit(0);
        }

        if(getBounds().getX() <= 0) {
            velX = speed;
        } else if(getBounds().getX() >= 500 - getBounds().getWidth()) {
            velX = -speed;
        }
    }

    public Rectangle getBounds() {
        return new Rectangle(x, y, getBallImg().getWidth(null), getBallImg().getHeight(null));
    }


}

Block class:

import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.Rectangle;

import javax.swing.ImageIcon;

public class Block {
    int x2, y2;
    public Block(int x2, int y2) {
        this.x2 = x2;
        this.y2 = y2;
    }

    public void update() {

    }

    public void draw(Graphics2D g2d){
        g2d.drawImage(getBlockImg(), x2, y2, null);
    }

    public static Image getBlockImg() {
        ImageIcon ic = new ImageIcon("C:/Users/Elliot/Desktop/Eclipse Game/block.png");
        return ic.getImage();
    }

    public Rectangle getBounds() {
        return new Rectangle(x2, y2, getBlockImg().getWidth(null), getBlockImg().getHeight(null));      
    }
}

I also have a folder called Eclipse Game on my desktop and I refer to it in my code.

Again, I understand this is alot but any idea with making it lag less is helpful. Also, watching the tutorial (look at the beginning for the link) on making what I have finished so far will help make it less confusing for you to understand how the code works. The game seriously lags so much I cannot play.

Answer 1

There are multiple issues.

The first, as I already mentioned in my comment, is that you're calling startGame() inside your timer action listener:

@Override
public void actionPerformed(ActionEvent arg0) {
    paddle.update();
    ball.update();
    for(int i = 0; i < blocks.size(); i++) {
        Block b = blocks.get(i);
        b.update();
    }
    repaint();
    startGame();
}

This is adding 3,200 blocks every second to the game, so you don't want that. I think the simplest place to put startGame() is at the end of the game constructor:

public Game() {
    setFocusable(true);
    paddle = new Paddle(250, 300);
    addKeyListener(new KeyAdapt(paddle));
    ball = new Ball(275, 280);
    mainTimer = new Timer(10, this);
    mainTimer.start();
    startGame();
}

The other really big problem is that you're constantly reloading the images all the time. For example, look at this snippet:

if(getBounds().intersects(b.getBounds()) && velX!=-speed) {
    velY=speed;
    velX =- speed;
    Game.removeBlock(b);
}
else if(getBounds().intersects(b.getBounds())) {
    velY=speed;
    velX = speed;
    Game.removeBlock(b);
}

That is 4 calls to getBounds(), and if we take a look at that:

return new Rectangle(x2, y2, getBlockImg().getWidth(null), getBlockImg().getHeight(null));

You are loading 2 images which in total is 4*2*blockCount images every 10ms, just for this one method. Instead of loading images all the time, do something like this:

class GameResources {
    static Image ballImage;
    static Image paddleImage;
    static Image blockImage;

    // call GameResources.loadResources() at the
    // beginning of main() or something
    static void loadResources() {
        // load all 3 images once here and be done
        ballImage = ...;
        paddleImage = ...;
        blockImage = ...;
}

Then finally, you have an issue with removing items from the list while iterating over it, Ball.checkCollisions:

for(int i = 0; i < Game.getBlockList().size(); i++) {
    Block b = Game.getBlockList().get(i);
    if(getBounds().intersects(b.getBounds()) && velX!=-speed) {
        velY=speed;
        velX =- speed;
        // removeBlock changes blocks.size()
        Game.removeBlock(b);
    }
    else if(getBounds().intersects(b.getBounds())) {
        velY=speed;
        velX = speed;
        // removeBlock changes blocks.size()
        Game.removeBlock(b);
    }
}

Instead you need to do something like this:

Iterator<Block> iter = Game.getBlockList().iterator();
while (it.hasNext()) {
    Block b = it.next();

    if(getBounds().intersects(b.getBounds()) && velX!=-speed) {
        velY=speed;
        velX =- speed;
        // safely removing
        it.remove();
    }
    else if(getBounds().intersects(b.getBounds())) {
        velY=speed;
        velX = speed;
        // safely removing
        it.remove();
    }
}

And another possible boundary issue in Game.paint:

//        using blockCount after possibly
//        removing items from the list
//                 vvvvvvvvvv
for(int i = 0; i < blockCount; i++) {
    Block b = blocks.get(i);
    b.draw(g2d);
}

For simple iterations like this, you should use for-each:

for(Block b : blocks) {
    b.draw(g2d);
}

After all of that the game runs pretty smoothly, except for some type of issue with the key listener which I didn't have time to figure out. I might look at it again after dinner.

---

edit:

I noticed a lot of other small things, so here is the program fixed up a bit more with my comments.

Some of the classes aren't public anymore just because I had them all in one source file.

import javax.swing.SwingUtilities;
import javax.swing.Timer;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JOptionPane;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyEvent;
import java.awt.event.KeyAdapter;
import java.util.ArrayList;
import java.util.Iterator;
import java.awt.Rectangle;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.Dimension;
import java.net.URL;
import javax.imageio.ImageIO;
import java.io.IOException;
import java.io.File;

public class BlockGame {
    public static void main(String[] args) {
        // Swing program should always begin on the Swing
        // thread with a call to invokeLater.
        // See https://docs.oracle.com/javase/tutorial/uiswing/concurrency/initial.html
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                try {
                    // change this to
                    //           .loadImages();
                    GameResources.loadInternetImages();
                } catch (IOException x) {
                    x.printStackTrace();
                    return;
                }

                JFrame frame = new JFrame("Game");
//                frame.setSize(500, 400);
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
//                frame.add(new Game());
                // Instead of calling setSize on the JFrame
                // directly, set a preferred size on the game
                // panel, then call pack() on the JFrame
                Game game = new Game();
                game.setPreferredSize(new Dimension(500, 400));
                frame.add(game);
                frame.pack();
                frame.setResizable(false);
                frame.setVisible(true);
                // I started the game here instead
                // of in the game loop, so the panel
                // is visible and stuff beforehand.
                game.startGame();
            }
        });
    }
}

class Game extends JPanel implements ActionListener {
    Timer mainTimer;
    Paddle paddle;
    Ball ball;

    // I removed this because it's only ever
    // used by startGame.
//    int blockCount = 16;

    // I changed this to an instance variable
    // (not static) and passed the game in to
    // update so the game objects can access
    // it.
    ArrayList<Block> blocks = new ArrayList<Block>();

    public Game() {
        setFocusable(true);
        paddle = new Paddle(250, 300);
        addKeyListener(new KeyAdapt(paddle));
        ball = new Ball(275, 280);
        mainTimer = new Timer(10, this);
        // I moved this to the startGame() method
//        mainTimer.start();
    }

    // Swing programs should override paintComponent
    // instead of paint.
    @Override
    protected void paintComponent(Graphics g) {
        super.paintComponent(g);
        // You should create a copy instead of
        // directly using the graphics object which
        // the component uses.
        // This is so any changes you make to it
        // don't affect the Swing paint routines.
        Graphics2D g2d = (Graphics2D) g.create();

//        ImageIcon ic = new ImageIcon("C:/Users/Elliot/Desktop/Eclipse Game/background.png");
//        g2d.drawImage(ic.getImage(), 0, 0, null);

        // Painting static resource.
        g2d.drawImage(GameResources.backgroundImage, 0, 0, null);

        paddle.draw(g2d);
        ball.draw(g2d);

        // This loop will throw an out of bounds
        // exception once the first block is removed.
        //                 vvvvvvvvvv
//        for(int i = 0; i < blockCount; i++) {
//            Block b = blocks.get(i);
//            b.draw(g2d);
//        }

        // using for each
        for (Block b : blocks) {
            b.draw(g2d);
        }

        // Dispose the copied graphics when you're done.
        g2d.dispose();
    }

    @Override
    public void actionPerformed(ActionEvent arg0) {
        paddle.update(this);
        ball.update(this);
//        for(int i = 0; i < blocks.size(); i++) {
//            Block b = blocks.get(i);
//            b.update();
//        }
        for (Block b : blocks) {
            b.update(this);
        }
        repaint();
        // I moved this to main
//        startGame();
    }

    public void addBlock(Block b) {
        blocks.add(b);
    }

    public void removeBlock(Block b) {
        blocks.remove(b);
    }

    public ArrayList<Block> getBlockList() {
        return blocks;
    }

    // I added this method so that the
    // ball can access the paddle without
    // static variables.
    public Paddle getPaddle() {
        return paddle;
    }

    public void startGame() {
        // So the method won't be called twice
        // and put the game in some unexpected
        // state.
        if (mainTimer.isRunning()) {
            throw new IllegalStateException("game already started");
        }

        int initialBlockCount = 16;
        for(int i = 0; i < initialBlockCount; i++) {
            addBlock(new Block(i*60 + 7, 20));
            addBlock(new Block(i*60 + 7, 0));
        }

        mainTimer.start();
    }
}

// Generally speaking you should use
// Swing key bindings now, instead of
// key listeners.
//
// Key listeners have problems with
// the focus system: Swing components
// only send out key events when they
// have the focus.
//
// Key bindings don't have this issue.
//
// You can set up key bindings so they
// trigger any time the key is pressed
// in the focused window.
//
// https://docs.oracle.com/javase/tutorial/uiswing/misc/keybinding.html
//
class KeyAdapt extends KeyAdapter {
    Paddle p;
    public KeyAdapt(Paddle paddle) {
        p = paddle;
    }
    public void keyPressed(KeyEvent e) {
        p.keyPressed(e);
    }
    public void keyReleased(KeyEvent e) {
        p.keyReleased(e);
    }
}

class Paddle {
    int velX;
    int speed = 3;
    // I changed these from static
    // to instance variables.
    int x1, y1;

    // I added these variables to
    // help with the key listener
    // logic.
    boolean leftPressed, rightPressed;

    public Paddle(int x1, int y1) {
        this.x1 = x1;
        this.y1 = y1;
    }

    public void update(Game game) {
        x1 += velX;
        checkCollisions();
    }

    public void draw(Graphics2D g2d) {
        g2d.drawImage(GameResources.paddleImage, x1, y1, null);
    }

//    public static Image getPaddleImg() {
//        ImageIcon ic = new ImageIcon("C:/Users/Elliot/Desktop/Eclipse Game/paddle.png");
//        return ic.getImage();
//    }

    public void keyPressed(KeyEvent e) {
        int key = e.getKeyCode();
        // This logic is a little more robust
        // because it handles cases where both
        // keys are being held at the same time.
        // Also see computeVelX().
        if (key == KeyEvent.VK_D) {
            leftPressed = true;
//            velX = speed;
        } else if (key == KeyEvent.VK_A) {
            rightPressed = true;
//            velX = -speed;
        }
        computeVelX();
    }

    public void keyReleased(KeyEvent e) {
        int key = e.getKeyCode();
        // This logic is a little more robust
        // because it handles cases where both
        // keys are being held at the same time.
        // Also see computeVelX().
        if (key == KeyEvent.VK_D) {
            leftPressed = false;
//            velX = 0;
        } else if (key == KeyEvent.VK_A) {
            rightPressed = false;
//            velX = 0;
        }
        computeVelX();
    }

    public void computeVelX() {
        // This way the keys will never
        // "stick". If both keys are
        // held at the same time, velX
        // is just 0 until one of the
        // keys is released.
        velX = 0;
        if (leftPressed) {
            velX += speed;
        }
        if (rightPressed) {
            velX -= speed;
        }
    }

    public void checkCollisions() {
        // I used a variable instead of calling
        // getBounds() repeatedly.
        Rectangle bounds = getBounds();

        if (bounds.getX() + bounds.getWidth() >= 500) {
            x1 = 440;
        } else if (bounds.getX() <= 0) {
            x1 = 0;
        }
    }

    // I change this from static to an instance method.
    public Rectangle getBounds() {
//        return new Rectangle(x1, y1 - 1, getPaddleImg().getWidth(null), getPaddleImg().getHeight(null));
        int width  = GameResources.paddleImage.getWidth(null);
        int height = GameResources.paddleImage.getHeight(null);
        return new Rectangle(x1, y1 - 1, width, height);
    }
}

class Ball {
    int velX;
    int velY;
    int speed = 3;
    int x, y;

    public Ball(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public void update(Game game) {
        x += velX;
        y += velY;
        checkCollisions(game);
    }

    public void draw(Graphics2D g2d) {
//        g2d.drawImage(getBallImg(), x, y, null);
        g2d.drawImage(GameResources.ballImage, x, y, null);
    }

//    public Image getBallImg() {
//        ImageIcon ic = new ImageIcon("C:/Users/Elliot/Desktop/Eclipse Game/ball.png");
//        return ic.getImage();
//    }

    public void checkCollisions(Game game) {
        // Using an iterator instead of looping with size()
        // directly, because we want to remove items from
        // the list while iterating.
        // The problem with removing while iterating with
        // size() is that once you remove an element, the
        // list shifts all the other elements back by 1,
        // so on the next iteration of the loop you end
        // up skipping an item.
        // (Say you remove the element at index 5. Then
        // all the elements shift back, so that e.g. the
        // element at index 6 is now at index 5. The variable
        // i is incremented, so you end up skipping the element
        // that was at index 6 before the removal.
        Iterator<Block> iter = game.getBlockList().iterator();
        Rectangle bounds = getBounds();

        while (iter.hasNext()) {
            Block b = iter.next();
            Rectangle bBounds = b.getBounds();

            if (bounds.intersects(bBounds) && velX != -speed) {
                velY = speed;
                velX =- speed;
//                Game.removeBlock(b);
                iter.remove();
            } else if (bounds.intersects(bBounds)) {
                velY = speed;
                velX = speed;
//                Game.removeBlock(b);
                iter.remove();
            }
        }
        //
        Rectangle pBounds = game.getPaddle().getBounds();

        if (bounds.intersects(pBounds)) {
            velY = -speed;
        } else if (bounds.getY() <= 0 && velX != speed) {
            velY = speed;
            velX =- speed;
        } else if (bounds.getY() <= 0 && velX != -speed) {
            velY = speed;
            velX = speed;
        } else if (bounds.getY() >= 400) {
            JOptionPane.showMessageDialog(null, "You Lost!  :( ");
            System.exit(0);
        }

        if (bounds.getX() <= 0) {
            velX = speed;
        } else if(bounds.getX() >= 500 - bounds.getWidth()) {
            velX = -speed;
        }
    }

    public Rectangle getBounds() {
//        return new Rectangle(x, y, getBallImg().getWidth(null), getBallImg().getHeight(null));
        int width  = GameResources.ballImage.getWidth(null);
        int height = GameResources.ballImage.getHeight(null);
        return new Rectangle(x, y, width, height);
    }
}

class Block {
    int x2, y2;

    public Block(int x2, int y2) {
        this.x2 = x2;
        this.y2 = y2;
    }

    public void update(Game game) {
    }

    public void draw(Graphics2D g2d){
//        g2d.drawImage(getBlockImg(), x2, y2, null);
        g2d.drawImage(GameResources.blockImage, x2, y2, null);
    }

//    public static Image getBlockImg() {
//        ImageIcon ic = new ImageIcon("C:/Users/Elliot/Desktop/Eclipse Game/block.png");
//        return ic.getImage();
//    }

    public Rectangle getBounds() {
//        return new Rectangle(x2, y2, getBlockImg().getWidth(null), getBlockImg().getHeight(null));
        int width  = GameResources.blockImage.getWidth(null);
        int height = GameResources.blockImage.getHeight(null);
        return new Rectangle(x2, y2, width, height);
    }
}

class GameResources {
    public static Image backgroundImage;
    public static Image blockImage;
    public static Image ballImage;
    public static Image paddleImage;

    public static void loadImages() throws IOException {
        // Load images once here.
        // I didn't test this method since I don't have the images, but it
        // should work. ImageIO.read will give better error messages than
        // using ImageIcon. ImageIcon.getImage() will just return null if
        // there was a problem, which doesn't tell you what the problem
        // actually was.
        paddleImage =
            ImageIO.read(new File("C:/Users/Elliot/Desktop/Eclipse Game/paddle.png"));
        ballImage =
            ImageIO.read(new File("C:/Users/Elliot/Desktop/Eclipse Game/ball.png"));
        blockImage =
            ImageIO.read(new File("C:/Users/Elliot/Desktop/Eclipse Game/block.png"));
        backgroundImage =
            ImageIO.read(new File("C:/Users/Elliot/Desktop/Eclipse Game/background.png"));
    }

    public static void loadInternetImages() throws IOException {
        // These images are from
        // http://stackoverflow.com/questions/19209650/example-images-for-code-and-mark-up-qas
        paddleImage =
            ImageIO.read(new URL("https://i.stack.imgur.com/gYxHm.png"));
        ballImage =
            ImageIO.read(new URL("https://i.stack.imgur.com/gJmeJ.png"));
        blockImage =
            ImageIO.read(new URL("https://i.stack.imgur.com/F0JHK.png"));
        backgroundImage =
            ImageIO.read(new URL("https://i.stack.imgur.com/P59NF.png"));
    }
}