Breakout clone in pygame

Question

As a guy who started coding a month ago, I would like an honest review of my version of breakout that I created using pygame.

import pygame
import random
import sys
#level1
global level1
level1 =[
        [0,0,0,0,1,1,1,0,0,0,0],
        [0,0,0,0,1,1,0,0,0,0,0],
        [0,0,1,1,1,1,1,1,0,0,0],
        [0,0,0,1,0,1,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,1,0,0,1,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,1,0,1,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0],
        [0,0,0,0,0,0,0,0,0,0,0]
        ]
def reset_array(array):
    for line in array:
        for i,num in enumerate(line):
            if num == 2:
                line[i] = 1

#colour
black = (0,0,0)
white = (255,255,255)
red = (255,0,0)
blue = (0,0,255)
#paddle
class paddle:
    def __init__(self,x,y,width,height,vel):
        self.x = x
        self.y = y
        self.width = width
        self.height = height
        self. vel = vel
Paddle = paddle(350,550,80,20,5)
def draw_paddle():
    pygame.draw.rect(screen,red,(Paddle.x,Paddle.y,Paddle.width,Paddle.height))
#ball
class ball:
    def __init__(self,x,y,rad,velx,vely):
        self.x = x
        self.y = y
        self.rad = rad
        self.velx = velx
        self.vely = vely
Ball = ball(30,30,10,2,5)
def ball_move():
    Ball.x -= Ball.velx
    Ball.y -= Ball.vely
#block
class block:
    def __init__(self,x,y,width,height):
        self.x = x
        self.y = y
        self.width = width
        self.height = height
Block = block(0,0,75,35)
def draw_block(self,array):
    x = self.x
    y = self.y
    for line in array:
        for character in line:
            if character == 1:
                pygame.draw.rect(screen,white,(x,y,self.width,self.height))
                pygame.draw.rect(screen,red,(x,y,self.width,self.height),1)
            x += self.width
        y += self.height
        x = 0
def draw_ball():
    pygame.draw.circle(screen,white,(Ball.x,Ball.y), Ball.rad)
#init   
pygame.init()
#window
swidth = 800
sheight = 600
screen = pygame.display.set_mode((swidth,sheight))

def main():
    #gamestart bool
    global gamestart
    gamestart = False
    #player lives
    global player_lives
    player_lives = 3
    #main loop    
    running = True
    while running:
        for event in pygame.event.get():

            if event.type == pygame.QUIT:
                sys.exit()   
        if player_lives <= 0:
            reset_array(level1)
            player_lives = 3
        #paddlemove
        key = pygame.key.get_pressed()
        if key[pygame.K_a]:
            Paddle.x -= Paddle.vel
        if key[pygame.K_d]:
            Paddle.x += Paddle.vel
        if Paddle.x + 80 < 0:
            Paddle.x += swidth
        if Paddle.x >= swidth:
            Paddle.x -= swidth
        #ballmove
        posx = Ball.y
        posy = Ball.x
        row = posx // 35
        column = posy // 75
        if level1[row][column] == 1:
           level1[row][column] = 2
           Ball.vely = -Ball.vely
        if gamestart == False:
            Paddle.x = 350
            Ball.x = Paddle.x + 40
            Ball.y = Paddle.y - 10
            Ball.velx = 2
            Ball.vely = 5    
        if key[pygame.K_SPACE]:
            gamestart = True
        if gamestart == True:
            ball_move()
        if Ball.y <= 0:
            Ball.vely = - Ball.vely
        if Ball.y >= 600:
            gamestart = False
            player_lives -= 1
            print(player_lives)
        if Ball.x <= 0:
            Ball.velx = - Ball.velx
        if Ball.x >= 800:
            Ball.velx = - Ball.velx
        #collision
        if Paddle.x <= Ball.x <= Paddle.x +44 and Paddle.y == Ball.y:
            Ball.vely = - Ball.vely
            if Ball.x > Paddle.x:
                Ball.velx =  Ball.velx
            if Ball.x < Paddle.x:
                Ball.velx = -Ball.velx
        if Paddle.x +45 <= Ball.x <= Paddle.x +80 and Paddle.y == Ball.y:
            Ball.vely = - Ball.vely
            if Ball.x < Paddle.x:
                Ball.velx = - Ball.velx
            if Ball.x > Paddle.x:
                Ball.velx = Ball.velx
        #game end
        victory = False
        for line in level1:
            for col in line:
                if all(all(item != 1 for item in item)for item in level1):
                    victory = True
        if victory == True:
            print("you win")
            break
        screen.fill(blue)
        draw_paddle()
        draw_ball()
        draw_block(Block,level1)
        pygame.display.flip()

main()

Answer 1

#level1
global level1

The comment here is not useful, if you have a variable level1, well... that's level1, right?

The same goes for all your other comments in the code. You should think of comments as a way to describe why you did things that specific way or the meaning of something. If you need them to describe what is happening, most likely you need to refactor something.

Also, as already pointed out in the other answers, this is a global variable, which in this case would not be terrible except that if it's level1, you probably also expect to have level2 and maybe level3, which would make it quite ugly, especially if you hardcode your levels in the module.

You might want to look into loading a text file into a variable. You might have different text files named level1.txt, level2.txt and so on, depending on the level.

class paddle:
    def __init__(self,x,y,width,height,vel):
        self.x = x
        self.y = y
        self.width = width
        self.height = height
        self. vel = vel
Paddle = paddle(350,550,80,20,5)
def draw_paddle():
    pygame.draw.rect(screen,red,(Paddle.x,Paddle.y,Paddle.width,Paddle.height))

You have a class paddle (as already noted it should be Paddle), so why not having the draw inside the class?

That way you'd have something like:

class Paddle:
    def __init__(self,x,y,width,height,vel):
        self.x = x
        self.y = y
        self.width = width
        self.height = height
        self. vel = vel

    def draw(self):
        pygame.draw.rect(screen,red,(self.x, self.y, self.width, self.height))

paddle = Paddle(350,550,80,20,5)
...
paddle.draw()

The same goes for block and ball.

Actually, you could go the extra mile and have a list of objects on which you loop to call the draw and maybe a possible update function.

Something like:

game_objects = [Paddle(350,550,80,20,5), Block(0,0,75,35), Ball(30,30,10,2,5)]
for game_object in game_objects:
    game_object.update()
    game_object.draw()

As already noted, these magic numbers (e.g. row = posx // 35) are better written as constants, even if you use them only once, to explain their meaning.

if Paddle.x <= Ball.x <= Paddle.x +44 and Paddle.y == Ball.y:
    Ball.vely = - Ball.vely
    if Ball.x > Paddle.x:
        Ball.velx =  Ball.velx
    if Ball.x < Paddle.x:
        Ball.velx = -Ball.velx
if Paddle.x +45 <= Ball.x <= Paddle.x +80 and Paddle.y == Ball.y:
    Ball.vely = - Ball.vely
    if Ball.x < Paddle.x:
        Ball.velx = - Ball.velx
    if Ball.x > Paddle.x:
        Ball.velx = Ball.velx

This is a block of code that is prone to errors, because of the multiple conditions check and the multiple nested ifs.

You don't need the Ball.velx = Ball.velx line, that doesn't do anything, which mean that you can remove that check.

There's also an issue with checking a strict Paddle.y == Ball.y because if you increase the speed of the ball, there's the chance that it will go past the paddle's y and not be detected, so you should check if the ball's y plus the ball's height is within the paddle's y plus the paddle's height.

The outer if would also be more readable if you put that in a function and called it with the specific parameters, something like:

if paddle_and_ball_collided(paddle, ball):
    Ball.vely = - Ball.vely
    if Ball.x < Paddle.x:
        Ball.velx = -Ball.velx

You should also look into the integrated collision detection functions from pygame, they can save you a bit of trouble (see e.g. https://stackoverflow.com/questions/29640685/how-do-i-detect-collision-in-pygame )

Answer 2

Welcome to CodeReview!

Formatting

Get an IDE such as PyCharm or VSCode that's capable of linting. It will give you several "PEP8" suggestions, such as newlines around functions, that will make your code more legible. Some particulars that will be suggested:

• paddle = Paddle (classes)
• black = BLACK (global constants)
• Ball = ball and vice versa - instance variables should be lowercase, classes uppercase

Avoid globals

They're a messy way of tracking information. In particular, ball and block instances, and the screen instance should be moved to local variables, maybe class members.

Simplify logic

if Paddle.x + 80 < 0:

becomes

if Paddle.x < -80:

Global code

pygame.init()
#window
swidth = 800
sheight = 600
screen = pygame.display.set_mode((swidth,sheight))

Stuff like this should move into your main function.

Long methods

main should be subdivided into several subroutines for legibility and maintainability.

Magic numbers

"44" doesn't mean much to us, and in six months it probably won't mean much to you either. It's best to put this and similar quantities into named constants.

Nested all

if all(all(item != 1 for item in item)for item in level1):

You don't need an inner all. One level of all and two inner comprehension loops will work.

Level data

Your level1 array is a reasonable representation of the level during program runtime. However, it's verbose to construct manually. You'd probably be better off representing this with a more compact string-based format and parsing it out on program load.