Disclaimer: This code is far from complete, I am aware of issues regarding my standards of naming, my lack of comments and readability; however, I am encountering a large issue which is preventing me from editing, testing and completing the game. This is not a functional issue, but a performance issue. If you would like a more detailed explanation, this is what I wrote previously on Stack Overflow:
I am building the following game in
tkinter
, it's essentially a snake game with two players. Where you would battle the other player until one of you hits the other, the wall or runs out of space. To create the 'game' I am using canvases, which are created in a grid. I think this may be the cause of the following issue: When attempting to close the program, python and the script will completely freeze, and take around 5-10 seconds to close. I am aware that tkinter may not be the best option for game creation, however this is the way I have chosen and would like to know if there is any way to fix my issue.To recap: Upon trying to close the following program - the window freezes for around 5-10 seconds. It then closes but obviously testing and actual gameplay is hindered.
One note I would like to make is that the game loads up much quicker than it closes which I find to be strange. Any optimisations that you could recommend would also be welcome however I realise that Stack Exchange is more the platform for this.
from tkinter import *
from random import randint
import os
velocity = [1,0]
velocity2 = [-1,0]
grid = []
coord = [[10,10]]
coord2 = [[20,20]]
time = 0
end = False
colour1 = "#d35400"
colour2 = "#9b59b6"
winner = 0
size1 = 0
size2 = 0
loopIds = []
speed1 = 50
speed2 = 50
powers = ["#2ecc71","#e74c3c","#ecf0f1"]
up1 = 0
up2 = 0
down1 = 0
down2 = 0
points1 = 0
points2 = 0
master = Tk()
master.config(bg="#222")
master.title("Trail Snake")
master.wm_state('zoomed')
master.resizable(0, 0)
def start():
global coord, coord2, colour1, colour2
for i in coord:
grid[i[0]][i[1]].config(bg=colour1)
for i in coord2:
grid[i[0]][i[1]].config(bg=colour2)
def genPower():
global h, w, powers
power = randint(0,2)
location = [randint(0,h-1), randint(0,h-1)]
while grid[location[0]][location[1]].cget("bg") != "#222":
location = [randint(0,h-1), randint(0,h-1)]
grid[location[0]][location[1]].config(bg=powers[power])
def clear():
for n in grid:
for i in n:
i.config(bg="#222")
def move():
global coord, coord2, colour1, colour2, size1, size2
moveSnake(coord, colour1, size1)
moveSnake(coord2, colour2, size2)
def moveSnake(snake, colour, size):
global end, h, w, colour1, winner, velocity, velocity2, loopIds, speed1, speed2, powers, up1, up2, down1, down2
if colour == colour1:
movement = velocity
speed = speed1
else:
movement = velocity2
speed = speed2
if up1 == 0 and down2 == 0:
speed1 = 50
if up2 == 0 and down1 == 0:
speed2 = 50
if not(snake[0][0]+movement[0] >= 0 and snake[0][0]+movement[0] <= (h-1) and snake[0][1]+movement[1] >= 0 and snake[0][1]+movement[1] <= (w-1)):
end = True
if colour == colour1:
winner = "Purple"
else:
winner = "Orange"
if not end:
for i in snake:
grid[i[0]][i[1]].config(bg="#222")
snake.insert(0, [snake[0][0] + movement[0], snake[0][1] + movement[1]])
if size == 0:
chance = randint(1,20)
if chance == 1:
size = randint(1,3)
if size != 0:
snake.pop()
path = snake[0]
size -= 1
else:
path = snake.pop()
for i in snake:
if grid[i[0]][i[1]].cget("bg") == "#222":
grid[i[0]][i[1]].config(bg=colour)
elif grid[i[0]][i[1]].cget("bg") in powers:
power = powers.index(grid[i[0]][i[1]].cget("bg"))
if power == 0:
if colour == colour1:
if up1 == 0:
speed1 = 1
up1 += 5
else:
if up2 == 0:
speed2 = 1
up2 += 5
elif power == 1:
if colour == colour1:
if down1 == 0:
speed2 = 200
down1 += 10
else:
if down2 == 0:
speed1 = 200
down2 += 10
else:
clear()
else:
end = True
if colour == colour1:
winner = "Purple"
else:
winner = "Orange"
grid[path[0]][path[1]].config(bg=colour)
chance = randint(1,100)
if chance == 1:
genPower()
loopId = master.after(speed, lambda snake=snake, colour=colour, size=size: moveSnake(snake, colour, size))
loopIds.append(loopId)
else:
pass # WINNER
def changeDir(changeDir):
global velocity
if velocity[0] == changeDir[1] or velocity[1] == changeDir[0]:
velocity = changeDir
def changeDir2(changeDir):
global velocity2
if velocity2[0] == changeDir[1] or velocity2[1] == changeDir[0]:
velocity2 = changeDir
def displayGame():
global h, w
try:
main.destroy()
except: pass
global wrapper, scoreCount, totalScore, bottom, game
wrapper = Frame(master, bg="#3498db")
wrapper.pack(fill=X)
gameTitle = Label(wrapper ,text="Trail Snake", bg="#3498db", fg="#fff", height=1, pady=10, font=('century gothic',(18)))
gameTitle.pack(side="top")
gameWrapper = Frame(bg="#222")
gameWrapper.pack(expand=True, fill=X, padx=200)
game = Frame(gameWrapper, bg="#eee", highlightbackground="#2980b9", highlightthickness=5)
game.pack(side="right")
game.propagate(0)
scoresMessage = Label(gameWrapper, anchor="w", fg="#fff", text="Scores this round: (5 games played)", font=('century gothic',(20)), bg="#222")
scoresMessage.pack(anchor="w")
playerOneScore = Frame(gameWrapper, padx=10, pady=10, bg="#9b59b6")
playerOneScore.pack(anchor="w")
playerScoreIncrease = Label(playerOneScore, text="+1", bg="#8e44ad")
playerScoreIncrease.pack(side="left")
WINDOW_WIDTH = master.winfo_screenwidth() / 3
WINDOW_HEIGHT = master.winfo_screenheight() / 2
game.config(width=WINDOW_WIDTH, height=WINDOW_HEIGHT)
SW = game.winfo_screenwidth()
SH = game.winfo_screenheight()
h = round(((SH-10) / 15)) - 10
w = round(((SW-10) / 15))
w = round(w / 1.8)
for n in range(0, h):
grid.append([])
for i in range(0,w):
grid[n].append(Canvas(game, bg="#222", height="15", width="15", highlightthickness="0"))
grid[n][i].grid(row=n, column=i)
start()
move()
def restart(event=None):
global coord, coord2, grid, velocity, velocity2, score, end, loopIds, time, timerId, speed1, speed2, up1, up2, down1, down2
for n in grid:
for i in n:
i.config(bg="#222")
for i in loopIds:
master.after_cancel(i)
loopIds = []
velocity = [1,0]
velocity2 = [-1,0]
coord = [[10,10]]
coord2 = [[40,50]]
score = 0
end = False
time = 0
speed1 = 50
speed2 = 50
up1 = 0
up2 = 0
down1 = 0
down2 = 0
master.after_cancel(timerId)
start()
move()
def mainMenu():
global main
main = Frame(master, pady=20, padx=20, bg="#333")
main.pack(expand=True, padx=20, pady=20)
mainLbl = Label(main ,text="Click Start To Begin!", bg="#3498db",anchor="n", fg="#fff", height=1, width=40, pady=20, font=('century gothic',(18)))
mainLbl.pack(pady=(0,0), fill=X)
startGame = Button(main, text="Start", command=displayGame, bg="#2ecc71", borderwidth=0, relief=FLAT, height=2, fg="#fff", font=('century gothic',(14)))
startGame.pack(pady=(10,0), fill=X)
mainMenu()
master.bind("<Up>", lambda Event=None: changeDir2([-1,0]))
master.bind("<Down>", lambda Event=None: changeDir2([1,0]))
master.bind("<Left>", lambda Event=None: changeDir2([0,-1]))
master.bind("<Right>", lambda Event=None: changeDir2([0,1]))
master.bind("<w>", lambda Event=None: changeDir([-1,0]))
master.bind("<s>", lambda Event=None: changeDir([1,0]))
master.bind("<a>", lambda Event=None: changeDir([0,-1]))
master.bind("<d>", lambda Event=None: changeDir([0,1]))
master.bind("<F5>", restart)
master.bind("<F6>", lambda x: master.destroy())
master.mainloop()
-
\$\begingroup\$ So to be clear, everything works correctly, it's just very slow to close? \$\endgroup\$Dan Oberlam– Dan Oberlam2018年03月08日 23:50:14 +00:00Commented Mar 8, 2018 at 23:50
-
\$\begingroup\$ Yeah, sorry for not explaining properly! I have a couple kinks to work out, like orange snake ALWAYS winning on a head on collision against purple, but the closing is more of a propriety for me. \$\endgroup\$Chris– Chris2018年03月09日 00:28:52 +00:00Commented Mar 9, 2018 at 0:28
-
\$\begingroup\$ Look at some of the points mentioned in codereview.stackexchange.com/questions/24267/… \$\endgroup\$hjpotter92– hjpotter922018年03月09日 03:44:02 +00:00Commented Mar 9, 2018 at 3:44
-
\$\begingroup\$ For clarity, you’re closing with f6 and it’s taking long, right? Did you try debugging to see what happens after f6? \$\endgroup\$Gürkan Çetin– Gürkan Çetin2018年03月09日 18:38:05 +00:00Commented Mar 9, 2018 at 18:38
-
\$\begingroup\$ @GürkanÇetin Not sure how I'd check this? [debugging after f6] However, what you've said is correct. \$\endgroup\$Chris– Chris2018年03月09日 19:11:14 +00:00Commented Mar 9, 2018 at 19:11
2 Answers 2
tkinter imports
The following line unnecessarily imports many unused items:
from tkinter import *
It is common to use the following:
import tkinter as tk
This requires you to prefix everything from tkinter
with tk
, such as:
master = tk.Tk()
main = tk.Frame(master, pady=20, padx=20, bg="#333")
However, the benefit is that the code is more self-documenting in the sense
that we don't need to guess where Frame
came from.
Unused
This line is unused and can be deleted:
import os
These lines are not needed and can be deleted:
else:
pass # WINNER
You could add a comment near the if not end:
line regarding the winner.
Simpler
These lines:
chance = randint(1,100)
if chance == 1:
can be combined:
if randint(1,100) == 1:
In this line:
for n in range(0, h):
there is no need for the start since 0 is the default:
for n in range(h):
Naming
Regarding the disclaimer, using good coding practices helps everyone to understand the code better, making it more likely to help with the performance issue. Also, code reviews on this site are targeted at everyone who reads them, not just the OP.
The PEP 8 style guide recommends snake_case (oddly enough) for function and variable names.
genPower
would be gen_power
.
Constants should use all caps:
size1 = 0
size2 = 0
would be:
SIZE1 = 0
SIZE2 = 0
Also, you should specify what 1 and 2 refer to.
try/except
Do not use bare except
. Use a specific exception type.
except: pass
Also, that should be 2 lines, not 1.
except:
pass
We can unpack each nested list, making accessing the indices "manually" like this unnecessary.
def start(): global coord, coord2, colour1, colour2 for i in coord: grid[i[0]][i[1]].config(bg=colour1) for i in coord2: grid[i[0]][i[1]].config(bg=colour2)
Better would be the following, with x
and y
used in place of more meaningful names.
def start():
global coord, coord2, colour1, colour2
for x, y in coord:
grid[a][b].config(bg=colour1)
for x, y in coord2:
grid[x][y].config(bg=colour2)
There are multiple places in your code this can be applied.
Taking a look at this snippet:
for i in snake: if grid[i[0]][i[1]].cget("bg") == "#222": grid[i[0]][i[1]].config(bg=colour) elif grid[i[0]][i[1]].cget("bg") in powers: power = powers.index(grid[i[0]][i[1]].cget("bg")) if power == 0: if colour == colour1: if up1 == 0: speed1 = 1 up1 += 5 else: if up2 == 0: speed2 = 1 up2 += 5 elif power == 1: if colour == colour1: if down1 == 0: speed2 = 200 down1 += 10 else: if down2 == 0: speed1 = 200 down2 += 10 else: clear() else: end = True if colour == colour1: winner = "Purple" else: winner = "Orange"
There's an opportunity to apply the earlier for x, y in ...:
lesson, but also to save grid[i[0]][i[1]].cget("bg")
to a variable rather than repeating this.
Also, rather than check if this value is in powers
and then find the index again, we can simply call index
and handle the ValueError
if it's not found.
And there is an opportunity to use the conditional expression for assigning winner
.
So that we get:
for x, y in snake:
bg = grid[x][y].cget("bg")
if bg == "#222":
grid[x][y].config(bg=colour)
continue
try:
power = powers.index(bg)
if power == 0:
if colour == colour1:
if up1 == 0:
speed1 = 1
up1 += 5
else:
if up2 == 0:
speed2 = 1
up2 += 5
elif power == 1:
if colour == colour1:
if down1 == 0:
speed2 = 200
down1 += 10
else:
if down2 == 0:
speed1 = 200
down2 += 10
else:
clear()
except ValueError:
end = True
winner = "Purple" if colour == colour1 else "Orange"
But even here it's possible to refine this. The else
branches followed by an if
can just be an elif
.
for x, y in snake:
bg = grid[x][y].cget("bg")
if bg == "#222":
grid[x][y].config(bg=colour)
continue
try:
power = powers.index(bg)
if power == 0:
if colour == colour1:
if up1 == 0:
speed1 = 1
up1 += 5
elif up2 == 0:
speed2 = 1
up2 += 5
elif power == 1:
if colour == colour1:
if down1 == 0:
speed2 = 200
down1 += 10
elif down2 == 0:
speed1 = 200
down2 += 10
else:
clear()
except ValueError:
end = True
winner = "Purple" if colour == colour1 else "Orange"
Explore related questions
See similar questions with these tags.