Simon game / Four tiles game

Question

When I read the community challange, I couldn't wait. So here I go with the very first Four tiles game:

import pygame as pg
import sys
import random
import time
import tkMessageBox

if sys.version_info.major == 3:
    import tkinter as tk
else:
    import Tkinter as tk

from pygame.locals import *


def game(LENGHT):
    pg.init()
    pg.display.set_caption("Four tiles game")

    WIDTH = 600
    HEIGHT = 400
    DISPLAY = pg.display.set_mode((WIDTH, HEIGHT), 0, 32)

    WHITE = (255, 255, 255)
    RED = (255, 0, 0)
    green = (0, 255, 0)
    blue = (0, 0, 255)
    yellow = (255, 255, 0)
    COLORS = [RED, blue, green, yellow]

    WAITING_TIME = 500

    def generate_correct_sequence(length):
        return [random.randint(1, 4) for _ in range(length)]

    def draw_button(n, color):
        if n == 1:
            pg.draw.rect(DISPLAY, color, (0, 0, WIDTH / 2, HEIGHT / 2))
        if n == 2:
            pg.draw.rect(DISPLAY,color,
            (WIDTH / 2, 0, WIDTH / 2, HEIGHT / 2))
        if n == 3:
            pg.draw.rect(DISPLAY,color,
            (0, HEIGHT / 2, WIDTH / 2, HEIGHT / 2))
        if n == 4:
            pg.draw.rect(DISPLAY,color,
            (WIDTH / 2, HEIGHT / 2, WIDTH / 2, HEIGHT / 2))


    def draw_buttons():

        for i in range(1, 5):
            draw_button(i, COLORS[i - 1])

    def what_button_is_clicked(mouse_pos):
        """
        The buttons are numbered in the following way
            1 | 2
            _____

            3 | 4
        """
        if mouse_pos[0] < WIDTH / 2 and mouse_pos[1] < HEIGHT / 2:
            return 1

        if mouse_pos[0] > WIDTH / 2 and mouse_pos[1] < HEIGHT / 2:
            return 2

        if mouse_pos[0] < WIDTH / 2 and mouse_pos[1] > HEIGHT / 2:
            return 3

        elif mouse_pos[0] > WIDTH / 2 and mouse_pos[1] > HEIGHT / 2:
            return 4


    def flash(button_number, waiting_time):
        draw_button(button_number, WHITE)
        pg.display.update()
        pg.time.wait(waiting_time)
        draw_button(button_number, COLORS[button_number - 1])
        pg.display.update()

    def inform_user(correct_sequence, delay):
        for button in correct_sequence:
            flash(button, delay)

    draw_buttons()
    correct_sequence = generate_correct_sequence(LENGHT)
    inform_user(correct_sequence, WAITING_TIME)

    while True:
        for event in pg.event.get():
            if event.type == QUIT:
                pg.quit()
            if event.type == pg.MOUSEBUTTONUP:
                mouse_pos = pg.mouse.get_pos()
                if what_button_is_clicked(mouse_pos) == correct_sequence[0]:
                    correct_sequence.pop(0)
                else:
                    tkMessageBox.showinfo("LOSE", "You are the loser...")
                    pg.quit()


        if correct_sequence == []:
            tkMessageBox.showinfo("WIN", "You are the winner!!!")
            pg.quit()


if __name__ == "__main__":
    root = tk.Tk()
    root.wm_title("Four tiles game")

    def play():
        game(w.get())

    INTRODUCTION = tk.StringVar()
    INTRODUCTION.set("""Welcome to the four tiles game.
You will see some tiles light up one after the other.
Afterwards, you must click them in the same order they lit up before.""")
    tk.Message(root, textvariable=INTRODUCTION, width=500, font=30).pack()

    tk.Label(root, text="Select the lenght down here", font=30).pack()
    w = tk.Scale(root, from_=1, to=15, orient=tk.HORIZONTAL, length=400)
    w.pack()

EDIT: If you want to look at the game with added functionalaties and features and with nicer style go to this GitHub repo.

Answer 1

Missing the specs

You're missing a very important rule of the game:

The pattern gets longer each time the player completes the pattern. If the player presses a wrong button, the game ends.

You implemented only a one-shot round, which is not so interesting.

Mutually exclusive conditions

These conditions are mutually exclusive:

if n == 1:
    # ...
if n == 2:
    # ...
if n == 3:
    # ...

So you should use elif for the 2nd and later conditions.

Use consistent names

It's odd to name some constants with all caps, and others with all lowercase like this:

WHITE = (255, 255, 255)
RED = (255, 0, 0)
green = (0, 255, 0)
blue = (0, 0, 255)
yellow = (255, 255, 0)

Be consistent, name all of them all caps!

Avoid hard-coding

This function relies on having 4 buttons:

def generate_correct_sequence(length):
    return [random.randint(1, 4) for _ in range(length)]

It would be better to avoid hardcoding the number 4. You could for example use len(COLORS) instead. Even better would be to use a dedicated constant, and derive both the elements of colors from it, and use it everywhere else where it's relevant, as in this function.

Use local variables to explain logic

In this code:

if mouse_pos[0] < WIDTH / 2 and mouse_pos[1] < HEIGHT / 2:
    return 1

if mouse_pos[0] > WIDTH / 2 and mouse_pos[1] < HEIGHT / 2:
    return 2
# ...

mouse_pos[0] and mouse_pos[1] are repeated many times, which is tedious, and not as clear as it can be. I don't actually have tkinter now, but I'm guessing these stand for x and y coordinates.

I would either change the method to take parameters named x and y, or introduce the local variables x and y early in the method:

x = mouse_pos[0]
y = mouse_pos[1]

The rest of the code would become shorter, and clearer.

Why not use 0-base indexing everywhere?

This method is slightly more complex than it needs to be due to the 1-based indexing:

def draw_buttons():
    for i in range(1, 5):
        draw_button(i, COLORS[i - 1])

I bet you could change to this simpler implementation, and adjust the rest of the code to work with it without sacrificing anything:

def draw_buttons():
    for i in range(4):
        draw_button(i, COLORS[i])

Notice the hardcoded number 4 again. This highlights again the importance to avoid hardcoding. The range(1, 5) came from the fact of the hard limit 4, and if you decided to change 4 at some point, searching in the code for "4" you would never find range(1, 5).

So, really, replace the number 4 with a constant. len(COLORS) may look tempting, but as I explained above, I suggest to use something else as the single authoritative point of control.