Swing timer only running on last event

Question

I am trying to build a Simon Says game which will flash the buttons to press. I am currently trying to work out how to make the buttons flash one after another. Following the documentation (to the best of my ability), I wrote the code below, but for some reason only the green button flashes. I tested it a bit further and found that only the last method inside the btnGo event works as desired. I think it is something to do with the way the timer is running and it is turning the red and blue buttons back to black before the timer has finished, but I'm not sure how or why?

public void flashRed(){
    btn1.setBackground(Color.red);
    btn2.setBackground(Color.black);
    btn3.setBackground(Color.black);
    btn4.setBackground(Color.black);
    repaint();
    t.start();
}
public void flashYellow(){
    btn1.setBackground(Color.black);
    btn2.setBackground(Color.yellow);
    btn3.setBackground(Color.black);
    btn4.setBackground(Color.black);
    repaint();
    t.start();
}
public void flashGreen(){
    btn1.setBackground(Color.black);
    btn2.setBackground(Color.black);
    btn3.setBackground(Color.green);
    btn4.setBackground(Color.black);
    repaint();
    t.start();
}
public void flashBlue(){
    btn1.setBackground(Color.black);
    btn2.setBackground(Color.black);
    btn3.setBackground(Color.black);
    btn4.setBackground(Color.blue);
    repaint();
    t.start();
}

@Override
public void actionPerformed(ActionEvent event) {
        if(event.getSource() == btnGo)
        {
            flashRed();
            flashBlue();
            flashGreen();

        }
        if(event.getSource() ==t){
            btn1.setBackground(Color.black); //resets btn1 to black
            btn2.setBackground(Color.black);
            btn3.setBackground(Color.black);
            btn4.setBackground(Color.black);

            repaint();
            t.stop(); //stops the timer
        }
    }

Answer 1

You need to have all of the waiting done in a background thread, and then put the colour change commands into the EDT with invoke later. If you do this, then you can do away with the timer construct altogether, as you can just sleep() the background thread. Think of it this way, the background thread is orchestrating the colour changes, and the EDT handles the actual screen widgets:

import java.awt.Color;
import java.awt.Dimension;
import java.awt.GridLayout;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;

import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.SwingUtilities;
import javax.swing.SwingWorker;

public class testFrame {

    static JButton btn1 = new JButton("red");
    static JButton btn2 = new JButton("yellow");
    static JButton btn3 = new JButton("green");
    static JButton btn4 = new JButton("blue");

    public static void showFrame() {
        JFrame frame = new JFrame();
        frame.setLocation(500, 500);
        frame.setSize(100, 100);
        final JButton button = new JButton("Test");
        button.setMaximumSize(new Dimension(80, 30));
        button.setSize(80, 20);
        frame.setLayout(new GridLayout(5, 1));
        frame.add(button);
        frame.add(btn1);
        frame.add(btn2);
        frame.add(btn3);
        frame.add(btn4);
        button.addMouseListener(new MouseAdapter() {
            @Override
            public void mouseClicked(MouseEvent e) {
                SwingWorker<Void, Void> worker = new SwingWorker<Void, Void>() {

                    @Override
                    public Void doInBackground() {
                        try {
                            flashRed();
                            Thread.sleep(500);
                            flashGreen();
                            Thread.sleep(500);
                            flashBlue();
                            Thread.sleep(500);
                            flashYellow();
                        } catch (InterruptedException e) {
                        }
                        return null;
                    }

                    @Override
                    public void done() {
                    }
                };
                worker.execute();
            }
        });
        frame.pack();
        frame.setVisible(true);
    }

    public static void flashRed() {
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                btn1.setBackground(Color.red);
                btn2.setBackground(Color.black);
                btn4.setBackground(Color.black);
                btn3.setBackground(Color.black);
            }
        });
    }

    public static void flashYellow() {
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                btn1.setBackground(Color.black);
                btn2.setBackground(Color.yellow);
                btn3.setBackground(Color.black);
                btn4.setBackground(Color.black);
            }
        });
    }

    public static void flashGreen() {
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                btn1.setBackground(Color.black);
                btn2.setBackground(Color.black);
                btn3.setBackground(Color.green);
                btn4.setBackground(Color.black);
            }
        });
    }

    public static void flashBlue() {
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                btn1.setBackground(Color.black);
                btn2.setBackground(Color.black);
                btn3.setBackground(Color.black);
                btn4.setBackground(Color.blue);
            }
        });
    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {

            @Override
            public void run() {
                showFrame();
            }
        });
    }
}

Answer 2

There are a couple of things to keep in mind:

• Swing is single threaded (thread is called the E(vent) D(ispatch) T(hread)). While you are doing stuff on that thread, your UI cannot repaint and vice versa.
• A call to repaint() does not actually performs a repaint. It just schedules one a repaint on the EDT. If multiple repaints are scheduled, they are simply grouped and only one is executed
• Most of the methods on JComponents do not change the component immediately. They just change the state of that component and schedule a repaint. When that repaint is done, you see the changes you made reflected in the UI.

So what happens when you press the JButton and your ActionListener is triggered:

flashRed();
flashBlue();
flashGreen();

This will change the background color of a lot of buttons, but there is simply no time to perform the repaint as your code is occupying the EDT. Only when your ActionListener has finished, the repaint can be executed. At that moment, all the button's background are changed back to black, except the green one. That is why you only see the green button flashing.

What you need to do to solve this is to flash them one by one and releasing the EDT in between, giving it time to perform the repaint. I would solve this by using a Timer. But why you are using it only to restore the background of the buttons to black, I would use it for the flashing as well.

In pseudo-code, the ActionListener would look like:

switch ( iteration ){
  case first:
    flashRed();
    increaseIteration();
    break;
  case second:
    flashBlue();
    increaseIteration();
    break;
  ...
  case last:
    restoreAllToBlack();
    timer.stop();
    break;
}