Please help me with my connect 4 game im beyond despair and I have it due in 4 days

[deanhystad]

I think it is a great idea that you test for the winner based on the last move, but your tests are all wrong. You need to count the number of contiguous markers in a line. You are kind of doing that but forgetting that there might be markers before and after the recently played marker. If the last marker was placed at (2, 2) four consecutive markers could be in (0,2), (1,2), (2,2), (3,2). You look in only one direction.

My test for checking if there are 4 markers in a row would look like this:

def get(row, column, board):
    """Return marker in board[row][column] if row and column are valid, else return None"""
    if 0 <= row < ROWS and 0 <= column <= COLUMNS:
        return board[row][column]
    return None

def winningRow(row, column, board):
    """Return True if there are 4 or more matching markers in a row"""
    marker = get(row, column, board)
    count = 1
    c = column-1
    while get(row, c) == marker:
        count += 1
        c -= 1
    c = column+1
    while get(row, c) == marker:
        count += 1
        c += 1
    return count >= 4

Using a pattern like this you could write a generic tester that would work for rows, columns and diagonals.


A few comments about your code:

It is good that you are breaking up your code into functions, but you lose much of the benefits of functions by using global variables to pass information to and from the function. For example:

def ColumnNumberValid():  # returns whether or not the column number is valid
    global ColumnNumber
    Valid = False
    if (ColumnNumber >= 0) and (ColumnNumber <= 6):
        if Board[6][ColumnNumber] == BLANK:  # at least 1 empty space in column
            Valid = True
    return Valid

should look like this:

def columnNumberValid(column, board):
    """"Returns True if the column is in range and has an empty spot"""
    if 0 <= column < COLUMNS:  # Do not sprinkle code with numerical constants like 6.
        return board[ROWS-1][column] == BLANK
    return False

This might be even better:

def columnNumberValid(column, board):
    try:
        return board[ROWS-1][column] == BLANK
    except IndexError:
        return False

Other than BLANK (and ROWS and COLUMNS in my example) I don't think any of your variables should be global. Initialize the variables in main() and pass them as arguments to your functions.

You should avoid functions that don't provide any real benefit. I would get rid of these functions and move the code back into main.

def InitialiseBoard():
    global Board
    Board = [[BLANK] * 7 for x in range(7)]  # Creates the array of blank values
 
 
def SetUpGame():  # Set up the parameters for the beginning of the game
    global ThisPlayer
    global GameFinished
    ThisPlayer = "O"  # Player O always starts
    GameFinished = False

None of these functions are used more than once, they don't reduce the length of the code, and I don't think they make your code easier to understand.

Do not duplicate code. This loop:

    print("Player ", ThisPlayer, "'s turn.")
    ColumnNumber = int(input("Enter a valid column number: ")) - 1
    while ColumnNumberValid() == False:  # check whether the column number is valid
        ColumnNumber = int(input("Enter a valid column number: ")) - 1
    return ColumnNumber

is better written as this:

    print("Player ", ThisPlayer, "'s turn.")
    while True:
        ColumnNumber = int(input("Enter a valid column number: ")) - 1
        if ColumnNumberValid():
            return ColumnNumber

Each duplicate line of code is a potential maintenance nightmare.

I don't understand the logic of this function.

def CheckForFullBoard():
    global GameFinished
    BlankFound = False
    ThisRow = 0
    while (ThisRow != 6) and (BlankFound == False):
        ThisColumn = 0
        ThisRow += 1
    while (ThisColumn != 7) and (BlankFound == False):
        ThisColumn += 1
        if Board[ThisRow][ThisColumn] == BLANK:
            BlankFound = True
 
    if BlankFound == False:
        print("It is a draw")
        GameFinished = True

The first while loops sets ThisColumn = 0 and ThisRow = 6. Why not just set ThisColumn=0 and thisRow=6 instead of looping. Better yet, you know how many moves it takes to fill the board. Why not get rid of the full board check and do something more like this:

for _ in range(ROWS*COLUMNS):
    ThisPlayerMakesMove()
    OutputBoard()
    CheckIfThisPlayerHasWon()
    SwapThisPlyer()
    if GameFinished:
        break;

If you have 7 rows and 7 columns, the board is full after players make 49 moves. There is no reason to check the board.