If anyone has the patience to take a look at my first python snake game, I'd be very grateful for any feedback. I am fairly new to programming and Python, but am looking to improve so any constructive feedback is appreciated.
import Tkinter
import time
import random
UP = u'\uf700'
DOWN = u'\uf701'
LEFT = u"\uf702"
RIGHT = u'\uf703'
TIMESTEP = 50
WIDTH = 300
HEIGHT = 300
colours = ['blue','red','pink','yellow','cyan','magenta','green']
class game(object):
def __init__(self):
self.window = Tkinter.Tk()
self.canvas = Tkinter.Canvas(self.window, width = WIDTH, height = HEIGHT)
self.canvas.pack()
self.leader = Rect(self.canvas,50,50,60,60,'red',10,0)
self.rect2 = Rect(self.canvas,40,50,50,60,'blue',10,0)
self.test1 = Rect(self.canvas,100,100,110,110,'pink',0,0)
self.followers = [self.rect2,self.test1,Rect(self.canvas,200,200,210,210,'yellow',0,0)]
def moveit(self):
# moves the leader and the followers follow!
prevpos = (self.leader.coords()[0],self.leader.coords()[1])
self.leader.move(self.leader.getvel()[0],self.leader.getvel()[1])
for rect in self.followers:
x = (rect.coords()[0],rect.coords()[1])
self.moveto(rect,prevpos)
prevpos = x
self.eatapple()
self.selfcollision()
self.throughwalls()
self.window.after(TIMESTEP,self.moveit)
def moveto(self,box1,pos):
#moves box1 to pos
box1.move(pos[0]-box1.coords()[0],pos[1]-box1.coords()[1])
def makeapple(self):
#This randomly puts an apple on the canvas
x1 = random.randrange(0,WIDTH,10)
y1 = random.randrange(0,HEIGHT,10)
self.apple = Rect(self.canvas,x1,y1,x1+10,y1+10, 'purple',0,0)
def eatapple(self):
#This check for eating apple, if eaten, creates new apple and grows the snake
if self.leader.coords() == self.apple.coords():
self.canvas.delete(self.apple.id)
self.makeapple()
self.grow()
def grow(self):
# grows the snake by appending a rectangle to the followers list
x1 = self.followers[-1].coords()[0]
y1 = self.followers[-1].coords()[1]
x2 = self.followers[-1].coords()[2]
y2 = self.followers[-1].coords()[3]
self.followers.append(Rect(self.canvas,x1,y1,x2,y2,colours[random.randint(0,len(colours)-1)],0,0))
def selfcollision(self):
# This will check for collision with itself
for follower in self.followers:
if self.leader.coords() == follower.coords():
print "You lose!"
self.window.destroy()
def throughwalls(self):
# This will put the snake on the klein bottle geometry!
x1,y1,x2,y2 = self.leader.coords()
if x1>=WIDTH:
self.moveto(self.leader,(0,HEIGHT-y1))
if x1<0:
self.moveto(self.leader,(WIDTH,HEIGHT-y1))
if y1>= HEIGHT:
self.moveto(self.leader,(x1,0))
if y1< 0:
self.moveto(self.leader,(x1,HEIGHT))
def keyup(self,event):
if self.leader.velocity != (0,10):
self.leader.velocity=(0,-10)
#print "UP!"
def keydown(self,event):
if self.leader.velocity != (0,-10):
self.leader.velocity = (0,10)
#print "DOWN!"
def keyright(self,event):
if self.leader.velocity != (-10,0):
self.leader.velocity = (10,0)
def keyleft(self,event):
if self.leader.velocity != (10,0):
self.leader.velocity = (-10,0)
def mainloop(self):
self.window.bind("<Key-Up>",self.keyup)
self.window.bind("<Key-Down>",self.keydown)
self.window.bind("<Key-Left>",self.keyleft)
self.window.bind("<Key-Right>",self.keyright)
self.makeapple()
self.moveit()
self.window.mainloop()
class Rect(object):
def __init__(self,canvas,x1,y1,x2,y2,color,vx,vy):
self.canvas = canvas
self.velocity = vx,vy
self.id = self.canvas.create_rectangle(x1,y1,x2,y2,fill = color)
def getvel(self):
return self.velocity
def coords(self):
return self.canvas.coords(self.id)
#self.root = Tkin
def move(self,vx,vy):
self.canvas.move(self.id,vx,vy)
canvas = game()
canvas.mainloop()
while True:
ans=raw_input("Another game? (y/n) ")
if ans== 'n':
break
elif ans == 'y':
canvas = game()
canvas.mainloop()
else:
print "What was that?"
Also, this snake lives on the Klein bottle, as opposed to the usual Torus snake.
One specific question:
Games such as this require a loop that moves the blocks around the canvas. One can achieve this with a while loop, however I read somewhere that this was not advised as there is already a mainloop() running. Instead, one should use Tkinter's "after" function. Can anyone explain why this is favourable?
-
1\$\begingroup\$ Does this currently run for you? I am not able to run it as is and am encountering several errors. \$\endgroup\$StrugglingProgrammer– StrugglingProgrammer2015年08月25日 20:56:15 +00:00Commented Aug 25, 2015 at 20:56
-
\$\begingroup\$ @benalbrecht Sorry. No it doesnt run for me either. I think copying it into the question has introduced some formatting errors. It there are better way to enter code than just copy and past? \$\endgroup\$Moss– Moss2015年08月25日 21:01:23 +00:00Commented Aug 25, 2015 at 21:01
-
\$\begingroup\$ Or perhaps I should ask, how does one correctly paste code into a SE question? \$\endgroup\$Moss– Moss2015年08月25日 21:05:14 +00:00Commented Aug 25, 2015 at 21:05
-
\$\begingroup\$ One easy way: paste the code, then select (highlight) the code using the mouse, and press Control-K : you will notice it shifts all the selected text to the right by 4 spaces \$\endgroup\$janos– janos2015年08月25日 21:07:41 +00:00Commented Aug 25, 2015 at 21:07
-
\$\begingroup\$ Got it, thanks. But I still seem to have a problem running the code... \$\endgroup\$Moss– Moss2015年08月25日 21:10:54 +00:00Commented Aug 25, 2015 at 21:10
1 Answer 1
Coding style
Please follow PEP8, the style guide of Python. The code will become so much easier to read.
More tuple-assignments please
Use tuple-assignments more aggressively. For example, instead of this:
x1 = self.followers[-1].coords()[0] y1 = self.followers[-1].coords()[1] x2 = self.followers[-1].coords()[2] y2 = self.followers[-1].coords()[3]
This is simpler:
x1, y1, x2, y2 = self.followers[-1].coords()
Avoid repeated calls
Use tuple/list slicing more aggressively. For example, these expressions are long and tedious:
prevpos = (self.leader.coords()[0],self.leader.coords()[1]) self.leader.move(self.leader.getvel()[0],self.leader.getvel()[1])
This is simpler:
prevpos = self.leader.coords()[:2]
self.leader.move(*self.leader.getvel()[:2])
Not only this is simpler, but it avoids making repeated method calls.
Use elif
These conditions are mutually exclusive:
if x1>=WIDTH: self.moveto(self.leader,(0,HEIGHT-y1)) if x1<0: self.moveto(self.leader,(WIDTH,HEIGHT-y1))
As they cannot happen at the same time, you should chain them with an elif
.
if x1>=WIDTH:
self.moveto(self.leader,(0,HEIGHT-y1))
elif x1<0:
self.moveto(self.leader,(WIDTH,HEIGHT-y1))
Strange API
Most of the functions of game
are not natural:
moveto
eatapple
grow
selfcollision
throughwalls
A game that moves? And eats apple?
Methods should represent actions that can be performed on the class. Since "selfcollision" and "throughwalls" are not verbs, it's not trivial what they represent. All these methods make for a very strange API.