Somehow my GUI got stuck when I click on a button to perform a method call

Question

I want to update the board configuration by clicking on a JButton. However, sometimes the image is displayed on the frame. sometimes is not. Everytime there is a significant delay after I click on the button. I tried debugging and found there might be a endless loop in :EventDispatchThread.class

(this is the class from java library)
void pumpEventsForFilter(int id, Conditional cond, EventFilter filter) {
    addEventFilter(filter);
    doDispatch = true;
while (doDispatch && cond.evaluate()) {
        if (isInterrupted() || !pumpOneEventForFilters(id)) {
            doDispatch = false;
        }
    }
    removeEventFilter(filter);
}

The endless loop is the while loop above.

Below is my listener class:

public class PlaceListener implements ActionListener{

private JTextField _text1;
private Board board;
private JTextField _text2;
private ArrayList<NewGUI> _guiList;
private int _numOfPlayer;
public PlaceListener(JTextField text1, JTextField text2, Board b,ArrayList<NewGUI> guiList, int numOfPlayer,NewGUI gui)
{
    _text1 = text1;
    _text2 = text2;
    board = b;
    _guiList = guiList;
    _numOfPlayer = numOfPlayer;
}
@Override
public void actionPerformed(ActionEvent e) {

    int x = Integer.parseInt(_text1.getText());
    int y = Integer.parseInt(_text2.getText());

    board.Place(y, x);

    for(int j = 0;j<_numOfPlayer;j++)
    {
          NewGUI gui = _guiList.get(j);
          gui.updateBoard();
          gui.updateCurrTile();
          gui.updateScore();
          gui.updateTurn();
    }
}

}

The basic idea is: I have an array of GUI. after each click, the listener will call all the GUIs within the array to update their configuration.

I also tried to update the board configuration within the GUI class directly, and it turns out to work well. I'm extremely confused! Can anyone help me out? Thanks!!

This is the main GUI class:

public class NewGUI {
private JFrame _frame;
    private Board _board;
private JLabel _turnLabel;
private JTextArea _textArea;
private JLabel _currTileLabel;
private JPanel _boardPanel;
public NewGUI(Board board,int whos,ArrayList<NewGUI> guiList,int numOfPlayer)
{
    _board = board;



   _frame = new JFrame("Metro");    


   //turnLabel
   _turnLabel = new JLabel();
   _turnLabel.setText("Current player is: "+_board.getCurrPlayer());
   _turnLabel.setSize(110, 40);
   _turnLabel.setLocation(0, 0);
   _frame.add(_turnLabel);


   //mainPlayerLabel
   JLabel mainPlayerLabel = new JLabel("Player"+whos+" 's window");
   mainPlayerLabel.setSize(120, 20);
   mainPlayerLabel.setLocation(400,0);
   _frame.add(mainPlayerLabel);

   //JTextArea to hold scores
   _textArea = new JTextArea();
   _textArea.setText(_board.displayScore());
   _textArea.setSize(160,140);
   _textArea.setLocation(730, 170);
   _frame.add(_textArea);

   _boardPanel = new JPanel();
   _boardPanel.setSize(560, 560);
   _boardPanel.setLocation(170, 80);
   _boardPanel.setLayout(null);
 //  _boardPanel.setBackground(java.awt.Color.BLACK);
   _frame.add(_boardPanel);


   //Button Panel
   JPanel buttonPanel = new JPanel();
   buttonPanel.setSize(300, 150);
   buttonPanel.setLocation(280, 650);
   buttonPanel.setBackground(java.awt.Color.blue);
   _frame.add(buttonPanel);

   //Current Tile Label
   _currTileLabel = new JLabel("Current Tile is: ");
   _currTileLabel.setIcon(new ImageIcon(NewGUI.class.getResource(_board.getCurrTile().tileType()+".png")));
   _currTileLabel.setSize(170, 60);
   _currTileLabel.setLocation(20, 620);
   _frame.add(_currTileLabel);


   //2 input JTextField
   JTextField text1 = new JTextField(3);
   JTextField text2 = new JTextField(3);
   text1.setSize(20, 20);
   text2.setSize(20, 20);
   text1.setLocation(620, 680);
   text2.setLocation(640, 680);
   _frame.add(text1);
   _frame.add(text2);


   //Buttons
   JButton buttonPlace = new JButton("Place");
   JButton buttonCommit = new JButton("Commit");
   JButton buttonRemove = new JButton("Remove");
   JButton buttonResign = new JButton("Resign");

   buttonPlace.addActionListener(new PlaceListener(text1,text2,_board,guiList,numOfPlayer,this));
   buttonCommit.addActionListener(new CommitListener(_board,guiList,numOfPlayer));
   buttonRemove.addActionListener(new RemoveListener(_board,guiList,numOfPlayer,this));
   buttonResign.addActionListener(new ResignListener(_board));

   //Add buttons onto buttonPanel
   buttonPanel.add(buttonCommit);
   buttonPanel.add(buttonResign);
   buttonPanel.add(buttonRemove);
   buttonPanel.add(buttonPlace);
   buttonPanel.setLayout(new FlowLayout());

   _frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
   _frame.setSize(900, 900);
   _frame.setLayout(null);
   _frame.setVisible(true);

}


public void updateBoard()
{
    _boardPanel.removeAll();
    //scan and refresh the board configuration.
    for(int i = 1; i<13;i++)
    {
        for(int j = 1; j<13;j++)
        {
            if(_board.getBoard()[i][j]!=null)
            {

              for(int e = 65; e<89;e++){
                  char temp = (char)e;
                  if(_board.getBoard()[i][j].tileType()==temp)
                  {

                JLabel label = new JLabel(new ImageIcon(NewGUI.class.getResource(temp+".png")));
                label.setSize(40,40);
                label.setLocation(40+(i-1)*40, 40+(j-1)*40);
                _boardPanel.add(label);
                break;
                  }
            }
        }
        }
    }
}

public void updateTurn()
{
    _turnLabel.setText("Current player is: "+_board.getCurrPlayer());
}
public void updateScore()
{
    _textArea.setText(_board.displayScore());
}
public void updateCurrTile()
{
    _currTileLabel.setIcon(new ImageIcon(NewGUI.class.getResource(_board.getCurrTile().tileType()+".png")));
}


public static void main(String[] args)
{
    Board b = new Board(3,true);
    NewGUI gui = new NewGUI(b,1);
    b.Place(4, 4);
    b.Commit();
    b.Place(12, 12);
    b.Commit();
    b.Place(3, 3);
    gui.updateBoard();
}

}

See the last static main class? when I test it , all the update methods work well! But when I use listener to perform the method all. The updateBoard refuses to work.

Answer 1

So it looks like your code might be the problem here. Again EventDispatchThread uses an endless loop to work to pump events so that is obvious, and can be disregarded as the actual problem. You're problem comes from using removeAll(), and instantiating a few thousand labels every time they click the button (What's 13 x 13 x 89-65? 4056!). That's going to cause a lot of redrawing and relayout unnecessarily. So the pause you see is the performance of your code because it's not efficient. Don't believe try this:

public void updateBoard() {
    long start = System.currentTimeInMillis();

    // existing code goes here

    long duration = System.currentTimeMillis() - start;
    System.out.printf("UpdateBoard timing: %,d ms%n", duration );
}

If your code is anywhere over 10-100ms it will feel glitchy. In fact 100ms is on the slow side and a human can detect a delay of 100ms.

You probably need to re-evaluate your design, and either reuse the existing labels and simply call setImage() to change them. After all that's the whole point of using a persistent UI component model over using raw paint calls. Instantiate them once and reuse.

You are also creating thousands of images with new ImageIcon() calls too. You probably only need one icon and just have all labels point to the same image that will dramatically reduce your memory usage as well. In fact if you follow my advice I think you'll see dramatic speed and memory improvements.

If you can't find a suitable way to reuse JLabel then consider writing your own component by subclassing JComponent or JPanel (if you plan on using a container) and override paintComponent(). I see you're NOT using a LayoutManager and instead choosing to do everything using absolute positioning. If you are going to do absolute positioning you might want to just paint it yourself. Painting is more lower level interface, but you have full control. You'll have to handle positioning, word wrapping, all yourself. But, it will be very efficient, and you can redraw from a data model.

Since all your doing is draw images in a grid pattern I think drawing those with the Java2D api would be a better idea than instantiating that many JLabels and ImageIcons. If you subclass JPanel you can add JComponents to the panel for things like score, etc. But, draw the grid in the paintComponent() method.