-5

I'm trying to implement a simple GUI for the game of Tic Tac Toe using Tkinter. As a first step, I'm trying to make an array of buttons which change from being unlabeled to having the "X" label when clicked. I've tried the following:

import Tkinter as tk

class ChangeButton:

    def __init__(self, master, grid=np.diag(np.ones(3))):
        frame = tk.Frame(master)
        frame.pack()
        self.grid = grid
        self.buttons = [[tk.Button()]*3]*3

        for i in range(3):
            for j in range(3):
                self.buttons[i][j] = tk.Button(frame, text="", command=self.toggle_text(self.buttons[i][j]))
                self.buttons[i][j].grid(row=i, column=j)

    def toggle_text(self, button):
        if button["text"] == "":
            button["text"] = "X"


root = tk.Tk()
root.title("Tic Tac Toe")

app = ChangeButton(root)

root.mainloop()

However, the resulting window looks like this:

enter image description here

and the buttons don't change when clicked. Any ideas why this does not work?

Kurt Peek
  • 43,920
  • 71
  • 247
  • 451
  • Briefly: `command` needs a function, but self.toggle_text(self.buttons[i][j])` is evaluated immediately (so it happens immediately) and returns nothing (so nothing happens when the button is clicked). A `lambda` wrapper is the preferred fix. – TigerhawkT3 Aug 09 '16 at 08:50
  • And the question you would ask when you fix that and find that clicking any button only affects the last button is probably [this one](http://stackoverflow.com/questions/10865116/python-tkinter-creating-buttons-in-for-loop-passing-command-arguments). – TigerhawkT3 Aug 09 '16 at 08:51
  • @TigerhawkT3 There are two more problems; the dupe addresses only one of them. – tobias_k Aug 09 '16 at 08:52
  • And the third problem, where you don't actually have as many unique `Button` widgets as you think you do, is resolved by [this question](http://stackoverflow.com/questions/240178/python-list-of-lists-changes-reflected-across-sublists-unexpectedly). – TigerhawkT3 Aug 09 '16 at 08:57
  • 2
    @tobias_k - I can only dupehammer with one target, a question is supposed to have one question, and I think it's tidier to link all the future duplicates like this instead of having to dup-close three separate questions. – TigerhawkT3 Aug 09 '16 at 08:58

1 Answers1

2

The primary problem is that with command=self.toggle_text(self.buttons[i][j])), you invoke the callback function and bind its result to command. Instead, you have to bind the function itself tocommand, or alambda` that will invoke that function with the right parameters. A naive way of doing this would look like this:

command=lambda: self.toggle_text(self.buttons[i][j])  # won't work!

But this will not work inside the loop, as the variables inside the lambda are evaluated when the function is executed, i.e. after the loop, i.e. i and j will take on the last value for each of the functions. For a more detailed explanation, see e.g. here. One way to fix this is to declare those variables as parameters to the lambda, and at the same time use the current values from the loop as their default values, i.e. lambda i=i, j=j: .... This way, i and j are evaluated when the function is declared, not when it is called.

Your command and the surrounding loop would then look like this:

    for i in range(3):
        for j in range(3):
            self.buttons[i][j] = tk.Button(frame, text="", 
                    command=lambda i=i, j=j: self.toggle_text(self.buttons[i][j]))
            self.buttons[i][j].grid(row=i, column=j)

And there is a another problem, unrelated to the first, with the way you initialize the self.buttons list. By doing [[tk.Button()]*3]*3, the list will hold three references to the same list, each holding three references to the same button. See e.g. here for a more in-depth discussion. Also, you do not need to initialize the buttons in the list at all, as you set those afterwards, in the loop. I'd suggest using a nested list-comprehension instead:

    self.buttons = [[None for _ in range(3)] for _ in range(3)]
Community
  • 1
  • 1
tobias_k
  • 78,071
  • 11
  • 109
  • 168
  • 1
    Your answer is literally just links to three different duplicates. – TigerhawkT3 Aug 09 '16 at 09:09
  • 1
    Which I provided for you. – TigerhawkT3 Aug 09 '16 at 09:09
  • @TigerhawkT3 I just felt that an actual answer would be more useful than three links in comments and a dupe. Yes, I included the links to the answers with more complete explanation, why not? It also has code that addresses all three problems. If you think this is "not useful" or "not an answer to the question", feel free to downvote, but it seems like you already have. – tobias_k Aug 09 '16 at 09:15
  • Yeah, I think making signposts harder to see and replicating them further down the page isn't too useful. However, I can see how the proper course of action for a question with multiple hammerable issues is ambiguous, so I've [asked](http://meta.stackoverflow.com/questions/332007/whats-the-best-response-to-question-with-multiple-dupe-hammerable-issues). – TigerhawkT3 Aug 09 '16 at 09:27
  • In retrospect, the question should have been (and stayed) closed, but not that the answer is accepted, I can't delete it, so instead I tried to at least improve it. Hope you don't mind. – tobias_k Aug 11 '16 at 18:23