Python 4 Players Snake Game

Question

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] != direction))

    def move(self):
        # debug
        # if self.direction:
        #     print(self.segments[0], WIDTH, HEIGHT)
        self.prev_pos = [self.segments[-1][0], self.segments[-1][1]]
        previous = self.segments[0][:]
        self.segments[0][0] += self.x
        self.segments[0][1] += self.y
        if PASSABLE_WALLS:
            # if walls are enabled, there's bug that you can hide in wall -- FIXED
            if WALLS:
                if self.segments[0][0] > WIDTH - TILE_SIZE * 2:
                    self.segments[0][0] = 0 + TILE_SIZE

                elif self.segments[0][0] < 0 + TILE_SIZE:
                    self.segments[0][0] = WIDTH - TILE_SIZE * 2

                elif self.segments[0][1] > HEIGHT - TILE_SIZE * 2:
                    self.segments[0][1] = 0 + TILE_SIZE

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

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

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

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

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

        self.is_collision()
        for i in range(1, len(self.segments)):
            self.segments[i], previous = previous, self.segments[i][:]
        self.moved = True
        # print(apples)

    def is_collision(self):
        # snakes are checked 1 by 1, if both collide, ONLY earlier snake from list will get destroyed
        if not PASSABLE_WALLS:
            if not 0 <= self.segments[0][0] <= WIDTH or not 0 <= self.segments[0][1] <= HEIGHT:
                self.destroy_snake()
        for snake in snakes:
            for segment in snake.segments:
                if segment == self.segments[0] and segment is not self.segments[0]:
                    self.destroy_snake()
                    break

    def destroy_snake(self):
        snakes.remove(self)
        print('DESTROYED', self.COLOR, self.score)
        is_game_over()


def create_apple(app):
    if WALLS:
        # apple can appear in walls -- FIXED
        apple_x = randint(1, WIDTH // TILE_SIZE - 2) * TILE_SIZE
        apple_y = randint(1, HEIGHT // TILE_SIZE - 2) * TILE_SIZE
        app.append([apple_x, apple_y])
    else:
        apple_x = randint(0, WIDTH // TILE_SIZE) * TILE_SIZE
        apple_y = randint(0, HEIGHT // TILE_SIZE) * TILE_SIZE
        app.append([apple_x, apple_y])


def draw_apples():
    for apple in apples:
        pygame.draw.rect(game_display, RED, [apple[0], apple[1], TILE_SIZE, TILE_SIZE])


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.init()

WIDTH = 800
HEIGHT = 600
game_display = pygame.display.set_mode((WIDTH, HEIGHT))

game_end = False

WHITE = (255, 255, 255)
BLACK = (0, 0, 0)
RED = (255, 0, 0)
GREEN = (0, 255, 0)
BLUE = (0, 0, 255)
YELLOW = (255, 255, 0)
GRAY = (200, 200, 200)
BACKGROUND_COLOR = GRAY
TILE_SIZE = 10
TOTAL_APPLES = 10
DIRECTION_LEFT = "LEFT"
DIRECTION_RIGHT = "RIGHT"
DIRECTION_UP = "UP"
DIRECTION_DOWN = "DOWN"
DIRECTIONS_REVERSE = {DIRECTION_LEFT: DIRECTION_RIGHT,
                      DIRECTION_RIGHT: DIRECTION_LEFT,
                      DIRECTION_UP: DIRECTION_DOWN,
                      DIRECTION_DOWN: DIRECTION_UP}
DIRECTIONS = {DIRECTION_LEFT: (-TILE_SIZE, 0),
              DIRECTION_RIGHT: (TILE_SIZE, 0),
              DIRECTION_UP: (0, -TILE_SIZE),
              DIRECTION_DOWN: (0, TILE_SIZE)}
apples = []
for _ in range(TOTAL_APPLES):
    create_apple(apples)
snakes = [Snake(YELLOW, 400, 400, [pygame.K_UP, pygame.K_RIGHT, pygame.K_DOWN, pygame.K_LEFT]),
          Snake(GREEN, 200, 200, [pygame.K_w, pygame.K_d, pygame.K_s, pygame.K_a]),
          Snake(BLUE, 300, 300, [pygame.K_i, pygame.K_l, pygame.K_k, pygame.K_j]),
          Snake(BLACK, 500, 500, [pygame.K_KP8, pygame.K_KP6, pygame.K_KP2, pygame.K_KP4])]


def main():
    global move_ticker
    global game_end

    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            game_end = True

        if event.type == pygame.KEYDOWN:
            # very rapid change can destroy snake -- FIXED with self.moved in change_direction & move
            for snake in snakes:
                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)

    game_display.fill(BACKGROUND_COLOR)
    draw_apples()
    if move_ticker == 0:
        move_ticker = GAME_SPEED
        for snake in snakes:
            snake.move()

    for snake in snakes:
        snake.draw_snake()

    for apple in apples:
        for snake in snakes:
            if snake.segments[0] == apple:
                snake.eat_apple(apple)
                create_apple(apples)

    if WALLS:
        draw_walls()

    pygame.display.update()
    if move_ticker > 0:
        move_ticker -= 1

move_ticker = 0
while not game_end:
    main()
quit()

Answer 1

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] < 0:
                self.segments[0][0] = WIDTH - TILE_SIZE

            elif self.segments[0][1] > 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