8
\$\begingroup\$

I am a total beginner at Python, so I wrote a very simple snake game using pygame to get some practice with it. I also don't have too much experience with object-oriented design. Does the code layout look okay? Is it easy to read and understand? I'm sure it can be simplified some. The update_segments section in particular seems too long and overly complicated. I know there's a way to shorten it, maybe by going through it in reverse order, but it's just not coming to me at the moment.

import pygame
import random
BOARD_SIZE_WIDTH = 25
BOARD_SIZE_HEIGHT = 25
# This will determine the size of the objects drawn on the board.
# Must be equal to resolution
BOARD_SPACE_SIZE = 20
DISPLAY_RESOLUTION_WIDTH = BOARD_SIZE_WIDTH * BOARD_SPACE_SIZE
DISPLAY_RESOLUTION_HEIGHT = BOARD_SIZE_HEIGHT * BOARD_SPACE_SIZE
PLAYER1_START_X = 12
PLAYER1_START_Y = 12
COBRA_START_DIRECTION = "Up"
# Number of milliseconds between player movements.
MOVEMENT_DELAY = 100
SEGMENT_SIZE = 20
# Define the objects that can occupy board spaces.
EMPTY = 0
COBRA = 1
FOOD = 2
EMPTY_RECT = pygame.Rect(0, 0, BOARD_SPACE_SIZE, BOARD_SPACE_SIZE)
COBRA_RECT = pygame.Rect(0, 0, BOARD_SPACE_SIZE, BOARD_SPACE_SIZE)
FOOD_RECT = pygame.Rect(0, 0, BOARD_SPACE_SIZE, BOARD_SPACE_SIZE)
RED = (0xFF, 0x00, 0x00)
YELLOW = (0xFF, 0xFF, 0x00)
BLACK = (0x00, 0x00, 0x00)
class board(object):
 foodOnBoard = False
 def __init__(self, width, height, spaceSize):
 self.width = width
 self.height = height
 self.spaceSize = spaceSize
 self.spaces = [[0 for y in range(0, BOARD_SIZE_HEIGHT)] for x in range(0, BOARD_SIZE_WIDTH)]
 def create_food(self):
 while self.foodOnBoard == False:
 x = random.randint(0, BOARD_SIZE_WIDTH - 1)
 y = random.randint(0, BOARD_SIZE_HEIGHT - 1)
 if self.spaces[x][y] == EMPTY:
 self.spaces[x][y] = FOOD
 self.foodOnBoard = True
 def draw_board(self, screen):
 for y in range(0, BOARD_SIZE_HEIGHT):
 for x in range(0, BOARD_SIZE_WIDTH):
 drawX = x * BOARD_SPACE_SIZE
 drawY = y * BOARD_SPACE_SIZE
 if self.spaces[x][y] == COBRA:
 pygame.draw.rect(screen, RED, pygame.Rect(drawX, drawY, BOARD_SPACE_SIZE, BOARD_SPACE_SIZE))
 elif self.spaces[x][y] == FOOD:
 pygame.draw.rect(screen, YELLOW, pygame.Rect(drawX, drawY, BOARD_SPACE_SIZE, BOARD_SPACE_SIZE))
 elif self.spaces[x][y] == EMPTY:
 pygame.draw.rect(screen, BLACK, pygame.Rect(drawX, drawY, BOARD_SPACE_SIZE, BOARD_SPACE_SIZE))
class cobra(object):
 numberOfSegments = 1
 sizeOfSegments = SEGMENT_SIZE
 segments = []
 def __init__(self, posX, posY, direction):
 self.posX = posX
 self.posY = posY
 self.direction = direction
 class segment(object):
 def __init__(self, posX, posY):
 self.posX = posX
 self.posY = posY
 def add_segment(self, segment):
 self.segments.append(segment)
 def move(self):
 if self.direction == "Up":
 self.posY -= 1
 if self.direction == "Down":
 self.posY += 1
 if self.direction == "Left":
 self.posX -= 1
 if self.direction == "Right":
 self.posX += 1 
def check_collision(board, cobra):
 if (cobra.posX < 0) or (cobra.posY < 0) or (cobra.posX > BOARD_SIZE_WIDTH - 1)\
 or (cobra.posY > BOARD_SIZE_HEIGHT - 1) or (board.spaces[cobra.posX][cobra.posY] == COBRA):
 collision = True
 else:
 collision = False
 return collision
def update_segments(board, cobra):
 posX = cobra.posX
 posY = cobra.posY
 if board.spaces[posX][posY] == FOOD:
 cobra.add_segment(cobra.segment(posX, posY))
 board.foodOnBoard = False
 for seg in cobra.segments:
 prevPosX = seg.posX
 prevPosY = seg.posY
 board.spaces[prevPosX][prevPosY] = EMPTY
 seg.posX = posX
 seg.posY = posY
 board.spaces[seg.posX][seg.posY] = COBRA
 posX = prevPosX
 posY = prevPosY
def main():
 # Set board size, cobra starting position, create first cobra segment, and initialize display
 board1 = board(BOARD_SIZE_WIDTH, BOARD_SIZE_HEIGHT, BOARD_SPACE_SIZE)
 cobra1 = cobra(PLAYER1_START_X, PLAYER1_START_Y, COBRA_START_DIRECTION)
 cobra1.add_segment(cobra.segment(cobra1.posX, cobra1.posY))
 pygame.init()
 screen = pygame.display.set_mode((DISPLAY_RESOLUTION_WIDTH, DISPLAY_RESOLUTION_HEIGHT))
 gameRunning = True
 paused = False
 while gameRunning == True:
 update_segments(board1, cobra1)
 board1.draw_board(screen)
 pygame.display.update()
 events = pygame.event.get()
 for event in events:
 if event.type == pygame.QUIT:
 pygame.quit()
 if event.type == pygame.KEYDOWN:
 if event.key == pygame.K_UP:
 cobra1.direction = "Up"
 if event.key == pygame.K_DOWN:
 cobra1.direction = "Down"
 if event.key == pygame.K_LEFT:
 cobra1.direction = "Left"
 if event.key == pygame.K_RIGHT:
 cobra1.direction = "Right"
 if event.key == pygame.K_SPACE:
 if not paused:
 paused = True
 else:
 paused = False
 if not paused:
 cobra1.move()
 collision = check_collision(board1, cobra1)
 if not board1.foodOnBoard:
 board1.create_food()
 if collision == True:
 gameRunning = False
 pygame.time.wait(MOVEMENT_DELAY)
main()
asked Oct 15, 2018 at 21:58
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

You've followed some conventions very well like CAPS for constants. Very nice. Here are some points:

Standard library imports precede 3rd party ones

import pygame
import random

should be changed to

import random
import pygame

The middle line is to differentiate between the two types of imports

Two lines after imports

There should be two lines after imports

import random
BOARD_SIZE_WIDTH = 25

to

import random
BOARD_SIZE_WIDTH = 25

Better equate False to not True

This

while self.foodOnBoard == False:

can be changed to

while not self.foodOnBoard:

While True

Better add if cond directly instead of if cond == True

From this

while gameRunning == True:

to this:

while gameRunning:

No object inheritance for classes in Python3

Though your actual code runs on Python 3.8, you can leave out object inheritance for classes. From this:

class cobra(object):

to this

class cobra:

Caps for classes

Class names should be PascalCase. cobra becomes Cobra. gold_mine becomes GoldMine for class name.

Don't nest classes

Nesting classes means putting one class inside the other

class cobra(object):
 numberOfSegments = 1
 sizeOfSegments = SEGMENT_SIZE
 segments = []
 def __init__(self, posX, posY, direction):
 self.posX = posX
 self.posY = posY
 self.direction = direction
 class segment(object):
 def __init__(self, posX, posY):
 self.posX = posX
 self.posY = posY
...

Putting it outside increases readability.

class segment(object):
 def __init__(self, posX, posY):
 self.posX = posX
 self.posY = posY
class cobra(object):
 numberOfSegments = 1

You then modify

cobra.add_segment(cobra.segment(posX, posY))

to

cobra.add_segment(segment(posX, posY))

in main and update_segments

Snake case for variables

numberOfSegments should be number_of_segments

Additional brackets remove the need for backslash

Adding () to conditionals eliminate the need for \:

From this:

 if (cobra.posX < 0) or (cobra.posY < 0) or (cobra.posX > BOARD_SIZE_WIDTH - 1)\
 or (cobra.posY > BOARD_SIZE_HEIGHT - 1) or (board.spaces[cobra.posX][cobra.posY] == COBRA):

to that:

 if ((cobra.posX < 0) or 
 (cobra.posY < 0) or 
 (cobra.posX > BOARD_SIZE_WIDTH - 1) or
 (cobra.posY > BOARD_SIZE_HEIGHT - 1) or 
 (board.spaces[cobra.posX][cobra.posY] == COBRA)):

Return True directly

In

def check_collision(board, cobra):
 ...
 collision = True
 else:
 collision = False
 return collision

returning True or False directly might be more readable

def check_collision(board, cobra):
 ...
 return True
 else:
 return False

or as @Linny suggested

def check_collision(board, cobra):
 return (
 (cobra.posX < 0) or 
 (cobra.posY < 0) or 
 (cobra.posX > BOARD_SIZE_WIDTH - 1) or
 (cobra.posY > BOARD_SIZE_HEIGHT - 1) or 
 (board.spaces[cobra.posX][cobra.posY] == COBRA))

Rearchitecture

I suggest an events() function to have all events in it then call it in main so that your main remains clutter-free

I also suggest a Food class and a display() method added to both Food and Cobra. Imagine if the characters were more then just rectangles, then you'd do:

 if self.spaces[x][y] == COBRA:
 Cobra.display(x, y)
 elif self.spaces[x][y] == FOOD:
 Food.display(x, y)

update_segments as well as check_collision can be a methods tied to board.

Miscellaneous

  • Two lines needed after classes
  • You might want to hide the welcome message and lib version. See here.
  • These three constants are better suited as functions:
EMPTY_RECT = pygame.Rect(0, 0, BOARD_SPACE_SIZE, BOARD_SPACE_SIZE)
COBRA_RECT = pygame.Rect(0, 0, BOARD_SPACE_SIZE, BOARD_SPACE_SIZE)
FOOD_RECT = pygame.Rect(0, 0, BOARD_SPACE_SIZE, BOARD_SPACE_SIZE)
Ben A
10.7k5 gold badges37 silver badges101 bronze badges
answered Jan 22, 2020 at 20:01
\$\endgroup\$
2
  • \$\begingroup\$ For check_collision, I'd have returned the boolean expression, instead of True/False depending on the result. So return (cobra.posX < 0) or .... \$\endgroup\$ Commented Jan 22, 2020 at 22:08
  • 1
    \$\begingroup\$ @Liny yes. Included suggestion. \$\endgroup\$ Commented Jan 23, 2020 at 7:11

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.