4
\$\begingroup\$

I'm working on a clicker game in Tkinter that is very different from Bubble Blaster:

from tkinter import *
from tkinter.messagebox import *
from time import sleep
import math

extraCashCost = 50

class AppUI(Frame):

    def __init__(self, master=None):
        self.saves = open("self.saves.log", "r")
        self.savesEdit = open("self.saves.log", "a")
        self.saveSring = ''.join(self.saves.read())

        Frame.__init__(self, master, relief=SUNKEN, bd=2)

        self.menubar = Menu(self)

        menu = Menu(self.menubar, tearoff=0)
        self.menubar.add_cascade(label="Game", menu=menu)
        menu.add_command(label="Open Store", command=lambda:storewin())
        menu.add_command(label="Browse Saves", command=lambda:showinfo("WIP", "This is a work in progress game"))

        menu = Menu(self.menubar, tearoff=0)
        self.menubar.add_cascade(label="Edit", menu=menu)
        menu.add_command(label="Open the developer window", command=lambda:devwin())

        try:
            self.master.config(menu=self.menubar)
        except AttributeError:
            # master is a toplevel window (Python 1.4/Tkinter 1.63)
            self.master.tk.call(master, "config", "-menu", self.menubar)

        self.money = 0
        self.cashGet = 1
        self.moneyShow = Label(root, text="$" + str(self.money))
        self.moneyGet = Button(root, text="Get Money!", command=lambda:cashin(self.cashGet))
        self.moneyGet.pack(side=BOTTOM)
        self.moneyShow.pack()

def cashin(cash):
    sleep(0.125)
    app.money += int(cash)
    app.moneyShow.config(text="$" + str(app.money))
def devcashadd(moneyz):
    app.money += int(moneyz)
    app.moneyShow.config(text="$" + str(app.money))
def devwin():
    dev = Tk()
    dl = Label(dev, text="Welcome to the developer window. \n Here you will have access to developer tools. \n This is available only in developer versions.").pack()
    mcl = Label(dev, text="Enter a number below and press the button under it to get free money!")
    mc = Entry(dev)
    mca = Button(dev, text="Get Money", command=lambda:devcashadd(mc.get())).pack()
    mc.pack()
    dev.mainloop()
def ec(cs):
    global extraCashCost
    if app.money >= extraCashCost:
        extraCashCost = math.ceil(extraCashCost * 2.2)
        app.money -= extraCashCost
        app.cashGet *= 1.6
        app.moneyShow.config(text="$" + str(app.money))
        cs.config(text="$" + str(extraCashCost))

def storewin():
    global extraCashCost
    store = Tk()
    extraCashShow = Label(store, text="$" + str(extraCashCost))
    extraCash = Button(store, text="More Money!", command=lambda:ec(extraCashShow))
    extraCashShow.pack()
    extraCash.pack()
    store.mainloop()

root = Tk()

app = AppUI(root)
app.pack()

root.mainloop()

It is going to be on GitHub soon. Anything that could be changed to make it run faster?
And also, how can I put custom styles on the widgets?
Like background gradients, images, etc...

\$\endgroup\$
  • 6
    \$\begingroup\$ You might want to consider a better description of the code too. In my experience, the more context you give people to work with, the more likely they are to give you a good review. \$\endgroup\$ – Carcigenicate Dec 9 '17 at 20:52
  • \$\begingroup\$ Your question currently leaves some things to be desired. I'd recommend taking a look at Simon's Guide to posting a good question. In particular, you could improve your description about the code, and add screenshots if possible. "Very different from Bubble Blaster" doesn't say much. \$\endgroup\$ – Simon Forsberg Dec 10 '17 at 16:16
5
\$\begingroup\$

Don't use wildcard imports

You had this same advice on your prior post. You apparently decided to ignore it. It doesn't make you look smarter.

Use import tkinter as tk if you must. But put some kind of prefix on Tk functions so that later readers of your code - including yourself months from now - will know when a function call or class reference is to an outside library:

class AppUI(Frame):   # ?

class AppUI(tk.Frame): # Aha!

Use super in __init__

Instead of calling Frame.__init__ you should call super like so:

super().__init__(...)

See this article by Raymond Hettinger for more details.

Don't hold open files you don't use

You open files in __init__ that you don't use and don't close. Avoid this. Instead, open/read/close or open/write/close them when you actually need to. The open operations are cheap compared to writing or reading from a file, and file handles are a scarce resource.

Also, doing your open/read or open/write at the same time allows you to use the preferred Python idiom, with, for your file I/O:

with open('savegame.txt', 'r') as save:
    savetext = save.read()
    ...

The with model handles errors well and automatically closes the resource when you're done, so you don't have to worry about it.

Move your GUI code out of __init__

Do what you need to do to set up member data, then delegate your GUI setup to another method. This makes init easier to understand, and you will likely find some code reuse once you have the "create a method" approach in mind.

Use bound methods rather than global variables

You have a callback like so:

def cashin(cash):
    ...
    app.money += int(cash)

But app is a global variable that points to your AppUI. How do you call it? With this line, set up in __init__:

    self.moneyGet = Button(root, text="Get Money!", command=lambda:cashin(self.cashGet))

So you're inside self.__init__, passing a function as a command to a button. But inside the function, you have to look up a global variable to figure out the value of self?

Try passing a bound method, or using a closure, for this kind of thing.

command=self.cashin  # no parens, it's a reference not a call

Wrap your top-level code in a function

There's a standard idiom for top-level code in Python. Use it!

def main():
    root = Tk()
    app = AppUI(root)
    app.pack()
    root.mainloop()

if __name__ == '__main__':
    main()

Encapsulate!

You pass root into AppUI() as a parameter. So why are you calling root.mainloop()? Why don't you encapsulate this in your AppUI?

app = AppUI()  # Let __init__ figure out to call tk.Tk()
app.run()      # app.run can call self.master.mainloop()
\$\endgroup\$
2
\$\begingroup\$

Only create one instance Tk

You're creating more than one instance of Tk and you are calling mainloop() more than once. Tkinter is designed to have exactly one instance of Tk(), and exactly one mainloop running. If you need multiple windows, create instances of Toplevel.

Don't call sleep

A GUI thread should never call sleep, since that's exactly what it does. Your entire UI will freeze. You're only doing it for a fraction of a second so it doesn't matter too much, but you should probably remove it.

I don't know what you're trying to accomplish with it, but if you just want some code to happen with a slight delay you can use after to schedule it to run in the future.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.