patternpythonMinor
Flappy Bird clone
Viewed 0 times
birdcloneflappy
Problem
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:
(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:\
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:\
Solution
Before you try to convert your code into objects and classes, there's a couple of other places that the code can be improved.
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).
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.
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:
There's also some parts that I thought could maybe use constants.
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.
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.
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 = 20I 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 = 4There'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.
Code Snippets
def getgoing():
...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")white = (255, 255, 255)
movey = 4
movex = 0
widthofwall = 20if 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)def anotherdraw(scorecarddisp, width, height,firstwall1,firstwall2,secondwall1,secondwall2,pillar1dest,pillar2dest,pillar3dest,pillar4dest,bird,birdimg,background):Context
StackExchange Code Review Q#146054, answer score: 3
Revisions (0)
No revisions yet.