0

I'm remaking my battleship game, and I have a constant variable called SEA which holds an empty board. However, the variable is being modified, and I don't know why (or where). I suspect it's being passed by reference to player_board and when player_board is modified, so is SEA. How do I stop that from happening? Here is my code. You'll see on the bottom I print out SEA, and it's been modified.

from random import randint #Constants and globals OCEAN = "O" FIRE = "X" HIT = "*" SIZE = 10 SHIPS = [5, 4, 3, 3, 2] player_radar = [] player_board = [] player_ships = [] ai_radar = [] ai_board = [] ai_ships = [] #Classes class Ship(object): def set_board(self, b): self.ship_board = b def edit(self, row, col, x): self.ship_board[row][col] = x def __repre__(self): return self.ship_board #Set up variables last_ship = Ship() #Holds the last ship made in make_ship() SEA = [] # Blank Board for x in range(SIZE): SEA.append([OCEAN] * SIZE) #Functions def print_board(): for row in range(SIZE): print " ".join(player_radar[row]), "||" , " ".join(player_board[row]) def random_row(is_vertical, size): if is_vertical: return randint(0, SIZE - size) else: return randint(0, SIZE -1) def random_col(is_vertical, size): if is_vertical: return randint(0, SIZE - 1) else: return randint(size-1, SIZE -1) def exists(row, col, b): # true if ocean if row < 0 or row >= SIZE: return 0 elif col < 0 or col >= SIZE: return 0 if b[row][col] == OCEAN: return 1 else: return 0 def make_ship(size, board): #Find an unoccupied spot, then place ship on board #Also put ship in last_ship temp = [] temp = board is_vertical = randint(0, 1) # vertical ship if true occupied = True while(occupied): occupied = False ship_row = random_row(is_vertical, size) ship_col = random_col(is_vertical, size) if is_vertical: for p in range(size): if not exists(ship_row+p, ship_col, temp): occupied = True else: for p in range(size): if not exists(ship_row, ship_col-p, temp): occupied = True #Place ship on boards last_ship.set_board(SEA) if is_vertical: last_ship.edit(ship_row, ship_col, "^") last_ship.edit(ship_row+size-1, ship_col, "v") temp[ship_row][ship_col] = "^" temp[ship_row+size-1][ship_col] = "v" for p in range(size -2): last_ship.edit(ship_row+p+1, ship_col, "+") temp[ship_row+p+1][ship_col] = "+" else: last_ship.edit(ship_row, ship_col, ">") last_ship.edit(ship_row, ship_col-size+1, "<") temp[ship_row][ship_col] = ">" temp[ship_row][ship_col-size+1] = "<" for p in range(size -2): last_ship.edit(ship_row, ship_col-p-1, "+") temp[ship_row][ship_col-p-1] = "+" return temp # Make the boards player_radar = SEA player_board = SEA ai_radar = SEA ai_board = SEA print_board() for x in SHIPS: player_board = make_ship(x, player_board) #player_ships.append(last_ship) #ai_board = make_ship(x, ai_board) #ai_ships.append(last_ship) print "Let's play Battleship!" for row in range(SIZE): print " ".join(SEA[row]) 
1

3 Answers 3

4

SEA and its members are lists, and lists in Python are mutable. When you say player_radar = SEA, etc., you're not making a copy of SEA; you're making a new reference to it. Any changes you then make to player_radar will be reflected in SEA.

copy.deepcopy is often used to recursively copy nested mutable data structures. Personally, however, I prefer to just copy the number of layers I know I'll need. For instance, to make a copy of a list of lists and all its members, you can do this:

player_radar = [sublist[:] for sublist in SEA] 

This is a list comprehension. Each sublist is copied using [:], which makes a shallow copy of each one.

Sign up to request clarification or add additional context in comments.

4 Comments

If you want a copy of list of lists, etc, use copy.deepcopy to make sure you really have copied all levels.
@9000: Not necessarily. If you're treating a list of lists as a 2-dimensional list, you frequently want to make copies of the sublists, but not of the elements of the sublists.
Important note: when you say x=y, you're never "making a copy," regardless of the type of y. If SEA were an int or a str, you would still be making a new reference, not a copy.
@user2357112 I'd still prefer to use the copy library for cleaner semantics. Your point is valid and correct none the less :)
1

SEA is a list, so make a copies of it:

player_radar = SEA[:] player_board = SEA[:] ai_radar = SEA[:] ai_board = SEA[:] 

or deeper copies of it, if you need to.

EDIT: By "deeper copies", I mean that if your list contains, for instance, other lists, then just making a top level copy will create a new list, but its members will be references to the same members that your original list had, so to create a deep copy, you'd also need to make copies of those members.

To illustrate:

>>> list1 = [[1,2,3]] >>> list2 = list1[:] # Make a shallow copy >>> print(list1) [[1,2,3]] >>> print(list2) [[1,2,3]] >>> list2[0][0] = 4 # Also changing list1's first member, here >>> print(list2) [[4,2,3]] >>> print(list1) [[4,2,3]] # So list1 has also changed. 

3 Comments

Can you explain the difference between a copy and a deeper copy?
I think [:] is not enough here, use copy.deepcopy.
0

Python variables are names for things, not places to put things.

player_radar = SEA 

doesn't make player_radar a copy of SEA, like it would in, say, C++. Rather, player_radar and SEA are both names for the same list object. If you change player_radar, you will see the changes on SEA, and on all other variables that refer to the same object.

3 Comments

In C++, both player_radar and SEA would just be pointers.
@9000: Only if you declared them as pointers and newed them instead of using something like a vector<vector<Ship>>.
Also, any time you write a copy constructor or assignment operator for a C++ class with pointer member variables, you have very similar issues to deal with, i.e. do you just want to copy the pointer (usually not, but sometimes you do) or do you want make a copy of the thing it's pointing to, and point to that.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.