Platformer Game: Arrow Key Player Movement and Collision Detection

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
3
down vote

favorite












I am in the middle of working on this game and I still need to add some things (e.g. score, coin counters, etc.) but I really feel like there is way too much unneeded code. Could you help me simplify this so I can keep adding things?






<!DOCTYPE html>
<html>
<head>
<title>Game</title>
<style>
html, body 
background: linear-gradient(to bottom,#7EC0EE,#C1E5FF);
height: 100%;
width: 100%;
margin: 0;

#player 
background: #AF1C1C;
position: fixed;
height: 20px;
bottom: 0px;
width: 8px;
z-index: 2;
left: 0px;
top: 0px;

#head 
background: #F2CE8C;
position: absolute;
height: 6px;
width: 8px;
left: 0px;
top: 0px;

#hair 
background: #000000;
position: absolute;
height: 2px;
width: 8px;
left: 0px;
top: 0px;

#hair-back 
background: #000000;
position: absolute;
height: 6px;
width: 2px;
left: 0px;
top: 0px;

#arm 
background: #F2CE8C;
position: absolute;
height: 5px;
width: 5px;
left: 0px;
top: 8px;

#feet 
background: #000000;
position: absolute;
bottom: 0px;
height: 4px;
width: 8px;
left: 0px;

#jetpack 
background: linear-gradient(to bottom,#FFF,transparent);
position: fixed;
height: 16px;
width: 8px;
opacity: 0;
z-index: 1;

#ground 
background: green;
position: fixed;
height: 10px;
width: 100%;
bottom: 0px;
z-index: 3;
left: 0px;

#platform 
background: #915921;
position: fixed;
height: 10px;
width: 15%;
z-index: 3;

.coin 
animation: coinflip 6s linear infinite;
transform: rotate(-90deg);
border: 2px inset #D69E10;
background: #EFB112;
position: fixed;
height: 8px;
width: 8px;
z-index: 0;

@keyframes coinflip 
0% transform: rotate(-90deg);filter: brightness(100%);
40% transform: rotate(-90deg);filter: brightness(100%);
50% transform: rotate(-90deg) rotateX(180deg);filter: brightness(60%);
60% transform: rotate(-90deg);filter: brightness(100%);
100% transform: rotate(-90deg);filter: brightness(100%);

</style>
</head>
<body onkeydown="getkey()" onkeyup="stopkey()">
<div id="player">
 <div id="head"></div>
 <div id="hair"></div>
 <div id="hair-back"></div>
 <div id="arm"></div>
 <div id="feet"></div>
</div>
<div id="jetpack"></div>
<div class="object" id="ground"></div>
<script>
var collision = false;
var player = document.getElementById("player");
var jetpack = document.getElementById("jetpack");
var ground = document.getElementById("ground");
var object = document.getElementsByClassName("object");
var platform = document.getElementsByClassName("platform");
var coin = document.getElementsByClassName("coin");
var up = 0;
var key = 0;
var counter = 0;
var addcounter = setInterval(createPlatform,1);
setInterval(collisionDetection,1);
setInterval(gravity,1);
setInterval(controller,1);
setInterval(followplayer,1);

function createPlatform() 
if (counter == 0) 
if (2 * Math.round((window.innerHeight / 15) / 2) < player.offsetHeight) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = player.offsetHeight + 2 + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;

else 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = 2 * Math.round((window.innerHeight / 15) / 2) + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;


if (counter > 0 && counter < 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = 2 * Math.round((newPlatform.offsetTop - 18) / 2) + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

counter++;

if (counter > 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = newPlatform.offsetTop - 18 + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

clearInterval(addcounter);



function gravity() 
if (collision === false) 
player.style.top = player.offsetTop + 1 - up + "px";



function collisionDetection() 
for (i = 0; i < object.length; i++) 
// Player Right to Object Left
if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
 player.offsetLeft + player.offsetWidth >= object[i].offsetLeft &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft - player.offsetWidth + "px"; 
collision = true; 

// Player Left to Object Right
if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
 player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px"; 
collision = true; 

// Player Bottom to Object Top
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
 player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
player.style.top = object[i].offsetTop - player.offsetHeight + "px"; 
collision = true;

// Player Top to Object Bottom
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
 player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
player.style.top = object[i].offsetTop + object[i].offsetHeight + "px"; 
collision = true;

else 
collision = false;




function controller() 
// Left
if (key == 37) 
player.style.left = player.offsetLeft - 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Right
if (key == 39) 
player.style.left = player.offsetLeft + 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Up
if (key == 38) 
up = 2;
jetpack.style.opacity = "1";

else 
up = 0;
jetpack.style.opacity = "0";



function followplayer() 
jetpack.style.top = player.offsetTop + player.offsetHeight + "px";
jetpack.style.left = player.offsetLeft + "px";


function getkey() 
key = event.keyCode;


function stopkey() 
key = 0;

</script>
</body>
</html>






javascript html game css collision




share|improve this question





















  • What happened to the indentation? Could you repaste the code? It's hard to read.
    – t3chb0t
    Jun 7 at 6:25
















up vote
3
down vote

favorite












I am in the middle of working on this game and I still need to add some things (e.g. score, coin counters, etc.) but I really feel like there is way too much unneeded code. Could you help me simplify this so I can keep adding things?






<!DOCTYPE html>
<html>
<head>
<title>Game</title>
<style>
html, body 
background: linear-gradient(to bottom,#7EC0EE,#C1E5FF);
height: 100%;
width: 100%;
margin: 0;

#player 
background: #AF1C1C;
position: fixed;
height: 20px;
bottom: 0px;
width: 8px;
z-index: 2;
left: 0px;
top: 0px;

#head 
background: #F2CE8C;
position: absolute;
height: 6px;
width: 8px;
left: 0px;
top: 0px;

#hair 
background: #000000;
position: absolute;
height: 2px;
width: 8px;
left: 0px;
top: 0px;

#hair-back 
background: #000000;
position: absolute;
height: 6px;
width: 2px;
left: 0px;
top: 0px;

#arm 
background: #F2CE8C;
position: absolute;
height: 5px;
width: 5px;
left: 0px;
top: 8px;

#feet 
background: #000000;
position: absolute;
bottom: 0px;
height: 4px;
width: 8px;
left: 0px;

#jetpack 
background: linear-gradient(to bottom,#FFF,transparent);
position: fixed;
height: 16px;
width: 8px;
opacity: 0;
z-index: 1;

#ground 
background: green;
position: fixed;
height: 10px;
width: 100%;
bottom: 0px;
z-index: 3;
left: 0px;

#platform 
background: #915921;
position: fixed;
height: 10px;
width: 15%;
z-index: 3;

.coin 
animation: coinflip 6s linear infinite;
transform: rotate(-90deg);
border: 2px inset #D69E10;
background: #EFB112;
position: fixed;
height: 8px;
width: 8px;
z-index: 0;

@keyframes coinflip 
0% transform: rotate(-90deg);filter: brightness(100%);
40% transform: rotate(-90deg);filter: brightness(100%);
50% transform: rotate(-90deg) rotateX(180deg);filter: brightness(60%);
60% transform: rotate(-90deg);filter: brightness(100%);
100% transform: rotate(-90deg);filter: brightness(100%);

</style>
</head>
<body onkeydown="getkey()" onkeyup="stopkey()">
<div id="player">
 <div id="head"></div>
 <div id="hair"></div>
 <div id="hair-back"></div>
 <div id="arm"></div>
 <div id="feet"></div>
</div>
<div id="jetpack"></div>
<div class="object" id="ground"></div>
<script>
var collision = false;
var player = document.getElementById("player");
var jetpack = document.getElementById("jetpack");
var ground = document.getElementById("ground");
var object = document.getElementsByClassName("object");
var platform = document.getElementsByClassName("platform");
var coin = document.getElementsByClassName("coin");
var up = 0;
var key = 0;
var counter = 0;
var addcounter = setInterval(createPlatform,1);
setInterval(collisionDetection,1);
setInterval(gravity,1);
setInterval(controller,1);
setInterval(followplayer,1);

function createPlatform() 
if (counter == 0) 
if (2 * Math.round((window.innerHeight / 15) / 2) < player.offsetHeight) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = player.offsetHeight + 2 + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;

else 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = 2 * Math.round((window.innerHeight / 15) / 2) + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;


if (counter > 0 && counter < 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = 2 * Math.round((newPlatform.offsetTop - 18) / 2) + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

counter++;

if (counter > 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = newPlatform.offsetTop - 18 + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

clearInterval(addcounter);



function gravity() 
if (collision === false) 
player.style.top = player.offsetTop + 1 - up + "px";



function collisionDetection() 
for (i = 0; i < object.length; i++) 
// Player Right to Object Left
if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
 player.offsetLeft + player.offsetWidth >= object[i].offsetLeft &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft - player.offsetWidth + "px"; 
collision = true; 

// Player Left to Object Right
if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
 player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px"; 
collision = true; 

// Player Bottom to Object Top
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
 player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
player.style.top = object[i].offsetTop - player.offsetHeight + "px"; 
collision = true;

// Player Top to Object Bottom
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
 player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
player.style.top = object[i].offsetTop + object[i].offsetHeight + "px"; 
collision = true;

else 
collision = false;




function controller() 
// Left
if (key == 37) 
player.style.left = player.offsetLeft - 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Right
if (key == 39) 
player.style.left = player.offsetLeft + 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Up
if (key == 38) 
up = 2;
jetpack.style.opacity = "1";

else 
up = 0;
jetpack.style.opacity = "0";



function followplayer() 
jetpack.style.top = player.offsetTop + player.offsetHeight + "px";
jetpack.style.left = player.offsetLeft + "px";


function getkey() 
key = event.keyCode;


function stopkey() 
key = 0;

</script>
</body>
</html>






javascript html game css collision




share|improve this question





















  • What happened to the indentation? Could you repaste the code? It's hard to read.
    – t3chb0t
    Jun 7 at 6:25












up vote
3
down vote

favorite









up vote
3
down vote

favorite











I am in the middle of working on this game and I still need to add some things (e.g. score, coin counters, etc.) but I really feel like there is way too much unneeded code. Could you help me simplify this so I can keep adding things?






<!DOCTYPE html>
<html>
<head>
<title>Game</title>
<style>
html, body 
background: linear-gradient(to bottom,#7EC0EE,#C1E5FF);
height: 100%;
width: 100%;
margin: 0;

#player 
background: #AF1C1C;
position: fixed;
height: 20px;
bottom: 0px;
width: 8px;
z-index: 2;
left: 0px;
top: 0px;

#head 
background: #F2CE8C;
position: absolute;
height: 6px;
width: 8px;
left: 0px;
top: 0px;

#hair 
background: #000000;
position: absolute;
height: 2px;
width: 8px;
left: 0px;
top: 0px;

#hair-back 
background: #000000;
position: absolute;
height: 6px;
width: 2px;
left: 0px;
top: 0px;

#arm 
background: #F2CE8C;
position: absolute;
height: 5px;
width: 5px;
left: 0px;
top: 8px;

#feet 
background: #000000;
position: absolute;
bottom: 0px;
height: 4px;
width: 8px;
left: 0px;

#jetpack 
background: linear-gradient(to bottom,#FFF,transparent);
position: fixed;
height: 16px;
width: 8px;
opacity: 0;
z-index: 1;

#ground 
background: green;
position: fixed;
height: 10px;
width: 100%;
bottom: 0px;
z-index: 3;
left: 0px;

#platform 
background: #915921;
position: fixed;
height: 10px;
width: 15%;
z-index: 3;

.coin 
animation: coinflip 6s linear infinite;
transform: rotate(-90deg);
border: 2px inset #D69E10;
background: #EFB112;
position: fixed;
height: 8px;
width: 8px;
z-index: 0;

@keyframes coinflip 
0% transform: rotate(-90deg);filter: brightness(100%);
40% transform: rotate(-90deg);filter: brightness(100%);
50% transform: rotate(-90deg) rotateX(180deg);filter: brightness(60%);
60% transform: rotate(-90deg);filter: brightness(100%);
100% transform: rotate(-90deg);filter: brightness(100%);

</style>
</head>
<body onkeydown="getkey()" onkeyup="stopkey()">
<div id="player">
 <div id="head"></div>
 <div id="hair"></div>
 <div id="hair-back"></div>
 <div id="arm"></div>
 <div id="feet"></div>
</div>
<div id="jetpack"></div>
<div class="object" id="ground"></div>
<script>
var collision = false;
var player = document.getElementById("player");
var jetpack = document.getElementById("jetpack");
var ground = document.getElementById("ground");
var object = document.getElementsByClassName("object");
var platform = document.getElementsByClassName("platform");
var coin = document.getElementsByClassName("coin");
var up = 0;
var key = 0;
var counter = 0;
var addcounter = setInterval(createPlatform,1);
setInterval(collisionDetection,1);
setInterval(gravity,1);
setInterval(controller,1);
setInterval(followplayer,1);

function createPlatform() 
if (counter == 0) 
if (2 * Math.round((window.innerHeight / 15) / 2) < player.offsetHeight) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = player.offsetHeight + 2 + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;

else 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = 2 * Math.round((window.innerHeight / 15) / 2) + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;


if (counter > 0 && counter < 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = 2 * Math.round((newPlatform.offsetTop - 18) / 2) + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

counter++;

if (counter > 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = newPlatform.offsetTop - 18 + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

clearInterval(addcounter);



function gravity() 
if (collision === false) 
player.style.top = player.offsetTop + 1 - up + "px";



function collisionDetection() 
for (i = 0; i < object.length; i++) 
// Player Right to Object Left
if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
 player.offsetLeft + player.offsetWidth >= object[i].offsetLeft &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft - player.offsetWidth + "px"; 
collision = true; 

// Player Left to Object Right
if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
 player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px"; 
collision = true; 

// Player Bottom to Object Top
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
 player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
player.style.top = object[i].offsetTop - player.offsetHeight + "px"; 
collision = true;

// Player Top to Object Bottom
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
 player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
player.style.top = object[i].offsetTop + object[i].offsetHeight + "px"; 
collision = true;

else 
collision = false;




function controller() 
// Left
if (key == 37) 
player.style.left = player.offsetLeft - 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Right
if (key == 39) 
player.style.left = player.offsetLeft + 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Up
if (key == 38) 
up = 2;
jetpack.style.opacity = "1";

else 
up = 0;
jetpack.style.opacity = "0";



function followplayer() 
jetpack.style.top = player.offsetTop + player.offsetHeight + "px";
jetpack.style.left = player.offsetLeft + "px";


function getkey() 
key = event.keyCode;


function stopkey() 
key = 0;

</script>
</body>
</html>






javascript html game css collision




share|improve this question













I am in the middle of working on this game and I still need to add some things (e.g. score, coin counters, etc.) but I really feel like there is way too much unneeded code. Could you help me simplify this so I can keep adding things?






<!DOCTYPE html>
<html>
<head>
<title>Game</title>
<style>
html, body 
background: linear-gradient(to bottom,#7EC0EE,#C1E5FF);
height: 100%;
width: 100%;
margin: 0;

#player 
background: #AF1C1C;
position: fixed;
height: 20px;
bottom: 0px;
width: 8px;
z-index: 2;
left: 0px;
top: 0px;

#head 
background: #F2CE8C;
position: absolute;
height: 6px;
width: 8px;
left: 0px;
top: 0px;

#hair 
background: #000000;
position: absolute;
height: 2px;
width: 8px;
left: 0px;
top: 0px;

#hair-back 
background: #000000;
position: absolute;
height: 6px;
width: 2px;
left: 0px;
top: 0px;

#arm 
background: #F2CE8C;
position: absolute;
height: 5px;
width: 5px;
left: 0px;
top: 8px;

#feet 
background: #000000;
position: absolute;
bottom: 0px;
height: 4px;
width: 8px;
left: 0px;

#jetpack 
background: linear-gradient(to bottom,#FFF,transparent);
position: fixed;
height: 16px;
width: 8px;
opacity: 0;
z-index: 1;

#ground 
background: green;
position: fixed;
height: 10px;
width: 100%;
bottom: 0px;
z-index: 3;
left: 0px;

#platform 
background: #915921;
position: fixed;
height: 10px;
width: 15%;
z-index: 3;

.coin 
animation: coinflip 6s linear infinite;
transform: rotate(-90deg);
border: 2px inset #D69E10;
background: #EFB112;
position: fixed;
height: 8px;
width: 8px;
z-index: 0;

@keyframes coinflip 
0% transform: rotate(-90deg);filter: brightness(100%);
40% transform: rotate(-90deg);filter: brightness(100%);
50% transform: rotate(-90deg) rotateX(180deg);filter: brightness(60%);
60% transform: rotate(-90deg);filter: brightness(100%);
100% transform: rotate(-90deg);filter: brightness(100%);

</style>
</head>
<body onkeydown="getkey()" onkeyup="stopkey()">
<div id="player">
 <div id="head"></div>
 <div id="hair"></div>
 <div id="hair-back"></div>
 <div id="arm"></div>
 <div id="feet"></div>
</div>
<div id="jetpack"></div>
<div class="object" id="ground"></div>
<script>
var collision = false;
var player = document.getElementById("player");
var jetpack = document.getElementById("jetpack");
var ground = document.getElementById("ground");
var object = document.getElementsByClassName("object");
var platform = document.getElementsByClassName("platform");
var coin = document.getElementsByClassName("coin");
var up = 0;
var key = 0;
var counter = 0;
var addcounter = setInterval(createPlatform,1);
setInterval(collisionDetection,1);
setInterval(gravity,1);
setInterval(controller,1);
setInterval(followplayer,1);

function createPlatform() 
if (counter == 0) 
if (2 * Math.round((window.innerHeight / 15) / 2) < player.offsetHeight) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = player.offsetHeight + 2 + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;

else 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = 2 * Math.round((window.innerHeight / 15) / 2) + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;


if (counter > 0 && counter < 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = 2 * Math.round((newPlatform.offsetTop - 18) / 2) + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

counter++;

if (counter > 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = newPlatform.offsetTop - 18 + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

clearInterval(addcounter);



function gravity() 
if (collision === false) 
player.style.top = player.offsetTop + 1 - up + "px";



function collisionDetection() 
for (i = 0; i < object.length; i++) 
// Player Right to Object Left
if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
 player.offsetLeft + player.offsetWidth >= object[i].offsetLeft &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft - player.offsetWidth + "px"; 
collision = true; 

// Player Left to Object Right
if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
 player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px"; 
collision = true; 

// Player Bottom to Object Top
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
 player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
player.style.top = object[i].offsetTop - player.offsetHeight + "px"; 
collision = true;

// Player Top to Object Bottom
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
 player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
player.style.top = object[i].offsetTop + object[i].offsetHeight + "px"; 
collision = true;

else 
collision = false;




function controller() 
// Left
if (key == 37) 
player.style.left = player.offsetLeft - 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Right
if (key == 39) 
player.style.left = player.offsetLeft + 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Up
if (key == 38) 
up = 2;
jetpack.style.opacity = "1";

else 
up = 0;
jetpack.style.opacity = "0";



function followplayer() 
jetpack.style.top = player.offsetTop + player.offsetHeight + "px";
jetpack.style.left = player.offsetLeft + "px";


function getkey() 
key = event.keyCode;


function stopkey() 
key = 0;

</script>
</body>
</html>








<!DOCTYPE html>
<html>
<head>
<title>Game</title>
<style>
html, body 
background: linear-gradient(to bottom,#7EC0EE,#C1E5FF);
height: 100%;
width: 100%;
margin: 0;

#player 
background: #AF1C1C;
position: fixed;
height: 20px;
bottom: 0px;
width: 8px;
z-index: 2;
left: 0px;
top: 0px;

#head 
background: #F2CE8C;
position: absolute;
height: 6px;
width: 8px;
left: 0px;
top: 0px;

#hair 
background: #000000;
position: absolute;
height: 2px;
width: 8px;
left: 0px;
top: 0px;

#hair-back 
background: #000000;
position: absolute;
height: 6px;
width: 2px;
left: 0px;
top: 0px;

#arm 
background: #F2CE8C;
position: absolute;
height: 5px;
width: 5px;
left: 0px;
top: 8px;

#feet 
background: #000000;
position: absolute;
bottom: 0px;
height: 4px;
width: 8px;
left: 0px;

#jetpack 
background: linear-gradient(to bottom,#FFF,transparent);
position: fixed;
height: 16px;
width: 8px;
opacity: 0;
z-index: 1;

#ground 
background: green;
position: fixed;
height: 10px;
width: 100%;
bottom: 0px;
z-index: 3;
left: 0px;

#platform 
background: #915921;
position: fixed;
height: 10px;
width: 15%;
z-index: 3;

.coin 
animation: coinflip 6s linear infinite;
transform: rotate(-90deg);
border: 2px inset #D69E10;
background: #EFB112;
position: fixed;
height: 8px;
width: 8px;
z-index: 0;

@keyframes coinflip 
0% transform: rotate(-90deg);filter: brightness(100%);
40% transform: rotate(-90deg);filter: brightness(100%);
50% transform: rotate(-90deg) rotateX(180deg);filter: brightness(60%);
60% transform: rotate(-90deg);filter: brightness(100%);
100% transform: rotate(-90deg);filter: brightness(100%);

</style>
</head>
<body onkeydown="getkey()" onkeyup="stopkey()">
<div id="player">
 <div id="head"></div>
 <div id="hair"></div>
 <div id="hair-back"></div>
 <div id="arm"></div>
 <div id="feet"></div>
</div>
<div id="jetpack"></div>
<div class="object" id="ground"></div>
<script>
var collision = false;
var player = document.getElementById("player");
var jetpack = document.getElementById("jetpack");
var ground = document.getElementById("ground");
var object = document.getElementsByClassName("object");
var platform = document.getElementsByClassName("platform");
var coin = document.getElementsByClassName("coin");
var up = 0;
var key = 0;
var counter = 0;
var addcounter = setInterval(createPlatform,1);
setInterval(collisionDetection,1);
setInterval(gravity,1);
setInterval(controller,1);
setInterval(followplayer,1);

function createPlatform() 
if (counter == 0) 
if (2 * Math.round((window.innerHeight / 15) / 2) < player.offsetHeight) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = player.offsetHeight + 2 + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;

else 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = 2 * Math.round((window.innerHeight / 15) / 2) + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;


if (counter > 0 && counter < 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = 2 * Math.round((newPlatform.offsetTop - 18) / 2) + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

counter++;

if (counter > 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = newPlatform.offsetTop - 18 + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

clearInterval(addcounter);



function gravity() 
if (collision === false) 
player.style.top = player.offsetTop + 1 - up + "px";



function collisionDetection() 
for (i = 0; i < object.length; i++) 
// Player Right to Object Left
if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
 player.offsetLeft + player.offsetWidth >= object[i].offsetLeft &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft - player.offsetWidth + "px"; 
collision = true; 

// Player Left to Object Right
if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
 player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px"; 
collision = true; 

// Player Bottom to Object Top
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
 player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
player.style.top = object[i].offsetTop - player.offsetHeight + "px"; 
collision = true;

// Player Top to Object Bottom
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
 player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
player.style.top = object[i].offsetTop + object[i].offsetHeight + "px"; 
collision = true;

else 
collision = false;




function controller() 
// Left
if (key == 37) 
player.style.left = player.offsetLeft - 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Right
if (key == 39) 
player.style.left = player.offsetLeft + 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Up
if (key == 38) 
up = 2;
jetpack.style.opacity = "1";

else 
up = 0;
jetpack.style.opacity = "0";



function followplayer() 
jetpack.style.top = player.offsetTop + player.offsetHeight + "px";
jetpack.style.left = player.offsetLeft + "px";


function getkey() 
key = event.keyCode;


function stopkey() 
key = 0;

</script>
</body>
</html>





<!DOCTYPE html>
<html>
<head>
<title>Game</title>
<style>
html, body 
background: linear-gradient(to bottom,#7EC0EE,#C1E5FF);
height: 100%;
width: 100%;
margin: 0;

#player 
background: #AF1C1C;
position: fixed;
height: 20px;
bottom: 0px;
width: 8px;
z-index: 2;
left: 0px;
top: 0px;

#head 
background: #F2CE8C;
position: absolute;
height: 6px;
width: 8px;
left: 0px;
top: 0px;

#hair 
background: #000000;
position: absolute;
height: 2px;
width: 8px;
left: 0px;
top: 0px;

#hair-back 
background: #000000;
position: absolute;
height: 6px;
width: 2px;
left: 0px;
top: 0px;

#arm 
background: #F2CE8C;
position: absolute;
height: 5px;
width: 5px;
left: 0px;
top: 8px;

#feet 
background: #000000;
position: absolute;
bottom: 0px;
height: 4px;
width: 8px;
left: 0px;

#jetpack 
background: linear-gradient(to bottom,#FFF,transparent);
position: fixed;
height: 16px;
width: 8px;
opacity: 0;
z-index: 1;

#ground 
background: green;
position: fixed;
height: 10px;
width: 100%;
bottom: 0px;
z-index: 3;
left: 0px;

#platform 
background: #915921;
position: fixed;
height: 10px;
width: 15%;
z-index: 3;

.coin 
animation: coinflip 6s linear infinite;
transform: rotate(-90deg);
border: 2px inset #D69E10;
background: #EFB112;
position: fixed;
height: 8px;
width: 8px;
z-index: 0;

@keyframes coinflip 
0% transform: rotate(-90deg);filter: brightness(100%);
40% transform: rotate(-90deg);filter: brightness(100%);
50% transform: rotate(-90deg) rotateX(180deg);filter: brightness(60%);
60% transform: rotate(-90deg);filter: brightness(100%);
100% transform: rotate(-90deg);filter: brightness(100%);

</style>
</head>
<body onkeydown="getkey()" onkeyup="stopkey()">
<div id="player">
 <div id="head"></div>
 <div id="hair"></div>
 <div id="hair-back"></div>
 <div id="arm"></div>
 <div id="feet"></div>
</div>
<div id="jetpack"></div>
<div class="object" id="ground"></div>
<script>
var collision = false;
var player = document.getElementById("player");
var jetpack = document.getElementById("jetpack");
var ground = document.getElementById("ground");
var object = document.getElementsByClassName("object");
var platform = document.getElementsByClassName("platform");
var coin = document.getElementsByClassName("coin");
var up = 0;
var key = 0;
var counter = 0;
var addcounter = setInterval(createPlatform,1);
setInterval(collisionDetection,1);
setInterval(gravity,1);
setInterval(controller,1);
setInterval(followplayer,1);

function createPlatform() 
if (counter == 0) 
if (2 * Math.round((window.innerHeight / 15) / 2) < player.offsetHeight) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = player.offsetHeight + 2 + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;

else 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.top = 2 * Math.round((window.innerHeight / 15) / 2) + "px";
newPlatform.style.left = "0px";
document.body.appendChild(newPlatform);
counter++;


if (counter > 0 && counter < 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = 2 * Math.round((newPlatform.offsetTop - 18) / 2) + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

counter++;

if (counter > 5) 
var newPlatform = document.createElement("DIV");
newPlatform.setAttribute("class", "object platform");
newPlatform.setAttribute("id", "platform");
newPlatform.style.left = (window.innerWidth / 5) * counter + "px";
newPlatform.style.top = 2 * Math.round(((window.innerHeight / 5) * counter + 4) / 2) + "px";
document.body.appendChild(newPlatform);
for (var c = 0; c < 5; c++) 
var newCoin = document.createElement("DIV");
newCoin.setAttribute("class", "coin");
newCoin.style.top = newPlatform.offsetTop - 18 + "px";
newCoin.style.left = newPlatform.offsetLeft + (((window.innerWidth / 31) + 4.5) * c) + "px";
document.body.appendChild(newCoin);

clearInterval(addcounter);



function gravity() 
if (collision === false) 
player.style.top = player.offsetTop + 1 - up + "px";



function collisionDetection() 
for (i = 0; i < object.length; i++) 
// Player Right to Object Left
if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
 player.offsetLeft + player.offsetWidth >= object[i].offsetLeft &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft - player.offsetWidth + "px"; 
collision = true; 

// Player Left to Object Right
if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
 player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px"; 
collision = true; 

// Player Bottom to Object Top
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
 player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
player.style.top = object[i].offsetTop - player.offsetHeight + "px"; 
collision = true;

// Player Top to Object Bottom
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
 player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
player.style.top = object[i].offsetTop + object[i].offsetHeight + "px"; 
collision = true;

else 
collision = false;




function controller() 
// Left
if (key == 37) 
player.style.left = player.offsetLeft - 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Right
if (key == 39) 
player.style.left = player.offsetLeft + 1 + "px";
up = 0;
jetpack.style.opacity = "0";

// Up
if (key == 38) 
up = 2;
jetpack.style.opacity = "1";

else 
up = 0;
jetpack.style.opacity = "0";



function followplayer() 
jetpack.style.top = player.offsetTop + player.offsetHeight + "px";
jetpack.style.left = player.offsetLeft + "px";


function getkey() 
key = event.keyCode;


function stopkey() 
key = 0;

</script>
</body>
</html>




javascript html game css collision





share|improve this question












share|improve this question




share|improve this question








edited Jun 7 at 16:09









Sam Onela

5,76961543




5,76961543









asked Jun 7 at 3:35









Josh Berger

185




185











  • What happened to the indentation? Could you repaste the code? It's hard to read.
    – t3chb0t
    Jun 7 at 6:25
















  • What happened to the indentation? Could you repaste the code? It's hard to read.
    – t3chb0t
    Jun 7 at 6:25















What happened to the indentation? Could you repaste the code? It's hard to read.
– t3chb0t
Jun 7 at 6:25




What happened to the indentation? Could you repaste the code? It's hard to read.
– t3chb0t
Jun 7 at 6:25










2 Answers
2






active

oldest

votes

















up vote
3
down vote



accepted










Right off the bat, using multiple calls to setInterval is a bad idea. setInterval is already kind of unreliable and not very well optimized, plus using multiple intervals separately can easily lead to your update operations falling out of sync.



Try reading up on game loops and requestAnimationFrame. MDN has a pretty decent article, but you can also find plenty of materials online if you google it.



https://developer.mozilla.org/en-US/docs/Games/Anatomy



Basically, in the end you'll want to turn this:



setInterval(createPlatform,1);
setInterval(collisionDetection,1);
setInterval(gravity,1);
setInterval(controller,1);
setInterval(followplayer,1);


Into something like this:



let lastTick = performance.now();

function tick(now) 
 window.requestAnimationFrame(tick);

 // Get the elapsed time since the last frame in seconds
 let delta = (now - lastTick) / 1000;

 controller(delta);
 gravity(delta);
 followPlayer(delta);
 createPlatform(delta);
 collisionDetection(delta);

 lastTick = performance.now();


window.requestAnimationFrame(tick);





share|improve this answer























  • Sorry for being so late to respond but I've been away from my computer. I did this and it works but it is extremely slow, is there any way to stop this?
    – Josh Berger
    Jun 10 at 4:32










  • You probably need to redesign your update logic somewhat. Using requestAnimationFrame will throttle your game to 60fps at most. The reason we calculate the delta value in the tick() function is so we know how much time has elapsed since the last frame, and you need to update the game world accordingly. For example, if you want your player to move at a speed of 100 pixels per second, you add 100 * delta to its position each frame.
    – Máté Safranka
    Jun 10 at 8:29










  • Thanks for all the help. I ended up keeping mainly what I had originally but I called the gravity function and the collisionDetection function in another function that runs every millisecond because of your input. It works great so thanks again.
    – Josh Berger
    Jun 17 at 15:55

















up vote
2
down vote













Feedback



The code caches the DOM lookups (e.g. player, jetpack, etc.) - which is wise. Also, wow that is a nice rendering of a person and spinning coins using CSS.



Suggestions



The advice below is not an exhaustive list, but hopefully a start in the right direction.




id attributes are supposed to be unique



It appears that every time createPlatform() is called and it makes a platform element, it sets the id attribute to platform, but the value must be unique in the whole document1. You could utilize counter to create unique values for that attribute.



Indentation



As was mentioned in comments, the indentiation is lacking - both in the CSS and JavaScript. That is a really important in terms of readability. The amount to indent can vary depending on developer/team opninions, but generally 2 or 4 spaces are standard. Doug Crockford reccomends 4 spaces2



Don't Repeat Yourself



createPlatform() has a lot of repeated lines. The common lines within the if and else blocks can be moved outside:



function createPlatform() {
 if (counter == 0) 
 var newPlatform = document.createElement("DIV");
 newPlatform.setAttribute("class", "object platform");
 newPlatform.setAttribute("id", "platform");
 if (2 * Math.round((window.innerHeight / 15) / 2) < player.offsetHeight) 
 newPlatform.style.top = player.offsetHeight + 2 + "px";
 else 
 newPlatform.style.top = 2 * Math.round((window.innerHeight / 15) / 2) + "px";
 
 newPlatform.style.left = "0px";
 document.body.appendChild(newPlatform);
 counter++;


Perhaps it would be best to make a function for those first three lines of the element creation and attribute setting, which could also be used later on in other places.



Also, in collisionDetection there appears to be a lot of redundancy in the conditions:




if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
 player.offsetLeft + player.offsetWidth >= object[i].offsetLeft &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft - player.offsetWidth + "px"; 
collision = true; 

// Player Left to Object Right
if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
 player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px"; 
collision = true; 

// Player Bottom to Object Top
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
 player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
player.style.top = object[i].offsetTop - player.offsetHeight + "px"; 
collision = true;

// Player Top to Object Bottom
if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
 player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
 player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
player.style.top = object[i].offsetTop + object[i].offsetHeight + "px"; 
collision = true;




This could be cleaned up by setting collision to false at the start of the function and then abstracting the common lines:



collision = false; //set initially
for (i = 0; i < object.length; i++) 
 if (player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
 player.offsetHeight + player.offsetTop > object[i].offsetTop) 
 // Player Right to Object Left
 if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
 player.offsetLeft + player.offsetWidth >= object[i].offsetLeft) 
 player.style.left = object[i].offsetLeft - player.offsetWidth + "px";
 collision = true;
 
 // Player Left to Object Right
 if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
 player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth) 
 player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px";
 collision = true;
 
 
 if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
 player.offsetLeft + player.offsetWidth > object[i].offsetLeft) 
 // Player Bottom to Object Top
 if (player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
 player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
 player.style.top = object[i].offsetTop - player.offsetHeight + "px";
 collision = true;
 
 // Player Top to Object Bottom
 if (player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
 player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
 player.style.top = object[i].offsetTop + object[i].offsetHeight + "px";
 collision = true;


Minimize reflows from appending elements



It might be wise to consider using a DocumentFragment or at least a container element to add the platforms and coins to, and then add that element to the body to minimize reflows. For more information on this and similar topics, check out this article Stop Writing Slow Javascript. I know it is a few years old but still has some relevant information.



1https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id2https://crockford.com/javascript/code.html#indentation






share|improve this answer























    Your Answer




    StackExchange.ifUsing("editor", function ()
    return StackExchange.using("mathjaxEditing", function ()
    StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
    StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
    );
    );
    , "mathjax-editing");

    StackExchange.ifUsing("editor", function ()
    StackExchange.using("externalEditor", function ()
    StackExchange.using("snippets", function ()
    StackExchange.snippets.init();
    );
    );
    , "code-snippets");

    StackExchange.ready(function()
    var channelOptions =
    tags: "".split(" "),
    id: "196"
    ;
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function()
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled)
    StackExchange.using("snippets", function()
    createEditor();
    );

    else
    createEditor();

    );

    function createEditor()
    StackExchange.prepareEditor(
    heartbeatType: 'answer',
    convertImagesToLinks: false,
    noModals: false,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: null,
    bindNavPrevention: true,
    postfix: "",
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    );



    );








     

    draft saved


    draft discarded


















    StackExchange.ready(
    function ()
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f195993%2fplatformer-game-arrow-key-player-movement-and-collision-detection%23new-answer', 'question_page');

    );

    Post as a guest

















    2 Answers
    2






    active

    oldest

    votes








    2 Answers
    2






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes








    up vote
    3
    down vote



    accepted










    Right off the bat, using multiple calls to setInterval is a bad idea. setInterval is already kind of unreliable and not very well optimized, plus using multiple intervals separately can easily lead to your update operations falling out of sync.



    Try reading up on game loops and requestAnimationFrame. MDN has a pretty decent article, but you can also find plenty of materials online if you google it.



    https://developer.mozilla.org/en-US/docs/Games/Anatomy



    Basically, in the end you'll want to turn this:



    setInterval(createPlatform,1);
    setInterval(collisionDetection,1);
    setInterval(gravity,1);
    setInterval(controller,1);
    setInterval(followplayer,1);


    Into something like this:



    let lastTick = performance.now();

    function tick(now) 
     window.requestAnimationFrame(tick);

     // Get the elapsed time since the last frame in seconds
     let delta = (now - lastTick) / 1000;

     controller(delta);
     gravity(delta);
     followPlayer(delta);
     createPlatform(delta);
     collisionDetection(delta);

     lastTick = performance.now();


    window.requestAnimationFrame(tick);





    share|improve this answer























    • Sorry for being so late to respond but I've been away from my computer. I did this and it works but it is extremely slow, is there any way to stop this?
      – Josh Berger
      Jun 10 at 4:32










    • You probably need to redesign your update logic somewhat. Using requestAnimationFrame will throttle your game to 60fps at most. The reason we calculate the delta value in the tick() function is so we know how much time has elapsed since the last frame, and you need to update the game world accordingly. For example, if you want your player to move at a speed of 100 pixels per second, you add 100 * delta to its position each frame.
      – Máté Safranka
      Jun 10 at 8:29










    • Thanks for all the help. I ended up keeping mainly what I had originally but I called the gravity function and the collisionDetection function in another function that runs every millisecond because of your input. It works great so thanks again.
      – Josh Berger
      Jun 17 at 15:55














    up vote
    3
    down vote



    accepted










    Right off the bat, using multiple calls to setInterval is a bad idea. setInterval is already kind of unreliable and not very well optimized, plus using multiple intervals separately can easily lead to your update operations falling out of sync.



    Try reading up on game loops and requestAnimationFrame. MDN has a pretty decent article, but you can also find plenty of materials online if you google it.



    https://developer.mozilla.org/en-US/docs/Games/Anatomy



    Basically, in the end you'll want to turn this:



    setInterval(createPlatform,1);
    setInterval(collisionDetection,1);
    setInterval(gravity,1);
    setInterval(controller,1);
    setInterval(followplayer,1);


    Into something like this:



    let lastTick = performance.now();

    function tick(now) 
     window.requestAnimationFrame(tick);

     // Get the elapsed time since the last frame in seconds
     let delta = (now - lastTick) / 1000;

     controller(delta);
     gravity(delta);
     followPlayer(delta);
     createPlatform(delta);
     collisionDetection(delta);

     lastTick = performance.now();


    window.requestAnimationFrame(tick);





    share|improve this answer























    • Sorry for being so late to respond but I've been away from my computer. I did this and it works but it is extremely slow, is there any way to stop this?
      – Josh Berger
      Jun 10 at 4:32










    • You probably need to redesign your update logic somewhat. Using requestAnimationFrame will throttle your game to 60fps at most. The reason we calculate the delta value in the tick() function is so we know how much time has elapsed since the last frame, and you need to update the game world accordingly. For example, if you want your player to move at a speed of 100 pixels per second, you add 100 * delta to its position each frame.
      – Máté Safranka
      Jun 10 at 8:29










    • Thanks for all the help. I ended up keeping mainly what I had originally but I called the gravity function and the collisionDetection function in another function that runs every millisecond because of your input. It works great so thanks again.
      – Josh Berger
      Jun 17 at 15:55












    up vote
    3
    down vote



    accepted







    up vote
    3
    down vote



    accepted






    Right off the bat, using multiple calls to setInterval is a bad idea. setInterval is already kind of unreliable and not very well optimized, plus using multiple intervals separately can easily lead to your update operations falling out of sync.



    Try reading up on game loops and requestAnimationFrame. MDN has a pretty decent article, but you can also find plenty of materials online if you google it.



    https://developer.mozilla.org/en-US/docs/Games/Anatomy



    Basically, in the end you'll want to turn this:



    setInterval(createPlatform,1);
    setInterval(collisionDetection,1);
    setInterval(gravity,1);
    setInterval(controller,1);
    setInterval(followplayer,1);


    Into something like this:



    let lastTick = performance.now();

    function tick(now) 
     window.requestAnimationFrame(tick);

     // Get the elapsed time since the last frame in seconds
     let delta = (now - lastTick) / 1000;

     controller(delta);
     gravity(delta);
     followPlayer(delta);
     createPlatform(delta);
     collisionDetection(delta);

     lastTick = performance.now();


    window.requestAnimationFrame(tick);





    share|improve this answer















    Right off the bat, using multiple calls to setInterval is a bad idea. setInterval is already kind of unreliable and not very well optimized, plus using multiple intervals separately can easily lead to your update operations falling out of sync.



    Try reading up on game loops and requestAnimationFrame. MDN has a pretty decent article, but you can also find plenty of materials online if you google it.



    https://developer.mozilla.org/en-US/docs/Games/Anatomy



    Basically, in the end you'll want to turn this:



    setInterval(createPlatform,1);
    setInterval(collisionDetection,1);
    setInterval(gravity,1);
    setInterval(controller,1);
    setInterval(followplayer,1);


    Into something like this:



    let lastTick = performance.now();

    function tick(now) 
     window.requestAnimationFrame(tick);

     // Get the elapsed time since the last frame in seconds
     let delta = (now - lastTick) / 1000;

     controller(delta);
     gravity(delta);
     followPlayer(delta);
     createPlatform(delta);
     collisionDetection(delta);

     lastTick = performance.now();


    window.requestAnimationFrame(tick);






    share|improve this answer















    share|improve this answer



    share|improve this answer








    edited Jun 7 at 8:48


























    answered Jun 7 at 8:43









    Máté Safranka

    3315




    3315











    • Sorry for being so late to respond but I've been away from my computer. I did this and it works but it is extremely slow, is there any way to stop this?
      – Josh Berger
      Jun 10 at 4:32










    • You probably need to redesign your update logic somewhat. Using requestAnimationFrame will throttle your game to 60fps at most. The reason we calculate the delta value in the tick() function is so we know how much time has elapsed since the last frame, and you need to update the game world accordingly. For example, if you want your player to move at a speed of 100 pixels per second, you add 100 * delta to its position each frame.
      – Máté Safranka
      Jun 10 at 8:29










    • Thanks for all the help. I ended up keeping mainly what I had originally but I called the gravity function and the collisionDetection function in another function that runs every millisecond because of your input. It works great so thanks again.
      – Josh Berger
      Jun 17 at 15:55
















    • Sorry for being so late to respond but I've been away from my computer. I did this and it works but it is extremely slow, is there any way to stop this?
      – Josh Berger
      Jun 10 at 4:32










    • You probably need to redesign your update logic somewhat. Using requestAnimationFrame will throttle your game to 60fps at most. The reason we calculate the delta value in the tick() function is so we know how much time has elapsed since the last frame, and you need to update the game world accordingly. For example, if you want your player to move at a speed of 100 pixels per second, you add 100 * delta to its position each frame.
      – Máté Safranka
      Jun 10 at 8:29










    • Thanks for all the help. I ended up keeping mainly what I had originally but I called the gravity function and the collisionDetection function in another function that runs every millisecond because of your input. It works great so thanks again.
      – Josh Berger
      Jun 17 at 15:55















    Sorry for being so late to respond but I've been away from my computer. I did this and it works but it is extremely slow, is there any way to stop this?
    – Josh Berger
    Jun 10 at 4:32




    Sorry for being so late to respond but I've been away from my computer. I did this and it works but it is extremely slow, is there any way to stop this?
    – Josh Berger
    Jun 10 at 4:32












    You probably need to redesign your update logic somewhat. Using requestAnimationFrame will throttle your game to 60fps at most. The reason we calculate the delta value in the tick() function is so we know how much time has elapsed since the last frame, and you need to update the game world accordingly. For example, if you want your player to move at a speed of 100 pixels per second, you add 100 * delta to its position each frame.
    – Máté Safranka
    Jun 10 at 8:29




    You probably need to redesign your update logic somewhat. Using requestAnimationFrame will throttle your game to 60fps at most. The reason we calculate the delta value in the tick() function is so we know how much time has elapsed since the last frame, and you need to update the game world accordingly. For example, if you want your player to move at a speed of 100 pixels per second, you add 100 * delta to its position each frame.
    – Máté Safranka
    Jun 10 at 8:29












    Thanks for all the help. I ended up keeping mainly what I had originally but I called the gravity function and the collisionDetection function in another function that runs every millisecond because of your input. It works great so thanks again.
    – Josh Berger
    Jun 17 at 15:55




    Thanks for all the help. I ended up keeping mainly what I had originally but I called the gravity function and the collisionDetection function in another function that runs every millisecond because of your input. It works great so thanks again.
    – Josh Berger
    Jun 17 at 15:55












    up vote
    2
    down vote













    Feedback



    The code caches the DOM lookups (e.g. player, jetpack, etc.) - which is wise. Also, wow that is a nice rendering of a person and spinning coins using CSS.



    Suggestions



    The advice below is not an exhaustive list, but hopefully a start in the right direction.




    id attributes are supposed to be unique



    It appears that every time createPlatform() is called and it makes a platform element, it sets the id attribute to platform, but the value must be unique in the whole document1. You could utilize counter to create unique values for that attribute.



    Indentation



    As was mentioned in comments, the indentiation is lacking - both in the CSS and JavaScript. That is a really important in terms of readability. The amount to indent can vary depending on developer/team opninions, but generally 2 or 4 spaces are standard. Doug Crockford reccomends 4 spaces2



    Don't Repeat Yourself



    createPlatform() has a lot of repeated lines. The common lines within the if and else blocks can be moved outside:



    function createPlatform() {
     if (counter == 0) 
     var newPlatform = document.createElement("DIV");
     newPlatform.setAttribute("class", "object platform");
     newPlatform.setAttribute("id", "platform");
     if (2 * Math.round((window.innerHeight / 15) / 2) < player.offsetHeight) 
     newPlatform.style.top = player.offsetHeight + 2 + "px";
     else 
     newPlatform.style.top = 2 * Math.round((window.innerHeight / 15) / 2) + "px";
     
     newPlatform.style.left = "0px";
     document.body.appendChild(newPlatform);
     counter++;


    Perhaps it would be best to make a function for those first three lines of the element creation and attribute setting, which could also be used later on in other places.



    Also, in collisionDetection there appears to be a lot of redundancy in the conditions:




    if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
     player.offsetLeft + player.offsetWidth >= object[i].offsetLeft &&
     player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
     player.offsetHeight + player.offsetTop > object[i].offsetTop) 
    player.style.left = object[i].offsetLeft - player.offsetWidth + "px"; 
    collision = true; 

    // Player Left to Object Right
    if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
     player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth &&
     player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
     player.offsetHeight + player.offsetTop > object[i].offsetTop) 
    player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px"; 
    collision = true; 

    // Player Bottom to Object Top
    if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
     player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
     player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
     player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
    player.style.top = object[i].offsetTop - player.offsetHeight + "px"; 
    collision = true;

    // Player Top to Object Bottom
    if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
     player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
     player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
     player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
    player.style.top = object[i].offsetTop + object[i].offsetHeight + "px"; 
    collision = true;




    This could be cleaned up by setting collision to false at the start of the function and then abstracting the common lines:



    collision = false; //set initially
    for (i = 0; i < object.length; i++) 
     if (player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
     player.offsetHeight + player.offsetTop > object[i].offsetTop) 
     // Player Right to Object Left
     if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
     player.offsetLeft + player.offsetWidth >= object[i].offsetLeft) 
     player.style.left = object[i].offsetLeft - player.offsetWidth + "px";
     collision = true;
     
     // Player Left to Object Right
     if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
     player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth) 
     player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px";
     collision = true;
     
     
     if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
     player.offsetLeft + player.offsetWidth > object[i].offsetLeft) 
     // Player Bottom to Object Top
     if (player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
     player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
     player.style.top = object[i].offsetTop - player.offsetHeight + "px";
     collision = true;
     
     // Player Top to Object Bottom
     if (player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
     player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
     player.style.top = object[i].offsetTop + object[i].offsetHeight + "px";
     collision = true;


    Minimize reflows from appending elements



    It might be wise to consider using a DocumentFragment or at least a container element to add the platforms and coins to, and then add that element to the body to minimize reflows. For more information on this and similar topics, check out this article Stop Writing Slow Javascript. I know it is a few years old but still has some relevant information.



    1https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id2https://crockford.com/javascript/code.html#indentation






    share|improve this answer



























      up vote
      2
      down vote













      Feedback



      The code caches the DOM lookups (e.g. player, jetpack, etc.) - which is wise. Also, wow that is a nice rendering of a person and spinning coins using CSS.



      Suggestions



      The advice below is not an exhaustive list, but hopefully a start in the right direction.




      id attributes are supposed to be unique



      It appears that every time createPlatform() is called and it makes a platform element, it sets the id attribute to platform, but the value must be unique in the whole document1. You could utilize counter to create unique values for that attribute.



      Indentation



      As was mentioned in comments, the indentiation is lacking - both in the CSS and JavaScript. That is a really important in terms of readability. The amount to indent can vary depending on developer/team opninions, but generally 2 or 4 spaces are standard. Doug Crockford reccomends 4 spaces2



      Don't Repeat Yourself



      createPlatform() has a lot of repeated lines. The common lines within the if and else blocks can be moved outside:



      function createPlatform() {
       if (counter == 0) 
       var newPlatform = document.createElement("DIV");
       newPlatform.setAttribute("class", "object platform");
       newPlatform.setAttribute("id", "platform");
       if (2 * Math.round((window.innerHeight / 15) / 2) < player.offsetHeight) 
       newPlatform.style.top = player.offsetHeight + 2 + "px";
       else 
       newPlatform.style.top = 2 * Math.round((window.innerHeight / 15) / 2) + "px";
       
       newPlatform.style.left = "0px";
       document.body.appendChild(newPlatform);
       counter++;


      Perhaps it would be best to make a function for those first three lines of the element creation and attribute setting, which could also be used later on in other places.



      Also, in collisionDetection there appears to be a lot of redundancy in the conditions:




      if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
       player.offsetLeft + player.offsetWidth >= object[i].offsetLeft &&
       player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
       player.offsetHeight + player.offsetTop > object[i].offsetTop) 
      player.style.left = object[i].offsetLeft - player.offsetWidth + "px"; 
      collision = true; 

      // Player Left to Object Right
      if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
       player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth &&
       player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
       player.offsetHeight + player.offsetTop > object[i].offsetTop) 
      player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px"; 
      collision = true; 

      // Player Bottom to Object Top
      if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
       player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
       player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
       player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
      player.style.top = object[i].offsetTop - player.offsetHeight + "px"; 
      collision = true;

      // Player Top to Object Bottom
      if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
       player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
       player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
       player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
      player.style.top = object[i].offsetTop + object[i].offsetHeight + "px"; 
      collision = true;




      This could be cleaned up by setting collision to false at the start of the function and then abstracting the common lines:



      collision = false; //set initially
      for (i = 0; i < object.length; i++) 
       if (player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
       player.offsetHeight + player.offsetTop > object[i].offsetTop) 
       // Player Right to Object Left
       if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
       player.offsetLeft + player.offsetWidth >= object[i].offsetLeft) 
       player.style.left = object[i].offsetLeft - player.offsetWidth + "px";
       collision = true;
       
       // Player Left to Object Right
       if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
       player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth) 
       player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px";
       collision = true;
       
       
       if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
       player.offsetLeft + player.offsetWidth > object[i].offsetLeft) 
       // Player Bottom to Object Top
       if (player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
       player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
       player.style.top = object[i].offsetTop - player.offsetHeight + "px";
       collision = true;
       
       // Player Top to Object Bottom
       if (player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
       player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
       player.style.top = object[i].offsetTop + object[i].offsetHeight + "px";
       collision = true;


      Minimize reflows from appending elements



      It might be wise to consider using a DocumentFragment or at least a container element to add the platforms and coins to, and then add that element to the body to minimize reflows. For more information on this and similar topics, check out this article Stop Writing Slow Javascript. I know it is a few years old but still has some relevant information.



      1https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id2https://crockford.com/javascript/code.html#indentation






      share|improve this answer

























        up vote
        2
        down vote










        up vote
        2
        down vote









        Feedback



        The code caches the DOM lookups (e.g. player, jetpack, etc.) - which is wise. Also, wow that is a nice rendering of a person and spinning coins using CSS.



        Suggestions



        The advice below is not an exhaustive list, but hopefully a start in the right direction.




        id attributes are supposed to be unique



        It appears that every time createPlatform() is called and it makes a platform element, it sets the id attribute to platform, but the value must be unique in the whole document1. You could utilize counter to create unique values for that attribute.



        Indentation



        As was mentioned in comments, the indentiation is lacking - both in the CSS and JavaScript. That is a really important in terms of readability. The amount to indent can vary depending on developer/team opninions, but generally 2 or 4 spaces are standard. Doug Crockford reccomends 4 spaces2



        Don't Repeat Yourself



        createPlatform() has a lot of repeated lines. The common lines within the if and else blocks can be moved outside:



        function createPlatform() {
         if (counter == 0) 
         var newPlatform = document.createElement("DIV");
         newPlatform.setAttribute("class", "object platform");
         newPlatform.setAttribute("id", "platform");
         if (2 * Math.round((window.innerHeight / 15) / 2) < player.offsetHeight) 
         newPlatform.style.top = player.offsetHeight + 2 + "px";
         else 
         newPlatform.style.top = 2 * Math.round((window.innerHeight / 15) / 2) + "px";
         
         newPlatform.style.left = "0px";
         document.body.appendChild(newPlatform);
         counter++;


        Perhaps it would be best to make a function for those first three lines of the element creation and attribute setting, which could also be used later on in other places.



        Also, in collisionDetection there appears to be a lot of redundancy in the conditions:




        if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
         player.offsetLeft + player.offsetWidth >= object[i].offsetLeft &&
         player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
         player.offsetHeight + player.offsetTop > object[i].offsetTop) 
        player.style.left = object[i].offsetLeft - player.offsetWidth + "px"; 
        collision = true; 

        // Player Left to Object Right
        if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
         player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth &&
         player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
         player.offsetHeight + player.offsetTop > object[i].offsetTop) 
        player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px"; 
        collision = true; 

        // Player Bottom to Object Top
        if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
         player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
         player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
         player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
        player.style.top = object[i].offsetTop - player.offsetHeight + "px"; 
        collision = true;

        // Player Top to Object Bottom
        if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
         player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
         player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
         player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
        player.style.top = object[i].offsetTop + object[i].offsetHeight + "px"; 
        collision = true;




        This could be cleaned up by setting collision to false at the start of the function and then abstracting the common lines:



        collision = false; //set initially
        for (i = 0; i < object.length; i++) 
         if (player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
         player.offsetHeight + player.offsetTop > object[i].offsetTop) 
         // Player Right to Object Left
         if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
         player.offsetLeft + player.offsetWidth >= object[i].offsetLeft) 
         player.style.left = object[i].offsetLeft - player.offsetWidth + "px";
         collision = true;
         
         // Player Left to Object Right
         if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
         player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth) 
         player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px";
         collision = true;
         
         
         if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
         player.offsetLeft + player.offsetWidth > object[i].offsetLeft) 
         // Player Bottom to Object Top
         if (player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
         player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
         player.style.top = object[i].offsetTop - player.offsetHeight + "px";
         collision = true;
         
         // Player Top to Object Bottom
         if (player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
         player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
         player.style.top = object[i].offsetTop + object[i].offsetHeight + "px";
         collision = true;


        Minimize reflows from appending elements



        It might be wise to consider using a DocumentFragment or at least a container element to add the platforms and coins to, and then add that element to the body to minimize reflows. For more information on this and similar topics, check out this article Stop Writing Slow Javascript. I know it is a few years old but still has some relevant information.



        1https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id2https://crockford.com/javascript/code.html#indentation






        share|improve this answer















        Feedback



        The code caches the DOM lookups (e.g. player, jetpack, etc.) - which is wise. Also, wow that is a nice rendering of a person and spinning coins using CSS.



        Suggestions



        The advice below is not an exhaustive list, but hopefully a start in the right direction.




        id attributes are supposed to be unique



        It appears that every time createPlatform() is called and it makes a platform element, it sets the id attribute to platform, but the value must be unique in the whole document1. You could utilize counter to create unique values for that attribute.



        Indentation



        As was mentioned in comments, the indentiation is lacking - both in the CSS and JavaScript. That is a really important in terms of readability. The amount to indent can vary depending on developer/team opninions, but generally 2 or 4 spaces are standard. Doug Crockford reccomends 4 spaces2



        Don't Repeat Yourself



        createPlatform() has a lot of repeated lines. The common lines within the if and else blocks can be moved outside:



        function createPlatform() {
         if (counter == 0) 
         var newPlatform = document.createElement("DIV");
         newPlatform.setAttribute("class", "object platform");
         newPlatform.setAttribute("id", "platform");
         if (2 * Math.round((window.innerHeight / 15) / 2) < player.offsetHeight) 
         newPlatform.style.top = player.offsetHeight + 2 + "px";
         else 
         newPlatform.style.top = 2 * Math.round((window.innerHeight / 15) / 2) + "px";
         
         newPlatform.style.left = "0px";
         document.body.appendChild(newPlatform);
         counter++;


        Perhaps it would be best to make a function for those first three lines of the element creation and attribute setting, which could also be used later on in other places.



        Also, in collisionDetection there appears to be a lot of redundancy in the conditions:




        if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
         player.offsetLeft + player.offsetWidth >= object[i].offsetLeft &&
         player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
         player.offsetHeight + player.offsetTop > object[i].offsetTop) 
        player.style.left = object[i].offsetLeft - player.offsetWidth + "px"; 
        collision = true; 

        // Player Left to Object Right
        if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
         player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth &&
         player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
         player.offsetHeight + player.offsetTop > object[i].offsetTop) 
        player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px"; 
        collision = true; 

        // Player Bottom to Object Top
        if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
         player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
         player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
         player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
        player.style.top = object[i].offsetTop - player.offsetHeight + "px"; 
        collision = true;

        // Player Top to Object Bottom
        if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
         player.offsetLeft + player.offsetWidth > object[i].offsetLeft &&
         player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
         player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
        player.style.top = object[i].offsetTop + object[i].offsetHeight + "px"; 
        collision = true;




        This could be cleaned up by setting collision to false at the start of the function and then abstracting the common lines:



        collision = false; //set initially
        for (i = 0; i < object.length; i++) 
         if (player.offsetTop < object[i].offsetTop + object[i].offsetHeight &&
         player.offsetHeight + player.offsetTop > object[i].offsetTop) 
         // Player Right to Object Left
         if (player.offsetLeft + player.offsetWidth <= object[i].offsetLeft + 2 &&
         player.offsetLeft + player.offsetWidth >= object[i].offsetLeft) 
         player.style.left = object[i].offsetLeft - player.offsetWidth + "px";
         collision = true;
         
         // Player Left to Object Right
         if (player.offsetLeft >= object[i].offsetLeft + object[i].offsetWidth - 2 &&
         player.offsetLeft <= object[i].offsetLeft + object[i].offsetWidth) 
         player.style.left = object[i].offsetLeft + object[i].offsetWidth + "px";
         collision = true;
         
         
         if (player.offsetLeft < object[i].offsetLeft + object[i].offsetWidth &&
         player.offsetLeft + player.offsetWidth > object[i].offsetLeft) 
         // Player Bottom to Object Top
         if (player.offsetTop + player.offsetHeight >= object[i].offsetTop &&
         player.offsetTop + player.offsetHeight <= object[i].offsetTop + 2) 
         player.style.top = object[i].offsetTop - player.offsetHeight + "px";
         collision = true;
         
         // Player Top to Object Bottom
         if (player.offsetTop <= object[i].offsetTop + object[i].offsetHeight &&
         player.offsetTop >= object[i].offsetTop + object[i].offsetHeight - 2) 
         player.style.top = object[i].offsetTop + object[i].offsetHeight + "px";
         collision = true;


        Minimize reflows from appending elements



        It might be wise to consider using a DocumentFragment or at least a container element to add the platforms and coins to, and then add that element to the body to minimize reflows. For more information on this and similar topics, check out this article Stop Writing Slow Javascript. I know it is a few years old but still has some relevant information.



        1https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id2https://crockford.com/javascript/code.html#indentation







        share|improve this answer















        share|improve this answer



        share|improve this answer








        edited Jun 7 at 23:55


























        answered Jun 7 at 17:00









        Sam Onela

        5,76961543




        5,76961543






















             

            draft saved


            draft discarded


























             


            draft saved


            draft discarded














            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f195993%2fplatformer-game-arrow-key-player-movement-and-collision-detection%23new-answer', 'question_page');

            );

            Post as a guest
































































            -

            Popular posts from this blog

            Greedy Best First Search implementation in Rust

            Image
            Clash Royale CLAN TAG #URR8PPP .everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0; up vote 1 down vote favorite I have implemented a Greedy Best First Search algorithm in Rust, since I couldn't find an already implemented one in the existing crates. I have a small pet project I do in Rust, and the Greedy BFS is at the core of it. The algorithm is designed to be as flexible as possible. Theoretically, this algorithm could be used as a greedy depth-first search or as a greedy a*. I want to design a good algorithm that I could later use in any other project needing it, not only this one. I am quite new to Rust and this is the second project I do in this language. Also, I am new to writing optimized algorithms. Since this search will be called often, my main concern is performance. What are some tips and tricks to make the algorithm perform better? Of course any other critique and comment is welcome. Bellow is the code: //! Here is
            Read more

            Function to Return a JSON Like Objects Using VBA Collections and Arrays

            Image
            Clash Royale CLAN TAG #URR8PPP .everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0; up vote 1 down vote favorite My goal is to create a compact function that can create a JSON Like object from JSON string. I want a function with a small footprint that I or anyone who wants to use it, can simply paste into a module and use. At 61 lines of code, I am happy with its size and portability. Here is an image of JSON Object created from string data using a ScriptControl . Although the Locals Window displays the properties and values correctly, the object itself is extremely difficult to work with. This image shows an object created using getJSONCollection . Because it is made of VBA Collections and Arrays, it is very easy to work with. Option Explicit Private Function getJSONCollection(ByVal Value As Variant, Optional ScriptEngine As Object) As Variant Const DELIMITER As String = "||" Dim col As Collection, JSON As Object, KeyNam
            Read more

            C++11 CLH Lock Implementation

            Image
            Clash Royale CLAN TAG #URR8PPP .everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0; up vote 1 down vote favorite 1 As a hobbyist programmer I have been reading "The Art of Multiprocessor Programming" (Herlihy, Maurice; Shavit, Nir) and converting the Java spin locks into C++ - I have enjoyed this and learned a lot. I have implemented the CLH spin queue lock (due to Travis Craig, Erik Hagersten, and Anders Landin) but I feel unhappy about the way that I have the raw pointers interacting with smart pointers and would be grateful for code review advice. As I understand it I must use a smart pointer, in this case std::unique_ptr , for my thread local node pointer or the lock will not deallocate properly and the clh_lock will leak. But... I also have no choice but to use a raw atomic tail queue pointer in order access the atomic exchange member function. Therefore, I have to come up with some combination of either, raw or smart
            Read more