Python 4 Players Snake Game

I would really like to hear out some suggestions as well as general response to the code I've written for 4 players snake game. It's my first time using pygame module, as well as trying to design a simple game, in general. I've encountered few bugs which I've managed to fix, but I would like to improve my code and refactor it to make it better. My biggest concern is about "apples" and random generation in general. For sure I repeat myself too much in my code, in few places.

```
import pygame
from random import randint

WALLS = True
PASSABLE_WALLS = True
GAME_SPEED = 120 # suggsted 120; lower value - faster game play, higher value - slower game play

def is_game_over():
global game_end
game_end = not snakes

class Snake:
def __init__(self, color, head_x, head_y, movement_keys):
self.segments = [[head_x, head_y]]
self.prev_pos = None
self.UP = movement_keys[0]
self.RIGHT = movement_keys[1]
self.DOWN = movement_keys[2]
self.LEFT = movement_keys[3]
self.direction = None
self.x = 0
self.y = 0
self.COLOR = color
self.score = 0
self.moved = True

def draw_snake(self):
for segment in self.segments:
self.draw_segment(segment)

def draw_segment(self, segment):
pygame.draw.rect(game_display, self.COLOR, [segment[0], segment[1], TILE_SIZE, TILE_SIZE])

def eat_apple(self, apple):
self.score += 1
apples.remove(apple)
self.segments.append(self.prev_pos)

def change_direction(self, direction):
if self.change_direction_helper(direction):
self.direction = direction
self.x = DIRECTIONS[direction][0]
self.y = DIRECTIONS[direction][1]
self.moved = False

def change_direction_helper(self, direction):
return self.moved and (self.direction is None or
(self.direction != direction and DIRECTIONS_REVERSE[self.direction]

is_x functions should not modify anything

As a widely accepted convention, if a function starts with is_ it just returns an information, without changing the state of the world.

As such your is_collision is surprising:

def is_collision(self):
    ....
            self.destroy_snake()

Just change it to return a boolean and destroy the snake from outside.

Use tuple unpacking

It is unnecessary to use a = x[0]; b = x[1]; #..., using , you can assign multiple values at once and simplify:

self.UP, self.RIGHT, self.DOWN, self.LEFT = movement_keys

and:

self.x, self.y = DIRECTIONS[direction]

Avoid changing the world if reasonable

In a game you must change the world a lot, but each change is a possible breaking point, so I suggest minimizing these.

I would change create_apple to return the newly create apple:

def create_apple():
    if WALLS:
        apple_x = randint(1, WIDTH // TILE_SIZE - 2)
        apple_y = randint(1, HEIGHT // TILE_SIZE - 2) 
    else:
        apple_x = randint(0, WIDTH // TILE_SIZE)
        apple_y = randint(0, HEIGHT // TILE_SIZE)
    return [apple_x * TILE_SIZE, apple_y * TILE_SIZE]

This also simplifies the code later because allows the use of a list comprehension:

apples = [create_apple() for _ in range(TOTAL_APPLES)]

Prefer dictionaries over long if elif chains

if event.key == snake.LEFT:
                snake.change_direction(DIRECTION_LEFT)
            elif event.key == snake.RIGHT:
                snake.change_direction(DIRECTION_RIGHT)
            elif event.key == snake.UP:
                snake.change_direction(DIRECTION_UP)
            elif event.key == snake.DOWN:
                snake.change_direction(DIRECTION_DOWN)

Can be written as:

KEY_TO_DIRECTION = {
     snake.LEFT : DIRECTION_LEFT,
     ...
}

and:

snake.change_direction(KEY_TO_DIRECTION[event.key])

This makes the logic clearer and complexity is handled better in data-structures rather than conditionals.

Reducing repetition

You can use loops to reduce repetition:

def draw_walls():
    color = BLACK
    pygame.draw.rect(game_display, color, [0, 0, WIDTH, TILE_SIZE])
    pygame.draw.rect(game_display, color, [0, HEIGHT - TILE_SIZE, WIDTH, TILE_SIZE])
    pygame.draw.rect(game_display, color, [0, 0, TILE_SIZE, HEIGHT])
    pygame.draw.rect(game_display, color, [WIDTH - TILE_SIZE, 0, TILE_SIZE, HEIGHT])

pygame.draw.rect(game_display, color, is repeated four times, instead you could write:

def draw_walls():
    color = BLACK
    for rect in ([0, 0, WIDTH, TILE_SIZE],
                 [0, HEIGHT - TILE_SIZE, WIDTH, TILE_SIZE],
                 [0, 0, TILE_SIZE, HEIGHT],
                 [WIDTH - TILE_SIZE, 0, TILE_SIZE, HEIGHT]):
        pygame.draw.rect(game_display, color, rect)

This has the advantage of making it obvious that between this calls only the rect changes.

Write additional functions to explain your code at a higher abstraction

The following b0lock of code:

if self.segments[0][0] > WIDTH - TILE_SIZE:
                self.segments[0][0] = 0

            elif self.segments[0][0]  HEIGHT - TILE_SIZE:
                self.segments[0][1] = 0

            elif self.segments[0][1] < 0:
                self.segments[0][1] = HEIGHT - TILE_SIZE

Handles the wrapping around the edges of the snake, but the reader has to guess/deduce it, instead you could write a function:

def wrap_around(x, y, width=WIDTH, height=HEIGHT, tile_size=TILE_SIZE):
    # implement

And write:

self.segments[0][0], self.segments[0][1] = wrap_around(self.segments[0][0], self.segments[0][1])

A separate pure function can also be documented and/or tested on its own to further improve the quality of the code.

Naming: helper

Naming a function helper does not help the reader, he can already see it is a helper by seeing that it is used inside a function only once.

Given that change_direction_helper verifies whether it is possible to change direction, I would rename it can_change_direction

StackExchange Code Review Q#121918, answer score: 3