I've been teaching myself python and I decided to make a Simon copy. Please let me know if there's anything notable that I should improve on. I understand my use of globals in some functions is considered poor, but I couldn't think of any other way to write them. Thank you for reading

import pygame
from random import randint
from time import sleep

pygame.init()
(width, height) = (450, 450)
screen = pygame.display.set_mode((width, height))
pygame.display.set_caption('Simon')
pygame.display.flip()
red = [255, 0, 0]
green = [0, 255, 0]
blue = [0, 0, 255]
yellow = [255, 255, 0]


class Button:
 def __init__(self, color, pos):
 self.color = color
 self.pos = pos
 self.shape = pygame.Rect
 self.draw()

 def draw(self):
 self.shape = pygame.draw.rect(screen, self.color, self.pos)

 def darken(self):
 index = 0
 for i in self.color:
 if i == 255:
 i -= 95
 self.color[index] = i
 index += 1
 self.draw()
 pygame.display.update()

 def lighten(self):
 index = 0
 for i in self.color:
 if i == 160:
 i += 95
 self.color[index] = i
 index += 1
 self.draw()
 pygame.display.update()

 def dark_then_light(self):
 self.darken()
 beep()
 sleep(.5)
 self.lighten()


red_square = Button(red, (25, 25, 200, 200))
green_square = Button(green, (225, 25, 200, 200))
blue_square = Button(blue, (25, 225, 200, 200))
yellow_square = Button(yellow, (225, 225, 200, 200))
pygame.display.update()

beep_sound = pygame.mixer.Sound('beep.wav')
intro_sound = pygame.mixer.Sound('intro.wav')


def intro():
 intro_sound.play()


def beep():
 beep_sound.play()


def click():
 global player_list
 pos = pygame.mouse.get_pos()
 if (25 < pos[0] < 225) and (25 < pos[1] < 225):
 press(0)
 player_list.append(0)
 elif (225 < pos[0] < 425) and (25 < pos[1] < 225):
 press(1)
 player_list.append(1)
 elif (25 < pos[0] < 225) and (225 < pos[1] < 425):
 press(2)
 player_list.append(2)
 elif (225 < pos[0] < 425) and (225 < pos[1] < 425):
 press(3)
 player_list.append(3)


def press(index):
 if index == 0:
 red_square.dark_then_light()
 elif index == 1:
 green_square.dark_then_light()
 elif index == 2:
 blue_square.dark_then_light()
 elif index == 3:
 yellow_square.dark_then_light()


font_name = pygame.font.match_font('arial')


def draw_text(surf, text, size, x, y):
 font = pygame.font.Font(font_name, size)
 text_surface = font.render(text, True, (255, 255, 255))
 text_rect = text_surface.get_rect()
 text_rect.midtop = (x, y)
 surf.blit(text_surface, text_rect)


def erase_text():
 pygame.draw.rect(screen, (0, 0, 0), (0, 0, 450, 25))


correct_list = 
player_list = 


def ai_turn():
 global AI_turn, player_turn, correct_list
 index = randint(0, 3)
 correct_list.append(index)
 for i in correct_list:
 press(i)
 sleep(.15)
 AI_turn = False
 player_turn = True


def check():
 global correct_list, player_list, DEFEAT
 if len(correct_list) == len(player_list):
 if correct_list == player_list:
 return True
 else:
 DEFEAT = True
 return False


END_SCREEN = True
running = True
AI_turn = True
player_turn = False
DEFEAT = False
score = 0

intro()
sleep(6)

while running:
 if AI_turn:
 erase_text()
 draw_text(screen, 'SIMON'S TURN', 20, 225, 2, )
 pygame.display.update()
 sleep(.5)
 ai_turn()
 if player_turn:
 erase_text()
 draw_text(screen, 'PLAYER'S TURN', 20, 225, 2, )
 pygame.display.update()
 for event in pygame.event.get():
 if not DEFEAT:
 if event.type == pygame.QUIT:
 running = False
 END_SCREEN = False
 if event.type == pygame.MOUSEBUTTONDOWN:
 click()
 if check():
 AI_turn = True
 player_turn = False
 player_list = 
 score += 1
 break
 else:
 break
 if DEFEAT:
 break

screen.fill((0, 0, 0))
draw_text(screen, 'YOU LOSE!!!', 50, 225, 200)
draw_text(screen, 'SCORE:', 50, 225, 255)
draw_text(screen, str(score), 50, 225, 305)
pygame.display.update()

while END_SCREEN:
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 END_SCREEN = False

Good job so far.

Major issues

• The most important (game breaking) problem is that the game will become unresponsive if you don't make some sort of call to the event queue regularly, and when that happens (for example when the correct_list becomes too large and sleep gets called too often) you can't see which buttons the AI is clicking anymore.

A quick fix would be to call pygame.event.pump() in the ai_turn function:

def ai_turn():
 global AI_turn, player_turn, correct_list
 index = randint(0, 3)
 correct_list.append(index)
 for i in correct_list:
 pygame.event.pump()
 press(i)
 sleep(.15)
 AI_turn = False
 player_turn = True

• I think it would be better to restructure the main while loop. In most games the event handling, game logic and the rendering are done separately, but here it's all intertwined.




• It would be a good idea to reduce the amount of global variables. Pass the variables to the functions that need them and then return something and bind the returned value to a variable.

Medium issues

• Instead of changing the colors in the Button's darken and lighten methods, you could just pass the light and dark color and make them attributes of the class. Then pass the desired color to the draw method.




• I'd also assign the rect to a self.rect attribute and remove the pos and shape attributes. You don't have to reassign the rect returned by pygame.draw.rect, since the rect shape won't change anyway.




• pygame.Rects have handy collision detection methods (collidepoint would be helpful here).




• The buttons can be put into a list. That makes it possible to iterate over the buttons with a for loop and you can shorten the click and press functions quite a bit.




• Actually, I'd replace click and press with a function that checks which button was clicked and then check if it was the correct button. In order to make that work, you have to append the button objects (references) to the correct_list instead of the indices.

Minor issues

• PEP 8 (capitalized words are for constants).




• Define a pygame.time.Clock instance and call clock.tick(frame_rate) every frame to limit the frame rate.




• time.sleep should usually not be called in games, since it stops the execution of the rest of the program and makes the game unresponsive. However, for this particular game, I think it's okay and it probably doesn't need to be replaced.




• One more tip: If you set os.environ['SDL_VIDEO_CENTERED'] = '1', the pygame window will be centered and I think that also moves it to the front (if other windows are open in the foreground).

Gameplay

• I think the game should stop when a wrong button is clicked, so that you don't have to keep clicking (unless the original worked in this way). I'm using the index variable in my example for this purpose and have removed the player_list.




• It would be nice if the game could be restarted when it's over instead of having to execute the program again. The while loops and the corresponding variables could be put into different functions to make it easier to restart the game.

Here's the complete example:

import os
from random import randint
from time import sleep

import pygame


class Button:
 def __init__(self, color, dark_color, pos):
 self.color = color
 self.dark_color = dark_color
 self.rect = pygame.Rect(pos)
 self.draw(screen, self.color)

 def draw(self, screen, color):
 pygame.draw.rect(screen, color, self.rect)
 pygame.display.update()

 def dark_then_light(self, screen):
 self.draw(screen, self.dark_color)
 sleep(.5)
 self.draw(screen, self.color)


def clicked_button(mouse_pos, buttons):
 """Return the clicked button or None if no button was clicked."""
 for button in buttons:
 if button.rect.collidepoint(mouse_pos):
 return button
 return None # Return None if no button was clicked.


# Takes a font object now instead of creating a new one.
def draw_text(surf, text, font, x, y):
 text_surface = font.render(text, True, (255, 255, 255))
 text_rect = text_surface.get_rect()
 text_rect.midtop = (x, y)
 surf.blit(text_surface, text_rect)


def erase_text():
 pygame.draw.rect(screen, (0, 0, 0), (0, 0, 450, 25))


os.environ['SDL_VIDEO_CENTERED'] = '1'
pygame.init()
screen = pygame.display.set_mode((450, 450))
clock = pygame.time.Clock() # A clock to limit the frame rate.
pygame.display.set_caption('Simon')
# Define the font objects once (for efficiency reasons).
FONT = pygame.font.Font(pygame.font.match_font('arial'), 50)
FONT_SMALL = pygame.font.Font(pygame.font.match_font('arial'), 20)

end_screen = True
running = True
ai_turn = True
score = 0
correct_list = 
index = 0
buttons = [
 Button(pygame.Color('red'), pygame.Color(160, 0, 0), (25, 25, 200, 200)),
 Button(pygame.Color('green'), pygame.Color(0, 160, 0), (225, 25, 200, 200)),
 Button(pygame.Color('blue'), pygame.Color(0, 0, 160), (25, 225, 200, 200)),
 Button(pygame.Color('yellow'), pygame.Color(160, 160, 0), (225, 225, 200, 200)),
 ]

while running:
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 running = False
 end_screen = False
 elif event.type == pygame.MOUSEBUTTONDOWN and not ai_turn:
 correct_button = correct_list[index]
 button = clicked_button(event.pos, buttons)
 if button is None: # No button pressed.
 break
 elif button == correct_button: # Correct button pressed.
 button.dark_then_light(screen)
 index += 1
 if index == len(correct_list): # All buttons clicked.
 ai_turn = True
 score += 1
 index = 0
 else: # Wrong button pressed.
 running = False

 if ai_turn:
 erase_text()
 draw_text(screen, 'SIMON'S TURN', FONT_SMALL, 225, 2)
 pygame.display.update()
 ai_turn = False
 sleep(.5)
 # Append references to the button objects instead of indices.
 correct_list.append(buttons[randint(0, 3)])
 for button in correct_list:
 pygame.event.pump() # Prevent the game from freezing.
 button.dark_then_light(screen)
 sleep(.15)

 erase_text()
 draw_text(screen, 'PLAYER'S TURN', FONT_SMALL, 225, 2)
 pygame.display.update()

 clock.tick(30) # Limit the frame rate to 30 FPS.


screen.fill((0, 0, 0))
draw_text(screen, 'GAME OVER!!!', FONT, 225, 200)
draw_text(screen, 'SCORE:', FONT, 225, 255)
draw_text(screen, str(score), FONT, 225, 305)
pygame.display.update()

while end_screen:
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 end_screen = False

 clock.tick(30)

In this example it's possible to click at the same time as the AI, but that should be impossible to prevent cheating. That needs to be fixed.

And again, you already did a pretty good job and of course you don't have to implement all of these suggestions.