3
\$\begingroup\$

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()
toolic
14.6k5 gold badges29 silver badges203 bronze badges
asked Mar 8, 2018 at 22:57
\$\endgroup\$
11
  • \$\begingroup\$ So to be clear, everything works correctly, it's just very slow to close? \$\endgroup\$ Commented 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\$ Commented Mar 9, 2018 at 0:28
  • \$\begingroup\$ Look at some of the points mentioned in codereview.stackexchange.com/questions/24267/… \$\endgroup\$ Commented 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\$ Commented 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\$ Commented Mar 9, 2018 at 19:11

2 Answers 2

2
\$\begingroup\$

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
answered Jul 13 at 13:21
\$\endgroup\$
2
\$\begingroup\$

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"
answered yesterday
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.