Beginner Battleship game in Python/terminal

Question

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:
 nose_index = int(ask_for_coordinates())
 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
 input('Your deployment phase has finished.')
 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
 def receive_deployment_info(self, opponent_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
 index = ask_for_coordinates()
 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')
 print('Bad luck!')
 Player.show_board(self.shooting_board) # refresh the board
 input('Your turn has finished')
 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
 else: # if evaded
 self.alternate_pixel(index, 'get miss')
 print('You have evaded the shot!\n')
 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()
def ask_for_ship_type_amount(ship_size, space_avaible):
 """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')
 return ask_for_ship_type_amount(ship_size, space_avaible)
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):
 value = ask_for_ship_type_amount(i, space_avaible)
 space_avaible -= i*value # each next placed ship takes space
 ship_list.append(value) # this also stores board_size indirectly
 return ship_list
def ask_for_coordinates():
 """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')
 return ask_for_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')
 a.receive_deployment_info(b.send_deployment_info())
 b.receive_deployment_info(a.send_deployment_info()) # deployment info exch.
 #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()

• user1149: 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.
• 200_success: @BarryCarter Comments are for seeking clarification to the question, and may be deleted. Please put all suggestions for improvements in answers.

Answer 1

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')
dog = Dog('ADog')

When I do this, this happens:

Animal talking BaseAnimal
Animal talking ADog

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')
dog = Dog('ADog')

Now the result is:

Animal talking BaseAnimal
Dog talking ADog

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!