6
\$\begingroup\$

You know that boring old beginner snake game that people make when they first start out? That's what this is but upgraded and multiplayer. Admittedly it's only on one computer with 'wasd' and 'up down left right' control, but it's a lot of fun. I am concerned with code styling and readability rather than performance (the code is a mess).


mps.py:

import pygame
import random


class game:
    def __init__(self, title, size):
        pygame.init()
        self.size = size
        self.screen = pygame.display.set_mode(self.size)
        pygame.display.set_caption(title)
        pygame.event.set_allowed([pygame.KEYDOWN, pygame.QUIT])
        self.clock = pygame.time.Clock()
        self.done = 0
        self.view_rect = pygame.Rect((0, 0), self.size)
        self.camx, self.camy = 0, 0

        self.cell = 10
        self.cols = self.size[0] // self.cell
        self.rows = self.size[1] // self.cell

        self.num_foods = 200

        self.reset()
        self.score1 = 0
        self.score2 = 0
        self.directions_clockwise = [
            (1, 0),
            (0, 1),
            (-1, 0),
            (0, -1),
        ]

    def reset(self):
        start_y = self.rows // 2
        start_x1 = self.cols // 3
        self.snake = [(start_x1 - i, start_y) for i in range(50)]
        self.direction = (1, 0)
        start_x2 = (self.cols * 2) // 3
        self.snake2 = [(start_x2 + i, start_y) for i in range(50)]
        self.direction2 = (-1, 0)

        self.foods = []
        self.spawn_foods()

        self.dead1 = False
        self.dead2 = False
        self.next_direction1 = None
        self.next_direction2 = None
        self.frame_count1 = 0
        self.frame_count2 = 0
        self.frames_per_move1 = 12
        self.frames_per_move2 = 12

        self.started = False
        self.game_over = False
        self.winner = None

    def spawn_food_one(self):
        tries = 0
        while True:
            fx = random.randrange(0, self.cols)
            fy = random.randrange(0, self.rows)
            pos = (fx, fy)
            if pos in self.foods:
                tries += 1
                if tries > 200:
                    return
                continue
            if pos in self.snake or pos in getattr(self, "snake2", []):
                tries += 1
                if tries > 200:
                    return
                continue
            self.foods.append(pos)
            return

    def spawn_foods(self):
        self.foods = []
        for _ in range(self.num_foods):
            self.spawn_food_one()

    def run_game(self):
        while not self.done:
            self.handle_events()
            if not self.started:
                keys = pygame.key.get_pressed()
                Red_hold = keys[pygame.K_q]
                Blue_hold = keys[pygame.K_p]
                if Red_hold and Blue_hold:
                    self.started = True

            self.frame_count1 += 1
            self.frame_count2 += 1

            if self.started and not self.game_over:
                ready1 = self.frame_count1 >= self.frames_per_move1
                ready2 = self.frame_count2 >= self.frames_per_move2

                if ready1:
                    self.frame_count1 = 0
                    self.move_one1()
                if ready2:
                    self.frame_count2 = 0
                    self.move_one2()

                if self.dead1 or self.dead2:
                    self.game_over = True
                    self.score1 += self.dead2 * 1
                    self.score2 += self.dead1 * 1

            if self.dead1 and not self.dead2:
                self.winner = "Blue wins"
            elif self.dead2 and not self.dead1:
                self.winner = "Red wins"
            elif self.dead1 and self.dead2:
                self.winner = "Tie"

            self.draw()

            pygame.display.update(self.view_rect)

            self.clock.tick(120)

        pygame.quit()

    def handle_events(self):
        for e in pygame.event.get():
            if e.type == pygame.QUIT:
                self.done = 1
            if e.type == pygame.KEYDOWN:
                if e.key == pygame.K_r and self.game_over:
                    self.reset()

                if not self.started or self.game_over:
                    continue
                if e.key == pygame.K_UP:
                    self.change_dir((0, -1), 2)
                elif e.key == pygame.K_DOWN:
                    self.change_dir((0, 1), 2)
                elif e.key == pygame.K_LEFT:
                    self.change_dir((-1, 0), 2)
                elif e.key == pygame.K_RIGHT:
                    self.change_dir((1, 0), 2)

                elif e.key == pygame.K_w:
                    self.change_dir((0, -1), 1)
                elif e.key == pygame.K_s:
                    self.change_dir((0, 1), 1)
                elif e.key == pygame.K_a:
                    self.change_dir((-1, 0), 1)
                elif e.key == pygame.K_d:
                    self.change_dir((1, 0), 1)

    def change_dir(self, new_dir, player):
        ndx, ndy = new_dir
        if player == 1:
            dx, dy = self.direction
            if (dx, dy) != (-ndx, -ndy):
                self.next_direction1 = new_dir
        else:
            dx, dy = self.direction2
            if (dx, dy) != (-ndx, -ndy):
                self.next_direction2 = new_dir

    def move_one1(self):
        if self.next_direction1 is not None:
            self.direction = self.next_direction1
            self.next_direction1 = None
            self.frames_per_move1 = min(
                15, self.frames_per_move1 + abs(self.frames_per_move1 - 20) / 15
            )
        else:
            self.frames_per_move1 = max(
                2, self.frames_per_move1 - (self.frames_per_move1**2) / 500
            )

        new_head1 = None
        head = self.snake[0]
        dx, dy = self.direction
        new_head1 = (head[0] + dx, head[1] + dy)

        if new_head1 is not None:
            if not (0 <= new_head1[0] < self.cols and 0 <= new_head1[1] < self.rows):
                self.dead1 = True
            elif new_head1 in self.snake:
                self.dead1 = True
            elif new_head1 in self.snake2:
                self.dead1 = True

        if not self.dead1:
            self.snake.insert(0, new_head1)
            for food_pos in self.foods:
                food_poses = [
                    (food_pos[0] + x, food_pos[1] + y)
                    for x in range(3)
                    for y in range(3)
                ]
                if new_head1 in food_poses:
                    try:
                        self.foods.remove(food_pos)
                    except ValueError:
                        pass
                    self.spawn_food_one()
                    break
            else:
                self.snake.pop()

    def move_one2(self):
        if self.next_direction2 is not None:
            self.direction2 = self.next_direction2
            self.next_direction2 = None
            self.frames_per_move2 = min(
                15, self.frames_per_move2 + abs(self.frames_per_move2 - 20) / 15
            )
        else:
            self.frames_per_move2 = max(
                2, self.frames_per_move2 - (self.frames_per_move2**2) / 500
            )

        new_head2 = None
        head2 = self.snake2[0]
        dx2, dy2 = self.direction2
        new_head2 = (head2[0] + dx2, head2[1] + dy2)

        if new_head2 is not None:
            if not (0 <= new_head2[0] < self.cols and 0 <= new_head2[1] < self.rows):
                self.dead2 = True
            elif new_head2 in self.snake2:
                self.dead2 = True
            elif new_head2 in self.snake:
                self.dead2 = True

        if not self.dead2:
            self.snake2.insert(0, new_head2)
            for food_pos in self.foods:
                food_poses = [
                    (food_pos[0] + x, food_pos[1] + y)
                    for x in range(3)
                    for y in range(3)
                ]
                if new_head2 in food_poses:
                    try:
                        self.foods.remove(food_pos)
                    except ValueError:
                        pass
                    self.spawn_food_one()
                    break
            else:
                self.snake2.pop()

    def draw(self):
        self.screen.fill((255, 255, 255))

        for fx, fy in self.foods:
            pygame.draw.rect(
                self.screen,
                (0, 200, 0),
                pygame.Rect(
                    fx * self.cell, fy * self.cell, self.cell * 3, self.cell * 3
                ),
            )

        for i, (sx, sy) in enumerate(self.snake):
            color = (255, 0, 0) if i != 0 else (200, 0, 0)
            pygame.draw.rect(
                self.screen,
                color,
                pygame.Rect(sx * self.cell, sy * self.cell, self.cell, self.cell),
            )

        for i, (sx, sy) in enumerate(self.snake2):
            color = (0, 0, 255) if i != 0 else (0, 0, 200)
            pygame.draw.rect(
                self.screen,
                color,
                pygame.Rect(sx * self.cell, sy * self.cell, self.cell, self.cell),
            )

        font = pygame.font.SysFont(None, 20)
        score_surf = font.render(
            f"Red (WASD) score: {int(self.score1)}", True, (20, 20, 20)
        )
        self.screen.blit(score_surf, (5, 5))
        score2_surf = font.render(
            f"Blue (arrows) score: {int(self.score2)}", True, (20, 20, 20)
        )
        self.screen.blit(score2_surf, (5, 25))

        if not self.started:
            instr = "Hold q + p to start"
            instr_s = font.render(instr, True, (20, 20, 20))
            self.screen.blit(
                instr_s,
                (self.size[0] // 2 - instr_s.get_width() // 2, instr_s.get_height()),
            )

        if self.game_over:
            msg = self.winner if self.winner else "Tie"
            go_surf = font.render(f"{msg} - press R to restart", True, (20, 20, 20))
            self.screen.blit(
                go_surf,
                (self.size[0] // 2 - go_surf.get_width() // 2, go_surf.get_height()),
            )


game_play = game("MPS", (1530, 760))
game_play.run_game()

mps - multiplayer snake game

\$\endgroup\$

4 Answers 4

6
\$\begingroup\$

A couple of points:

  • Class names should be capitalized. game should be Game.
  • You should wrap your code to execute in a guard so that your file can be treated as a module and imported without the game actually running.
if __name__ == '__main__':
    game_play = Game("MPS", (1530, 760))
    game_play.run_game()

Magic numbers

Your code is littered with magic numbers. Creating named constants for these makes your code more self-documenting, but also provides you with a convenient place to change them once without having to track them down in your code.

Documentation

  • Use docstrings to explain the purpose of modules, classes, and functions as well as side-effects they may have.
  • Use type hints where applicable to make expected parameter types and function return values explicit.

Control flow

  • The continue statements are really just taking the place of elif and else.
    def spawn_food_one(self):
        tries = 0
        while True:
            fx = random.randrange(0, self.cols)
            fy = random.randrange(0, self.rows)
            pos = (fx, fy)
            if pos in self.foods:
                tries += 1
                if tries > 200:
                    return
                continue
            if pos in self.snake or pos in getattr(self, "snake2", []):
                tries += 1
                if tries > 200:
                    return
                continue
            self.foods.append(pos)
            return

Revised:

    def spawn_food_one(self):
        tries = 0
        while True:
            fx = random.randrange(0, self.cols)
            fy = random.randrange(0, self.rows)
            pos = (fx, fy)
            if pos in self.foods:
                tries += 1
                if tries > 200:
                    return
            elif pos in self.snake or pos in getattr(self, "snake2", []):
                tries += 1
                if tries > 200:
                    return
            else:    
                self.foods.append(pos)
                return

Both our if and elif branches take the exact same action.

    def spawn_food_one(self):
        tries = 0
        while True:
            fx = random.randrange(0, self.cols)
            fy = random.randrange(0, self.rows)
            pos = (fx, fy)
            if pos in self.foods or (pos in self.snake or pos in getattr(self, "snake2", [])):
                tries += 1
                if tries > 200:
                    return
            else:    
                self.foods.append(pos)
                return

Further evaluating this, we either increment tries by one, or we return from the function, making the value of tries irrelevant. Once tries is greater than 200, we also exit the function.

    def spawn_food_one(self):
        for tries in range(201):
            fx = random.randrange(0, self.cols)
            fy = random.randrange(0, self.rows)
            pos = (fx, fy)
            if pos in self.foods or (pos in self.snake or pos in getattr(self, "snake2", [])):
                pass
            else:    
                self.foods.append(pos)
                return

Now we could just invert that condition and in that situation append the new food and return.

Complexity

But that has a pretty indeterminate complexity given the randomness of picking a set of coordinates. I would instead suggest maintaining a set of coordinates that are occupied and not and randomly sampling that set, then updating it.

\$\endgroup\$
3
  • \$\begingroup\$ "Use type hints" - or don't. Almost all methods of the class are void (-> None) and accept zero non-self parameters, which means that you'll get very little help from mypy, and that could be not worth the hassle. Type hints become really helpful as your code grows, but are much less so for a single-file quick project with just tens of functions and methods. \$\endgroup\$ Commented 6 hours ago
  • 1
    \$\begingroup\$ @STerliakov While I would agree with you as a general rule, anyone posting their code here is presumably trying to learn about what best practices are and how they can apply to their code, and type hinting definitely falls into that category. \$\endgroup\$ Commented 3 hours ago
  • \$\begingroup\$ @Abion47 no, my point is exactly that type hinting is not always a "best practice", and I would not say that the same code as posted, but with correct type hints added, is better overall. Not all large-scale codebase rules apply to small programs, and the code in question is a small program and not an extract from something bigger. \$\endgroup\$ Commented 5 mins ago
3
\$\begingroup\$

Inflexible controls

You should make these configurable:

            elif e.key == pygame.K_w:
                self.change_dir((0, -1), 1)
            elif e.key == pygame.K_s:
                self.change_dir((0, 1), 1)
            elif e.key == pygame.K_a:
                self.change_dir((-1, 0), 1)
            elif e.key == pygame.K_d:
                self.change_dir((1, 0), 1)

On my keyboard, those are all over the place, whereas player 2's controls are in a tight block and can be reached with one hand. Make it easier for players by making the controls configurable, rather than assuming that everyone has your (QWERTY? AZERTY?) layout.

\$\endgroup\$
2
\$\begingroup\$

More Efficient and Flexible Event Handling

Currently you have:

    def handle_events(self):
        for e in pygame.event.get():
            if e.type == pygame.QUIT:
                self.done = 1
            if e.type == pygame.KEYDOWN:
                if e.key == pygame.K_r and self.game_over:
                    self.reset()

                if not self.started or self.game_over:
                    continue
                if e.key == pygame.K_UP:
                    self.change_dir((0, -1), 2)
                elif e.key == pygame.K_DOWN:
                    self.change_dir((0, 1), 2)
                ... # etc.

First it should be noted that if e.type equals pygame.QUIT it cannot equal pygame.KEYDOWN. Thus, the second if statement in the method should be elif.

Then what follows is a series if if/elif tests on the key that was entered. One improvement would be to replace these with a match statement. But there is a more efficient approach that also allows you to easily have multiple key-to-action mappings that could be offered to the user if they wish to use a different key to move left for example. That is, the arguments to method change_dir can be stored in a dictionary movements whose keys are the keyboard keys to be used for movement:

    ...

    movements = {
        pygame.K_UP: ((0, -1), 2),
        pygame.K_DOWN: ((0, 1), 2),
        pygame.K_LEFT: ((-1, 0), 2),
        # etc.
    }


    def handle_events(self):
        for e in pygame.event.get():
            if e.type == pygame.QUIT:
                self.done = 1
            elif e.type == pygame.KEYDOWN:
                if e.key == pygame.K_r and self.game_over:
                    self.reset()
                if not self.started or self.game_over:
                    continue
                movement = movements.get(e.key)
                if movement:
                    self.change_dir(*movement)
\$\endgroup\$
1
\$\begingroup\$

UX

When I run the code, I get a nice looking GUI. After staring for a while, I see some tiny instructions at the top center of the window. After I follow those instructions, I don't know what to do. It would be nice to make the instructions more prominent, and it would also be helpful to explicitly instruct the user to use the arrow keys, etc.

Documentation

The PEP 8 style guide recommends adding docstrings for classes and functions.

Consider using type hints to describe input and return types of the functions to make the code even more self-documenting.

I realize another answer mentions these points, but I figured I'd post some links for you.

Magic numbers

I see the constant 200 used in several places. It would be better to give it a name, or names if 200 has different meaning in different parts of the code.

Simpler

This type of if/else:

if self.next_direction1 is not None:

else:

could be simplified by removing not and swapping the statements under each clause:

if self.next_direction1 is None:

else:

Similarly for:

color = (255, 0, 0) if i != 0 else (200, 0, 0)

as:

color = (200, 0, 0) if i == 0 else (255, 0, 0)
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.