11
\$\begingroup\$

I'm still pretty new to JavaScript. I'm making a game to get a better understanding. Although it's working exactly how I want it to right now, the code could definitely be less repetitive and more organized. I'm just not sure how to go about that. I'm hoping someone can take a look and simply give me some pointers about ways to make this more efficient, especially the repetitive collision functions.

Basic things about the game: The goal is to collect as many of the white nodes as possible before your cursor moves 5000 pixels. The big purple nodes slow you down, but still count you pixels normally. The red nodes add 1000 pixels to your distance counter, the green ones subtract 1000 pixels.

http://codepen.io/sketchcrush/pen/rOyvya

$(document).ready(function () {
  
//CREATES NEW COLLECTABLE NODES
    (function makeDiv() {
        var divsize = ((Math.random() * 100) + 50).toFixed();
        var colors = Array('FFF');
        var color = '#' + colors[Math.floor(Math.random() * colors.length)];
        $newdiv = $('<div/>').css({
            'filter': 'blur(100px)',
                'width': '40px',
                'height': '40px',
                'border-radius': '100%',
                'z-index': '-1',
                'filter': 'blur(2px)',
                '-webkit-filter': 'blur(2px)',
                'background-color': color
        });

        var posx = (Math.random() * ($(document).width() - divsize)).toFixed();
        var posy = (Math.random() * ($(document).height() - divsize)).toFixed();


        $newdiv.css({
            'position': 'absolute',
                'left': posx + 'px',
                'top': posy + 'px',
                'display': 'none'
        }).appendTo('#track').fadeIn(200).attr('id', 'node');

 //DETECTS COLLITION WITH COLLECTABLE NODES
        function collision_node($cursor, $node) {
            var x1 = $cursor.offset().left;
            var y1 = $cursor.offset().top;
            var h1 = $cursor.outerHeight(true);
            var w1 = $cursor.outerWidth(true);
            var b1 = y1 + h1;
            var r1 = x1 + w1;
            var x2 = $node.offset().left;
            var y2 = $node.offset().top;
            var h2 = $node.outerHeight(true);
            var w2 = $node.outerWidth(true);
            var b2 = y2 + h2;
            var r2 = x2 + w2;

            if (b1 < y2 || y1 > b2 || r1 < x2 || x1 > r2) {
                return false;
            } else {
              
            //REMOVES A NODE WHEN COLLECTED, CREATES A NEW ONE
                $('#node').fadeOut(100, function () {
                    $('#node').remove();
                    makeDiv();
                });
                //return true;
            }
        }

        window.setInterval(function () {
            $('#result').text(collision_node($('#cursor'), $('#node')));
        }, 200);

    })();

});

//A COUNTER FOR THE NODES COLLECTED WILL BE CREATED LATER

//DETECT THE CURSOR'S COORDINATES
(function cursorMapping() {

    var $mouseX = 0,
        $mouseY = 0;
    var $xp = 0,
        $yp = 0;

    $(document).mousemove(function (e) {
        $mouseX = e.pageX;
        $mouseY = e.pageY;
    });

    function showCoords(event) {
        var x = event.clientX;
        var y = event.clientY;
        var coor = "X: " + x + ", Y: " + y;
    }


//DECLARING VARS THAT WILL BE USED TO DETECT POSITION AND SPEED
    var mrefreshinterval = 500; // update display every 500ms
    var lastmousex = -1;
    var lastmousey = -1;
    var lastmousetime;
    var mousetravel = 0;
    var lastmousetravel = 0;



    var speed;
    var marker1 = 1;
    var marker2 = 1;

    var timer = setInterval(function () {
        marker1;
        marker2;
    }, 20);

  
//THIS FUNCTION CHANGES THE CURSOR'S COLOR DEPENDING ON ITS SPEED (NOT AVAILABLE IN THIS CODEPEN)
    var thisInterval = setInterval(function FXInterval() {

        speed = $('#speed').text();
        if (marker1 === marker2 && lastmousetravel === mousetravel || speed < 1.5) {
            $('#cursor').attr('class', 'cursor_transition_revert');
            $('#cursor').css({
                'background-color': '#7AA8CE'
            });
            clearInterval(timer);
            var timer;
        } else {
            $('#cursor').attr('class', 'cursor_transition');
            $('#cursor').css({
                'background-color': '#CE7A7A'
            });
        }

        if (mousetravel > 5000) {
            alert('Done!');
            clearInterval(thisInterval);
        }

    }, 20);

//DETERMINES THE AMOUNT OF PIXELS TRAVELED
    $('html').mousemove(function (e) {
        var mousex = e.pageX;
        var mousey = e.pageY;
        if (lastmousex > -1) mousetravel += Math.max(Math.abs(mousex - lastmousex), Math.abs(mousey - lastmousey));
        lastmousex = mousex;
        lastmousey = mousey;
        var speed = lastmousex + lastmousey;

        setTimeout(function () {
            lastmousetravel = mousetravel;
        }, 20);

        document.getElementById("mousetravel").innerHTML = 'Distance: ' + mousetravel;
    });

    var newSpeed;
    var newColor;
  
//DETECTS COLLISION WITH THE PURPLE NODES THAT SLOW YOUR CURSOR
    setInterval(function () {
        function collision_thing($cursor, $thing) {
            var x1 = $cursor.offset().left;
            var y1 = $cursor.offset().top;
            var h1 = $cursor.outerHeight(true);
            var w1 = $cursor.outerWidth(true);
            var b1 = y1 + h1;
            var r1 = x1 + w1;
            var collides = false;

            $thing.each(function (i) {
                var x2 = $(this).offset().left;
                var y2 = $(this).offset().top;
                var h2 = $(this).outerHeight(true);
                var w2 = $(this).outerWidth(true);
                var b2 = y2 + h2;
                var r2 = x2 + w2;

                collides = collides || !(b1 < y2 || y1 > b2 || r1 < x2 || x1 > r2);
            });

            newSpeed = collides ? 200 : 20;
            //newColor = collides ? '#000' : '#fff';
            changeCursorPosition();

            function changeCursorPosition() {
                $xp += (($mouseX - $xp) / newSpeed);
                $yp += (($mouseY - $yp) / newSpeed);
                $("#cursor").css({
                    left: $xp + 'px',
                    top: $yp + 'px'
                });
                //$($thing).css({'background-color': newColor}); 
            }
        }
        $(collision_thing($('#cursor'), $('.thing')));
    }, 20);

//DETECTS COLLISION WITH THE RED NODES THAT ADD 1000 PIXELS TO YOUR DISTANCE COUNT
    setInterval(function () {
        function collision_add1000($cursor, $add1000) {
            var x1 = $cursor.offset().left;
            var y1 = $cursor.offset().top;
            var h1 = $cursor.outerHeight(true);
            var w1 = $cursor.outerWidth(true);
            var b1 = y1 + h1;
            var r1 = x1 + w1;
            var collides = false;

            $add1000.each(function (i) {
                var x2 = $(this).offset().left;
                var y2 = $(this).offset().top;
                var h2 = $(this).outerHeight(true);
                var w2 = $(this).outerWidth(true);
                var b2 = y2 + h2;
                var r2 = x2 + w2;

                if (b1 < y2 || y1 > b2 || r1 < x2 || x1 > r2) {
                    //return false;
                } else {
                    mousetravel -= 1000;
                    $($add1000).remove();
                    //return true;
                }

            });

        }
        $(collision_add1000($('#cursor'), $('#add1000_1')));
        $(collision_add1000($('#cursor'), $('#add1000_2')));
    }, 20);

//DETECTS COLLISION WITH THE GREEN NODES THAT SUBTRACT 1000 PIXELS TO YOUR DISTANCE COUNT
    setInterval(function () {
        function collision_sub1000($cursor, $sub1000) {
            var x1 = $cursor.offset().left;
            var y1 = $cursor.offset().top;
            var h1 = $cursor.outerHeight(true);
            var w1 = $cursor.outerWidth(true);
            var b1 = y1 + h1;
            var r1 = x1 + w1;
            var collides = false;

            $sub1000.each(function (i) {
                var x2 = $(this).offset().left;
                var y2 = $(this).offset().top;
                var h2 = $(this).outerHeight(true);
                var w2 = $(this).outerWidth(true);
                var b2 = y2 + h2;
                var r2 = x2 + w2;

                if (b1 < y2 || y1 > b2 || r1 < x2 || x1 > r2) {} else {
                    mousetravel += 1000;
                    $($sub1000).remove();
                }

            });

        }
        $(collision_sub1000($('#cursor'), $('#sub1000_1')));
        $(collision_sub1000($('#cursor'), $('#sub1000_2')));
    }, 20);

})();
html, body {
    margin: 0;
    padding: 0;
    height: 100%;
    width: 100%;
    overflow: hidden;
    cursor: none;
}
#cursor {
    width: 50px;
    height: 50px;
    position: fixed;
    border-radius: 25px;
}
#cursor_info, #db_info {
    color: #000;
    position: absolute;
    top: 0;
}
#db_info {
    right: 0;
}
#flex_container {
    width: 100%;
    height: 100%;
    display: flex;
    flex-direction: row;
    justify-content: center;
    align-items: center;
}
#track {
    width: 100%;
    height: 100%;
}
.container {
    width: 100%;
    height: 100%;
    position: relative;
    background: linear-gradient(-45deg, #333, #181818 50%, #333);
    z-index: -1;
}
.cursor_transition_revert {
    background-color: #7AA8CE;
    box-shadow: 0px 0px 30px #7AA8CE;
    animation-name: cursor_transition_revert;
    animation-duration: 2s;
    animation-play-state: running;
}
@keyframes cursor_transition_revert {
    0% {
        background-color: #CE7A7A;
        box-shadow: 0px 0px 30px #CE7A7A;
    }
    100% {
        background-color: #7AA8CE;
        box-shadow: 0px 0px 30px #7AA8CE;
    }
}
.hud {
    font-size: 2em;
    color: #fff;
    font-family: Arial;
    -webkit-animation: neon1 1.5s ease-in-out infinite alternate;
    -moz-animation: neon1 1.5s ease-in-out infinite alternate;
    animation: neon1 1.5s ease-in-out infinite alternate;
}
@keyframes neon1 {
    from {
        text-shadow: 0 0 10px #fff, 0 0 20px #fff, 0 0 30px #fff, 0 0 40px #FF1177, 0 0 70px #FF1177, 0 0 80px #FF1177, 0 0 100px #FF1177, 0 0 150px #FF1177;
    }
    to {
        text-shadow: 0 0 5px #fff, 0 0 10px #fff, 0 0 15px #fff, 0 0 20px #FF1177, 0 0 35px #FF1177, 0 0 40px #FF1177, 0 0 50px #FF1177, 0 0 75px #FF1177;
    }
}
.thing {
    border-radius: 35px;
    animation: thing_glow 1.5s ease-in-out infinite alternate;
}
@keyframes thing_glow {
    from {
        box-shadow: 0px 0px 30px #4625AB;
        filter: blur(2px);
        -webkit-filter:blur(2px);
    }
    to {
        box-shadow: 0px 0px 100px #4625AB;
        filter: blur(12px);
        -webkit-filter:blur(12px);
    }
}
.add1000 {
    border-radius: 20px;
    animation: add1000_glow 1s ease-in-out infinite alternate;
}
@keyframes add1000_glow {
    from {
        box-shadow: 0px 0px 20px #37CF89;
        filter: blur(2px);
        -webkit-filter:blur(2px);
    }
    to {
        box-shadow: 0px 0px 40px #37CF89;
        filter: blur(12px);
        -webkit-filter:blur(12px);
    }
}
.sub1000 {
    border-radius: 20px;
    animation: sub1000_glow 1.5s ease-in-out infinite alternate;
}
@keyframes sub1000_glow {
    0% {
        box-shadow: 0px 0px 20px #D42440;
        filter: blur(2px);
        -webkit-filter:blur(2px);
    }
    50% {
        box-shadow: 0px 0px 20px #D42440;
        filter: blur(2px);
        -webkit-filter:blur(2px);
    }
    to {
        box-shadow: 0px 0px 40px #D42440;
        filter: blur(12px);
        -webkit-filter:blur(12px);
    }
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id='track'>
    <div class='container'>
        <div id='cursor'>&nbsp;</div>
        <div class='thing' style='width:70px; height:70px; background: #4625AB; position: absolute; bottom: 400px; right: 300px; z-index: -1;'>&nbsp;</div>
        <div class='thing' style='width:70px; height:70px; background: #4625AB; position: absolute; bottom: 200px; right: 400px; z-index: -1;'>&nbsp;</div>
        <div class='add1000' id='add1000_1' style='width:40px; height:40px; background: #fff; position: absolute; bottom: 200px; right: 700px; z-index: -1;'>&nbsp;</div>
        <div class='add1000' id='add1000_2' style='width:40px; height:40px; background: #fff; position: absolute; bottom: 100px; right: 600px; z-index: -1;'>&nbsp;</div>
        <div class='sub1000' id='sub1000_1' style='width:40px; height:40px; background: #fff; position: absolute; bottom: 150px; right: 150px; z-index: -1;'>&nbsp;</div>
        <div class='sub1000' id='sub1000_2' style='width:40px; height:40px; background: #fff; position: absolute; bottom: 250px; right: 500px; z-index: -1;'>&nbsp;</div>
    </div>
</div>
<div id='cursor_info'>
    <p id='mousetravel' class='hud'></p>
</div>

\$\endgroup\$
16
\$\begingroup\$

I will only address the repetition in your collision detection code in this answer.

The following lines of code are repeated several times:

var x1 = $el.offset().left;
var y1 = $el.offset().top;
var h1 = $el.outerHeight(true);
var w1 = $el.outerWidth(true);
var b1 = y1 + h1;
var r1 = x1 + w1;

The only thing that is changed is the value of $el. Therefore, you can move this code to a seperate function which receives $el. To get the 4 values you need out of one function call, you can return an object with four fields: (I renamed the variables to improve readability)

function getBoundingBox($el) {
    var left = $el.offset().left;
    var top = $el.offset().top;
    var height = $el.outerHeight(true);
    var width = $el.outerWidth(true);
    var bottom = top + height;
    var right = left + width;

    return {
        left: left,
        top: top,
        bottom: bottom,
        right: right
    };
}

Using this function, the code for collision_node becomes:

function collision_node($cursor, $node) {
    var cursorBB = getBoundingBox($cursor);
    var nodeBB = getBoundingBox($node);

    if (cursorBB.bottom < nodeBB.top || cursorBB.top > nodeBB.bottom || cursorBB.right < nodeBB.left || cursorBB.left > nodeBB.right) {
        return false;
    } else {

    //REMOVES A NODE WHEN COLLECTED, CREATES A NEW ONE
        $('#node').fadeOut(100, function () {
            $('#node').remove();
            makeDiv();
        });
    }
}

The following condition is also repeated:

cursorBB.bottom < nodeBB.top || cursorBB.top > nodeBB.bottom || cursorBB.right < nodeBB.left || cursorBB.left > nodeBB.right

Since the condition depends on two objects (the two bounding boxes), it can be factored out to a seperate function with two arguments:

function testCollision(bb1, bb2) {
    return !(bb1.bottom < bb2.top || bb1.top > bb2.bottom || bb1.right < bb2.left || bb1.left > bb2.right)
}

Which can be rewritten to avoid inverting the condition: (the original condition in collision_node checked if there was no collision)

function testCollision(bb1, bb2) {
    return (bb1.bottom >= bb2.top && bb1.top <= bb2.bottom && bb1.right >= bb2.left && bb1.left <= bb2.right)
}

Now collision_node looks like this:

function collision_node($cursor, $node) {
    var cursorBB = getBoundingBox($cursor);
    var nodeBB = getBoundingBox($node);

    if (!testCollision(cursorBB, nodeBB)) {
        return false;
    } else {

        //REMOVES A NODE WHEN COLLECTED, CREATES A NEW ONE
        $('#node').fadeOut(100, function () {
                $('#node').remove();
                makeDiv();
        });
    }
}

Now change the other two places where you have collision testing code to use these two functions. (and add a comment with a link to an updated codepen snippet!)


The important change here was to group several variables into a single object. When you have several variables that are always used together, they should usually be grouped into a single object. This allows you to treat all of them as one whole instead of a collection of seperate variables. For example, The testCollision function receives two bounding boxes instead of eight numbers.

Good luck!

(Also, go over your code and get rid of unused stuff, e.g. you don't have a #result element and you don't need the colors arrays if it has one color, also remove commented-out code)

\$\endgroup\$
  • 9
    \$\begingroup\$ Wow, nice first answer, welcome to Code Review, and please keep 'em coming! \$\endgroup\$ – janos Oct 3 '15 at 21:15
  • \$\begingroup\$ This is a beautiful answer. Thank you so much! I will post an updated codepen here tonight so everyone can see it in action. \$\endgroup\$ – jack_of_all_trades Oct 4 '15 at 21:56
  • 1
    \$\begingroup\$ I would also remove the else from the collision_node function.It's unnecessary. once you return false no more of the functions code gets exectuted anyway. \$\endgroup\$ – Gary Storey Oct 5 '15 at 16:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.