10
\$\begingroup\$

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?

\$\endgroup\$
  • 1
    \$\begingroup\$ Does this currently run for you? I am not able to run it as is and am encountering several errors. \$\endgroup\$ – StrugglingProgrammer Aug 25 '15 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 Aug 25 '15 at 21:01
  • \$\begingroup\$ Or perhaps I should ask, how does one correctly paste code into a SE question? \$\endgroup\$ – Moss Aug 25 '15 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\$ – Stop ongoing harm to Monica Aug 25 '15 at 21:07
  • \$\begingroup\$ Got it, thanks. But I still seem to have a problem running the code... \$\endgroup\$ – Moss Aug 25 '15 at 21:10
9
\$\begingroup\$

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.

\$\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.