Text-mode 2048 in Python (using numpy)

Question

Good evening. I am studying mathematics at the moment, so I have little to no formal education in actual computer engineering. However, I am trying my hands at learning Python because I will need lots of it for my future career. I hope you can point out any bad practices or mistakes on my part, so that I can fix them early on in my journey. I thank all of you in advance.

I have coded a command line implementation of the "2048" game. I have written a "game engine", implemented as the Game class of the two048.py python module. This module is imported by main.py and an instance of Game is then used to actually play the game.

The Game class is implemented to be as general as possible: it can be used to play the "2048" game on a nxn game table with the goal of going over any positive m. The game table is stored as a numpy array to make the code as efficient as possible.

Here comes the code:

file1 - main.py

"""Play a command line "2048" game.""" from two048 import * # Create a new "2048" game, set a random entry and display it. game = Game(4, 2048) game.display() # As long as the game is not won or lost, allow the player to make moves by inputting characters. # Illegal moves or meaningless strings are ignored. while game.status == Status.IN_PROGRESS: c = input() if c == "w": game.shift_up() elif c == "d": game.shift_right() elif c == "s": game.shift_down() elif c == "a": game.shift_left() else: continue game.insert_random_2() game.update_status() game.display() if game.status == Status.LOST: print("You lost!") elif game.status == Status.WON: print("You won!") 

file2 - two048.py

"""Game engine for "2048". This module defines a class Game whose instances are 2048 games. """ # Import useful libraries: # - numpy: the "2048" game is played on a numpy array # - random: random handles # - enum: the status of the "2048" game is memorized as an Enum # - re: we use the sub function to print the game table import numpy as np import random from enum import Enum from re import sub random.seed() Status = Enum("Status", ["WON", "LOST", "IN_PROGRESS"]) class Game: """Each instance represents a 2048 game. The game table is represented as a nxn numpy array (n is provided by the user an is usually set = 4). Use the shift_left, shift right, shift_up, shift_down methods to shift the numbers on the table. These methods represent moves the player can make. A move is legal if it results in at least one number shifting or changing. Every time the player makes a legal move, a 2 is randomly inserted on the table. If the player cannot make any legal move, they lose. If the player manages to obtain a number >= winning number (winning number is provided by the user and is usually set = 2048), they win. The status of the game (WON - LOST - IN_PROGRESS) is memorized as a Status Enum. """ def __init__(self, n, winning_number): """Initialize a 2048 game and set up all the instance variables. Arguments: self n -- integer representing the size of the game table winning_number -- integer representing the number that the user should exceed to win """ self.n = n self.table = np.zeros((n, n), dtype=int) self.winning_number = winning_number self.status = Status.IN_PROGRESS self.insert_random_2() def shift_left(self): """Shift the table left. The same algorithm is applied to every row: - compact the row to the left by removing empty space - replace consecutive equal integers with a single integer equal to their sum """ # i -- row index # j -- column index for i in range(self.n): # We need to keep track of the number eliminated elements to avoid getting stuck in a loop. num_eliminated_elements = 0 # Iterate over the i-th row to compact the row by eliminating null entries. j = 0 while j < self.n - num_eliminated_elements - 1: if self.table[i, j] == 0: # Compact the array by "eliminating" self.table[i, j]. The concatenate function is used for the sake # of efficiency. self.table[i, j:] = np.concatenate((self.table[i, j + 1:], np.zeros(1))) num_eliminated_elements += 1 else: j += 1 # Iterate over the i-th row again to sum up consecutive equal entries. j = 0 while j < self.n - num_eliminated_elements - 1: if self.table[i, j] == self.table[i, j + 1]: self.table[i, j + 1] = 2*self.table[i, j + 1] self.table[i, j:] = np.concatenate((self.table[i, j + 1:], np.zeros(1))) num_eliminated_elements += 1 else: j += 1 def shift_right(self): """Shift the table right. The same algorithm is applied to every row: - compact the row to the right by removing empty space - replace consecutive equal integers with a single integer equal to their sum """ # i -- row index # j -- column index for i in range(self.n): # We need to keep track of the number eliminated elements to avoid getting stuck in a loop. num_eliminated_elements = 0 # Iterate through the i-th row in reverse order to compact the row by eliminating null entries. j = self.n - 1 while j > num_eliminated_elements: if self.table[i, j] == 0: # Compact the array by "eliminating" self.table[i, j]. The concatenate function is used for the sake # of efficiency. self.table[i, :j + 1] = np.concatenate((np.zeros(1), self.table[i, :j])) num_eliminated_elements += 1 else: j -= 1 # Iterate over the i-th row again to sum up consecutive equal entries. j = self.n - 1 while j > num_eliminated_elements: if self.table[i, j] == self.table[i, j - 1]: self.table[i, j - 1] = 2 * self.table[i, j - 1] self.table[i, :j + 1] = np.concatenate((np.zeros(1), self.table[i, :j])) num_eliminated_elements += 1 else: j -= 1 def shift_up(self): """Shift the table up. The same algorithm is applied to every column: - compact the column above by removing empty space - replace consecutive equal integers with a single integer equal to their sum """ # i -- column index # j -- row index for i in range(self.n): # We need to keep track of the number eliminated elements to avoid getting stuck in a loop. num_eliminated_elements = 0 # Iterate through the i-th column in reverse order to compact the row by eliminating null entries. j = 0 while j < self.n - num_eliminated_elements - 1: if self.table[j, i] == 0: # Compact the array by "eliminating" self.table[i, j]. The concatenate function is used for the sake # of efficiency. self.table[j:, i] = np.concatenate((self.table[j + 1:, i], np.zeros(1))) num_eliminated_elements += 1 else: j += 1 # Iterate over the i-th column again to sum up consecutive equal entries. j = 0 while j < self.n - num_eliminated_elements - 1: if self.table[j, i] == self.table[j + 1, i]: self.table[j + 1, i] = 2 * self.table[j + 1, i] self.table[j:, i] = np.concatenate((self.table[j + 1:, i], np.zeros(1))) num_eliminated_elements += 1 else: j += 1 def shift_down(self): """Shift the table down. The same algorithm is applied to every column: - compact the column below by removing empty space - replace consecutive equal integers with a single integer equal to their sum """ # i -- column index # j -- row index for i in range(self.n): # We need to keep track of the number eliminated elements to avoid getting stuck in a loop. num_eliminated_elements = 0 # Iterate through the i-th column in reverse order to compact the row by eliminating null entries. j = self.n - 1 while j > num_eliminated_elements: if self.table[j, i] == 0: # Compact the array by "eliminating" self.table[i, j]. The concatenate function is used for the sake # of efficiency. self.table[:j + 1, i] = np.concatenate((np.zeros(1), self.table[:j, i])) num_eliminated_elements += 1 else: j -= 1 # Iterate over the i-th column again to sum up consecutive equal entries. j = self.n - 1 while j > num_eliminated_elements: if self.table[j, i] == self.table[j - 1, i]: self.table[j - 1, i] = 2 * self.table[j - 1, i] self.table[:j + 1, i] = np.concatenate((np.zeros(1), self.table[:j, i])) num_eliminated_elements += 1 else: j -= 1 def update_status(self): """Check whether the game has been won or lost and update the self.status instance variable if necessary.""" # If there is at least one element in self.table >= self.winning_number, update the game status to WON. if np.any(self.table >= self.winning_number): self.status = Status.WON return # If any entry of the table is null, the game is certainly not lost. Since at this point we know that the game # is not lost, any subsequent check becomes unnecessary. if np.any(self.table == 0): return # At this point, we know that the game is not won and that no entry of self.table is null. Check if the player # can make any legal move, i.e. if there are two consecutive equal elements of self.table along either axis. # If this is the case, no further check is needed and we can return. If the player can make no legal move, # the game is lost. for i in range(self.n): for j in range(self.n): # The indexing may fail due to out of bounds errors and thus a try block is required. If the indexing # fail, just try a different pair of indeces. try: if self.table[i,j] == self.table[i+1,j]: return except: pass try: if self.table[i,j] == self.table[i,j+1]: return except: pass self.status = Status.LOST def display(self): """Display the game table in a legible format.""" # We use the array_str numpy method to convert the table to a string, and then we use a regular expression to # remove the brackets. The first bracket needs to be replaced by a space to preserve indentation. # A blank line is printed before and after the table. print() print(sub("[\[\]]", "", " " + np.array_str(self.table))) print() def insert_random_2(self): """Replace a random null entry of self.table with the number 2. Nothing is done if there is no null entry.""" if np.any(self.table == 0): # Select random indices i,j until self.table[i, j] == 0. Then set self.table[i, j] = 2. i = random.randrange(self.n) j = random.randrange(self.n) while self.table[i, j] != 0: i = random.randrange(self.n) j = random.randrange(self.n) self.table[i, j] = 2 

The following aspects of the code concern me the most:

1. The four methods that shift the numbers on the table are all fairly similar and feel quite repetitive. Nevertheless, any other solution I can come up with would make the code much more complicated and difficult to understand. Is this amount of repetition normal in professional code?
2. I'm not 100% sure if making main.py handle the game loop is a good idea. On one hand, the game loop feels like part of the game logic, and thus I think should be in two048.py. On the other hand, I would like the Game class to be quite generic. For instance, I wouldn't mind reusing it as is to implement a GUI version of the game in the future, and the game loop wood look very different there.
3. This is the first Python project I have taken seriously enough to document everything properly. How did I do? Are the docstrings and comments explicative enough? Is my code understandable?

Any feedback and advice is welcome, regardless of how harsh. Thanks everyone and have nice day.

Answer 1

Yay! Modules have docstrings, excellent. And same for classes.

Recommend that you routinely include .idea/ in .gitignore, so files peculiar to certain IDE version won't leak out into the public repo.

wildcard import

Don't do it.

from two048 import * 

Prefer:

from two048 import Game, Status 

When writing source code we always want to support Local Analysis. We want it to be easy for the reader to understand all the pieces in motion. The * star is opaque; the reader would have to pause, go analyze that entire source file, then return here. Be kind, list the symbols explicitly, it's a good habit.

constructor

I like the Game ctor, including the call to randomly insert a "2".

There's a line in the docstring which just says "self" and doesn't make much sense. Simply elide it.

There's two occurrences of the "integer representing" phrase. They make sense, but often such qualifiers will be redundant with the signature, and it's best to say a thing just once. I would prefer to elide both phrases, and use type annotation, so mypy can read it, too.

 def __init__(self, n: int, winning_number: int): 

insert_random_2

This randomized algorithm does work -- eventually it will stumble upon that last empty square. But late in the game it takes much longer than necessary to do so.

Better that you not keep retrying a non-empty cell. Scan the n ** 2 cells, emitting (i, j) coordinates for any zeros found. Now pick a random coordinate from that list, and set the cell to 2.

display

The comments mostly describe the "how", rather than the "why", and should be elided. OTOH this comment is insightful and should be preserved:

# The first bracket needs to be replaced by a space to preserve indentation. 

Consider using import re rather than from re import sub, so the re.sub( ... ) call will more clearly be about regular expressions.

update_status

Please lint your code every now and again. We see this bit of lint advice:

E722 Do not use bare `except` 

Never write except:, as it catches "too many" exceptions, including the one corresponding to CTRL-C. Prefer except Exception:. Or something more narrowly focused. Here, we care about except IndexError:

user prompt

 elif c == "a": game.shift_left() else: continue 

I anticipated that the game would offer a brief line of instructions before starting its loop. Or maybe that's not needed, since repeat players already know what to do.

But at the continue, to silently taunt the player seems cruel. This is the perfect opportunity to remind them that the wasd keys control game play.

questions

1. The four methods that shift the numbers on the table are all fairly similar and feel quite repetitive. ... Is this amount of repetition normal in professional code?

No. (But sometimes yes, at least in a first draft.)

The four shift_DIRECTION() methods work and are a good first step. You could certainly stop here. There are several refactoring avenues for improvement. If you begin following my suggestions and then lose interest, that's fine, you can stop at any time and will still have working code. I will focus on shift_up just because it's the first one called (for w key).

You have a working shift_up, good. Now write a test named test_shift_up, so it shows Green bar.

Return to shift_up, look at those loops, and notice that it's doing more than one thing. Break out helpers that adhere to the single responsibility principle. Writing the test for a helper is usually simpler than for the bigger function. Verify that you still see a Green bar.

Now visit shift_down. How could we delete much of this code? Could we define a dy delta y, with value +1 or -1, which could be passed in for the helpers to use? Use copy-n-paste (this is normal for tests, which are dirt simple) to produce a test_shift_down method. Verify that you still see a Green bar.

Now let's think about the remaining shift_left and shift_right. Recall that numpy treats rows as axis 0 and columns as axis 1. Is there a small integer we could pass in to the helpers that would turn the existing column operations into row operations? Verify that you still see a Green bar.

There may be a detail of worrying about IndexError at edge of square. One approach worth considering is to embed the game of side N in the center of an N+2 square, with sentinel values on the edges. Now there's no need to probe too near the edge.

Yes, making main.py handle the game loop is a good idea.

The only thing I would change is to bury the existing code within def main():, and then add the usual main guard at end:

if __name__ == "__main__": main() 

We usually want just class + function definitions in a source file, for the most part. Nothing with side effects, such as print(). That lets other modules, including unit tests, silently import those definitions. The main guard handles the case where we actually want to run the script, rather than import it. Again, following that convention is a good habit to get into.

3. Are the docstrings and comments [appropriate]?

The Game class docstring is brilliant.

I tend to like most of your comments. Some of them can be a little on the obvious or verbose side. Focus on explaining the "why", since the code explains the "how". When you're about to write an explanation in the middle of a function, consider whether introducing a well named helper, maybe def sum_up_consecutive_equal_entries():, would obviate the need for a comment.

---

This codebase achieves its design goals.

I would be willing to delegate or accept maintenance tasks on it.