A Simple Battleship Game

Question

I have just finished learning basic-intermediate Python subjects and wanted to test myself. This Battleship game is the third game i have written.

import random


def create_random_ship():
    return random.randint(0, 5), random.randint(0, 5)


def play_again():
    try_again = input("Wanna play again? <Y>es or <N>o? >: ").lower()
    if try_again == "y":
        play_game()
    else:
        print("Goodbye!")
        return


print("Welcome to the Battleship game!"
      "\nYour main objective is to find and destroy all the hidden ships on map!\n")

print("""\nIntroductions:
\nYou have 10 ammo and there are 3 hidden ships on map.
In order to hit them, you have to enter specific numbers for that location. For example:
For the first row and first column, you have to write 1 and 1.
I wish you good fortune in wars to come!\n""")


def play_game():
    game_board = [["O", "O", "O", "O", "O"],
                  ["O", "O", "O", "O", "O"],
                  ["O", "O", "O", "O", "O"],
                  ["O", "O", "O", "O", "O"],
                  ["O", "O", "O", "O", "O"]]

    for i in game_board:
        print(*i)

    ship1 = create_random_ship()
    ship2 = create_random_ship()
    ship3 = create_random_ship()
    ships_left = 3
    ammo = 10

    while ammo:
        try:
            row = int(input("Enter a row number between 1-5 >: "))
            column = int(input("Enter a column number between 1-5 >: "))
        except ValueError:
            print("Only enter number!")
            continue

        if row not in range(1,6) or column not in range(1, 6):
            print("\nThe numbers must be between 1-5!")
            continue

        row = row - 1 # Reducing number to desired index.
        column = column - 1 # Reducing number to desired index.

        if game_board[row][column] == "-" or game_board[row][column] == "X":
            print("\nYou have already shoot that place!\n")
            continue
        elif (row, column) == ship1 or (row, column) == ship2 or (row, column) == ship3:
            print("\nBoom! You hit! A ship has exploded! You were granted a new ammo!\n")
            game_board[row][column] = "X"
            ships_left -= 1
            if ships_left == 0:
                print("My my, i didn't know you were a sharpshooter! Congratz, you won!")
                play_again()
        else:
            print("\nYou missed!\n")
            game_board[row][column] = "-"
            ammo -= 1

        for i in game_board:
            print(*i)

        print(f"Ammo left: {ammo} | Ships left: {ships_left}")

    play_again()


if __name__ == "__main__":
    play_game()

What do you think? How can I make it better? What are my mistakes?

Answer 1

Magic Numbers

Your biggest issue is too many magic numbers and hard-coded information.

If you added at the top of your script:

ROWS = 5
COLUMNS = 5

then you could have:

def create_random_ship():
    return random.randrange(ROWS), random.randrange(COLUMNS)

Game board can be created with:

game_board = [["O"] * COLUMNS for _ in range(ROWS)]

And you can ask for input from the user with:

row = int(input(f"Enter a row number between 1-{ROWS} >: "))

Along the same vein, you could have:

print(f"""\nIntroductions:
\nYou have {INIT_AMMO} ammo and there are {INIT_ENEMY} hidden ships on map.
...""")

so your welcome instructions can keep up-to-date with any changes to your game initial conditions.

Instructions

You have a if __name__ == "__main__": guard to prevent the code from running if imported into another script, but your instruction are unconditionally printed. You should move the instruction printing into an instructions() function, and call that only if appropriate.

Infinite Recursion

play_game() calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), which calls play_again(), which can call play_game(), ...

Traceback (most recent call last):
  [...omitted...]
RecursionError: maximum recursion depth exceeded

Don't do that. Much simpler is to have play_again() return a True/False result, and loop based on the return value:

def play_many_games():

    instructions()

    play_game()

    while play_again():
        play_game()

    print("Goodbye!")

if __name__ == "__main__":
    play_many_games()

No more recursion.

In the comments, @Graipher suggests an alternate method to avoid the recursion:

while True:
    play_game()
    if not play_again():
        break

Yet another alternative introduces a loop condition variable, which is pre-set before the loop:

 playing = True
 while playing:
     play_game()
     playing = play_again()

All three approaches avoid the infinite recursion.

I Saw Three Ships...

ship1 = create_random_ship()
ship2 = create_random_ship()
ship3 = create_random_ship()

# ...

    elif (row, column) == ship1 or (row, column) == ship2 or (row, column) == ship3:

What are you going to do if you change this to four ships, or even 5 ships?

With computers, there are 3 important numbers: zero, one and many. If you have more than one ship, you have "many" ships, and should put them in a container, such as a list():

ship_coordinates = []
for _ in range(ships_left):
    ship_coordinates.append(create_random_ship())

And when you want to see if a particular row/column matches any one of the ship coordinates in the container, you use the in function:

    elif (row, column) in ship_coordinates:

There are other container types you can use, such as a set() which has faster in performance. However, it cannot add the same value more than once, and using it would result in slightly different behaviour to what you currently have. You've mentioned that you have a bug where ships can have the same coordinates, and I am trying very hard not to accidentally fix that. (Code Review is for reviewing working code only, not help debugging, so I am tiptoeing around the bug, so that you can fix it yourself.)

Answer 2

How can i prevent that ships may have the same coordinates?

In other words, you want to pick three distinct locations on the board randomly; one for each ship. Sounds like you want a random sample (without replacement) of size three from the population of the game board coordinates:

from random import sample
from itertools import product

ROWS = 5
COLUMNS = 5
NUM_SHIPS = 3

# game_coordinates = [
#   (0, 0), (0, 1), (0, 2), (0, 3), (0, 4),
#   (1, 0), (1, 1), (1, 2), (1, 3), (1, 4),
#   (2, 0), (2, 1), (2, 2), (2, 3), (2, 4),
#   (3, 0), (3, 1), (3, 2), (3, 3), (3, 4),
#   (4, 0), (4, 1), (4, 2), (4, 3), (4, 4)
# ]
game_coordinates = list(product(range(ROWS), range(COLUMNS)))

ship1, ship2, ship3 = sample(game_coordinates, NUM_SHIPS)

Explanation of the above code:

First we generate a list of all the game coordinates using itertools.product to calculate the Cartesian product of the row and column indices.

Then using random.sample, we take a random sample (without replacement) of size three from game_coordinates to get the coordinates of the three ships.

EDIT: To expand on the above example (and to address @TemporalWolf's comment), I should clarify that creating the list of game coordinates only needs to happen once per program run.

Near the top of the program, you can declare game constants like so:

ROWS = 5
COLUMNS = 5
NUM_SHIPS = 3
GAME_COORDINATES = list(product(range(ROWS), range(COLUMNS)))

Then in play_game:

# set of ships the player must hit to win the game
ships = set(sample(GAME_COORDINATES, NUM_SHIPS))

# ...

# player hit a ship!
elif (row, column) in ships:
    # ...
    game_board[row][column] = "X"
    ships.remove((row, column))

    # player hit all the ships -- victory!
    if not ships:
        # print congratulatory message

The list of coordinates GAME_COORDINATES is created only once at the beginning of the program, and used (and re-used) in each call to play_game within the same program run.

Answer 3

I'd say that

game_board = [["O", "O", "O", "O", "O"],
            ["O", "O", "O", "O", "O"],
            ["O", "O", "O", "O", "O"],
            ["O", "O", "O", "O", "O"],
            [ "O", "O", "O", "O", "O"]]

for i in game_board:
    print(*i)

produces a neat display:

O O O O O
O O O O O
O O O O O
O O O O O
O O O O O

which is an awesome way of translating array to display.

elif (row, column) == ship1

is also an intelligent way of coupling function with comparison where initialisation is class like:

ship1 = create_random_ship()

but, a ship class might be better

The Ship class

class Ship:
    def __init__(self):
        self.coord = (random.randint(0, 5), random.randint(0, 5))

    def __eq__(self, other):
        return self.coord == other

with initialisations as

ship1 = Ship()
ship2 = Ship()
ship3 = Ship()

This also helps if you want to add more info to your ships like let's say you want to make 3 teams

Messages Display

We might want to hold messages in a structure like

class Message:
    welcome = ("Welcome to the Battleship game!\n"
                "Your main objective is to find and destroy all the hidden ships on map!\n")
    instructions = ("\nIntroductions:\n"
                "You have 10 ammo and there are 3 hidden ships on map.\n"
                "In order to hit them, you have to enter specific numbers for that location. For example:\n"
                "For the first row and first column, you have to write 1 and 1.\n"
                "I wish you good fortune in wars to come!\n")
    # ...

and use as

print(Message.welcome)
print(Message.instructions)

Ship Initialisations

Ships could be better instantiated, maybe a function which modifies the game coordinates to represent the ship with a new symbol. Could be useful in the case where one ship occupies more than one tile

Game Class

A game class might be better suited with initialisations of the board size and number of random ships.

battle_ship = Game(board_size=(5,5), rand_ships=3)

Then you have methods as

battle_ship.play()
battle_ship.play_again()

Miscellaneous

• +1 for use of f strings.
• The game is also pretty smooth, maybe an Exception to catch KeyboardInterrupt might enhance it better.
• I find it tiring to add row and col line by line, maybe a format like 1 5 might be used to specify it at once.

Answer 4

First things first: random.randint() is inclusive of it's second term, so (0, 5) can generate 5 (this is different than range(), for example, which is exclusive of the 2nd term). If you want to maintain the similarity with range(), there is random.randrange() which acts like you expect. This is reflected in the docs:

random.randint(a, b):

Return a random integer N such that a <= N <= b. Alias for randrange(a, b+1).

There are multiple ways to deconflict ships, the simplest is to just regenerate when you detect a conflict:

import random

number_of_ships = 3
boardsize = 5

def generate_ships(number_of_ships, max_x, max_y):
    while True:
        ships = [(random.randint(0, max_x - 1), random.randint(0, max_y - 1)) for _ in range(number_of_ships)]
        if len(set(ship for ship in ships)) == number_of_ships:
            return ships  # return once we have a list of unique ships

ship_locations = generate_ships(number_of_ships, boardsize, boardsize)

In this case I just make a set of all ship coordinates and see if that set is the same length as the number of ships requested. If any share coordinates, the set will be short, so I regenerate the list.

I'm using a list comprehension, which is equivalent to:

ships = []
for _ in range(number_of_ships):
    ships.append((random.randint(0, max_x - 1), random.randint(0, max_y - 1)))

It's worth noting _ is not a special variable, but by convention shows I'm not using that value (I just want the code to repeat number_of_ships times.)

I ran a quick test and over 1 million attempts to generate ships there was a conflict about 13% of the time on a 5x5 board, which is generally consistent with the expected overlap rate for a 25 choose 3 problem. On an 8x8 that drops to 5%.

if game_board[row][column] == "-" or game_board[row][column] == "X":

elif (row, column) == ship1 or (row, column) == ship2 or (row, column) == ship3

These can both be simplified with in notation, which checks whether something is contained in a list, doubly so if you put the ships into a list like I showed above:

if game_board[row][column] in "-X":  # If board shows either a dash or X
    ...
elif (row, column) in ship_locations:  # If ship at location
    ...

if ships_left == 0:

Numbers are truthy/falsey, so you can instead say

if not ships_left:
    ...

Answer 5

This is how i have fixed the bug:

def create_random_ships():
    ship_coordinates = []

    while len(ship_coordinates) < 3:
        ship = [random.randrange(5), random.randrange(5)]
        if ship not in ship_coordinates:
            ship_coordinates += [ship]

    return ship_coordinates

First, i have redefined the ship creation function like above.

Then,

        elif [row, column] in ship_coordinates:
            print("\nBoom! You hit! A ship has exploded! You were granted a new ammo!\n")
            game_board[row][column] = "X"
            ships_left -= 1
            if not ships_left:
                print("My my, i didn't know you were a sharpshooter! Congratz, you won!")
                break

I have changed the coordinate checking statement like above. Now, the bug is fixed and my code is much more improved compared to the previous code. Thank you all.