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 10
s 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.