# Beginner Battleship game in Python/terminal

I am learning how to program and during the Python course on Codecademy I was encouraged to rewrite it all with new features (2 players, various ship sizes, etc.).

In summary this works as follows:

1. Board configuration is set
2. Players and their deployment is initialized
3. Deployment info is exchanged between players
4. Each player takes a turn to look at the state of his boards and to take a shot. When all ship tiles are shot, the opposite player wins.

My question is (if anyone got patience to dig through it), how can I improve my coding skills, style and simplify this? Is it good idea to trash it and rewrite with ships and boards as classes?

from textwrap import dedent
from copy import copy

class Player():

def alternate_pixel(self, index, mode):
"""This method, after receiving a call with pixel index
will change its graphical representation. Depending on mode ofc.
Also, this function does not mess with active_indexes."""
# print('alternating %d' % index)
if mode == 'deploy':               # . - empty, free space
self.own_board[index] = '$' #$ - healthy ship part
elif mode == 'get miss':
self.own_board[index] = ' '  #   - hit in empty space
elif mode == 'get hit':
self.own_board[index] = 'X'  # X - destroyed ship part
elif mode == 'make miss':
if self.shooting_board[index] == 'X':   # do not override X
pass
self.shooting_board[index] = ' '
elif mode == 'make hit':
self.shooting_board[index] = 'X'

@staticmethod
def show_board(board):
"""Processes list of pixels into graph, without messing with them"""
temp = copy(board)
c = -1      # compensate for inserted '\n\t' units
for i in range(board_size):
c += 1
temp.insert((i+1)*board_size + c, '\n\t')
print('\t ' + ' '.join(temp))

@staticmethod
def are_indexes_allowed(occupied, proposed):
"""Periodicity (being on two ends), out of board deploys and
stacking on tile is disallowed."""
for i in range(1, board_size):
if i*board_size - 1 in proposed and i*board_size in proposed:
return False
for index in proposed:
if index not in range(board_size**2) or index in occupied:
return False
return True

def ship_settler(self, ship_size):
"""The back end to deploy_fleet(). This will put given ship size on
board(horizontaly or vertically)."""
while True:
if nose_index not in self.active_indexes: break  # just faster

if ship_size == 1:  # mono ships do not need to face the incoming
self.active_indexes.append(nose_index)
Player.alternate_pixel(self, nose_index, 'deploy'); return

proposed_indexes = []
direction = input('NSWE?\n')

for i in range(ship_size):      # for each hull segment
if direction == 'N' or direction == 'n':
proposed_indexes.append(nose_index - i*board_size)
if direction == 'S' or direction == 's':
proposed_indexes.append(nose_index + i*board_size)
if direction == 'W' or direction == 'w':
proposed_indexes.append(nose_index - i)
if direction == 'E' or direction == 'e':
proposed_indexes.append(nose_index + i)

if Player.are_indexes_allowed(self.active_indexes, proposed_indexes):
for index in proposed_indexes:  # run the updates
self.active_indexes.append(index)
Player.alternate_pixel(self, index, 'deploy')
else:
print('Invalid, try again.')    # if not met - rerun
del proposed_indexes [:]    # if not emptied it will stay filled
Player.ship_settler(self, ship_size)

def deploy_fleet(self):
"""The front end function that fills active_indexes"""
print(dedent('\
------------------------------\n\
Deployment phase of %s\n\
------------------------------\n' % (self.name)))
ship_size = 0   # going form smallest to largest

for ship_amount in ship_list:
ship_size += 1

for i in range(ship_amount):    # for each ship of size
print('Deploying %d sized ship %d/%d\n'
% (ship_size, i+1, ship_amount))
Player.show_board(self.own_board)
Player.ship_settler(self, ship_size)
Player.show_board(self.own_board)       # refresh board at the end

def __init__(self, name):
"""Each player has 2 boards, first shows own ships and tiles shot by
the opponent, while second shows the same about him, but active ships
are hidden. Note that list of active_indexes (which marks non destroyed
ship parts) has no effect on graphics (in the shooting phase)"""
self.name = name
self.active_indexes = []  # filled by deploy(), emptied by get_shot()
self.own_board = ['.'] * board_size**2
self.shooting_records = []
self.shooting_board = copy(self.own_board)
Player.deploy_fleet(self)       # deployment is part of initialization
print(chr(27) + "[2J")  # clear terminal screen

def send_deployment_info(self):
"""Some method of trasfering info whether player hit somthing must be
in place. This seems suboptimal and maybe it could be resolved
diffrently, but perhaps rebuilding is nesscesary to do so."""
active_indexes = copy(self.active_indexes)
return active_indexes

self.opponent_indexes = opponent_indexes

def make_shot(self):
"""Essentially handles players turn"""
print(dedent('\
--------------------------------------\n\
\tFiring phase of %s\n\
--------------------------------------\n' % (self.name)))
Player.show_board(self.shooting_board)
while True:               # cannot strike same tile twice
if index not in self.shooting_records:
self.shooting_records.append(index)
break

if index in self.opponent_indexes:      # if guessed right
print('You got him!')
self.alternate_pixel(index, 'make hit')
else:                                   # if guessed wrong
self.alternate_pixel(index, 'make miss')

Player.show_board(self.shooting_board)  # refresh the board
print(chr(27) + "[2J")      # clears the terminal window
return index            # pass shot coordinate

def get_shot(self, index):
"""This has nothing to do with input, it only displays result
of opponents turn."""
print(dedent('\
--------------------------------------\n\
\tRaport phase phase of %s\n\
--------------------------------------\n' % (self.name)))
if index in self.active_indexes:       # if got hit
self.alternate_pixel(index, 'get hit')
print('The opponent got your ship hit!\n')
Player.show_board(self.own_board)
self.active_indexes.remove(index)   # for finishing the game
self.alternate_pixel(index, 'get miss')
Player.show_board(self.own_board)

def configure_board_size():
print(
'What size of board would you like to play on?\n\
Expected range: 4 to 20')
size = input()
if size.isdigit():
if 4 <= int(size) <= 20:
return int(size)
print('Invalid board size')
return configure_board_size()

"""Makes sure that there wont be much problems with fitting ship
coordinates in deployment phase. It is ensured bu restricting
the amount of ships size based on left free space."""
if ship_size > space_avaible:   # speeds up prompting
return 0
value = input('How much ships sized %d are to be placed?\n' % ship_size)
if value.isdigit():
if int(value)*ship_size <= space_avaible:
return int(value)
print('Invalid amount')

def configure_ships(board_size):        # gets called second
"""Gets called second and with help of ask_for_ship_type_amount
generates list of ships to be placed in deployment phases.
This ship_list stores amount of each next (in size) ship."""
ship_list = []
space_avaible = ((board_size)**2) // 2.2  # preserve arbitrary 60% freespace
print('Generating ships, ')
for i in range(1, board_size):
space_avaible -= i*value    # each next placed ship takes space
ship_list.append(value)     # this also stores board_size indirectly
return ship_list

"""Asks for row and column and outputs the index"""
coords = str(input('Select tile\n'))
if coords.isdigit() and len(coords) == 2:
if 0 < (int(coords[0]) and int(coords[1])) <= board_size:
index = int(coords[0]) + board_size * (int(coords[1])-1)
return index - 1      # the above doesnt account for 0th item
print('Invalid coordinates')

def main():

global board_size; global ship_list
board_size = configure_board_size()
ship_list = configure_ships(board_size)
#ship_list = [0, 2, 0]
#board_size = len(ship_list) + 1

a = Player('A')
b = Player('B')
#print(a.active_indexes, b.active_indexes)

while True:
a.get_shot(b.make_shot())    # Player B turn
if not a.active_indexes:
print('B wins!'); break
b.get_shot(a.make_shot())    # Player A turn
if not b.active_indexes:
print('A wins!'); break

main()

• This is good code. Minor: in alternate_pixel() you use if/elif/elif/... whereas in ship_settler() in for i in range(ship_size): you use if without elif (the conditions are mutually exclusive in both cases). Consider dropping the elif from alternate_pixel() or use case/switch in both cases. – user1149 Nov 5 '17 at 13:06
• @BarryCarter Comments are for seeking clarification to the question, and may be deleted. Please put all suggestions for improvements in answers. – 200_success Nov 9 '17 at 6:17

I got two basic things to say

You got a typo, in 'Raport' I guess it should be 'Report'. (Actually runned the code, that doesn't count as a thing)

--------------------------------------
Raport phase phase of A
--------------------------------------


1.- I didn't go through the implementation just readed your post and actually runned the code. OOP is supposed to model the world and help us code. In the world you have a single board and two players. Instead of that you got 2 players and 4 boards this is weird. I'm not saying that you should make an object named Board, it could be a just a list but players should not have knowledge of their opponents information. Like in the real world you dont have information about your opponents ships.

2.- You are doing a weird thing when calling methods inside your classes, for instance: (Got rid of some code for redability)

class Player:
...
def __init__(self, name):
...
Player.deploy_fleet(self)       # THIS LINE IS THE PROBLEM
...


This is a problem because you are not calling 'your' method you are calling the method defined in Player class with 'you' as an argument. This is not how you call your own methods in a class, you do it like this:

class Player:
...
def __init__(self, name):
...
self.deploy_fleet()       # Ou yeah! Much better
...


You may think it is the same but it isnt. It may be a little bit complex for the level you are in right now (I'm guessing here), but this becomes trouble when someone tries to inherit from your Player class. And again, this may be complex but if you understand it, it is a pretty good tool to have in your back.

I'll try to demostrate it with a little example, when 'you' inherit from a class suddently you become that class, you have the same attributes and the same methods (well in python a method is actually an attribute, but that is WAY out of the scope). And you can choose whether or not you keep your "parent's" attributes, if you dont like it, you can define your own, lets see with an example.

Yes, I'm sorry, but here we go with the animals example.

class Animal:
def __init__(self, name):
self.name = name
Animal.talk(self)  # This is not supposed to be done this way, you'll see
def talk(self):
print('Animal talking ' + self.name)


I think a dog is also an animal right? So lets do it!

class Dog(Animal):
def talk(self):
print('Dog talking ' + self.name)


I dont like a dog saying that it's just an animal, he's a dog (man's best friend), so I override the method talk while I let the method __init__ be the same as the Animal class. Let's have a look at what happens...

some_animal = Animal('BaseAnimal')


When I do this, this happens:

Animal talking BaseAnimal


I overided the talk method in the Dog class, but in the __init__ method which Dog inherit from Animal, a hardcoded call to the class Animal is preventing the Dog to call its own talk method. To solve this, we do:

class Animal:
def __init__(self, name):
self.name = name
self.talk()
def talk(self):
print('Animal talking ' + self.name)


Without touching the Dog class, if we repeat the previous statements:

some_animal = Animal('BaseAnimal')


Now the result is:

Animal talking BaseAnimal


Much better right! This is a pretty complex thing, but to summarize, dont call a method within a class with the name of the class, use self (Unless you are looking for that behaviour.)

Hope it helped!