animated rocks not spawning correctly

Question

I am creating a game where a caveman is throwing rocks, and when you click, a rock spawns. the first five or so work fine, then it waits until the rocks get off the screen and then they can spawn again. I would like them to spawn whenever i click.

Thanks in advance

Code:

 package com.russell.raphael.birds;

import java.awt.Toolkit;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;

import javax.swing.ImageIcon;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JOptionPane;
import javax.swing.JPanel;

@SuppressWarnings("serial")

public class Start extends JFrame {

    ImageIcon landImage, manImage, skyImage, RockPileImage, RockImage;
    JLabel skylbl, manlbl, landlbl, rockPilelbl;
    Bird[] birds = new Bird[10];
    Rock[] rocks = new Rock[10000];

    public static MouseListener throwrock;

    public static void main(String[] args){

        new Start();

    }

    public Start() {

        setVisible(true);

        initComp();
        setSize(1000, 1100);
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setLocationRelativeTo(null);
        setTitle("Not the Birds!!!");
        setIconImage(Toolkit.getDefaultToolkit().getImage(Start.class.getResource("/com/russell/raphael/images/Icon.png")));

    }

    private void initComp() {

        throwrock = new MouseAdapter() {
            @Override
            public void mouseClicked(MouseEvent e) {
                int eventX = 0, eventY = 0, sourceX = 420, sourceY = 840;
                int rise = 0, run = 0;

                try {
                    JLabel source = (JLabel) e.getSource();

                    eventX = (source.getLocation().x) + (e.getX());
                    eventY = (source.getLocation().y) + (e.getY());
                    rise = Math.abs(eventY - sourceY);
                    run = eventX - sourceX;


                    nextRock().start(rise, run);

                } catch (Exception ex) {

                    ex.printStackTrace();
                }

            }

        };

        setResizable(false);

        for(int counter =0; counter < rocks.length; counter++){

            rocks[counter] = new Rock();
            getContentPane().add(rocks[counter]);

        }

        landImage = new ImageIcon(
                Start.class.getResource("/com/russell/raphael/images/land.png"));
        manImage = new ImageIcon(
                Start.class.getResource("/com/russell/raphael/images/man.png"));
        skyImage = new ImageIcon(
                Start.class.getResource("/com/russell/raphael/images/sky.jpg"));
        RockPileImage = new ImageIcon(
                Start.class
                        .getResource("/com/russell/raphael/images/rockpile.png"));

        getContentPane().setLayout(null);

        skylbl = new JLabel(skyImage);
        skylbl.addMouseListener(throwrock);
        skylbl.setLocation(0, 0);
        skylbl.setSize(1010, 983);
        skylbl.setVisible(true);

        manlbl = new JLabel(manImage);
        manlbl.setSize(200, 300);
        manlbl.addMouseListener(throwrock);
        manlbl.setLocation(400, 700);

        landlbl = new JLabel(landImage);
        landlbl.setBounds(0, 725, 1000, 400);
        manlbl.addMouseListener(throwrock);

        rockPilelbl = new JLabel();
        rockPilelbl.setIcon(RockPileImage);
        rockPilelbl.setBounds(236, 782, 220, 174);
        getContentPane().add(rockPilelbl);
        manlbl.addMouseListener(throwrock);

        getContentPane().add(manlbl);
        getContentPane().add(landlbl);
        getContentPane().add(skylbl);

    }

    public Rock nextRock(){

        for(int counter = 0; counter < rocks.length; counter++){

            if(!rocks[counter].hasBeenUsed){

                return rocks[counter];

            }

        }
        return null;
    }
}

next class:

package com.russell.raphael.birds;

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

    import javax.swing.ImageIcon;
    import javax.swing.JLabel;
    import javax.swing.JOptionPane;
    import javax.swing.Timer;

    @SuppressWarnings("serial")
    public class Rock extends JLabel {

        Timer timer;

        Thread thread;

        boolean hasBeenUsed = false;

        public Rock() {

            super();

            setBounds(415, 840, 37, 35);
            setIcon(new ImageIcon(
                    Rock.class.getResource("/com/russell/raphael/images/Rock.png")));

            setVisible(true);
        }

        public void start(final int rise, final int run) {

            hasBeenUsed = true;

            thread = new Thread() {

                public void run() {

                    timer = new Timer(30, new ActionListener() {
                        @Override
                        public void actionPerformed(ActionEvent ae) {

                            setBounds(getBounds().x + run / 20, getBounds().y
                                    + -rise / 20, getBounds().width,
                                    getBounds().height);

                            if (getBounds().x < 0 || getBounds().y < 0
                                    || getBounds().y > 1000) {

                                timer.stop();

                                hasBeenUsed = false;

                                setBounds(415, 840, 37, 35);

                                thread.stop();

                            }

                        }

                    });
                    timer.start();
                }
            };

            thread.start();

        }
    }
</code>

Answer 1

I have no issue with getting the pool of rocks to empty and then waiting for them to disapear, but I have plenty of other issues...

Lets start with...

nextRock().start(rise, run);

This will potentially return a null object, which will cause a NullPointerException

As I stated before, you shouldn't need to mix Thread and Timer, here's my simple take on the rock..

public class Rock extends JLabel {

    Timer timer;
    private volatile boolean hasBeenUsed = false;

    public Rock() {
        super();
        setIcon(new ImageIcon(Rock.class.getResource("/Rock.png")));
        setBounds(415, 840, getPreferredSize().width, getPreferredSize().height);
        setVisible(false);
    }

    public void start(final int rise, final int run) {
        hasBeenUsed = true;
        setVisible(true);
        timer = new Timer(30, new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent ae) {
                setBounds(getBounds().x + run / 20, getBounds().y
                                + -rise / 20, getBounds().width,
                                getBounds().height);

                if (getBounds().x < 0 || getBounds().y < 0
                                || getBounds().y > 1000) {
                    timer.stop();
                    hasBeenUsed = false;
                    setBounds(415, 840, getPreferredSize().width, getPreferredSize().height);
                }
            }
        });
        timer.start();
    }
}

Your rock pool contains 10,000 rocks!! I don't think you can click fast enough to exhaust this pile.

Try reducing it down to something like 2 or 3 for testing...

Side note

This is a very expensive call...

public Rock nextRock() {
    for (int counter = 0; counter < rocks.length; counter++) {
        if (!rocks[counter].hasBeenUsed) {
            return rocks[counter];
        }
    }
    return null;
}

If you have 9, 999 rocks in the air, it's going to take a noticeable amount of time to get to the last one as you must iterate through all the rocks that are currently unavailable.

It might be easier to add all the rocks to a java.util.List (lets call it the rockPile). As rocks are thrown, you could check this list to see if there are any available rocks, if not, return, otherwise, remove the first rock from the list and place into a "inFlight" list. This will make it faster to check the rock pile (using something like List#isEmpty and List#get(0) are much faster).

When a rock becomes "available" again, you would remove it from the "inFlight" list and place back into the rockPile list.

This would also mean you could potentially get down to one Timer that would be responsible for iterating through the inFlight list and updating there positions...

UPDATE using a List

Basically, you need some kind of "manager" that maintain all the required information and can be shared between the interested parties...

public class RockManager {

    private List<Rock> rockPile;
    private List<Rock> inFlight;
    public static final int MAX_ROCKS = 2;

    public RockManager() {
        rockPile = new ArrayList<Rock>(MAX_ROCKS);
        inFlight = new ArrayList<Rock>(MAX_ROCKS);
        for (int index = 0; index < MAX_ROCKS; index++) {
            rockPile.add(new Rock(this));
        }
    }

    public Rock[] getRocksOnPile() {
        return rockPile.toArray(new Rock[rockPile.size()]);
    }

    public Rock[] getRocksInFlight() {
        return inFlight.toArray(new Rock[inFlight.size()]);
    }

    public Rock pickRockOfPile() {
        Rock rock = null;
        if (!rockPile.isEmpty()) {
            rock = rockPile.remove(0);
            inFlight.add(rock);
        }
        return rock;
    }

    public void putRockBackOnPile(Rock rock) {
        if (inFlight.contains(rock)) {
            inFlight.remove(rock);
            rockPile.add(rock);
        }
    }
}

NB: Technically, you probably don't "need" inFlight, but I'm using as a trap to ensure that rocks can't be added to the pile that aren't in flight ;). You could also use a Set for the rockPile as this will ensure that only unique references are contained within the list...

Next, I added a instance field of RockManager to the Start class.

In the initComp method of your Start class, you need to change the way you construct your rocks...

So, instead of

for(int counter =0; counter < rocks.length; counter++){
    rocks[counter] = new Rock();
    getContentPane().add(rocks[counter]);
}

You would now have...

rockManager = new RockManager();
for (Rock rock : rockManager.getRocksOnPile()) {
    getContentPane().add(rock);
}

You're nextRock method would become...

public Rock nextRock() {
    return rockManager.pickRockOfPile();
}

And finally, your Rock class would require a reference to the RockManager, basically, I pass a reference in via the constructor...

private RockManager manager;

public Rock(RockManager manager) {
    super();
    this.manager = manager;

Then, when the rock leaves the playing field, you can add it back to the pile via the RockManager...

manager.putRockBackOnPile(Rock.this);