Problems With Connect 4 [C++]

Started by
13 comments, last by Gage64 14 years ago
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[0]=BORDER;//left...
    }

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

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

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


void displayBoard()
{
    for(int i=0; i<=ROWS; i++)
    {
        for(int j=0; j<=COLUMNS; j++)
        {
            cout<<board[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!
Advertisement
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.
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.

Quote:Original post by BosskIn Soviet Russia, you STFU WITH THOSE LAME JOKES!
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.
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 wantconst int COLUMNS=10;const char BORDER='@';      //Marks the borderconst char COMPUTER='O';    //Computers piececonst char HUMAN='X';       //Humans piececonst char SPACE=' ';       //Un-occuppied place on the boardconst char C='C';           //This is returned from the game function if the computer has wonconst char H='H';           //This is returned from the game function if the human has wonconst char T='T';           //This is returned from the game function if it's a tie//Declaring all the functionsvoid populate(vector <vector <char> > &board);             //Populates the board for the first timevoid displayBoard(const vector <vector <char> > &board);   //Displays the board. Should use a const ref...int isLegal(int &move);                                    //Checks if a move is legalint humanMove();                                           //Gets the humans movevoid updateBoard(int move, char piece, vector <vector <char> > &board); //Updates the board with a moveint aiMove();                             //Finds the computers move. Find algorithim...char changeTurn(char &piece);             //Changes the turn from human to computer or vice-versachar game();                              //Returns whether the game has been won, drawn or neithervoid populate(vector <vector <char> > &board){    //Populate the borders    for (int i=0; i<COLUMNS; i++)    {        board[0]=BORDER;//fill in the left border    }    for (int i=0; i<COLUMNS; i++)    {        board[ROWS-1]=BORDER;//fill in the bottom border    }    for (int i=0; i<COLUMNS; i++)    {        board[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[j]!=BORDER)  //Loops through the whole board. If a space isn't occupied by a border, fill it with a space            {                board[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[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!
Step through the program with a debugger to find out what line causes the problem.
Still can't find anything, but I might be useing the debugger wrong...
Any thoughts on the AI or finding out if the game has been won?
Thanks
You still have buffer overuns.
Your board is not square, so this is not right:
    for (int i=0; i<COLUMNS; i++)    {        board[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.

"In order to understand recursion, you must first understand recursion."
My website dedicated to sorting algorithms
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.
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 wantconst int COLUMNS=10;const char BORDER='@';      //Marks the borderconst char COMPUTER='O';    //Computers piececonst char HUMAN='X';       //Humans piececonst char SPACE=' ';       //Un-occuppied place on the boardconst char C='C';           //This is returned from the game function if the computer has wonconst char H='H';           //This is returned from the game function if the human has wonconst char T='T';           //This is returned from the game function if it's a tie//Declaring all the functionsvoid populate(vector <vector <char> > &board);                //Populates the board for the first timevoid 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 legalint humanMove(int &move, const vector <vector <char> > &board);          //Gets the humans movevoid updateBoard(int &move, char &piece, vector <vector <char> > &board); //Updates the board with a moveint aiMove();                                     //Finds the computers move. Find algorithim...char changeTurn(char &piece);                     //Changes the turn from human to computer or vice-versachar game(const vector <vector <char> > &board);  //Returns whether the game has been won, drawn or neithervoid populate(vector <vector <char> > &board){    //Populate the borders    for (int i=0; i<ROWS; i++)    {        board[0]=BORDER;//fill in the left border    }    for (int i=1; i<COLUMNS; i++)    {        board[ROWS-1]=BORDER;//fill in the bottom border    }    for (int i=0; i<ROWS; i++)    {        board[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[j]!=BORDER)  //Loops through the whole board. If a space isn't occupied by a border, fill it with a space            {                board[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[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!

This topic is closed to new replies.

Advertisement