7
\$\begingroup\$

I am a beginner in Pygame (and programming in general) and tried to make a Flappy Bird game. It works fine, but I'm sure the code has a lot of scope for improvement. It is completely procedural, and while I do want to make it object oriented to clean it up, I'm not sure how to go about it.

For example: Bird will be an object, but what about the wall pairs? Will all wall pairs be one object? Will one wall pair be one object? It is all getting a bit confusing.

(Also, I'm aware they are not device independent paths.)

import pygame,sys,random
pygame.init()
def getgoing():
    size=width,height=300,510
    screen = pygame.display.set_mode(size)
    pygame.display.set_caption("GK Flappy bird")
    x, y = 10, height / 2
    bird = pygame.Rect(x, y, 36, 26)
    white = (255, 255, 255)
    movey = 4
    movex = 0
    widthofwall = 20
    clock = pygame.time.Clock()
    firstrectlength = random.randint(50, height / 2)
    secondrectlength = height - 150 - firstrectlength
    firstrect = pygame.Rect(200, 0, widthofwall, firstrectlength)
    secondrect = pygame.Rect(200, height - secondrectlength, widthofwall, secondrectlength)
    walllist = [firstrect, secondrect]
    # len=1
    pygame.time.set_timer(25, 2000)
    flapsound=pygame.mixer.Sound("C:\Users\GGK\Downloads\\flap.wav")
    crashsound=pygame.mixer.Sound("C:\Users\GGK\Downloads\hurt.wav")
    scoresound=pygame.mixer.Sound("C:\Users\GGK\Downloads\score.wav")
    filx = 0
    ct = 12
    score = 0
    count = 1
    black = (0, 0, 0)
    frame = 0

    qa = 1

    birdimg = pygame.image.load("C:\\Users\\GGK\Downloads\\bird_sing.png")

    background = pygame.image.load("C:\\Users\\GGK\Downloads\\bg.png")
    background = pygame.transform.scale(background, (width, height))

    pillar1 = pygame.image.load("C:\Users\GGK\Downloads\\tube1.png")
    pillar2 = pygame.image.load("C:\Users\GGK\Downloads\\tube2.png")
    pillar3 = pygame.image.load("C:\Users\GGK\Downloads\\tube1.png")
    pillar4 = pygame.image.load("C:\Users\GGK\Downloads\\tube2.png")
    pillar5 = pygame.image.load("C:\Users\GGK\Downloads\\tube1.png")
    pillar6 = pygame.image.load("C:\Users\GGK\Downloads\\tube2.png")

    for i in range(5000):
        firstrectlength = random.randint(50, height / 2)
        secondrectlength = height - 150 - firstrectlength
        firstrect = pygame.Rect(200, 0, widthofwall, firstrectlength)
        secondrect = pygame.Rect(200, height - secondrectlength, widthofwall, secondrectlength)
        walllist.append(firstrect)
        walllist.append(secondrect)

    backrect = pygame.Rect(0, 0, width, height)
    firstwall1 = walllist[filx]
    firstwall2 = walllist[filx + 1]
    secondwall1 = walllist[filx + 2]
    secondwall2 = walllist[filx + 3]
    thirdwall1 = walllist[filx + 4]
    thirdwall2 = walllist[filx + 5]

    secondwall1 = secondwall1.move(150, 0)
    secondwall2 = secondwall2.move(150, 0)
    thirdwall1 = thirdwall1.move(300, 0)
    thirdwall2 = thirdwall2.move(300, 0)
    pillar1dest = pygame.transform.scale(pillar1, (widthofwall, firstwall1.height))
    pillar3dest = pygame.transform.scale(pillar3, (widthofwall, secondwall1.height))
    pillar5dest = pygame.transform.scale(pillar5, (widthofwall, thirdwall1.height))

    pillar2dest = pygame.transform.scale(pillar2, (widthofwall, firstwall2.height))
    pillar4dest = pygame.transform.scale(pillar4, (widthofwall, secondwall2.height))
    pillar6dest = pygame.transform.scale(pillar6, (widthofwall, thirdwall2.height))

    # pygame.key.set_repeat(5,5)

    while True:

        clock.tick(120)
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                sys.exit()
            if event.type == pygame.KEYDOWN:
                if event.key == pygame.K_SPACE:
                    movey = -90   
                    bird = bird.move(movex, movey)
                    flapsound.play()
                    movey = 4
            if event.type == 25:
                firstrectlength = random.randint(50, height / 2)
                secondrectlength = height - 50 - firstrectlength
                firstrect = pygame.Rect(200, 0, widthofwall, firstrectlength)
                secondrect = pygame.Rect(200, height - secondrectlength, widthofwall, secondrectlength)

        if bird.colliderect(firstwall1) or bird.colliderect(firstwall2):
            scorecarddisp=collided(score,width,height)
            crashsound.play()
            anotherdraw(scorecarddisp, width, height,firstwall1,firstwall2,secondwall1,secondwall2,pillar1dest,pillar2dest,pillar3dest,pillar4dest,bird,birdimg,background)
            break
        elif bird.left > firstwall1.right and qa == 1:
            score += 1
            scoresound.play()
            qa = 0

        frame += 1
        if frame == 2:
            bird = bird.move(movex, movey)
            frame = 0

        movey = 4

        firstwall1 = firstwall1.move(-1, 0)
        firstwall2 = firstwall2.move(-1, 0)

        secondwall1 = secondwall1.move(-1, 0)
        secondwall2 = secondwall2.move(-1, 0)
        thirdwall1 = thirdwall1.move(-1, 0)
        thirdwall2 = thirdwall2.move(-1, 0)

        if firstwall1.left == -widthofwall:
            filx = filx + 6
            firstwall1 = secondwall1
            firstwall2 = secondwall2
            secdist = secondwall1.left
            secondwall1 = thirdwall1
            secondwall2 = thirdwall2
            dist = thirdwall1.left
            thirdwall1 = walllist[filx]
            thirdwall2 = walllist[filx + 1]

            thirdwall1.move_ip(230, 0)
            # thirdwall1.left=dist
            # thirdwall2.left=dist

            thirdwall2.move_ip(230, 0)

            pillar1dest = pygame.transform.scale(pillar1, (widthofwall, firstwall1.height))
            pillar3dest = pygame.transform.scale(pillar3, (widthofwall, secondwall1.height))
            pillar5dest = pygame.transform.scale(pillar5, (widthofwall, thirdwall1.height))

            pillar2dest = pygame.transform.scale(pillar2, (widthofwall, firstwall2.height))
            pillar4dest = pygame.transform.scale(pillar4, (widthofwall, secondwall2.height))
            pillar6dest = pygame.transform.scale(pillar6, (widthofwall, thirdwall2.height))
            qa = 1

        if bird.bottom >= height:
            bird.bottom = height
        if bird.top <= 0:
            bird.top = 0



            # count+=1
            #  if count==9:
            #     if bird.left>firstwall1.right or bird.left>secondwall1.right or bird.left>thirdwall1.right:
            #        score+=1

            #        count=0
        print score

        screen.fill((0, 0, 0))
        screen.blit(background, (0, 0))
        #    pygame.draw.rect(screen,black,bird,0)
        #    pygame.draw.rect(screen,white,firstwall1,0)
        #    pygame.draw.rect(screen, white,firstwall2, 0)
        #    pygame.draw.rect(screen, white, secondwall1, 0)
        #    pygame.draw.rect(screen, white, secondwall2, 0)
        #    pygame.draw.rect(screen, white, thirdwall1, 0)
        #    pygame.draw.rect(screen, white, thirdwall2, 0)
        screen.blit(pillar1dest, firstwall1)
        screen.blit(pillar3dest, secondwall1)
        screen.blit(pillar5dest, thirdwall1)
        screen.blit(pillar2dest, firstwall2)
        screen.blit(pillar4dest, secondwall2)
        screen.blit(pillar6dest, thirdwall2)

        screen.blit(birdimg, bird)

        pygame.display.flip()
def collided(score,width,height):
    scorefont=pygame.font.get_default_font()
    scorecard=pygame.font.Font(scorefont,50)
    scorecarddisp=scorecard.render(str(score),0,(0,0,0))
    return scorecarddisp






def anotherdraw(scorecarddisp, width, height,firstwall1,firstwall2,secondwall1,secondwall2,pillar1dest,pillar2dest,pillar3dest,pillar4dest,bird,birdimg,background):
    screen=pygame.display.set_mode()
    scoreimg = pygame.image.load("C:\Users\GGK\Downloads\scoresprite.bmp")
    scoreimg=pygame.transform.scale(scoreimg,(300,200))
    while True:
        for event in pygame.event.get():
            if event.type == pygame.QUIT: sys.exit()
            if event.type == pygame.KEYDOWN:
                if event.key==pygame.K_r:
                    getgoing()

        screen.blit(background,(0,0))
        screen.blit(pillar1dest, firstwall1)
        screen.blit(pillar3dest, secondwall1)
        screen.blit(pillar4dest, secondwall2)

        screen.blit(pillar2dest, firstwall2)
        screen.blit(birdimg,bird)
        #screen.blit(scoreimg,(0,height/2-150))
        screen.blit(scorecarddisp, (width / 2, height / 2))
        pygame.display.flip()


getgoing()
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Before you try to convert your code into objects and classes, there's a couple of other places that the code can be improved.

def getgoing(): 
      ...

This function is absolutely huge. It would probably be helpful to separate the function into separate logically sound functions. For example, you can at your setup operations and your game loop (more separation would probably be good).

pillar3 = pygame.image.load("C:\Users\GGK\Downloads\\tube1.png")
pillar4 = pygame.image.load("C:\Users\GGK\Downloads\\tube2.png")
pillar5 = pygame.image.load("C:\Users\GGK\Downloads\\tube1.png")
pillar6 = pygame.image.load("C:\Users\GGK\Downloads\\tube2.png")

You mentioned already that these are not device independent paths. Aside from that, these string values can placed into a constant somewhere earlier. That way, when you change what "tube1.png" or "tube2.png" are, you can change a single variable - and the change will be reflected everywhere else in your code.

white = (255, 255, 255)
movey = 4
movex = 0
widthofwall = 20

I took these as being constants, but I wasn't able to tell at first glance. Normally, const values are differentiated based on some naming conventions (all uppercase andunderscores to separate words). For example:

MOVE_Y = 4   

There's also some parts that I thought could maybe use constants.

if event.type == 25:
    firstrectlength = random.randint(50, height / 2)
    secondrectlength = height - 50 - firstrectlength
    firstrect = pygame.Rect(200, 0, widthofwall, firstrectlength)
    secondrect = pygame.Rect(200, height - secondrectlength, widthofwall, secondrectlength)  

It wasn't clear what some of these numbers meant (they felt a bit magic). Referring to a constant for these values is probably helpful for improving readability.

def anotherdraw(scorecarddisp, width, height,firstwall1,firstwall2,secondwall1,secondwall2,pillar1dest,pillar2dest,pillar3dest,pillar4dest,bird,birdimg,background):  

That's an awesome lot of arguments. I think I had two points to make about this. It's probably helpful to pass some of these arguments inside of a collection (e.g., a list). Also, it would probably be good to start including docstrings for your functions (and comments in general...but more emphasis on the docstrings).

It is completely procedural, and while I do want to make it object oriented to clean it up

It's not necessarily bad that's it's procedural, and following an OOP format for your code isn't sure to clean it up. I think that starting off with separating your code into smaller functions would be a good direction.

Best of luck.

\$\endgroup\$
2
  • \$\begingroup\$ Hey, thanks for the detailed analysis. I rewrote the code using classes and replaced some unnecessary methods with concise ones, and have improved the physics, so I think I have made it better. I am just adding comments in now, and will be posting it too soon. Thanks again :) \$\endgroup\$ Nov 6, 2016 at 18:02
  • \$\begingroup\$ Happy to help, and welcome to Stack Overflow. If this answer or any other one solved your issue, please mark it as accepted. If I miss your updated post, feel free to send me a message, and I'll be glad to look it over. \$\endgroup\$
    – abrarisme
    Nov 7, 2016 at 2:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.