Hello Hello - 5 months ago 19
Java Question

Java Connect4 Game with GUI bug issues

http://imgur.com/a/V7LPP
Grid images are in the link

I have 2 main issues with my current connect4 code, sometimes the "AI" places 2 discs in one turn and the player can't fill the entire board without the "AI" entering an infinite loop. Any help is appreciated.

/**
* Auto Generated Java Class.
*/
import java.awt.Dimension;
import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.BoxLayout;
import javax.swing.ImageIcon;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;

public class EmptyFrame1 implements ActionListener
{
//Array of JButtons
static JButton [] mnuBtn = new JButton[2];
static JButton [] btnArray = new JButton[7];
static JLabel [][] board = new JLabel[6][7];
static int [][] numBoard = new int[6][7];
static int choice = 0;
static int playerColour = 1;
static int computerColour = 2;
static int computerTurn = 0;
static ImageIcon emptyGrid = new ImageIcon("EmptyGrid.png");
static ImageIcon yellowGrid = new ImageIcon("YellowGrid.png");
static ImageIcon redGrid = new ImageIcon("RedGrid.png");
static JPanel pnlBoard;
static boolean validTurn;
static int pieceCount = 42;

public EmptyFrame1()
{

JFrame game = new JFrame("Connect 4");

//mainFrame panel to hold all components
JPanel mainFrame = new JPanel();

pnlBoard = new JPanel();

pnlBoard.setLayout(new GridLayout(7,6));

//Change mainFrame layout to vertical BoxLayout
mainFrame.setLayout(new BoxLayout(mainFrame, BoxLayout.Y_AXIS));

//For every single button
//Set the button text to its index value, add an action listener and add it to the pnlButtons
for (int i = 0; i < btnArray.length; i++)
{
btnArray[i] = new JButton("place");
btnArray[i].addActionListener(this);
pnlBoard.add(btnArray[i]);
}//end for

for (int r = 0; r < board.length; r++)
{
for (int c = 0; c < board[r].length; c++)
{
board[r][c] = new JLabel(emptyGrid);
board[r][c].setPreferredSize(new Dimension(69, 69));
pnlBoard.add(board[r][c]);
}//end for
}//end for

//Add all the panels to the mainFrame panel
mainFrame.add(pnlBoard);

//add mainFrame to the JFrame
game.add(mainFrame);
game.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE); // Added this for security
game.pack();
game.setVisible(true);
game.setResizable(false);
}

//*Action listener for all buttons*
public void actionPerformed(ActionEvent e)
{
for (int i = 0; i < btnArray.length; i++)
{
//if the current button was triggered, set the choice to the button index
if (btnArray[i] == e.getSource())
{
choice = i;
colCheck(choice, 1);
//System.out.println("User turn used");
pieceCount--;

for (int j = 0; j < btnArray.length; j++)
{
if (numBoard[0][j] == 0) btnArray[j].setEnabled(true);
}
if (pieceCount > 0)
{
validTurn = false;
while(!validTurn)
{
computerTurn = (int) (Math.random() * 7);

System.out.print(computerTurn + " ");
validTurn = colCheck(computerTurn, 2);
//System.out.println("CPU tried to move");
}
}
System.out.println();
setGrid();
pieceCount--;
System.out.println(pieceCount);

validTurn = false;
}//end if
}//end for

pnlBoard.repaint();

}//end actionPerformed
public static boolean colCheck(int choice, int currentTurn)
{
int row = -1;
for (int r = 5; r >= 0; r--)
{
if (numBoard[r][choice] == 0)
{
row = r;
break;
}
}
//System.out.println("Row That CPU Chooses: " + row);
if (row > -1)
{
numBoard[row][choice] = currentTurn;
if (row == 0)
{
btnArray[choice].setEnabled(false);
return false;
}
return true;
}
return false;
}

public static void setGrid()
{
for (int r = 0; r < numBoard.length; r++)
{
for (int c = 0; c < numBoard[r].length; c++)
{
if (numBoard [r][c] == 0)
{
board[r][c].setIcon(emptyGrid);
}
else if (numBoard [r][c] == 1)
{
board[r][c].setIcon(redGrid);
}
else if (numBoard [r][c] == 2)
{
board[r][c].setIcon(yellowGrid);
}
}
}
}

/**
* Inner helper class that defines the graphics
*/


public static void main(String[] args)
{
new EmptyFrame1();
}
//end constructor

Answer

Some problems and suggestions:

  • Your colCheck method and the while (!validTurn) { loop in the actionPerformed method is where the problem is located.
    • This method should not set numBoard value or disable buttons. In other words, it should not have any "side effects".
    • Instead it should only check for validity and then return a boolean value, nothing more or less
    • You should have another method that is called first, one that checks if no valid columns are available, if the game is effectively over. This should be called before the while loop above and should prevent the while loop from entering. This will end your endless loop problem because it's looping because no valid columns can be found for the computer turn.
    • You should have another method for setting the numBoard state and for enabling and disabling JButtons.

Change your colCheck to this to see where the loop is coming from:

public static boolean colCheck(int choice, int currentTurn) {
    int row = -1;
    for (int r = 5; r >= 0; r--) {
        if (numBoard[r][choice] == 0) {
            row = r;
            break;
        }
    }
    // System.out.println("Row That CPU Chooses: " + row);
    if (row > -1) {
        numBoard[row][choice] = currentTurn;
        if (row == 0) {
            btnArray[choice].setEnabled(false);
            System.out.printf("debug 1 row %d %n", row);
            return false;
        }
        System.out.printf("debug 2 row %d %n", row);
        return true;
    }
    System.out.printf("debug 3 row %d %n", row);
    return false;
}

Other issues not directly related to your problem at hand, but which should be addressed

  • You're grossly over-using the static modifier, and in fact none of your fields should be static. All should be private instance fields.
  • You've got your program logic code, the model, mixed in the same class as the GUI, the view, making it hard to debug problems and enhance the program. Much better if you could make your model a completely separate class, one that is "view-agnostic" meaning that it is testable on its own and can work with any view, be it a command line or Swing or Android UI.
  • Your question depends on images, EmptyGrid.png, YellowGrid.png.... that we have no access to, preventing us from testing it.
  • You're using magic numbers, such as 0, 1, 2 for position values, and need to avoid doing this.
  • Instead use an enum for empty, user and computer
  • This is Java not Javascript. You'll want to be clear on the difference because they're two completely different languages.
Comments