Sign in to follow this  
soitsthateasy

Problems With Connect 4 [C++]

Recommended Posts

soitsthateasy    100
I've been trying to make a game of connect four for the last while but I'm having some stupid problems. 1)When I compile the code, it just goes into an infinite loop of "Pick a place to move". 2)My code is an absoloute mess. 3)I have no AI. So what can I do to fix these problems. For 2) Should I use references / pointers or will I leave it with a load of global variables? I haven't optimized this yet and I am not really looking for optimization tips just yet. Here's the code:
#include <iostream>
using namespace std;

const int ROWS=5;
const int COLUMNS=6;
char board [ROWS][COLUMNS];

const char BORDER='@';
const char COMPUTER='O';
const char HUMAN='X';
const char SPACE=' ';
const char C='C';
const char H='H';
const char T='T';

char piece;
int move;


void populate();
void displayBoard();
int isLegal(int move);
int humanMove();
void updateBoard(int move, char piece);
int aiMove();
char changeTurn(char piece);
char game();


void populate()
{
    //Populate the borders
    for (int i=0; i<=COLUMNS; i++)
    {
        board[i][0]=BORDER;//left...
    }

    for (int i=0; i<=COLUMNS; i++)
    {
        board[ROWS][i]=BORDER;//bottom...
    }

    for (int i=0; i<=COLUMNS; i++)
    {
        board[i][COLUMNS]=BORDER;//right...
    }

//Fill in spaces...
    for (int i=0; i<=ROWS; i++)
    {
        for (int j=0; j<=COLUMNS; j++)
        {
            if(board[i][j]!=BORDER)
            {
                board[i][j]=SPACE;
            }
        }
    }
}


void displayBoard()
{
    for(int i=0; i<=ROWS; i++)
    {
        for(int j=0; j<=COLUMNS; j++)
        {
            cout<<board[i][j];
        }
        cout<<"\n";
    }
}


int isLegal(int move)
{
    if(board[ROWS][move]==SPACE)
        return 1;

    else
        return 0;
}


int humanMove()
{
    do
    {
        cout<<"\nPick a place to move: ";
        cin>>move;
    }while(!isLegal(move));

    return move;
}


void updateBoard(int move, char piece)
{
    int rows=ROWS;
    while(board[rows][move]==SPACE)
    {
        --rows;
    }

    if(board[rows][move]!=SPACE)
    {
        board[rows++][move]=piece;
    }
}


int aiMove()
{
    return 3;
}


char changeTurn(char piece)
{
    if(move==HUMAN)
        return COMPUTER;

    else
        return HUMAN;
}


char game()
{
    if(board[4][1]==HUMAN)
    return H;
}

int main()
{
    piece=HUMAN;
    char gameState;
    populate();
    displayBoard();

    do
    {
        if(piece==HUMAN)
        {
            move = humanMove();
            updateBoard(move, HUMAN);
        }

        else
        {
            move = aiMove();
            updateBoard(move, COMPUTER);
        }

        piece = changeTurn(piece);
        gameState = game();
    }while(gameState!=T && gameState!=T && gameState!=T);

    if(gameState==T)
    {
        cout<<"\n\nThe game was a tie, Well done.\n\n";
        return 0;
    }

    if(gameState==C)
    {
        cout<<"\n\nUnlucky, I won this time!\n\n";
        return 0;
    }

    else
    {
        cout<<"\n\nWell done, You won!\n\n";
        return 0;
    }

}

Sorry that my code is a mess!

Share this post


Link to post
Share on other sites
Gage64    1235
I think this line:

while(gameState!=T && gameState!=T && gameState!=T);

was supposed to be:

while(gameState!=T && gameState!=C && gameState!=H);

And definitely try to get rid of the global variables. Limiting the scope of variables and passing each function just what it needs can greatly simplify your code and reduce the chance to make errors.

Share this post


Link to post
Share on other sites
grekster    640
const int ROWS=5;
const int COLUMNS=6;
char board [ROWS][COLUMNS];

This creates a 'board' array with usable indices as follows:

board[0-4][0-5]

But you repeatedly use indices outside of this range (by indexing with ROW and column (5 and 6) ). This would be the first thing id fix.

Share this post


Link to post
Share on other sites
Narf the Mouse    322
Comment *Everything*. After you're done that, you should have a much better idea of what your code actually does and where it needs to be fixed.

If a piece of code doesn't seem to do what it's supposed to, note both what it's supposed to do and what it seems to be doing.

Share this post


Link to post
Share on other sites
soitsthateasy    100
I've put in comments, changed the array indexes and changed the thing that checks for a win, tie or other.
I changed the array to a 2d vector and I'm passing by references / const references when I can or when I need to. Now the code compiles but then a little Windows window comes up saying a problem caused the program to stop working correctly. I have no idea why or how this could be happening.

Help?


#include <iostream>
#include <vector>
using namespace std;

const int ROWS=7; //These can be changed as you want
const int COLUMNS=10;

const char BORDER='@'; //Marks the border
const char COMPUTER='O'; //Computers piece
const char HUMAN='X'; //Humans piece
const char SPACE=' '; //Un-occuppied place on the board
const char C='C'; //This is returned from the game function if the computer has won
const char H='H'; //This is returned from the game function if the human has won
const char T='T'; //This is returned from the game function if it's a tie


//Declaring all the functions
void populate(vector <vector <char> > &board); //Populates the board for the first time
void displayBoard(const vector <vector <char> > &board); //Displays the board. Should use a const ref...
int isLegal(int &move); //Checks if a move is legal
int humanMove(); //Gets the humans move
void updateBoard(int move, char piece, vector <vector <char> > &board); //Updates the board with a move
int aiMove(); //Finds the computers move. Find algorithim...
char changeTurn(char &piece); //Changes the turn from human to computer or vice-versa
char game(); //Returns whether the game has been won, drawn or neither


void populate(vector <vector <char> > &board)
{
//Populate the borders
for (int i=0; i<COLUMNS; i++)
{
board[i][0]=BORDER;//fill in the left border
}

for (int i=0; i<COLUMNS; i++)
{
board[ROWS-1][i]=BORDER;//fill in the bottom border
}

for (int i=0; i<COLUMNS; i++)
{
board[i][COLUMNS-1]=BORDER;//fill in the right border
}

//Fill in spaces...
for (int i=0; i<ROWS; i++)
{
for (int j=0; j<COLUMNS; j++)
{
if(board[i][j]!=BORDER) //Loops through the whole board. If a space isn't occupied by a border, fill it with a space
{
board[i][j]=SPACE;
}
}
}


}


void displayBoard(const vector <vector <char> > &board)
{
for(int i=0; i<ROWS; i++)
{
for(int j=0; j<COLUMNS; j++)
{
cout<<board[i][j]; //displays the board
}
cout<<"\n";
}
}


int isLegal(int move, const vector <vector <char> > &board)
{
if(board[ROWS-1][move]==SPACE)
{
//checks if the top row in the column specified by the move is free. If it is, It's legal. Else it's not
return 1;
}

else
return 0;
}


int humanMove(int &move, const vector <vector <char> > &board)
{
do
{
cout<<"\nPick a place to move: ";
cin>>move; //asks for a move of the player
}while(!isLegal(move, board)); //keep asking while the move isn't legal

return move;
}


void updateBoard(int move, char piece, vector <vector <char> > &board)
{
int rows=ROWS-1; //If I used ROWS it would be outside the range?
while(board[rows][move]==SPACE)//While the move hasn't hit another piece or the bottom
{
--rows; //make the piece drop down another row
}
//If it gets to here, it must be on the bottom or another piece so add one to the rows to make it sit on top of it
if(board[rows][move]!=SPACE)
{
board[rows++][move]=piece;
}
}


int aiMove()
{
return 3; //Yeah... Find an Algorithim
}


char changeTurn(char &piece)
{
if(piece==HUMAN) //Change the piece
return COMPUTER;

else
return HUMAN;
}


char game(const vector <vector <char> > &board)
{
if(board[4][1]==HUMAN) //Don't know what to do here... How will I get this to work?
return H;
}

int main()
{
char piece=HUMAN;//Human goes first
char gameState; //stores the win, draw or neither
int move; //stores a move for both players (computer and human)
vector <vector <char> > board (ROWS, vector <char> (COLUMNS) ); //2d vector for the board
populate(board); //call function to populate the board
displayBoard(board); //display it

do //game loop
{
if(piece==HUMAN)
{
move = humanMove(move, board); //get human move
updateBoard(move, piece, board);//update board
}

else
{
move = aiMove(); //get computer move
updateBoard(move, piece, board);//update board
}

displayBoard(board); //display board
piece = changeTurn(piece); //change the board
gameState = game(board); //check for win
}while(gameState!=T && gameState!=C && gameState!=H);

if(gameState==T)//If its a tie
{
cout<<"\n\nThe game was a tie, Well done.\n\n";
return 0;
}

if(gameState==C)//If the computer won
{
cout<<"\n\nUnlucky, I won this time!\n\n";
return 0;
}

else //Human must have won
{
cout<<"\n\nWell done, You won!\n\n";
return 0;
}

}



Thanks!

Share this post


Link to post
Share on other sites
iMalc    2466
You still have buffer overuns.
Your board is not square, so this is not right:
    for (int i=0; i<COLUMNS; i++)
{
board[i][COLUMNS-1]=BORDER;//fill in the right border
}
The last cell it touches is board[COLUMNS-1][COLUMNS-1] oops!

Why do you have numerous function protoptypes that do not match their definition?! It seems that this is not the actual compileable code any more.

What is it about your code that feels messy? For me one part of that is the lack of using OOP. Perhaps you could consider coming up with a plan on how to use OOP here. It looks to me like the board could be a class.

Statements like "Comment *Everything*" are just plain wrong. They lead to absurdities such as
        displayBoard(board);   //display board

Don't go overboard. Commenting is a tool just like everything else. Use it when it makes the most sense, and otherwise simply make the code self-explanatory.
Am I right that you don't feel that you "have a much better idea of what your code actually does"? Commenting mostly only helps others understand your code, or yourself if you haven't worked on it for a while. Since this code is fresh in your head, I expect that you gained very little from the effort.
One aim was obviously to get you to re-examine your code, in the hopes that you would spot some bugs. However I don't really see that this happened, and would have been surprised if it had.

Share this post


Link to post
Share on other sites
Narf the Mouse    322
Strangely enough, whenever I have written code that doesn't seem to work the way I want, commenting it leads to a greater understanding.

granted, your mileage may vary, but I don't post solutions when I don't think I know what I'm talking about.

Share this post


Link to post
Share on other sites
soitsthateasy    100
I was getting some weird errors trying to access this site over the last few days... Gamedev didn't renew their domain name? Anyways:

I can't get the isLegal function to work. It takes a reference for the move and a constant reference for a 2d vector of the board.
I want it to first check if the number is out of the range (less than 0 or greater than COLUMNS) and then I want it to check if the top row in the specified column is free. If it is, the move is legal. Else the move is illegal.
The function should return 1 if it's legal and 0 if it's illegal. I'll post the whole code for claritys sake but I'll make the relavent functions noticable.

Anyways, here's the code:

#include <iostream>
#include <vector>
using namespace std;

const int ROWS=7; //These can be changed as you want
const int COLUMNS=10;

const char BORDER='@'; //Marks the border
const char COMPUTER='O'; //Computers piece
const char HUMAN='X'; //Humans piece
const char SPACE=' '; //Un-occuppied place on the board
const char C='C'; //This is returned from the game function if the computer has won
const char H='H'; //This is returned from the game function if the human has won
const char T='T'; //This is returned from the game function if it's a tie


//Declaring all the functions
void populate(vector <vector <char> > &board); //Populates the board for the first time
void displayBoard(const vector <vector <char> > &board); //Displays the board. Should use a const ref...
int isLegal(int &move, const vector <vector <char> > &board); //Checks if a move is legal
int humanMove(int &move, const vector <vector <char> > &board); //Gets the humans move
void updateBoard(int &move, char &piece, vector <vector <char> > &board); //Updates the board with a move
int aiMove(); //Finds the computers move. Find algorithim...
char changeTurn(char &piece); //Changes the turn from human to computer or vice-versa
char game(const vector <vector <char> > &board); //Returns whether the game has been won, drawn or neither


void populate(vector <vector <char> > &board)
{

//Populate the borders
for (int i=0; i<ROWS; i++)
{
board[i][0]=BORDER;//fill in the left border
}

for (int i=1; i<COLUMNS; i++)
{
board[ROWS-1][i]=BORDER;//fill in the bottom border
}

for (int i=0; i<ROWS; i++)
{
board[i][COLUMNS-1]=BORDER;//fill in the right border
}

//Fill in spaces...
for (int i=0; i<ROWS; i++)
{
for (int j=0; j<COLUMNS; j++)
{
if(board[i][j]!=BORDER) //Loops through the whole board. If a space isn't occupied by a border, fill it with a space
{
board[i][j]=SPACE;
}
}
}


}


void displayBoard(const vector <vector <char> > &board)
{
cout<<"\n\n";
for(int i=0; i<ROWS; i++)
{
for(int j=0; j<COLUMNS; j++)
{
cout<<board[i][j]; //displays the board
}
cout<<"\n";
}
}


/******************************************************************************/
int isLegal(int &move, const vector <vector <char> > &board)
{
if(move>=COLUMNS-1 || move<=0)
{
return 0;
}

if(board[ROWS-1][move]==SPACE)
{
//checks if the top row in the column specified by the move is free. If it is, It's legal. Else it's not
return 1;
}

else
return 0;
}
/******************************************************************************/


/******************************************************************************/
int humanMove(int &move, const vector <vector <char> > &board)
{
do
{
cout<<"\nPick a place to move: ";
cin>>move; //asks for a move of the player
}while(!isLegal(move, board)); //keep asking while the move isn't legal

return move;
}
/******************************************************************************/

void updateBoard(int &move, char &piece, vector <vector <char> > &board)
{
int rows=ROWS-1; //If I used ROWS it would be outside the range?
while(1)//While the move hasn't hit another piece or the bottom
{
if(board[rows][move]==SPACE)
{
--rows;
}

//If it gets to here, it must be on the bottom or another piece so add one to the rows to make it sit on top of it
if(board[rows][move]!=SPACE)
{
board[++rows][move]=piece;
}

}
}


int aiMove()
{
return 3; //Yeah... Find an Algorithim
}


char changeTurn(char &piece)
{
if(piece==HUMAN) //Change the piece
return COMPUTER;

else
return HUMAN;
}


char game(const vector <vector <char> > &board)
{
if(board[4][1]==HUMAN) //Don't know what to do here... How will I get this to work?
return H;
}

/******************************************************************************/
int main()
{
char piece=HUMAN;//Human goes first
char gameState; //stores the win, draw or neither
int move; //stores a move for both players (computer and human)
vector <vector <char> > board (ROWS, vector <char> (COLUMNS) ); //2d vector for the board
populate(board); //call function to populate the board
displayBoard(board); //display it

do //game loop
{
if(piece==HUMAN)
{
move = humanMove(move, board); //get human move
updateBoard(move, piece, board);//update board
}

else
{
move = aiMove(); //get computer move
updateBoard(move, piece, board);//update board
}

displayBoard(board); //display board
piece = changeTurn(piece); //change the board
gameState = game(board); //check for win
}while(gameState!=T && gameState!=C && gameState!=H);

if(gameState==T)//If its a tie
{
cout<<"\n\nThe game was a tie, Well done.\n\n";
return 0;
}

if(gameState==C)//If the computer won
{
cout<<"\n\nUnlucky, I won this time!\n\n";
return 0;
}

else //Human must have won
{
cout<<"\n\nWell done, You won!\n\n";
return 0;
}

}
/******************************************************************************/



They're might be some buffer overflows somewhere in there too but I think I've found most of them!

Thanks!

Share this post


Link to post
Share on other sites
iMalc    2466
Some random thoughts:

populate fills a border like this:
    for (int i=1; i<COLUMNS; i++)
{
board[ROWS-1][i]=BORDER;//fill in the bottom border
}
But you expect cells to contain a space here:
int isLegal(int &move, const vector <vector <char> > &board)
{
if(move>=COLUMNS-1 || move<=0)
{
return 0;
}

if(board[ROWS-1][move]==SPACE)
{
//checks if the top row in the column specified by the move is free. If it is, It's legal. Else it's not
return 1;
}

else
return 0;
}


Perhaps you want to check ROWS-2?

Returning an int 0 or 1 is what people do in C, where there is no native bool type. In C++ you should really be using bool for things like this. The function is also way longer than it should be. You could simplify it to more like this:

bool isLegal(int &move, const vector <vector <char> > &board)
{
return (move > 0) && (move < COLUMNS-1) && (board[ROWS-2][move] == SPACE);
}



Why is updateBoard explicitly an infinite loop? Clearly the loop will need to exit at some point. You seem to be using 1 for true again. Actually, infinite loops are best coded as for(;;) since that prevents any warnings about the loop expression being constant on some compilers.

Personally I would not store the border as part of the board data. I'd just make sure I wrote it out as part of the printed output.

Share this post


Link to post
Share on other sites
soitsthateasy    100
Ok, unless I'm reading the array upside down This is really weird...

In the populate function, if I make the bottom border board[0][i]=BORDER; It fills in the top of the board and the game works perfectly (apart from the AI and the Checking for the winner...) Is my representation of a 2d vector messed up? Here's what I think:

1|2|3
4|5|6
7|8|9

1=board[2][0];
5=board[1][1];
9=board[0][2];

Is that right? For a 2d vector or array [3][3]?

Bool would work but my compiler doesn't really support it that well...

updateBoard() was a infinite loop. Thanks. I've fixed it now.

Any thoughts on the messed up vector?

Share this post


Link to post
Share on other sites
Zahlman    1682
Quote:
Original post by soitsthateasy
Is my representation of a 2d vector messed up?


Actually, defining the representation is your job. What matters is that you interpret it the same way at all times - when you add a marker, when you check for a win, and when you display the board. You've been using it everywhere except populate() and displayBoard() such that row 0 is at the bottom and row ROWS-1 is at the top. That's why you check board[ROWS-1] in isLegal() (i.e. is there space to drop the marker in from the top), and decrease the row value in updateBoard(). But in displayBoard(), because each line you output to the standard output will be from top to bottom, you put row 0 at the top.

Quote:
Bool would work but my compiler doesn't really support it that well...


Sure it does. Why don't you think it does?

Share this post


Link to post
Share on other sites
soitsthateasy    100
My compiler hasn't been working right for the last few days... If I want to make a new file I have to save it in Public or else it won't recongnise iostream or anything else. It randomly won't output anything and sometimes it crashes the computer.
Anyways...

I think I understand what you're saying, and I updated my code accordingly:


#include <iostream>
#include <vector>
using namespace std;

const int ROWS=7; //These can be changed as you want
const int COLUMNS=10;

const char BORDER='@'; //Marks the border
const char COMPUTER='O'; //Computers piece
const char HUMAN='X'; //Humans piece
const char SPACE=' '; //Un-occuppied place on the board
const char C='C'; //This is returned from the game function if the computer has won
const char H='H'; //This is returned from the game function if the human has won
const char T='T'; //This is returned from the game function if it's a tie


//Declaring all the functions
void populate(vector <vector <char> > &board); //Populates the board for the first time
void displayBoard(const vector <vector <char> > &board); //Displays the board. Should use a const ref...
int isLegal(int &move, const vector <vector <char> > &board); //Checks if a move is legal
int humanMove(int &move, const vector <vector <char> > &board); //Gets the humans move
void updateBoard(int &move, char &piece, vector <vector <char> > &board); //Updates the board with a move
int aiMove(); //Finds the computers move. Find algorithim...
char changeTurn(char &piece); //Changes the turn from human to computer or vice-versa
char game(const vector <vector <char> > &board); //Returns whether the game has been won, drawn or neither


void populate(vector <vector <char> > &board)
{

//Populate the borders
for (int i=0; i<ROWS; i++)
{
board[i][0]=BORDER;//fill in the left border
}

for (int i=1; i<COLUMNS; i++)
{
board[ROWS-1][i]=BORDER;//fill in the bottom border
}

for (int i=0; i<ROWS; i++)
{
board[i][COLUMNS-1]=BORDER;//fill in the right border
}

//Fill in spaces...
for (int i=0; i<ROWS; i++)
{
for (int j=0; j<COLUMNS; j++)
{
if(board[i][j]!=BORDER) //Loops through the whole board. If a space isn't occupied by a border, fill it with a space
{
board[i][j]=SPACE;
}
}
}


}


void displayBoard(const vector <vector <char> > &board)
{
cout<<"\n\n";
for(int i=0; i<ROWS; i++)
{
for(int j=0; j<COLUMNS; j++)
{
cout<<board[i][j]; //displays the board
}
cout<<"\n";
}
}


int isLegal(int &move, const vector <vector <char> > &board)
{
if(move>=COLUMNS-1 || move<=0)
{
return 0;
}

if(board[0][move]==SPACE)
{
//checks if the top row in the column specified by the move is free. If it is, It's legal. Else it's not
return 1;
}

else
return 0;
}


int humanMove(int &move, const vector <vector <char> > &board)
{
do
{
cout<<"\nPick a place to move: ";
cin>>move; //asks for a move of the player
}while(!isLegal(move, board)); //keep asking while the move isn't legal

return move;
}


void updateBoard(int &move, char &piece, vector <vector <char> > &board)
{
int rows=0;
while(1)//While the move hasn't hit another piece or the bottom
{
if(board[rows][move]==SPACE)
{
++rows;
}

//If it gets to here, it must be on the bottom or another piece so add one to the rows to make it sit on top of it
if(board[rows][move]!=SPACE)
{
board[--rows][move]=piece;
goto END;
}

}
END:;
}


int aiMove()
{
return 3; //Yeah... Find an Algorithim
}


char changeTurn(char &piece)
{
if(piece==HUMAN) //Change the piece
return COMPUTER;

else
return HUMAN;
}


char game(const vector <vector <char> > &board)
{
if(board[0][1]==HUMAN) //Don't know what to do here... How will I get this to work?
return H;
}

int main()
{
char piece=HUMAN;//Human goes first
char gameState; //stores the win, draw or neither
int move; //stores a move for both players (computer and human)
vector <vector <char> > board (ROWS, vector <char> (COLUMNS) ); //2d vector for the board
populate(board); //call function to populate the board
displayBoard(board); //display it

do //game loop
{
if(piece==HUMAN)
{
move = humanMove(move, board); //get human move
updateBoard(move, piece, board);//update board
}

else
{
move = aiMove(); //get computer move
updateBoard(move, piece, board);//update board
}

displayBoard(board); //display board
piece = changeTurn(piece); //change the board
gameState = game(board); //check for win
}while(gameState!=T && gameState!=C && gameState!=H);

if(gameState==T)//If its a tie
{
cout<<"\n\nThe game was a tie, Well done.\n\n";
return 0;
}

if(gameState==C)//If the computer won
{
cout<<"\n\nUnlucky, I won this time!\n\n";
return 0;
}

else //Human must have won
{
cout<<"\n\nWell done, You won!\n\n";
return 0;
}

}




Now all that's left is to find/make some AI and a function to check for a winner.
Thanks guys! Couldn't have got this far without you!!!

Also, Am I passing the function arguments correctly? As in consts and the like.

Thanks again

Share this post


Link to post
Share on other sites
Gage64    1235
If you're passing a primitive type (int, char, etc.) as an argument, and the function doesn't need to change it, pass it by value. In some of your functions (like isLegal()) you are passing it by non-const reference, which is potentially dangerous.

Also, I would add a typedef for the board:

typedef vector<vector<char> > Board;

It makes the code more readable.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this