西蒙记忆游戏

Question

我最近开始学习Python，我已经决定将到目前为止学到的东西应用于创建一个西蒙记忆游戏克隆。我现在正在寻求改进代码的帮助。任何帮助改进此代码将是非常感谢和批评是欢迎的。我正在使用Python3.5。

import tkinter
from random import choice
class Simon() :
    def __init__(self, master) :
        # Configure tkinter.Tk with some basic settings.
        self.master = master
        self.master.minsize(640, 480)
        self.master.resizable(False, False)
        self.master.title("Simon Memory Game")
        self.master.update() # Complete any outstanding tkinter tasks.

        # Create the canvas to be used to draw the rectangles that make up the game. Have it take up the entire window.
        self.game_canvas = tkinter.Canvas(self.master, width = self.master.winfo_width(), height = self.master.winfo_height(), highlightthickness = 0)
        self.game_canvas.pack()

        # Set up the four colors that will be used throughout the game.
        self.idle_colors = ("red", "blue", "green", "yellow")
        self.tinted_colors = ("#ff4d4d", "#4d4dff", "#4dff4d", "#ffff4d")
        self.current_colors = [color for color in self.idle_colors]

        self.rectangle_ids = []

        # Sequence of the colors for the current game and the position in the sequence for use when showing it to the user.
        self.sequence = [choice(self.idle_colors)]
        self.sequence_position = 0

        self.draw_canvas()

        self.show_sequence()

        self.master.mainloop()

        # Show the sequence to the player, so the player can repeat it.
    def show_sequence(self) :
        # Pass the current color of the sequence to the flash function.
        self.flash(self.sequence[self.sequence_position])
        # Check that we have not reached the end of the sequence.
        if(self.sequence_position < len(self.sequence) - 1) :
            # Since we haven't reached the end of the sequence increment the postion and schedule a callback.
            self.sequence_position += 1
            self.master.after(1250, self.show_sequence)
        else :
            self.sequence_position = 0 # Reset the position for next round.

        # Flash a single rectangle once. Used to indicate it is apart of the sequence to the player.
    def flash(self, color) :
        index = self.idle_colors.index(color) # Find the position in the tuple that the specified color is at.
        if self.current_colors[index] == self.idle_colors[index] : # Use the position of the color specified to compare if the current color on screen matches the idle one.
            # If so set the current color equal to that of the tinted color.
            self.current_colors[index] = self.tinted_colors[index]
            self.master.after(1000, self.flash, color) # Call this function again in 1 seconds time to revert back to the idle color
        else :
            self.current_colors[index] = self.idle_colors[index] # Revert the current color back to the idle color.
        self.draw_canvas() # Draw the canvas to reflect this change to the player.

    def check_choice(self) :
        color = self.idle_colors[self.rectangle_ids.index(self.game_canvas.find_withtag("current")[0])]
        if(color == self.sequence[self.sequence_position]) :
            if(self.sequence_position < len(self.sequence) - 1) :
                self.sequence_position += 1
            else :
                self.master.title("Simon Memory Game - Score: {}".format(len(self.sequence))) # Update the score.
                # Reached the end of the sequence append new color to sequence and play that.
                self.sequence.append(choice(self.idle_colors))
                self.sequence_position = 0
                self.show_sequence()
        else :
            # Game Over for the player as they got the sequence wrong reset the game back to level one.
            self.master.title("Simon Memory Game - Game Over! | Final Score: {}".format(len(self.sequence)))
            self.sequence[:] = [] # Empty the list of sequences
            self.sequence.append(choice(self.idle_colors)) # Add the first sequence of the new game to the list of sequences.
            self.sequence_position = 0
            self.show_sequence()

    def draw_canvas(self) :
        self.rectangle_ids[:] = [] # Empty out the list of ids.
        self.game_canvas.delete("all") # Clean the frame ready for the new one
        for index, color in enumerate(self.current_colors) : # Iterate over the colors in the list drawing each of the rectangles their respective place.
            if index <= 1 :
                self.rectangle_ids.append(self.game_canvas.create_rectangle(index * self.master.winfo_width(), 0, self.master.winfo_width() / 2, self.master.winfo_height() / 2, fill = color, outline = color))
            else :
                self.rectangle_ids.append(self.game_canvas.create_rectangle((index - 2) * self.master.winfo_width(), self.master.winfo_height(), self.master.winfo_width() / 2, self.master.winfo_height() / 2, fill = color, outline = color))
        for id in self.rectangle_ids :
            self.game_canvas.tag_bind(id, '<ButtonPress-1>', lambda e : self.check_choice())

def main() :
    root = tkinter.Tk()
    gui = Simon(root)

if __name__ == "__main__" : main()

Answer 1

风格

除了@syb0rg的S回答之外，正文中还有一些评论旨在描述下面的方法，这些应该是文档字符串。

您还碰巧拥有神奇的数字和序列，它们作为常量或至少是类级属性更好：

class Simon:
    IDLE = ("red", "blue", "green", "yellow")
    TINTED = ("#ff4d4d", "#4d4dff", "#4dff4d", "#ffff4d")

    FLASH_ON = 1000
    FLASH_OFF = 250

    def __init__(self):
        ...

重新发明车轮

tkinter没有在画布上绘制矩形并将事件绑定到它们上，而是提供了允许大量开箱即用的Button小部件:更改颜色是用button.config(background='green')完成的，绑定操作是用command属性完成的；您还可以禁用按钮，以便用户在显示序列时不能进行交互。

唯一要处理的事情是将正确的按钮检索到check_choice中:您需要添加某种index参数。但是，在构建按钮时使用的command关键字需要可调用的无参数。您可以通过使用使用固定索引调用check_choice、使用lambda或使用functools.partial的助手方法来克服这一问题。

闪烁瓷砖

我发现你向玩家展示顺序的方式有些不寻常。让flash在show_sequence的两个调用之间被调用两次(on + off)感觉很奇怪。让show_sequence启动一些东西，然后让flash_on和flash_off方法使用self.master.after (以及使用self.FLASH_ON和self.FLASH_OFF常量)相互调用，会更加简单。

在序列

上迭代

考虑到这些约束，您的做法并不是那么糟糕，但我只想展示另一种方法:使用iter和next。不是存储当前索引，而是使用iter和应该按下的当前按钮将迭代器存储到序列中。成功按下按钮后，可以使用next更新当前值。如果StopIteration被引发，那么就意味着玩家到达了序列的末尾。

此方法既可用于等待用户输入，也可用于闪烁按钮。

Nitpicks

我发现把瓷砖闪烁一秒有点慢。我个人会用600或750毫秒。

我还发现，有色的颜色与闲置的颜色太接近了(除了绿色)。您应该使用更清晰的颜色，这样玩家就可以更好地看到序列中的下一个。

最后，我觉得运行self.master.mainloop()甚至在类之外创建根都不允许进行大量的定制，我会将内容保持在内部，并提供一种启动游戏的方法(这将运行self.master.mainloop() )。

提议的改进

import tkinter as tk
import random
from functools import partial


class Simon:
    IDLE = ('red', 'blue', 'green', 'yellow')
    TINTED = ('#ff4d4d', '#4d4dff', '#4dff4d', '#ffff4d')

    FLASH_ON = 750  #ms
    FLASH_OFF = 250  #ms

    def __init__(self, title='Simon Memory Game'):
        self.master = tk.Tk()
        self.master.title(title)
        self.master.resizable(False, False)
        self.title = title
        self.buttons = [
            tk.Button(
                self.master,
                height=15,
                width=25,
                background=c,
                activebackground=c,  # remove this line if you want the player to see which button is hovered
                command=partial(self.push, i))
            for i, c in enumerate(self.IDLE)]
        for i, button in enumerate(self.buttons):
            button.grid({'column': i % 2, 'row': i // 2})

    def reset(self):
        self.sequence = []
        self.new_color()

    def push(self, index):
        if index == self.current:
            try:
                self.current = next(self.iterator)
            except StopIteration:
                self.master.title('{} - Score: {}'
                                  .format(self.title, len(self.sequence)))
                self.new_color()
        else:
            self.master.title('{} - Game Over! | Final Score: {}'
                              .format(self.title, len(self.sequence)))
            self.reset()

    def new_color(self):
        for button in self.buttons:
            button.config(state=tk.DISABLED)
        color = random.randrange(0, len(self.buttons))
        self.sequence.append(color)
        self.iterator = iter(self.sequence)
        self.show_tile()

    def show_tile(self):
        try:
            id = next(self.iterator)
        except StopIteration:
            # No more tiles to show, start waiting for user input
            self.iterator = iter(self.sequence)
            self.current = next(self.iterator)
            for button in self.buttons:
                button.config(state=tk.NORMAL)
        else:
            self.buttons[id].config(background=self.TINTED[id])
            self.master.after(self.FLASH_ON, self.hide_tile)

    def hide_tile(self):
        for button, color in zip(self.buttons, self.IDLE):
            button.config(background=color)
        self.master.after(self.FLASH_OFF, self.show_tile)

    def run(self):
        self.reset()
        self.master.mainloop()


if __name__ == '__main__':
    game = Simon()
    game.run()

Answer 2

大部分内容将归结为阅读PEP8，但无论如何，我还是会在这里说明它们。

• if语句不需要括号，所以不要将它们放在它们周围。你似乎介于做这件事和不做这件事之间，记住也要保持一致。
• 模块级的函数应该在它们和其他事情之间有两条新的行，而不是像现在这样有一条。
• 在函数调用中，在关键字/参数赋值周围有意想不到的空格。
• import语句和其他代码之间也应该有两行空白行。
• 在内联注释之前，应该至少有两个空格。
• 你的许多行都太长了，最多只能有79个字符。
• 在表达式和语句中的空白中，在下列情况下避免使用无关的空格：在逗号、分号或冒号之前:是:如果x == 4:打印x，y；x，y= y，x No:如果x == 4:打印x，y；x，y=y，x
• 您不应该在一行上有多个语句(冒号后面应该有一个换行符)
• 如图所示，文件末尾没有换行符。