HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Ping-pong game with JavaScript and Canvas

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pongwithjavascriptpinggameandcanvas

Problem

I have created my first ping-pong game with JavaScript and Canvas. I will be glad if anybody can tell me how to optimize the code and functionality.

https://github.com/nikitalarionov/js-pong

First collision detection:

//Function to check collision between ball and one of
//the paddles
function collides(b, p) {

    if(b.x + ball.r >= p.x + p.w && b.x - ball.r = p.y && b.y + b.r  W / 2) {
                hit2++;
            }
            return true;
        }
        else if(b.y - b.r = p.y - p.h && b.x + b.r >= p.x) {
            paddleHit = 2;
            if(b.x  W / 2) {
                hit2++;
            }                
            return true;
        }
        else return false;

    }
}


I experience bad performance when the ball hits an angle of one of the paddles:

// Function that updates everything, score, positions, main game logic
function update(){
    // Move the padles
    for(var i = 0; i = H) {
                p.y = H - p.h;
            }
            else {
                p.y += 25;
            }
        }
        if (mouse.y && i === 1) {
            if (p.y + p.h >= H && mouse.y > p.y) {
                p.y = H - p.h;
            }
            else {
                p.y = (mouse.y -50) - p.w / 10;    
            }
        }
    }

            // ... some code

}


There I have very lazy animation and moving of the left paddle. I use keys for it outside of the animation cycle:

// Add Global Event to handle keyboards buttons
window.addEventListener('keydown', function(e) {
    if (e.keyCode === 87) {
        players[0].moveUp = true;
    } else
    if (e.keyCode === 83) {
        players[0].moveDown = true;
    }
}, false);

window.addEventListener('keyup', function(e) {
    if (e.keyCode === 87) {
        players[0].moveUp = false;
    } else
    if (e.keyCode === 83) {
        players[0].moveDown = false;
    }
}, false);

Solution

From a once-over,

  • Short 1 letter variables are terrible ( except for x and y ), b, p and W are just terrible



  • Also please spellcheck your variables: padles -> paddles



-
You should only detect collision when the ball is going to the left and gets in the danger-zone or when it is going to the right and getting in the danger-zone. So instead of doing 8 accesses, 1 AND and 4 sums (if(b.x + ball.r >= p.x + p.w && b.x - ball.r

  • Please don't use magical constants, in this case I see 25` all over the place



  • Same for the keycodes, you should use constants for those

Code Snippets

if( deltaX < 0 && ball.x < someLimit ){
  testCollision( ball , leftPaddle );
} else if( ball.x > someOtherLimit  ) {
  testCollision( ball , rightPaddle );
}

Context

StackExchange Code Review Q#45618, answer score: 6

Revisions (0)

No revisions yet.