Page 1 of 1

Project pong! (opinions on code needed!)

Posted: Thu Jun 06, 2013 2:53 pm
by RunningGamesStudios
Hello all I would like to show you guys my first ever game in love!

Please tell me how I can improve my code because I think it looks ugly.

Re: Project pong! (opinions on code needed!)

Posted: Thu Jun 06, 2013 3:27 pm
by micha
Hi,

first of all: congratulation for you first game.

Here are my suggestion for better code:
  • Try to avoid numbers in your code. By that, of course, I don't mean numbers in general, but those, that might change, when you change your code. For example, in your code the condition for one player to get points is

    Code: Select all

    if ball.x > 799 then
    This is better:

    Code: Select all

    if ball.x > w-1
    because you saved the window width in the variable w. Now, if you change screen resolution, it will still work correctly. The idea is to remove redundancy.
  • Use dt consistently, that means either everywhere or nowhere. Better is everywhere. For example this:

    Code: Select all

    if love.keyboard.isDown("s") then
      p1.y = p1.y + p1.speed
    end
    Should be

    Code: Select all

    if love.keyboard.isDown("s") then
      p1.y = p1.y + p1.speed*dt
    end
    And the p1.speed should be adapted (you need a larger value).
  • For the ball movement, you should use velocity. Right now you store the balls speed in one variable and the balls direction in four booleans leftDown, leftUp, rightDown, and rightUp. This is again redundant and you could construct contradictions, if two or more of the booleans are true. So better introduce two numbers to store the x- and y-velocity. The updating of the ball position is then

    Code: Select all

    ball.x = ball.x + ball.vx * dt
    ball.y = ball.y + ball.vy * dt
    No if/then statement is needed then. To reflect the ball, say on the top you go

    Code: Select all

    ball.vy = -ball.vy
    or a bit more robust:

    Code: Select all

    ball.vy = math.abs(ball.vy)
  • For the paddles, better use a rectangle

    Code: Select all

    love.graphics.rectangle('fill',p1.x,p1.y,p1.w,p1.h)
  • And last, the collision check with the paddle

    Code: Select all

    if ball.x < p1.x + 10 and ball.y > p1.y and ball.x > p1.x + p1.w and ball.y < p1.y + p1.h then
    It does not check what radius the ball has (10 is hard coded). You should add a variable for the balls radius.
Hope that helps.
Edit: One last suggestion:
  • If you are planing to make a larger game, you should put some operations/calculations into functions. That makes the code more readable.

Re: Project pong! (opinions on code needed!)

Posted: Thu Jun 06, 2013 4:37 pm
by RunningGamesStudios
micha wrote:Hi,

first of all: congratulation for you first game.

Here are my suggestion for better code:
  • Try to avoid numbers in your code. By that, of course, I don't mean numbers in general, but those, that might change, when you change your code. For example, in your code the condition for one player to get points is

    Code: Select all

    if ball.x > 799 then
    This is better:

    Code: Select all

    if ball.x > w-1
    because you saved the window width in the variable w. Now, if you change screen resolution, it will still work correctly. The idea is to remove redundancy.
  • Use dt consistently, that means either everywhere or nowhere. Better is everywhere. For example this:

    Code: Select all

    if love.keyboard.isDown("s") then
      p1.y = p1.y + p1.speed
    end
    Should be

    Code: Select all

    if love.keyboard.isDown("s") then
      p1.y = p1.y + p1.speed*dt
    end
    And the p1.speed should be adapted (you need a larger value).
  • For the ball movement, you should use velocity. Right now you store the balls speed in one variable and the balls direction in four booleans leftDown, leftUp, rightDown, and rightUp. This is again redundant and you could construct contradictions, if two or more of the booleans are true. So better introduce two numbers to store the x- and y-velocity. The updating of the ball position is then

    Code: Select all

    ball.x = ball.x + ball.vx * dt
    ball.y = ball.y + ball.vy * dt
    No if/then statement is needed then. To reflect the ball, say on the top you go

    Code: Select all

    ball.vy = -ball.vy
    or a bit more robust:

    Code: Select all

    ball.vy = math.abs(ball.vy)
  • For the paddles, better use a rectangle

    Code: Select all

    love.graphics.rectangle('fill',p1.x,p1.y,p1.w,p1.h)
  • And last, the collision check with the paddle

    Code: Select all

    if ball.x < p1.x + 10 and ball.y > p1.y and ball.x > p1.x + p1.w and ball.y < p1.y + p1.h then
    It does not check what radius the ball has (10 is hard coded). You should add a variable for the balls radius.
Hope that helps.
Edit: One last suggestion:
  • If you are planing to make a larger game, you should put some operations/calculations into functions. That makes the code more readable.
Thank you for the very good tips! I will keep them in mind with my next project. Thank you!