4
\$\begingroup\$

My attempt at creating a snake game, in pygame. I tried to make it as simple as possible, but i feel it could be better in terms of best practice, and efficiency, along the idea of avoiding redundant code. It would be appreciated, if anyone can give me advice in that regard.

import pygame
import time
import random
pygame.init()
pygame.font.init()

WINDOW = pygame.display.set_mode((500, 500))

pygame.display.set_caption('snake')

FOOD_COORS = []

TICK = 15


RUN = True
SNAKE_COMP = [[50, 50, 2], [40, 50, 2], [30, 50, 2], [20, 50, 2], [10, 50, 2]]
f = [random.randint(0, 50)*10, random.randint(0, 50)*10]
d = 2
CLOCK = pygame.time.Clock()

def hit():
    time.sleep(3)
    pygame.quit()

class snake():
    def __init__(self, SNAKE_COMP):
        self.x, self.y = SNAKE_COMP[0][0:2]
    def draw(self, SNAKE_COMP):
        self.SNAKE_COMP = SNAKE_COMP
        for i in range(0, len(SNAKE_COMP)):
            pygame.draw.rect(WINDOW, (255, 255, 255), (SNAKE_COMP[i][0], SNAKE_COMP[i][1], 10, 10))

    def hit_check(self, SNAKE_COMP):
        self.SNAKE_COMP = SNAKE_COMP
        if SNAKE_COMP[0][0] >= 500 or SNAKE_COMP[0][0] < 0:
            hit()
        if SNAKE_COMP[0][1] >= 500 or SNAKE_COMP[0][1] < 0:
            hit()
        test_l = [[]]
        for i in range(0, len(SNAKE_COMP)):
            test_l.append(tuple(SNAKE_COMP[i][0:2]))
        for i in range(0, len(test_l)):
            if test_l.count(test_l[i]) > 1:
                hit()

class food():
    global FOOD_COORS
    def draw(self):
        x, y = self.x, self.y
        pygame.draw.rect(WINDOW, (255, 0, 0), (x, y, 10, 10))
    def spawn(self, SNAKE_COMP):
        global FOOD_COORS
        self.SNAKE_COMP = SNAKE_COMP
        test_l = [[]]
        for i in range(0, len(SNAKE_COMP)):
            test_l.append(SNAKE_COMP[i][0:2])
        g = True
        while g:
            x = random.randint(0, 49)*10
            y = random.randint(0, 49)*10
            if [x, y] not in test_l:
                g = False
        FOOD_COORS = [x, y]
        self.x, self.y = x, y
snek = snake(SNAKE_COMP)
apple = food()
apple.spawn(SNAKE_COMP)
s = False
g = False
while RUN:
    CLOCK.tick(TICK)

    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            RUN = False

    keys = pygame.key.get_pressed()
    if keys[pygame.K_UP] and d != 3:
        d = 1
    elif keys[pygame.K_RIGHT] and d != 4:
        d = 2
    elif keys[pygame.K_DOWN] and d != 1:
        d = 3
    elif keys[pygame.K_LEFT] and d != 2:
        d = 4
    if g != True and SNAKE_COMP[0][0:2] != FOOD_COORS:
        last = len(SNAKE_COMP) - 1
        for i in range(1, len(SNAKE_COMP)):
            SNAKE_COMP[len(SNAKE_COMP)-i][2] = SNAKE_COMP[len(SNAKE_COMP)-i-1][2]
        SNAKE_COMP[0][2] = d
        for i in range(0, len(SNAKE_COMP)):
            if SNAKE_COMP[i][2] == 1:
                SNAKE_COMP[i][1] -= 10
            elif SNAKE_COMP[i][2] == 2:
                SNAKE_COMP[i][0] += 10
            elif SNAKE_COMP[i][2] == 3:
                SNAKE_COMP[i][1] += 10
            elif SNAKE_COMP[i][2] == 4:
                SNAKE_COMP[i][0] -= 10
    else:
        k = SNAKE_COMP[0][2]
        FOOD_COORS.append(k)
        if k == 1:
            FOOD_COORS[1] -= 10
        elif k == 2:
            FOOD_COORS[0] += 10
        elif k == 3:
            FOOD_COORS[1] += 10
        elif k == 4:
            FOOD_COORS[0] -= 10
        SNAKE_COMP.insert(0, FOOD_COORS)
        apple.spawn(SNAKE_COMP)
    snek.hit_check(SNAKE_COMP)
    apple.draw()
    snek.draw(SNAKE_COMP)
    pygame.display.update()
    WINDOW.fill((0, 0, 0))

pygame.quit()
\$\endgroup\$
3
\$\begingroup\$

On the whole, the game works and has a solid UI that you've correctly kept basic and mostly out of play as you focus on the game engine logic. It looks like you've gone for a partially-OOP approach by putting focus on the two entities that are drawable and their necessary data but leaving the main game logic outside of a class. I think that's a reasonable fundamental design. I do have a variety of suggestions for tightening up this design and improving code style though.

Tight coupling

Although your classes are potentially useful abstractions, the snake's movement update is done outside of the snake class, breaking encapsulation. A snake.move or snake.change_direction function is the correct delegation of responsibility for this rather than reaching into the class from main and messing with its internals.

The state of code with classes and functions that rely on and mutate global state is difficult to predict. For example, I'd reasonably expect a snake or food class to be able to create multiple instances with separate positions. But in this design, instances are tightly coupled to a single global FOOD_COOR or SNAKE_COMP variable. Creating more than a single instance of either class breaks it.

There are other more subtle violations of modularity, like calling global hit inside of snake.hit_check. This function should return true or false if a hit occurred and let the calling code invoke hit how, when and if they please rather than creating a dependency between the class and exterior code. pygame could be removed or at least injected into the object so the same snake logic could hook to any visual interface.

High cyclomatic complexity

The main while loop that runs the game has very high cyclomatic complexity with over 18 branches and 3 layers of nesting. These giant blocks of conditionals and loops make the code very hard to understand (and by extension, maintain and debug) and should be broken out into functions or otherwise refactored.

Abusing literals

The code abuses hard-coded literal values throughout. If you want to change the size of the grid, for example, you'd need to walk the whole file hunting for all the 10s that happen to be related to the grid size to make them a different number. This is tedious and error-prone even in a tiny program.

Same applies to window size and a few other things. Storing these values as variables in one place and referencing them means everything just works when you need to change a value, helping to eliminate typo bugs and ease refactoring. If classes or functions need to know the window size, this information should be injected in a parameter to the initializer or appropriate method.

d (really direction) has 4 possible values: 1, 2, 3 and 4. The problem is that "1" has no semantic meaning here. It's not obvious whether a "1" entails up, down, left or sideways. The classical way to handle this is an enumeration, but even direction = "up" would increase the expressiveness of the code and reduce bugs (did you type in 2 when you meant 1 somewhere?).

Magical boolean flags

Variables like

s = False
g = False

are unclear. Pick descriptive names and avoid boolean flags in favor of functions that can return true/false to handle control flow. The reason functions are cleaner than flags is because they result in less state for the caller to keep track of and support modularity. Less state means the code is easier to understand. Modularity means it's easier to isolate problems if they occur and handle refactors locally without causing a domino effect throughout the code base.

Complex logic

Much of the logic can be simplified considerably. To pick one example, code that checks whether the head segment is colliding with the tail,

test_l = [[]]
for i in range(0, len(SNAKE_COMP)):
    test_l.append(tuple(SNAKE_COMP[i][0:2]))
for i in range(0, len(test_l)):
    if test_l.count(test_l[i]) > 1:
        hit()

could be something like

if any(SNAKE_COMP[0][:2] == x[:2] for x in SNAKE_COMP[1:]):
    hit()

Even here, it's unusual that SNAKE_COMP needs 3 elements in its coordinates. The slice is a code smell because it's non-obvious. If the third element is direction, it's not necessary. Only the head needs an explicit direction.

Style and naming

  • Classes should be UpperCamelCase. You don't need the () after the class name unless you're inheriting. Making the class Snake as it should be means you can call the instance snake instead of the awkward intentional typo snek to avoid the alias.

  • Use ALL_CAPS variable names sparingly, if at all, and only to designate program constants.

  • Never use single-letter variables unless the purpose is overwhelmingly obvious from context.

    f = [random.randint(0, 50)*10, random.randint(0, 50)*10]
    d = 2
    

    are not obvious. f is never used in the program which an editor with static analysis should warn you of and d should be called direction.

  • Alphabetize imports.

  • Use vertical whitespace more liberally, particularly around functions and blocks.

  • In addition to confusing single-letter boolean flags, names like SNAKE_COMP are unclear. What's COMP? Something like snake_coordinates, snake_body or snake_tail seems a bit clearer here. Even better in a class like snake.tail.

Follow PEP-8 and your style will sparkle.

UX

After a collision, the game freezes for 3 seconds and dies. As a player, I might feel like the game crashed or is buggy. A message or visual indication of the collision would communicate the snake's death better. Even just exiting instantly feels like a smoother experience.

Efficiency

This is totally premature, but it's worth keeping in mind that nearly all of your snake and apple operations that are O(n) like SNAKE_COMP.insert(0, FOOD_COORS) can be made O(1) using a deque and set. When you move the snake forward, you can rotate the deque. When you check for collision between the head and the body, you can use a set lookup.

Rewrite suggestion, round 1

This requires Python 3.8 because of the whisker assignments, but you can easily move those outside of the blocks.

I'm using __iter__ in both classes. Since I'm doing all of the drawing and pygame interaction in the main (the point is to keep UI and game logic separate), making the snake iterable is a nice way to get all of its body segments, but I cast to a tuple to avoid the caller mutating its position accidentally.

On the other hand, I trust that the caller will abide by the vector input for the turn function since we're all consenting adults. If you don't trust the client to behave, you can validate this coordinate pair and raise an error.

There's still lots of room for improvement: the main code is a bit bloated so this refactor is mostly an exercise in class organization and trying to keep everything loosely coupled. Docstrings are pretty cursory and could better explain the parameters and return values.

I don't really know Pygame so I may have gaffed--I find the key handlers pretty awkward, but I notice you can slice out the arrow keys and make a nice, indexable list of flags that hopefully doesn't break anything.

import pygame
from random import randrange

class Snake:
    def __init__(self, size, direction, body):
        self.size = size
        self.direction = direction
        self.body = list(map(tuple, body))

    def __iter__(self):
        return map(tuple, self.body)

    def in_bounds(self, width, height):
        """ Returns whether the snake's head is in the height/width bounds """
        x, y = self.body[0]
        return x >= 0 and y >= 0 and x < width and y < height

    def move(self):
        """ Moves the snake in the direction it's facing """
        self.body.insert(0, (self.body[0][0] + self.direction[0] * self.size,
                             self.body[0][1] + self.direction[1] * self.size))

    def remove_tail(self):
        """ Destroys the snake's last tail segment """
        del self.body[-1]

    def touching_point(self, point):
        """ Returns whether point is somewhere in the snake's body """
        return point in self.body

    def touching_tail(self):
        """ Returns whether the snake's head point is in the snake's body """
        return self.body[0] in self.body[1:]
    
    def set_direction(self, x, y):
        """ Sets the snake's direction given a cardinal unit-vector facing 
            in a non-opposite direction from the snake's current direction
        """
        if (-x, -y) != self.direction:
            self.direction = x, y

class Food:
    def __init__(self, x=0, y=0):
        self.x, self.y = x, y
        
    def __iter__(self):
        yield self.x, self.y

    def reposition(self, size, width, height, used_squares):
        """ Repositions the apple on the size grid within the bounds avoiding 
            certain used squares. Infinitely loops if no squares are available.
        """
        while point := (randrange(0, width, size), randrange(0, height, size)):
            if point not in used_squares:
                self.x, self.y = point
                break

if __name__ == "__main__":
    class Color:
        white = 255, 255, 255
        red = 255, 0, 0
        black = 0, 0, 0

    width = height = 500
    game_speed = 15
    cell_size = 10
    directions = (0, -1), (0, 1), (1, 0), (-1, 0)
    initial_body = (50, 50), (40, 50), (30, 50), (20, 50), (10, 50)
    initial_direction = (1, 0)
    pyg_arrow_key_loc = slice(273, 277)
    pygame.init()
    pygame.display.set_caption("snake")
    pyg_window = pygame.display.set_mode((width, height))
    pyg_clock = pygame.time.Clock()
    snake = Snake(cell_size, initial_direction, initial_body)
    apple = Food()
    apple.reposition(cell_size, width, height, snake)
    
    while not any(event.type == pygame.QUIT for event in pygame.event.get()):
        if any(arrows := pygame.key.get_pressed()[pyg_arrow_key_loc]):
            snake.set_direction(*directions[arrows.index(1)])

        snake.move()

        if snake.touching_point(*apple):
            apple.reposition(cell_size, width, height, snake)
        else:
            snake.remove_tail()

        if snake.touching_tail() or not snake.in_bounds(width, height):
            pygame.quit()
    
        pygame.draw.rect(pyg_window, Color.black, (0, 0, width, height))
        apple_rect = (apple.x, apple.y, cell_size, cell_size)
        pygame.draw.rect(pyg_window, Color.red, apple_rect)

        for x, y in snake:
            pygame.draw.rect(pyg_window, Color.white, (x, y, cell_size, cell_size))

        pyg_clock.tick(game_speed)
        pygame.display.update()

Rewrite suggestion, round 2

I wasn't entirely happy with main in the above rewrite so I gave a shot at cleaning it up a bit. It's still not perfect and adds code but it'd be a likely next step if you wanted to scale up the app. Breaking render into entity-specific functions is a potential next step as the app grows.

Notice that the Snake and Food classes don't need to be touched thanks to the earlier refactor and we can treat them as black boxes. After this refactor, the main function can treat SnakeGame as a black box as well and just specify its configuration. You can see how the abstractions build up: we can tuck these classes into another file like snake.py and use it as a library.

import pygame
from snake import Food, Snake

class SnakeGame:
    class Color:
        white = 255, 255, 255
        red = 255, 0, 0
        black = 0, 0, 0

    def __init__(self, width, height, cell_size, 
                 initial_body, initial_direction, game_speed):
        pygame.init()
        pygame.display.set_caption("snake")
        self.pyg_window = pygame.display.set_mode((width, height))
        self.pyg_clock = pygame.time.Clock()
        self.snake = Snake(cell_size, initial_direction, initial_body)
        self.apple = Food()
        self.cell_size = cell_size
        self.width = width
        self.height = height
        self.game_speed = game_speed
        self.apple.reposition(cell_size, width, height, self.snake)

    def run(self):    
        pyg_arrow_key_loc = slice(273, 277)
        directions = (0, -1), (0, 1), (1, 0), (-1, 0)
        
        while not any(event.type == pygame.QUIT for event in pygame.event.get()):
            if any(arrows := pygame.key.get_pressed()[pyg_arrow_key_loc]):
                self.snake.set_direction(*directions[arrows.index(1)])
        
            self.snake.move()
        
            if self.snake.touching_point(*self.apple):
                self.apple.reposition(self.cell_size, self.width, 
                                      self.height, self.snake)
            else:
                self.snake.remove_tail()
        
            if (self.snake.touching_tail() or 
                not self.snake.in_bounds(self.width, self.height)):
                pygame.quit()
            
            self.render()
        
    def render(self):
        pygame.draw.rect(self.pyg_window, SnakeGame.Color.black, 
                              (0, 0, self.width, self.height))
        apple_rect = (self.apple.x, self.apple.y, self.cell_size, self.cell_size)
        pygame.draw.rect(self.pyg_window, SnakeGame.Color.red, apple_rect)
        
        for x, y in self.snake:
            pygame.draw.rect(self.pyg_window, SnakeGame.Color.white, 
                                  (x, y, self.cell_size, self.cell_size))
        
        self.pyg_clock.tick(self.game_speed)
        pygame.display.update()

if __name__ == "__main__":
    SnakeGame(width=500, 
              height=500, 
              cell_size=10, 
              initial_body=((50, 50), (40, 50), (30, 50), (20, 50), (10, 50)), 
              initial_direction=(1, 0), 
              game_speed=15).run()

Suggested exercises

  • Add a score.
  • Improve graphics/UI/messaging/text.
  • Try making multiple apples.
  • Make the snake periodically "poop", creating an obstacle that it cannot touch but fades away over time.
  • Add walls.
  • Add levels.
  • Try adding a second snake that uses the wasd keys on the same keyboard.
  • Use Flask SocketIO to make a real time network game.
  • Try writing a snake AI.
\$\endgroup\$

Your Answer

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