5
\$\begingroup\$

I wrote a python model for the game "snake" that I'm not really satisfied with.

My intention was to separate the game logic from the drawing and input handling (which worked quite well) but I've got several points which I don't know how to solve better.

  1. To use this model one needs to initialize it with callbacks that fire when the score increases and when the game is over. They pass the score as an integer. I don't like this solution because it feels a lot like C, passing around function pointers.

  2. For the drawing the user needs to access the head, tail and food variables in the model. Should I write explicit getters for that? I prefixed everything the user shouldn't directly access with an underscore. How can I make this interface more clear?

  3. One needs to call _move_snake(dx, dy) with the direction dx, dy the snake should move although the model already has a variable that holds the snake's direction. But I guess this way it's easier for testing?


import random


class SnakeModel(object):

    """
    Model for a snake-like game.
    """

    def __init__(self, width, height,
                 gameover_callback, atefood_callback):
        """
        Initializes the game

        gameover_callback and atefood_callback are callback functions. They
        are passed the current player score (length of the snake's tail) as an
        integer.
        """
        self.width = width
        self.height = height
        self._gameover = gameover_callback
        self._ateFood = atefood_callback
        self.reset()

    def reset(self):
        """
        Starts a new game.

        Sets the starting position, length and direction of the snake as well
        as the position of the first food item.
        """
        self._dir = (0, -1)

        # You can read head, tail and food directly for drawing
        self.head = (self.width / 2, self.height - 2)
        self.tail = [(self.width / 2, self.height - 1)]
        self.food = (self.width / 2, self.height / 3)

    def step(self):
        """
        Advances the game one step
        """
        # check for collisions
        if self.head in self.tail:
            self._gameover(len(self.tail))

        # eat food
        if self.head == self.food:
            self.tail.insert(0, self.head)
            self.food = self._new_food_location()
            self._ateFood(len(self.tail))

        # move snake
        self._move_snake(*self._dir)

    def set_snake_direction(self, dx, dy):
        """
        Sets the direction the snake will move on the next step
        """
        self._dir = (dx, dy)

    def _move_snake(self, dx, dy):
        """
        Moves the snake one step in the given direction.

        The snake can move through the game area's borders and comes out on the
        other side.
        """
        x, y = self.head
        newX = (x + dx) % self.width
        newY = (y + dy) % self.height
        self.head = (newX, newY)
        self.tail.insert(0, (x, y))
        self.tail.pop()

    def _new_food_location(self):
        """
        Returns a new possible food position which is not in the snake.
        """
        game_field = set([(x, y) for x in range(self.width)
                          for y in range(self.height)])
        possible = game_field - set(self.tail) - set([self.head])
        new_position = random.choice(list(possible))
        return new_position
\$\endgroup\$
  • \$\begingroup\$ You might be interested in this question and its answer. \$\endgroup\$ – Gareth Rees Dec 23 '13 at 0:18
3
\$\begingroup\$

1. Answers to your questions

  1. Callbacks. Why not use a return value intead?

    def step(self):
        """Advance the game one step.
        Return a pair of Booleans (game over, ate food).
    
        """
        game_over = self.head in self.tail
        ate_food = self.head == self.food
        if ate_food:
            self.tail.insert(0, self.head)
            self.food = self._new_food_location()
        self._move_snake(*self._dir)
        return game_over, ate_food
    

    (But see below for two bugs in this function.)

  2. Getters are normally used in three scenarios. First, to provide a simple interface to a complex data structure. Second, to give the implementer freedom to change the implementation in future. Third, in combination with setters when writing a public API whose internal data structures have an invariant that needs to be preserved, so as to protect the internal data structures from corruption by a well-meaning but incompetent client.

    None of these scenarios applies here. The internal data structures are straightforward, so the first scenario does not apply. And this is all your own code (not a public API) so the second and third scenarios do not apply.

  3. Redundancy in _move_snake. It doesn't seem like a big deal either way. No opinion here.

2. Other comments on your code

  1. There's a bug in step: you move the snake even if the game is over. Surely the snake should stop moving when it's dead?

  2. Another bug in step, this time in the "eat food" logic. You insert the head into the tail:

    self.tail.insert(0, self.head)
    

    but then you call _move_snake which does the same thing. So the snake ends up containing two copies of the head location.

    The usual way that "snake" games work is that when the snake eats some food, it does not grow a new tail segment immediately. Instead, it waits until the next time it moves and grows a new tail segment in the position where its old tail used to be. This is easily implemented by incrementing a counter each time the snake eats food:

    self.growth_pending += 1
    

    and then decrementing the counter instead of deleting the tail segment:

    if self.growth_pending:
        self.growth_pending -= 1
    else:
        self.tail.pop()
    

    Note that these bugs would have been easy for you to spot had you actually finished writing the game and tried to run it.

  3. Inserting an item at the beginning of a list:

    self.tail.insert(0, (x, y))
    

    takes time proportional to the length of the list (see the TimeComplexity page on the Python wiki). It would be better to use a collections.deque and call the appendleft method.

  4. This line of code:

    game_field = set([(x, y) for x in range(self.width)
                      for y in range(self.height)])
    

    doesn't need the list comprehension, as you can pass a generator expression directly to set:

    game_field = set((x, y) for x in range(self.width)
                     for y in range(self.height))
    

    This avoids constructing a list and then immediately throwing it away again.

  5. You can further simplify that line of code using itertools.product:

    from itertools import product
    game_field = set(product(range(self.width), range(self.height)))
    
  6. In _new_food_location you convert possible into a list so that you can call random.choice on it. But if you used random.sample instead, you wouldn't have to do that.

  7. In _new_food_location you build a set containing every location in the game not occupied by the snake. It would be better to build this set just once per game, and keep it up to date as the snake moves.

    In detail, in reset you'd write something like:

    self.growth_pending = 0
    self.empty = set(product(range(self.width), range(self.height)))
    self.empty.remove(self.head)
    self.empty.difference_update(self.tail)
    

    (Note the use of difference_update to avoid having to convert self.tail to a set.)

    In _move_snake you'd write something like:

    self.tail.appendleft(self.head)
    self.head = newX, newY
    self.empty.remove(self.head)
    if self.growth_pending:
        self.growth_pending -= 1
    else:
        self.empty.add(self.tail.pop())
    

    And finally, in _new_food_location you'd write:

    new_position = random.sample(self.empty, 1)[0]
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.