4
\$\begingroup\$

I'm working for a month or so on a Snake game in Python. It's working well (I got some good help on it already), but since I'm a beginner in pygame, I would like some feedback from better programmers than me. Anything that improves performance, readability and/organization is welcomed.

Keep in mind that the sounds are in a separate folder so this code will not work on your computer unless you have the files. I just want a code review.

GitHub

import pygame
import sys
import time
import random
import collections
import itertools
import os
def main():
    """Snake v 1.59"""
    score = 0  # Initial score
    speed = pygame.time.Clock()
    direction = "R"  # Initial direction
    snake_position = collections.deque([100, 50])  # Initial snake position
    snake_body = collections.deque([[100, 50], [90, 50], [100, 50]])  # Initial snake body
    # It places the food randomly, excluding the border
    food_position = [random.randrange(1, 72) * 10, random.randrange(1, 46) * 10]
    food_spawn = True
    # Will define the colors
    white = pygame.Color("white")
    red = pygame.Color("red")
    green = pygame.Color("green")
    black = pygame.Color("black")
    orange = pygame.Color("orange")
    grey = pygame.Color("light grey")
    # Game surface
    player_screen = pygame.display.set_mode((720, 460))  # Set screen size
    pygame.display.set_caption("Snake v.1.38")  # Set screen title and version

    def initializer():
        """ Checks the mistakes, and closes the program if it does while
        printing on the console how many bugs it has, also initializes
        the mixers, and game """
        pygame.mixer.pre_init(44100, -16, 1, 512)
        pygame.mixer.init()
        bugs = pygame.init()
        if bugs[1] > 0:
            print("There are", bugs[1], "bugs! quiting.....")
            time.sleep(3)
            sys.exit("Closing program")
        else:
            print("The game was initialized")

    def game_sound(s):
        """ Include the game sfx and music"""
        if s == 0:
            directory = os.path.dirname(os.path.realpath(sys.argv[0]))
            full_path = os.path.join(directory, "background.ogg")
            pygame.mixer.music.load(full_path)
            pygame.mixer.music.play(-1)
        elif s == 1:
            directory = os.path.dirname(os.path.realpath(sys.argv[0]))
            full_path = os.path.join(directory, "eating.wav")
            pygame.mixer.Sound(full_path).play()
        elif s == 2:
            directory = os.path.dirname(os.path.realpath(sys.argv[0]))
            full_path = os.path.join(directory, "game-over.wav")
            pygame.mixer.Sound(full_path).play()

    def you_lose():
        """ When the players loses, it will show a red message in times new
         roman font with 44 px size in a rectangle box"""
        font_game_over = pygame.font.SysFont("times new roman", 44)
        game_over_surface = font_game_over.render("Game over :(", True, red)
        game_over_position = game_over_surface.get_rect()
        game_over_position.midtop = (360, 15)
        player_screen.blit(game_over_surface, game_over_position)
        game_sound(2)
        scoring()
        pygame.display.flip()  # Updates the screen, so it doesnt freeze
        quiting()

    def pause_menu():
        """It displays the pause menu"""
        player_screen.fill(white)
        pause_font = pygame.font.SysFont("times new roman", 44)
        pause_surface = pause_font.render("Paused", True, black)
        pause_position = pause_surface.get_rect()
        pause_position.midtop = (360, 15)
        player_screen.blit(pause_surface, pause_position)
        pygame.display.flip()

    def quiting():
        """ When this function is called, it will wait 4 seconds and exit"""
        time.sleep(4)
        pygame.quit()
        sys.exit()

    def scoring():
        """ It will shows the score after the game over in times new
        roman font with 16px size and black color in a rectangle box"""
        score_font = pygame.font.SysFont("times new roman", 16)
        score_surface = score_font.render("Score : {}".format(score), True, black)
        score_position = score_surface.get_rect()
        score_position.midtop = (360, 80)
        player_screen.blit(score_surface, score_position)

    initializer()
    game_sound(0)
    paused = False
    while True:
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                quiting()
            elif event.type == pygame.KEYDOWN:
                if event.key == pygame.K_p:  # Pausing/ unpausing
                    paused = not paused
                    if paused:
                        pygame.mixer.music.pause()
                        pause_menu()
                    else:
                        pygame.mixer.music.unpause()
                # Choose direction by user input, block opposite directions
                key_right = event.key in (pygame.K_RIGHT, pygame.K_d)
                key_left = event.key in (pygame.K_LEFT, pygame.K_a)
                key_down = event.key in (pygame.K_DOWN, pygame.K_s)
                key_up = event.key in (pygame.K_UP, pygame.K_w)
                if key_right and direction != "L":
                    direction = "R"
                elif key_left and direction != "R":
                    direction = "L"
                elif key_down and direction != "U":
                    direction = "D"
                elif key_up and direction != "D":
                    direction = "U"
                elif event.key == pygame.K_ESCAPE:
                    quiting()  # It will quit when esc is pressed

        # Simulates the snake movement(together with snake_body_pop)
        if not paused:
            if direction == "R":
                snake_position[0] += 10
            elif direction == "L":
                snake_position[0] -= 10
            elif direction == "D":
                snake_position[1] += 10
            elif direction == "U":
                snake_position[1] -= 10
            # Body mechanics
            snake_body.appendleft(list(snake_position))
            if snake_position == collections.deque(food_position):
                score += 1  # Every food taken will raise the score by 1
                game_sound(1)
                food_spawn = False  # It removes the food from the board
            else:
                # If the food is taken it will not remove the last body piece(raising snakes size)
                snake_body.pop()
            if food_spawn is False:  # When a food is taken it will respawn randomly
                food_position = [random.randrange(1, 72) * 10, random.randrange(1, 46) * 10]
            food_spawn = True  # It will set the food to True again, to keep the cycle
            # Drawing
            player_screen.fill(grey)  # Set the background to grey
            for position in snake_body:  # Snake representation on the screen
                pygame.draw.rect(player_screen, green, pygame.Rect(position[0], position[1], 10, 10))
            # Food representation on the screen
            pygame.draw.rect(player_screen, orange, pygame.Rect(food_position[0], food_position[1], 10, 10))
            if snake_position[0] not in range(0, 711) or snake_position[1] not in range(0, 451):
                you_lose()  # Game over when the Snake hit a wall
            for block in itertools.islice(snake_body, 1, None):
                if snake_position == collections.deque(block):
                    you_lose()  # Game over when the Snake hits itself
            pygame.display.flip()  # It constantly updates the screen
            speed.tick(26)  # It sets the speed to a playable value


if __name__ == "__main__":
    main()
\$\endgroup\$
  • \$\begingroup\$ Please explain your github comment... now I read.. I've copy/paste someones others code here and need a revamp...(confused)... where is your code? \$\endgroup\$ – ZF007 Dec 21 '17 at 13:02
  • \$\begingroup\$ @ZF007 mine code is there, its not showing? I copy pasted it. The github comment is that I dont know if its allowed to just link it from here, so I just copy pasted the code without linking it. \$\endgroup\$ – Mateus Dec 21 '17 at 13:38
  • \$\begingroup\$ I see code but I don't understand your remark : obs- The code is big(ish), so I think it would be better to post the GitHub page,. To me this looks like copy/past other persons work based on your comment. If so that should be stated... if its all yours and you posted it on github...then modify that particular phrase... now its confusing and for multiple ways interpretative. \$\endgroup\$ – ZF007 Dec 21 '17 at 13:41
  • \$\begingroup\$ It's all mine. I don't know how this could be confusing, why would I want a review for other person code? My doubt was if I could or not post the link to mine github page here, since I didn't know I just copied and pasted mine code here \$\endgroup\$ – Mateus Dec 21 '17 at 13:46
  • \$\begingroup\$ Now it makes sense :-) You have an option to attach files as well. That's preferred here due to the fact outside link may expire over time. Posting your code here is also just fine. \$\endgroup\$ – ZF007 Dec 21 '17 at 13:52
2
\$\begingroup\$

Thank you for sharing. Much of this looks nice, and the code is understandable. Your identifiers are well chosen.

Please run flake8. It will ask you to add a few blank lines, that sort of thing. Also, PEP-008 asks that one-line docstrings end with a '.' period.

Keeping variables out of global scope is good. But "main() is the new global" is an odd style. Consider moving at least some def's to top level, or creating a SnakeGame class.

You have a lot of magic numbers. Some are mostly OK, like pre_init(44100, -16, 1, 512). But things like grid resolution of 10 pixels, and grid size, really need to be constants, like GRID_WIDTH = 72 or GRID_SIZE = (72, 46). Refactor this. Refactor it now.

There is maybe an opportunity to introduce a Display class that knows the "secret" of grid resolution being 10px. So much of the game would manipulate food + snake in game space coordinates less than GRID_SIZE, and would ask Display to plot points in the much larger screen space. With that in hand, imagine adding one blue pill (food) and one red pill to the board. Eating blue causes Display to flip mirror image around X axis, and red, around Y axis. The rest of your game wouldn't know or care about the visual effect.

This is perfectly nice: bugs = pygame.init(). But consider re-phrasing it as pass, fail = pygame.init(), as bugs[1] is slightly obscure (I had to consult the docs).

DRY: don't repeat yourself. Consider writing a helper function for game_sound(), to factor out the common expressions.

Referring to sys.argv[0] in game_sound is an anti-pattern. Parse it out early and store it in a named variable.

Your comments are weirdly specific, e.g. on you_lose(). Please don't say in comments what the code already says clearly. At some point you'll make it 42px Arial font, and then either you'll remember to update the comment, or you'll have a comment that lies. Put generalities into English language comments, and put specifics into the code.

The same literal text is repeated several times. You should add a manifest constant like FONT = 'times new roman'.

Things like pause_position.midtop and score_position.midtop could use an expression like GRID_WIDTH * 0.5.

It appears food_spawn is misnamed, and should be food_spawned. Spawning more food when it is False is surprising. Perhaps a name like out_of_food would be clearer.

You defined food_position to be a list, [x, y]. Please make it a tuple, (x, y). Lists are variable length, unlike tuples. (At some point you may support a list of multiple food locations, and the spawn check would test if its length is zero.)

algorithm refinement

modeling direction

I recommend you parse key_{up,right,down,left} into dx and dy values drawn from {-1, 0, 1}. And model current direction in the same way. This would reduce "going backwards?" to two tests. And if you store direction as an index 0..3 into this:

    delta = [(0, -1), (1, 0), (0, 1), (-1, 0)]

then "going backwards?" is just a test of (direction + 2) % 4. (That's plus len(delta)/2, mod len(delta).)

Advancing the snake is merely adding (dx, dy) to snake_position.

detecting collision

This seems more expensive than necessary:

        for block in itertools.islice(snake_body, 1, None):
            if snake_position == collections.deque(block):
                you_lose()  # Game over when the Snake hits itself

Consider maintaining a set alongside snake_body. (This is an opportunity to define a Snake class.) When pixel is added to head of snake_body, add it to the set as well (constant time, independent of snake length.) When pixel is removed from tail, remove it from set. Now instead of linear scan you can do a single set membership query.

Of course, there is already a data structure that can be queried in constant (less than linear) time. The screen. Ask the display if we're moving onto a snake-colored pixel when detecting collision.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot for this detailed review just a few things, thanks for the flake8 recommendation its amazing im already using it now. I'm not sure how to make constants in python, Ill research, the sys.argv[0] was the only way I found to make the path to the songs relative( dont really know what it does) \$\endgroup\$ – Mateus Dec 21 '17 at 16:18
  • \$\begingroup\$ also I don't fully understand what you want me to do with the directions. On the other things, I already changed thanks a lot again \$\endgroup\$ – Mateus Dec 21 '17 at 16:29
  • 1
    \$\begingroup\$ I was suggesting creating a variable, maybe named dir, and store the directory in that. Or use __file__, as in stackoverflow.com/questions/28569328/… . For the direction, you're currently modeling it with four distinct unrelated values, with no relationship between e.g. 'L' & 'R'. By using delta x of +/- 1 the relationship would become clear, and simple arithmetic operations could help you out. \$\endgroup\$ – J_H Dec 21 '17 at 19:10

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.