Simple Python Battleship game

Question

I recently started learning python and decided to try and make my first project. I'm trying to make a battleship game that randomly places two 3 block long ships on a board. But it doesn't work quite right. I made a while loop for ship #2 that's supposed to check and see if two spaces next to it are free, then build itself there. But sometimes it just slaps itself on top of where ship #1 already is. can someone help me out?

Here's the first part of the code:

from random import randint

###board:

board = []

for x in range(7):
    board.append(["O"] * 7)

def print_board(board):
    for row in board:
        print " ".join(row)

###ships' positions:
#ship 1
def random_row(board):
    return randint(0, len(board) - 1)
def random_col(board):
    return randint(0, len(board[0]) - 1)
row_1 = random_row(board)
col_1 = random_col(board)

#ship 2
row_2 = random_row(board)
col_2 = random_col(board)
def make_it_different(r,c):
    while r == row_1 and c == col_1:
        r = random_row(board)
        c = random_col(board)
        row_2 = r
        col_2 = c
make_it_different(row_2,col_2)


### Makes the next two blocks of the ships:
def random_dir():
    n = randint(1,4)
    if n == 1:
        return "up"
    elif n == 2:
        return "right"
    elif n == 3:
        return "down"
    elif n == 4:
        return "left"
#ship one:
while True:
    d = random_dir() #reset direction
    if d == "up":
        if row_1 >= 2:
            #building...
            row_1_2 = row_1 - 1
            col_1_2 = col_1
            row_1_3 = row_1 - 2
            col_1_3 = col_1
            break
    if d == "right":
        if col_1 <= len(board[0])-3:
            #building...
            row_1_2 = row_1
            col_1_2 = col_1 + 1
            row_1_3 = row_1
            col_1_3 = col_1 + 2
            break
    if d == "down":
        if row_1 <= len(board)-3:
            #building...
            row_1_2 = row_1 + 1
            col_1_2 = col_1
            row_1_3 = row_1 + 2
            col_1_3 = col_1
            break
    if d == "left":
        if col_1 >= 2:
            #building...
            row_1_2 = row_1
            col_1_2 = col_1 - 1
            row_1_3 = row_1
            col_1_3 = col_1 - 2
            break
ship_1 = [(row_1,col_1),(row_1_2,col_1_2),(row_1_3,col_1_3)]

And here's where the ship 2 part is:

#ship two:
while True:
    d = random_dir() #reset direction
    if d == "up":
        if row_2 >= 2:
            if (row_2 - 1,col_2) not in ship_1 and (row_2 - 2,col_2) not in ship_1:
                #building...
                row_2_2 = row_2 - 1
                col_2_2 = col_2
                row_2_3 = row_2 - 2
                col_2_3 = col_2
                break
    if d == "right":
        if col_2 <= len(board[0])-3:
             if (row_2 ,col_2 + 1) not in ship_1 and (row_2,col_2 + 2) not in ship_1:
                #building...
                row_2_2 = row_2
                col_2_2 = col_2 + 1
                row_2_3 = row_2
                col_2_3 = col_2 + 2
                break
    if d == "down":
        if row_2 <= len(board)-3:
            if (row_2 + 1 ,col_2) not in ship_1 and (row_2 + 2,col_2) not in ship_1:
                #building...
                row_2_2 = row_2 + 1
                col_2_2 = col_2
                row_2_3 = row_2 + 2
                col_2_3 = col_2
                break
    if d == "left":
        if col_2 >= 2:
            if (row_2 ,col_2 - 1) not in ship_1 and (row_2,col_2 - 2) not in ship_1:
                #building...
                row_2_2 = row_2
                col_2_2 = col_2 - 1
                row_2_3 = row_2
                col_2_3 = col_2 - 2
                break

###test
board[row_1][col_1] = "X"
board[row_1_2][col_1_2] = "X"
board[row_1_3][col_1_3] = "X"
board[row_2][col_2] = "Y"
board[row_2_2][col_2_2] = "Y"
board[row_2_3][col_2_3] = "Y"
#Ship1 = X's and Ship2 = Y's
print_board(board)

Answer 1

I'd recommend allowing your placing code to run through simply and without if statements, it will be much cleaner. Then, at the end, you can check to see if any of the pieces overlap, and if they do reset.

Depending on how you end up deciding to store the points that the individual ships lie in, maybe a list of tuples. You could do this

the place ship method could return a list of tuples (points)

def placeShip():
    points = []

    # put random point generation here

    for point in points:
        if point in otherShipPoints:
            return placeShip()         # overlap detected, redo ship placement

    return points

Put your placement code in a single function so that it can be called this way simply. Your code is starting to get messy and I recommend going to an approach like this to keep from running into spaghetti code problems.

You could also give placeShip() a parameter for the size of ship you want to add, then this method could be your all-in-one ship placer. Just make your function look like this placeShip(size) and then randomly generate that many points within your grid

Answer 2

You are writing a lot of code for this, maybe another approach would be better. Try writing a function like

def ship_points(rowcol = (3,3), shiplength = 4, direction = (1,0), boardsize=(7,7)):
    points = []
    for i in xrange(shiplength):
        points.append((rowcol[0]+i*direction[0], rowcol[1]+i*direction[1]))
        if points[i][0] >= boardsize[0] or points[i][1] >= boardsize[1]:
            return None
    return points

Now you can just generate points for each ship and check for identical points directly. Much less code, and way more reusable.

Answer 3

Assigning to row_2 and col_2 in make_it_different doesn't assign to the global variables by those names. Python's rule for determining a function's local variables is that anything a function assigns to without a global declaration is local; assigning to row_2 and col_2 creates new local variables instead of changing the globals. You could fix this by declaring row_ and col_2 global, but it'd probably be better to instead pass the new values to the caller and let the caller assign them.

(Why does make_it_different take initial values of row_2 and col_2 at all? Why not just have it generate coordinates until if finds some that work?)