Python Turtle based Pong game

Question

I'm new to Python but I've coded in other languages mainly for hardware. I made Pong in Python using turtle but it's a little glitchy. I was wondering if any of you guys could check it out and give advice. If you have any advice on how to improve collision detection let me know please.

import os
import math
import random
import time
import turtle

#set up screen
screen = turtle.Screen()
screen.bgcolor("green")
screen.title("Pong")

# set up border
border_pen = turtle.Turtle()
border_pen.speed(0)
border_pen.color("white")
border_pen.penup()
border_pen.setposition(-300,-300)
border_pen.pendown()
border_pen.pensize(3)
for side in range(4):
    border_pen.fd(600)
    border_pen.lt(90)
border_pen.hideturtle()

#set score to 0
score = 0

#set time to zero
time = 0
seconds = 0

#Draw score
score_pen = turtle.Turtle()
score_pen.speed(0)
score_pen.color("white")
score_pen.penup()
score_pen.setposition(-290, 310)
scorestring = "Score %s" %score
score_pen.write(scorestring, False, align="left", font=     ("Arial", 14, "normal"))
score_pen.hideturtle()

#Draw timer
time_pen = turtle.Turtle()
time_pen.speed(0)
time_pen.color("white")
time_pen.penup()
time_pen.setposition(260, 310)
timestring = "Time %s" %time
time_pen.write(timestring, False, align="left", font= ("Arial", 14, "normal"))
time_pen.hideturtle()

#create the player turtle
player = turtle.Turtle()
player.color("blue")
player.shape("square")
player.shapesize(0.5, 4)
player.penup()
player.speed(0)
player.setposition(-280,-250)#(x,y)
player.setheading(90)
playerspeed = 15

#create the AIplayer turtle
AIplayer = turtle.Turtle()
AIplayer.color("black")
AIplayer.shape("square")
AIplayer.shapesize(0.5, 4)
AIplayer.penup()
AIplayer.speed(0)
AIplayer.setposition(280,250)#(x,y)
AIplayer.setheading(90)
AIplayerspeed = 15


#create the pong
pong = turtle.Turtle()
pong.color("red")
pong.shape("circle")
pong.shapesize(0.5, 0.5)
pong.penup()
pong.speed(10)
pong.setposition(0,0)#(x,y)
pongspeed = 15
pong.goto(0, 265)
pong.dy = -5
pong.dx = 5


#Move player up and down
def move_up():
    y = player.ycor()
    y += playerspeed
    if y > 265:
        y = 260
    player.sety(y)

def move_down():
    y = player.ycor()
    y -= playerspeed
    if y < -265:
        y = -260
    player.sety(y)

#keyboard bindings
turtle.listen()
turtle.onkey(move_up, "Up")
turtle.onkey(move_down, "Down")
#turtle.onkey(fire_bullet, "space")

def isCollision(t1, t2):
    distance = math.sqrt(math.pow(t1.xcor()-    t2.xcor(),2)+math.pow(t1.ycor()-t2.ycor(),2))
    if distance < 20:
        return True
    else:
        return False

#main game loop
while True:

    #move pong ball
    pong.sety(pong.ycor() +pong.dy)
    pong.setx(pong.xcor() +pong.dx)

    #check for bounce and redirect it
    if pong.ycor() < -300:
        pong.dy *= -1
    if pong.ycor() > 300:
        pong.dy *= -1
    if pong.xcor() < -300:
        pong.dx *= -1
        print("Game Over")
        exit()
    if pong.xcor() > 300:
        pong.dx *= -1

    #move AI paddle (might speed up pong movement)
    y = pong.ycor()
    y += AIplayerspeed
    AIplayer.sety(y)
    if AIplayer.ycor() > 265:
        AIplayerspeed *= -1       
    if AIplayer.ycor() < -250:
        AIplayerspeed *= -1

    #collision pong and player
    if isCollision(pong, player):
        pong.dy *= -1
        pong.dx *= -1
        #Update the score
        score += 10
        scorestring = "Score: %s" %score
        score_pen.clear()
        score_pen.write(scorestring, False, align="left", font=("Arial", 14, "normal"))

    #collision pong and AIplayer
    if isCollision(pong, AIplayer):
        pong.dy *= -1
        pong.dx *= -1

    #updates timer and increases ball speed
    if seconds > 29:
        pong.dy *= -2
        pong.dx *= -2

    if seconds > 59:
        pong.dy *= -3
        pong.dx *= -3

     #displays timer but makes game laggy
#    seconds += 0.1
#    time = seconds
#    timestring = "Time: %s" %time
#    time_pen.clear()
#    time_pen.write(timestring, False, align="Left", font=("Arial", 14, "normal"))

Answer 1

First - You have numeric constants scattered all over the code.

Try to change the board width to 500 and you will immediately understand. Numeric constants like board width shall appear exactly once when assigned to a constant which is used throughout the code lateron. This applies to board size, pong size and player size.

define

board_width = 600
board_height = 600

and use it to draw the border like

for side in range(2):
    border_pen.fd(board_width)
    border_pen.lt(90)
    border_pen.fd(board_height)
    border_pen.lt(90)

Second - you have calculated dependent numeric constants

If constants depend on others you shall not evaluate these. Try to change the bong size and you will have a hell of a work to find all depending values. You are hiding the dependencies and again make your code unmaintainable. All constants that have dependencies shall be initialized not by numeric values, but by expressions using the first class constants.

replace

score_pen.setposition(-290, 310)

with

score_pen.setposition(-board_width/2 + 10, board_height/2 + 10)

Third - structure your code

Whenever you feel you need a comment to explain what a block of code is doing you probably found a function (or a class). if you chose nice function names comments may be obsolete. Also you have less name collisions on local variables leading to better names.

# set up border
border_pen = turtle.Turtle()
#[...]
border_pen.hideturtle()

should be

def set_up_border():
    pen = turtle.Turtle()
    #[...]
    pen.hideturtle()

also

#Draw score
score_pen = turtle.Turtle()
#[...]
score_pen.hideturtle()

should be

def create_score():
    pen = turtle.Turtle()
    #[...]
    pen.hideturtle()
    return pen

score_pen = create_score()

Fourth - reuse code and do not repeat yourself

When you now look at your new functions create_score() and create_timer() you will notice they are nearly identical. They only differ in position and the string displayed. But they share font, color and so on. So we add some parameters and unify these

def create_text(pos, txt):
    pen = turtle.Turtle()
    pen.speed(0)
    pen.color("white")
    pen.penup()
    pen.setposition(pos)
    pen.write(txt, False, align="left", font= ("Arial", 14, "normal"))
    pen.hideturtle()
    return pen

which we use like

score_pen = create_text(score_position, "Score %s" % score)
time_pen = create_text(time_position, "Time %s" % time)

the same way we can unify player and AIplayer. If you want to change player size there is now a single line of code to change. Actually these players, text fields and the bong should be classes but for now we leave it like that.

All the points up to now are valid for any programming language and absolutely important. Maintainability, readability and testability count.

Some algorithmic issues

Bouncing

An object bouncing on another one may not penetrate it. so it is not sufficient to reverse the direction when a collision is detected, but you also have to exit the foreign object immediately. this is done by "mirroring" the entered object to the outside again. So instead of

#move pong ball
pong.sety(pong.ycor() +pong.dy)
#[...]

#check for bounce and redirect it
if pong.ycor() < -300:
    pong.dy *= -1
#[...]

we do not draw immediately but fix penetration before updating the screen

#move pong ball
y = pong.ycor() + pong.dy
#[...]

#check for bounce and redirect it
if y < -board_height/2:
    pong.dy *= -1
    y = -board_height - y
#[...]

pong.sety(y)
#[...]

BTW: Game over is not recognised if the AI player would miss the bong.

Tower running

Setting x and y coordinates ii separate calls where the pen is down results in the pong performing tower running instead of smooth angular movement.

AIplayerspeed

Your AI player does not move with its own speed but with bong speed as it directly takes bong y position. AIplayerspeed acts as an offset to bong y position. This should be fixed to

• either rename and calculated this offset from bong and player size to be center aligned
• or implemented as a max speed to approach bong y position (again centeraligned)

math.sqrt

square root is an expensive function which in game design is avoided wherever possible. In your case you coud simply square the max distance

distance = math.sqrt(math.pow(t1.xcor()-    t2.xcor(),2)+math.pow(t1.ycor()-t2.ycor(),2))
if distance < 20:
    #[...]

becomes

sq_distance = math.pow(t1.xcor() - t2.xcor(), 2) + math.pow(t1.ycor() - t2.ycor(), 2)
if distance < math.pow(20, 2):
    #[...]

Answer 2

Okay, thanks for fixing up the code. Now, to make it more pythonic, we need to move all the statements (not functions) into the entry point, which is a

if __name__ == "__main__":
    # set up screen
    screen = turtle.Screen()
    screen.bgcolor("green")
    screen.title("Pong")

etc. The code still runs as expected, and now we can look at what needs improving with the code. My IDE says you have 3 imports which are unused. I've ripped those out.
I also see a lot of comments which state the obvious, for example, above "set up screen", and below:

 # main game loop
 while True:

You don't need comments in your code if you've written code which expresses what you're doing. The comment # check for bounce and redirect it is useful, as the code doesn't explain in clear english what it does (so :thumbsup: there).

Now we get a little deeper, and see all the if statements. A famous programmer said (in many ways) "every if statement is a function waiting to happen". Let's look at one chunk of your code:

    # collision pong and player
    if isCollision(pong, player):
        pong.dy *= -1
        pong.dx *= -1
        # Update the score
        score += 10
        scorestring = "Score: %s" % score
        score_pen.clear()
        score_pen.write(scorestring, False, align="left", font=("Arial", 14, "normal"))

Here we see four things - first is the check for collision, next is a modification to the x and y, then we update the score, and write something to the display.

def isCollision(t1, t2):
    distance = math.sqrt(math.pow(t1.xcor() - t2.xcor(), 2) + math.pow(t1.ycor() - t2.ycor(), 2))
    if distance < 20:
        return True
    else:
        return False

This looks pretty straight forward, if the distance is less than 20, return a bool. However, if we follow the Single Responsibility Principal, this function is actually doing 3 thing. Calculation, determination and returning a result. Let's split it up:

def collision_calculation(t1, t2):
    return math.sqrt(math.pow(t1.xcor() - t2.xcor(), 2) + math.pow(t1.ycor() - t2.ycor(), 2))

def is_collision(t1, t2):
    return collision_calculation(t1, t2) < 20

Now we have a calculation function that only performs a calculation, and the isCollision function returning True or False. I also renamed your function into snake_case, this is preferred when using Python.

As we've updated that, we can now go back to the large if statement. Given a collision, we update the pong, so let's extract that into a separate function:

def change_pong_direction(pong):
    pong.dy *= -1
    pong.dx *= -1
    return pong

As we're trying to keep the variables in the main loop the same, we need to pass them into the function and get them out again after the state has been modified. I don't like the name "change_pong_direction", but for the moment it will do.

    if is_collision(pong, player):
        pong = change_pong_direction(pong)

Let's look at the scoring function now. As mentioned above, we are actually doing several things. Incrementing the score, and changing the display. Let's separate those commands into functions.

def increment_score(score):
    new_score = score + 10
    return new_score

I've actually added another variable as part of this function, just to make it a little clearer. Of course you can improve it, but it looks like there might be an opportunity to adjust the 10 at a later stage. Such as for games that last longer than a minute, to increase the per-score points. Anyway, continuing on... we need to now write the score to the display:

def write_score(message):
    score_pen.clear()
    score_pen.write("Score: {}".format(message), False, align="left", font=("Arial", 14, "normal"))

See how the string has been embedded into the function? This (might) make it easier to introduce a "display" interface later on, if the score_pen changes to something else, you can easily inject the message into a different function.
Alright! Here is the if statement in its final form:

if is_collision(pong, player):
    pong = change_pong_direction(pong)
    score = increment_score(score)
    write_score(score)

See how easy this code is to read? This is the sort of thing you can do with Python (or all code). Obviously this is only a small part of your code and you have a lot more to change, but I can already see some code duplication right below the if statement we just improved:

    # collision pong and AIplayer
    if is_collision(pong, AIplayer):
        pong.dy *= -1
        pong.dx *= -1

What would you change the last 2 lines into? (only a single line now, yes?)

Hope this helps you to improve your code.