First Game: Simon Game

Question

I am working on a Simon game using python and tkinter, just now it is incomplete, but I don't know if I am doing it right because it looks messy, and I would like to have a check on my code for corrections and suggestions on how to improve it.

This is the first game I'm making, and I don't have to much experience in programming.

This is my actual entire code:

from Tkinter import *
import Tkinter
import random

base = Tkinter.Tk()

fr = Tkinter.Frame(base, bg="black", width="238", height="250")


score = Tkinter.Label(base, bg="black", fg="white", text="Score:")
score.place(x = 30, y = 15)

s = 0

scoreNum = Tkinter.Label(base, bg="black", fg="white", text = s)
scoreNum.place(x = 70, y = 15)


best = Tkinter.Label(base, bg="black", fg="white", text="Best:")
best.place(x = 155, y = 15)

b = 0

bestNum = Tkinter.Label(base, bg="black", fg="white", text = b)
bestNum.place(x = 188, y = 15)


clicks = []
color = 0

def yellowClick():

    yellow.configure(activebackground="yellow3")
    yellow.after(500, lambda: yellow.configure(activebackground="yellow"))

    global clicks
    global color

    color = 1
    clicks.append(color)

yellow = Tkinter.Button(base, bd="0", highlightthickness="0",
                       width="7", height="5", activebackground="yellow",  
                       bg="yellow3", command = yellowClick)

yellow.place(x = 30, y = 50)


def blueClick():

    blue.configure(activebackground="medium blue")
    blue.after(500, lambda: blue.configure(activebackground="blue"))

    global clicks
    global color

    color = 2
    clicks.append(color)

blue = Tkinter.Button(base, bd="0", highlightthickness="0",
                     width="7", height="5", activebackground="blue",
                     bg="medium blue", command = blueClick)

blue.place(x = 125, y = 50)


def redClick():

    red.configure(activebackground="red3")
    red.after(500, lambda: red.configure(activebackground="red"))

    global clicks
    global color 

    color = 3
    clicks.append(color)

red = Tkinter.Button(base, bd="0", highlightthickness="0",
                    width="7", height="5", activebackground="red",
                    bg = "red3", command = redClick)    

red.place(x = 30, y = 145)


def greenClick():

    green.configure(activebackground="dark green")
    green.after(500, lambda: green.configure(activebackground="green4"))

    global clicks
    global color

    color = 4
    clicks.append(color)

green = Tkinter.Button(base, bd="0", highlightthickness="0",
                      width="7", height="5", activebackground="green4",
                      bg="dark green", command = greenClick)

green.place(x = 125, y = 145)   


def scoreUp():

    global s

    s = s + 1

    scoreNum.configure(text = s)


r = random.randint(1, 4)

sequence = []

def checkSequence():

    global clicks
    global sequence

    if clicks == sequence:

        scoreUp()

def showSequence():

    global r
    global sequence

    if r == 1:

        yellow.configure(bg="yellow")
        yellow.after(1000, lambda: yellow.configure(bg="yellow3"))

        sequence.append(r)

        base.after(5000, checkSequence)

    elif r == 2:

        blue.configure(bg="blue")
        blue.after(1000, lambda: blue.configure(bg="medium blue"))  

        sequence.append(r)

        base.after(5000, checkSequence)

    elif r == 3:

        red.configure(bg="red")
        red.after(1000, lambda: red.configure(bg="red3"))   

        sequence.append(r)

        base.after(5000, checkSequence)

    elif r == 4:

        green.configure(bg="green4")
        green.after(1000, lambda: green.configure(bg="dark green")) 

        sequence.append(r)      

        base.after(5000, checkSequence)


base.after(2000, showSequence)

fr.pack()

base.resizable(False, False)
base.mainloop()

Actually I'm checking the sequence with time because I can't find another to do it.

Answer 1

Python has an official style-guide, PEP8, which you are encouraged to follow to improve readability (mostly by others, but also yourself). On e of its recommendations is using spaces around operators, but no spaces around = when assigning keywords. So these are good:

var = "value"
l = [2, 1, 3]
l.sort(reverse=True)

while these are bad:

var="value"
l=[ 1,2,3 ]
l.sort( reverse = True)

---

In general it is not best practice to use global, because it makes the state your program is in potentially undefined.

For example the function

def f():
    global c
    return c

It is not defined what this function will output at any given time, because anybody else might modify the global c.

Most of the times, global can be avoided entirely, just by making functions accept the values as parameters and returning the results where necessary.

In your case this is a bit more tricky, because many of these functions cannot take any parameter (because Tkinter just calls them as they are).

There is one work-around for it and one solution, though.

One is using lambdas, like this:

buttons = []
for i in range(10):
    buttons.append(Tkinter.Button(base, bd="0", highlightthickness="0",
                               width="7", height="5", activebackground=color,
                               bg=alt_color, command=lambda i=i: print i)

Here the i=i is evaluated whenever the function is called. A simple lambda: print i would have failed.

The actual solution to this problem, though is to use a class. This way you can pack all those global variables as attributes, can access them within the functions, and so on.

I won't go into details about how to write a class, because there are very good tutorials about this (add links).

There are a few more noteworthy changes.

I made functions to create and place a label, which is pretty straight-forward, and to create and place a button. The latter is a bit more complicated, because, as explained above, the button needs a parameter-less function. For this I used the fact that in python functions are first-class objects. This means that you can return functions from other functions. I therefore wrote a button click handler factory, that produces a custom function given its parameters (with the right color, highlightcolor, etc).

To save the data of the different buttons I created a collections.namedtuple, which is just a tuple, where you can access the members also by name (but also still by index if you need to).

Then I made the game slightly more complete by adding a variable length of the shown sequence, an actual saving and verification of the user sequence and the correct handling of checking whether it is correct.

The attribute self.sequence contains the generated (and shown) sequence, while if a user clicks a button it is added to self.user_sequence if the two are identical the user wins, score is updated (and best if needed). A new sequence can be generated, with a length one longer than before.

If the user clicks and the sequence is right so far, nothing happens. But if the user clicks a button which is not the next in the sequence, the game resets (keeping only best).

I also placed the actual execution of the game in a if __name__ == "__main__": guard to allow importing parts of the code from other scripts (So you can e.g. do from simon_game import Button, if this code is saved as simon_game.py).

---

Final code:

import Tkinter
import random
from collections import namedtuple

Button = namedtuple("Button", "color alt_color x y")
BUTTONS_DATA = [Button("yellow", "yellow3", 30, 50),
                Button("blue", "medium blue", 125, 50),
                Button("red", "red3", 30, 145),
                Button("green", "dark green", 125, 145)]


class SimonGame:
    def __init__(self, buttons_data=BUTTONS_DATA):
        self.base = Tkinter.Tk()
        self.frame = Tkinter.Frame(self.base, bg="black", width="238", height="250")

        self.score = 0
        self.score_label = self.place_label(30, 15, "Score:")
        self.score_val = self.place_label(70, 15, self.score)

        self.best = 0
        self.best_label = self.place_label(155, 15, "Best:")
        self.best_val = self.place_label(188, 15, self.best)

        self.start = Tkinter.Button(self.base, bd="0", text="start", command=self.show_sequence)
        self.start.place(x=90, y=12)

        self.sequence = []
        self.user_sequence = []

        self.buttons_data = buttons_data
        self.buttons = [self.create_button(*button) for button in self.buttons_data]

        self.frame.pack()
        self.base.resizable(False, False)
        self.base.mainloop()

    def place_label(self, x, y, text, fg="white", bg="black"):
        label = Tkinter.Label(self.base, bg=bg, fg=fg, text=text)
        label.place(x=x, y=y)
        return label

    def click_factory(self, button, color, alt_color):
        def click():
            button.configure(activebackground=alt_color)
            button.after(500, lambda: button.configure(activebackground=color))
            self.user_sequence.append(color)
            self.check_sequence()
        return click

    def create_button(self, color, alt_color, x, y):
        button = Tkinter.Button(self.base, bd="0", highlightthickness="0",
                           width="7", height="5", activebackground=color,
                           bg=alt_color)
        button['command'] = self.click_factory(button, color, alt_color)
        button.place(x=x, y=y)
        return button

    def show_sequence(self):
        self.reset_gui()
        for _ in range(self.score + 1):
            i = random.randrange(len(self.buttons))
            button = self.buttons[i]
            button_data = self.buttons_data[i]
            button.configure(bg=button_data.color)
            button.after(750, lambda: button.configure(bg=button_data.alt_color))
            self.sequence.append(button_data.color)
        print self.sequence

    def check_sequence(self):
        if self.sequence == self.user_sequence:
            self.score += 1
            self.best = max(self.best, self.score)
            self.reset_gui()
        elif not self.sequence[:len(self.user_sequence)] == self.user_sequence:
            self.score = 0
            self.reset_gui()

    def reset_gui(self):
        self.sequence = []
        self.user_sequence = []
        for button, data in zip(self.buttons, self.buttons_data):
            button.configure(bg=data.alt_color, activebackground=data.color)
        self.best_val.configure(text=self.best)
        self.score_val.configure(text=self.score)

if __name__ == "__main__":
    game = SimonGame()

---

What is left to do: The graphical part is not very clean. Some buttons get stuck on on (in sequences where a button appears an even number of times) and the sequence plays out too fast. Also more visual feedback when getting a whole sequence right or messing it up would help.