AlexanderBeloev
AlexanderBeloev

Reputation: 21

Adding multiple Jlabels to Jframe with for loop, removing them and printing them on a new positions

I am currently creating a Flappy bird clone in java and I have problems reprinting the pipes. What I mean is, that in the while loop I print a pipe image on the right, then I print old pipes if any (in the first loop there aren't any pipes). Then add the first pipe's X and Y coordinates to two separate lists (for storing pipes coordinates) then I change all the x coordinates in the list with 50 pixels left with a for loop. The very next step is to go to the top of the loop and delete all the pipes and start over. The problem is that I cannot remove all the pipes added with the for loop. here is the code in the while loop that I use (with two words frame.remove(pipeOld); is not removing all the old pipes, but just the first one.):

        while (true) {

        frameCounter++;
        int curentFrame = animationFrame;
        animationFrame = calculateAnimationFrame(curentFrame);

        frame.remove(pipe);
        frame.remove(pipeOld); 

        pipeY = rnd.nextInt(100); //declared already
        pipe = showPipes(pipeX,pipeY,animationFrame); // method

        frame.add(pipe); // printing pipe

        for (int i = 0; i < pipeListX.size(); i++) {            
            pipeOld = showPipes(pipeListX.get(i),pipeListY.get(i),animationFrame);
            frame.add(pipeOld);             
        }

        pipeListX.add(pipeX); // adding current pipe X to list
        pipeListY.add(pipeY); // adding current pipe y to list

        for (int i = 0; i < pipeListX.size(); i++) { // declaring new x for all the pipes, so they can move
            int pipeNew = pipeListX.get(i);
            pipeListX.set(i, pipeNew - 50);
        }


        frame.validate(); // Validates the window

        frame.repaint(); // Repaints the window

        Thread.sleep(15); // Game speed setting
    }

Upvotes: 1

Views: 261

Answers (1)

Ordous
Ordous

Reputation: 3884

Following advice from metaSO:

Answer to your particular problem
pipeOld stores only the last pipe. You overwrite it in the for loop, so it only ever contains the last pipe added, hence that is what gets removed on the next cycle iteration. If you want to keep track of all old pipes, you should use a collection, not a single pipe reference, and remove all of them in a loop.

for (JComponent pipe : oldPipes) {
    frame.remove(pipe);
}
oldPipes.clear();

<other code>

for (int i = 0; i < pipeListX.size(); i++) {            
    pipeOld = showPipes(pipeListX.get(i), pipeListY.get(i), animationFrame);
    frame.add(pipeOld);             
    oldPipes.add(pipeOld);
}

However you should not be doing that at all!
Adding and removing components in a loop, as well as using Thread.sleep in EDT are extremely bad practices!

  • As was already mentioned in comments, using Thread.sleep in the EDT will mean GUI freezes and unresponsiveness. Andrew Thompson is completely right in saying that a Timer or SwingWorker are much better for this. I will repost his link here so it doesn't get lost: Concurrency in Swing. You should look into switching to a MVC architecture as well, so that your game speed is not tied to the frame rate.
  • Adding and removing components are very expensive operations! The parent component completely recalculates its layout every time you do it. Most games that are real time (and hence have moving objects and need consistent frame rates) use a canvas on which they do custom painting.

Upvotes: 1

Related Questions