Setting a label text in Swing undos all button location movements

Question

I have the weirdest bug ever.

I have this puzzle game that moves puzzle pieces (which really are buttons with images attached to them). Everything worked fine until I tried to change the text of some label (to indicate how many steps the player has done).

Everytime I call someControl.setText("text");, the puzzle pieces that moved are set back to the their first position. I have no idea why, but they just do.

Here's my window:

It consists of two panels, each uses a GridBagLayout. The main frame uses a gridBagLayout as well, which consists of the two panels.

I know it's weird as hell, but I can't figure out what may cause this GUI bug. Any idea?

The pieces of code:

increaseSteps which is called everytime I click a puzzle button

void increaseSteps() {
        _steps++;
        _lblSteps.setText("Steps: " + _steps);
    }

Creation of the puzzle panel (the left panel)

private JPanel puzzlePanel() {
        JPanel panel = new JPanel(new GridBagLayout());

        GridBagConstraints gbc = new GridBagConstraints();

        for (int i = 0; i < _splitImage.getSize(); i++)
            for (int j = 0; j < _splitImage.getSize(); j++) {
                int valueAtPos = _board.getMatrix()[i][j];
                if (valueAtPos == 0)
                    continue;

                int imageRow = _board.getImageRowFromValue(valueAtPos);
                int imageCol = _board.getImageColFromValue(valueAtPos);

                ImageIcon imageIcon = new ImageIcon(_splitImage.getImages()[imageRow][imageCol]);

                JButton btn = new JButton(imageIcon);
                _tileButtons[i][j] = new TileButton(btn, i, j);
                btn.setPreferredSize(new Dimension(_splitImage.getImages()[i][j].getWidth(null),
                        _splitImage.getImages()[i][j].getHeight(null)));

                // add action listener
                btn.addActionListener(this);
                btn.addKeyListener(this);

                gbc.gridx = j;
                gbc.gridy = i;
                panel.add(_tileButtons[i][j].getButton(), gbc);
            }

        return panel;
    }

actionPerformed:

@Override public void actionPerformed(ActionEvent e) { if (!(e.getSource() instanceof JButton)) return;

JButton btn = (JButton) e.getSource();

TileButton tile = getTileButtonFromBtn(btn);
if (tile == null)
    return;

// check if we can move the tile
String moveDir = _board.canMoveTile(tile.getRow(), tile.getCol());

if (moveDir.equals("no"))
    return;

increaseSteps();

int dirx = 0;
int diry = 0;

if (moveDir.equals("left")) {
    dirx = -1;
    _board.move("left", true);
    tile.setCol(tile.getCol() - 1);
} else if (moveDir.equals("right")) {
    dirx = 1;
    _board.move("right", true);
    tile.setCol(tile.getCol() + 1);
} else if (moveDir.equals("up")) {
    diry = -1;
    _board.move("up", true);
    tile.setRow(tile.getRow() - 1);
} else { // down
    diry = 1;
    _board.move("down", true);
    tile.setRow(tile.getRow() + 1);
}

moveButton(btn, dirx, diry, MOVE_SPEED);

if (_board.hasWon())
    win();

}

moveButton: (moves the button in a seperate thread, calling btn.setLocation())

private void moveButton(JButton btn, int dirx, int diry, int speed) {
        Point loc = btn.getLocation();

        // get start ticks, calculate distance etc...
        StopWatch stopper = new StopWatch();
        int distance;
        if (dirx != 0)
            distance = _splitImage.getImages()[0][0].getWidth(null) * dirx;
        else
            distance = _splitImage.getImages()[0][0].getHeight(null) * diry;

        if (speed > 0) {
            // run the animation in a new thread
            Thread thread = new Thread() {
                public void run() {
                    int currentTicks;

                    int elapsed;
                    do {
                        int newX = loc.x;
                        int newY = loc.y;
                        elapsed = stopper.getElapsed();

                        int moved = (int) ((double) distance * (double) (elapsed / (double) speed));

                        if (dirx != 0)
                            newX += moved;
                        else
                            newY += moved;

                        btn.setLocation(newX, newY);
                    } while (elapsed <= MOVE_SPEED);

                    // make sure the last location is exact
                    btn.setLocation(loc.x + (dirx == 0 ? 0 : distance), loc.y + (diry == 0 ? 0 : distance));
                }
            };

            thread.start();
        }

        else
            btn.setLocation(loc.x + (dirx == 0 ? 0 : distance), loc.y + (diry == 0 ? 0 : distance));
    }

Answer 1

You're trying to set the absolute position of a component via setLocation(...) or setBounds(...), one that is held by a container that uses a layout manager. This may work temporarily, but will fail if the container's layout manager is triggered to re-do the layout of its contained components. When that happens, the GridBagConstraints will take over and the components will move to their gridbag constraints assigned location.

The solution is to not do this, and instead to place the location of your components in concert with the layout managers used.

Another problem is that your current code is not Swing thread-safe since you're making Swing state changes from within a background thread. This won't always cause problems, but since it's a threading issue, risks causing intermittent hard to debug problems (ones that usually only occur when your boss or instructor are trying to run your code).

Possible solutions:

• For a grid of images, you could use a grid of JLabels (or JButtons if you must) held in a container that uses GridLayout. When you need to reposition components, remove all components held by that JPanel, and then re-add, using the order of addition to help you position the components.
• Easiest though would be to use a grid of non-moving JLabels, give them MouseListeners, and instead of moving the JLabels, remove and add Icons to them, including a blank Icon.
• If you need to do Swing animation, use a Swing Timer to drive the animation. This will allow your code to make repetitive calls with delay between the calls, and with these calls being made on the Swing event thread, the EDT (event dispatch thread).

Demo proof of concept example code that shows swapping icons, but without animation, and without test of solution yet:

import java.awt.GridLayout;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import java.awt.image.BufferedImage;
import java.io.IOException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import javax.imageio.ImageIO;
import javax.swing.*;

@SuppressWarnings("serial")
public class ImageShuffle extends JPanel {
    private static final int SIDES = 3;
    public static final String IMG_PATH = "https://upload.wikimedia.org/wikipedia/commons/"
            + "thumb/5/5a/Hurricane_Kiko_Sep_3_1983_1915Z.jpg/"
            + "600px-Hurricane_Kiko_Sep_3_1983_1915Z.jpg";
    private List<Icon> iconList = new ArrayList<>(); // shuffled icons
    private List<Icon> solutionList = new ArrayList<>();  // in order
    private List<JLabel> labelList = new ArrayList<>();  // holds JLabel grid
    private Icon blankIcon;

    public ImageShuffle(BufferedImage img) {
        setLayout(new GridLayout(SIDES, SIDES, 1, 1));
        fillIconList(img); // fill array list with icons and one blank one
        Collections.shuffle(iconList); 
        MyMouseListener myMouse = new MyMouseListener();
        for (Icon icon : iconList) {
            JLabel label = new JLabel(icon);
            label.addMouseListener(myMouse);
            add(label);
            labelList.add(label);
        }
    }

    private class MyMouseListener extends MouseAdapter {
        @Override
        public void mousePressed(MouseEvent e) {
            JLabel selectedLabel = (JLabel) e.getSource();
            if (selectedLabel.getIcon() == blankIcon) {
                return; // don't want to move the blank icon
            }
            // index variables to hold selected and blank JLabel's index location
            int selectedIndex = -1;
            int blankIndex = -1;
            for (int i = 0; i < labelList.size(); i++) {
                if (selectedLabel == labelList.get(i)) {
                    selectedIndex = i;                    
                } else if (labelList.get(i).getIcon() == blankIcon) {
                    blankIndex = i;
                }
            }

            // get row and column of selected JLabel
            int row = selectedIndex / SIDES;
            int col = selectedIndex % SIDES;

            // get row and column of blank JLabel
            int blankRow = blankIndex / SIDES;
            int blankCol = blankIndex % SIDES;

            if (isMoveValid(row, col, blankRow, blankCol)) {
                Icon selectedIcon = selectedLabel.getIcon();
                labelList.get(selectedIndex).setIcon(blankIcon);
                labelList.get(blankIndex).setIcon(selectedIcon);

                // test for win here by comparing icons held by labelList
                // with the solutionList
            } 
        }

        private boolean isMoveValid(int row, int col, int blankRow, int blankCol) {
            // has to be on either same row or same column
            if (row != blankRow && col != blankCol) {
                return false;
            }
            // if same row
            if (row == blankRow) {
                // then columns must be off by 1 -- they're next to each other
                return Math.abs(col - blankCol) == 1;
            } else {
                // or else rows off by 1 -- above or below each other
                return Math.abs(row - blankRow) == 1;
            }
        }

        public void shuffle() {
            Collections.shuffle(iconList);
            for (int i = 0; i < labelList.size(); i++) {
                labelList.get(i).setIcon(iconList.get(i));
            }
        }
    }

    private void fillIconList(BufferedImage img) {
        // get the width and height of each individual icon
        // which is 1/3 the image width and height
        int w = img.getWidth() / SIDES;
        int h = img.getHeight() / SIDES;
        for (int row = 0; row < SIDES; row++) {
            int y = (row * img.getWidth()) / SIDES;
            for (int col = 0; col < SIDES; col++) {
                int x = (col * img.getHeight()) / SIDES;
                // create a sub image
                BufferedImage subImg = img.getSubimage(x, y, w, h);
                // create icon from the image
                Icon icon = new ImageIcon(subImg);
                // add to both icon lists
                iconList.add(icon);
                solutionList.add(icon);
            }
        }

        // create a blank image and corresponding icon as well.
        BufferedImage blankImg = new BufferedImage(w, h, BufferedImage.TYPE_INT_ARGB);
        blankIcon = new ImageIcon(blankImg);
        iconList.remove(iconList.size() - 1);  // remove last icon from list
        iconList.add(blankIcon);   // and swap in the blank one
        solutionList.remove(iconList.size() - 1);  // same for the solution list
        solutionList.add(blankIcon);
    }


    private static void createAndShowGui(BufferedImage img) {
        JFrame frame = new JFrame("ImageShuffle");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.getContentPane().add(new ImageShuffle(img));
        frame.pack();
        frame.setLocationRelativeTo(null);
        frame.setVisible(true);
    }

    public static void main(String[] args) {
        URL imgUrl = null;
        BufferedImage img;
        try {
            imgUrl = new URL(IMG_PATH);
            img = ImageIO.read(imgUrl);
            SwingUtilities.invokeLater(() -> createAndShowGui(img));
        } catch (IOException e) {
            e.printStackTrace();
        }        
    }
}

If I wanted animation, again, I'd raise the icon into the JFrame's glasspane, animate it to the new position using a Swing Timer, and then place the icon into the new JLabel. I'd also disable the MouseListener using a boolean field, a "flag", until the animation had completed its move.