6
\$\begingroup\$

I'm looking for advice how to improve the code. It's a mess, but I don't know how to improve it. There are some mathematical errors, projectile movements are behaving strangely, code is messy in general, unit positions are off, etc.

var canvas = document.createElement("canvas");
canvas.id = "canvas";
var ctx = canvas.getContext("2d");
canvas.width = 1480;
canvas.height = 900;
document.body.appendChild(canvas);

var units = [];
var bgReady = false;
var bgImage = new Image();
bgImage.onload = function () {
    bgReady = true;
};
bgImage.src = "images/background.bmp";

camera = new Object();
camera.x = -50;
camera.y = -50;
camera.zoom = 1;
mouse = new Object();
mouse.x=0;
mouse.y=0;
select = new Object();
select.x1=0;
select.y1=0;
select.x2=0;
select.y2=0;
selected = [];
var change = 0;
var x1=0;
var x2=0;
var y1=0;
var y2=0;
var explosions=[];
var projectiles=[];
var count = 0;
var debug = 0;
function unit(name,health,letter,team,x,y,speed,accel,range,rate,type,damage,speed2,shield)
    {
        this.name = name;
        this.letter = letter;
        this.x = x;
        this.y = y;
        this.viewx = 0;
        this.viewy = 0;
        this.team = team;
        this.isselected = 0;
        this.state = ""; //idle
        this.maxspeed = speed;
        this.speed = 0;
        this.accel = accel;
        this.movex = 0;
        this.movey = 0;
        this.health = health;
        this.maxhealth = health;
        this.prange = range;
        this.pdamage = damage;
        this.prate = rate;
        this.ptype = type;
        this.pspeed = speed2;
        this.ang = 0;
        this.size = 5;
        this.shield = shield;
        this.maxshield = shield;
    }
function explosion(x,y,size,power,fade)
    {
        this.x = x;
        this.y = y;
        this.viewx = 0;
        this.viewy = 0;
        this.size = size;
        this.power = power;
        this.timer = 100;
        this.fade = fade;
    }       
function projectile(type,damage,x,y,targetx,targety,speed)
    {
        this.type = type;
        this.damage = damage;
        this.x = x;
        this.y = y;
        this.viewx = 0;
        this.viewy = 0;
        this.movex = targetx;
        this.movey = targety;
        this.speed = speed;
        this.timer = calcDist(x,y,targetx,targety)/speed;
        this.ang = Math.atan2((targety - y),(targetx - x)) * (180 / Math.PI);
        this.radius = 20;
    }
function explode(x,y)
    {
        explosions.push(Object.create(new explosion(x,y,1,0.25,1)));
    }

for(var i=0;i<40;i++)
    {
        units.push(Object.create(new unit("tank",100,"T","player2",150+Math.random()*200,150+Math.random()*200,0.2,0.002,175,4,"bullet",10,2,0)));
    }

for(var i=0;i<15;i++)
    {
        units.push(Object.create(new unit("mech",125,"M","player1",400+Math.random()*200,200+Math.random()*200,0.2,0.002,225,1,"bullet",4,2,100)));
    }

function calcDist(x1,y1,x2,y2)
    {
        var a = x2-x1;
        var b = y2-y1;
        return Math.sqrt((a*a)+(b*b));
    }
document.onkeydown = checkKey;
function checkKey(e) {

    e = e || window.event;

    if (e.keyCode == '38'&&camera.y>(-100*camera.zoom*1)) {
        // up arrow
        camera.y-=50;
    }
    if (e.keyCode == '40'&&camera.y<(0+(bgImage.height*(camera.zoom-1)))) {
        // down arrow
        camera.y+=50;
    }
    if (e.keyCode == '39'&&camera.x<(bgImage.width*0.7)*camera.zoom) {
        // right arrow
        camera.x+=50;
    }
    if (e.keyCode == '37'&&camera.x>(-100*camera.zoom)) {
        // left arrow
        camera.x-=50;
    }
}

this.canvas.onmousewheel = function(ev)
{
    //perform your own Event dispatching here
    if(ev.wheelDelta>0)
        {           
            if(camera.zoom<6)
                {
                    camera.x+=bgImage.width*0.2;
                    camera.y+=bgImage.height*0.2;
                    camera.zoom+=0.6;
                }
        }
        else
        {   
            if(camera.zoom>1)
                {
                    camera.x-=bgImage.width*0.2;
                    camera.y-=bgImage.height*0.2;
                    camera.zoom-=0.6;
                }
        }
    return false;
};


function getSelection()
    {

        if(select.x2>select.x1)
            {
                var x1 = select.x1;
                var x2 = select.x2;
            }
        else
            {
                var x1 = select.x2;
                var x2 = select.x1;
            }

        if(select.y2>select.y1)
                {
                    var y1 = select.y1;
                    var y2 = select.y2;
                }
            else
                {
                    var y1 = select.y2;
                    var y2 = select.y1;
                }
        if(change==1)
        {
            ctx.fillRect(x1-8,y1-8,x2-x1,y2-y1);
            ctx.beginPath();
            //ctx.stroke();
            ctx.closePath();
            ctx.fill();
        }

    }

function moveUnit(unit,x,y)
{
unit.movex = x-50;
unit.movey = y-50;
unit.state = "moving";
}

this.canvas.onmouseup = function(ev)
{

    if(select.x2>select.x1)
        {
            var x1 = select.x1;
            var x2 = select.x2;
        }
    else
        {
            var x1 = select.x2;
            var x2 = select.x1;
        }

    if(select.y2>select.y1)
            {
                var y1 = select.y1;
                var y2 = select.y2;
            }
        else
            {
                var y1 = select.y2;
                var y2 = select.y1;
            }

    for(var i=0;i<units.length;i++)
        {
            if (units[i].viewx>x1
            &&units[i].viewx<x2
            &&units[i].viewy>y1
            &&units[i].viewy<y2
            &&units[i].team=="player1"){
            selected.push(units[i]);
            }
        }
    change = 0; 
}

this.canvas.onmousedown = function(ev)
{   
    if(selected.length>0)
    {
        for(var i=0;i<selected.length;i++)
        {
            moveUnit(selected[i],mouse.x+((i%(selected.length/3))*40)-((selected.length/3)*20),mouse.y+((i/selected.length)*40)-40);
        }
    }

    selected.length = 0;
    select.x1 = ev.clientX;
    select.y1 = ev.clientY;
    change = 1;
}

window.onmousemove = function(ev)
{
    select.x2 = ev.clientX;
    select.y2 = ev.clientY;
    mouse.x = ev.clientX;
    mouse.y = ev.clientY;
}

function paard()
    {
        drawUnits();
        if(change==0)
        {
            for(var i=0;i<selected.length;i++)
                {
                    ctx.beginPath();
                    ctx.arc(selected[i].viewx+(camera.zoom*7*0.5),selected[i].viewy+(camera.zoom*9*0.5),25*camera.zoom,0,2*Math.PI);
                    ctx.strokeStyle = "rgba(0, 255, 0, 25)";
                    ctx.lineWidth = 5;
                    ctx.closePath();
                    ctx.stroke();
                }
        }                   
        getSelection();
        drawExplosions();
        drawProjectiles();              
    }
function drawUnits()
{
//  ctx.fillStyle = "rgb(25, 25, 25)";
    ctx.font = 12*camera.zoom+"px Helvetica";
    ctx.textAlign = "left";
    ctx.textBaseline = "top"; //top
    ctx.clearRect(-500,-500,2000,2000);
    ctx.drawImage(bgImage,-camera.x,-camera.y,bgImage.width*camera.zoom,bgImage.height*camera.zoom);
    for(var i=0;i<units.length;i++)
        {   
            if(units[i].team=="player1")
                {
                    ctx.fillStyle = "rgb(25, 25, 255)";
                }
                else
                {
                    ctx.fillStyle = "rgb(255, 25, 25)";
                }
            if(units[i].health<=0)
            {
                for(var z=0;z<5;z++)
                    {
                        explode(units[i].x+Math.random()*15,units[i].y+Math.random()*15);
                    }
                units.splice(i,1);
                break;
                return;
            }
            if(units[i].state=="moving")
                {
                    if(units[i].x<units[i].movex+1
                        &&units[i].x>units[i].movex-1
                        &&units[i].y<units[i].movey+1
                        &&units[i].y>units[i].movey-1)
                            {
                                units[i].state="";
                                units[i].speed=0;
                            }
                            else
                            {
                                var a = units[i].movex-units[i].x;
                                var b = units[i].movey-units[i].y;
                                var angleDeg = Math.atan2((units[i].movey - units[i].y),(units[i].movex - units[i].x)) * (180 / Math.PI);
                                units[i].ang = angleDeg;
                                if(Math.sqrt((a*a)+(b*b))>30)
                                    {
                                        if(units[i].speed<units[i].maxspeed)
                                        {
                                            units[i].speed+=units[i].accel;
                                        }
                                    }
                                else
                                    {
                                        if(units[i].speed>0.1)
                                        {
                                            units[i].speed-=units[i].accel*4;
                                        }
                                    }
                                units[i].x+=Math.sin(angleDeg)*units[i].speed;
                                units[i].y-=Math.cos(angleDeg)*units[i].speed;  
                            }
                }
            units[i].viewx = (units[i].x*camera.zoom)-camera.x;
            units[i].viewy = (units[i].y*camera.zoom)-camera.y;
            ctx.fillText(units[i].letter,units[i].viewx,units[i].viewy);
            if(units[i].shield<units[i].maxshield)
            {
            units[i].shield+=0.03*(units[i].maxshield/100);
            }
        }
}
function drawExplosions()
{
    for(var i=0;i<explosions.length;i++)
        {   
            ctx.strokeStyle = "rgba(255, "+explosions[i].timer*2.5+", 0, "+explosions[i].timer/100+")";

            if(explosions[i].timer>0)
                {
                    explosions[i].size+=explosions[i].power;
                    explosions[i].timer-=explosions[i].fade;
                    explosions[i].viewx = (explosions[i].x*camera.zoom)-camera.x;
                    explosions[i].viewy = (explosions[i].y*camera.zoom)-camera.y;

                    ctx.beginPath();
                    ctx.strokeStyle = "rgba(255, "+explosions[i].timer*2.5+", 0, "+explosions[i].timer/100+")";

                    ctx.arc(explosions[i].viewx,explosions[i].viewy,explosions[i].size*camera.zoom,0,2*Math.PI);
                    ctx.lineWidth = (explosions[i].size/3)*camera.zoom;
                    ctx.closePath();
                    ctx.stroke();
                }
            else
                {
                    explosions.splice(i,1);
                }

        }
}
function drawProjectiles()
{
    ctx.fillStyle = "rgb(255, 255, 25)";
    for(var i=0;i<projectiles.length;i++)
    {
        //var ang = Math.atan2((projectiles[i].movey - projectiles[i].y),(projectiles[i].movex - projectiles[i].x)) * (180 / Math.PI);
        projectiles[i].ang = Math.atan2((projectiles[i].movey - projectiles[i].y),(projectiles[i].movex - projectiles[i].x)) * (180 / Math.PI);
        projectiles[i].x += Math.sin(projectiles[i].ang)*projectiles[i].speed;
        projectiles[i].y -= Math.cos(projectiles[i].ang)*projectiles[i].speed;
        projectiles[i].viewx = (projectiles[i].x*camera.zoom)-camera.x;
        projectiles[i].viewy = (projectiles[i].y*camera.zoom)-camera.y;
        projectiles[i].timer -= 1;
        ctx.fillText(".",projectiles[i].viewx,projectiles[i].viewy-(10*camera.zoom));
        if(projectiles[i].timer <= 0)
        {
            if(projectiles[i].type=="bullet")
            {
            for(var u=0;u<units.length;u++)
                {
                    if(units[u].x<projectiles[i].x+units[u].size
                    &&units[u].x>projectiles[i].x-units[u].size
                    &&units[u].y<projectiles[i].y+units[u].size
                    &&units[u].y>projectiles[i].y-units[u].size)
                    {
                        if(projectiles[i].damage<units[u].shield)
                            {
                                units[u].shield-=projectiles[i].damage;
                            }
                            else
                            {
                                units[u].health-=projectiles[i].damage;
                            }
                        break;
                    }
                }   
            }
            if(projectiles[i].type=="explosive")
            {
            for(var u=0;u<units.length;u++)
                {
                    if(units[u].x<projectiles[i].x+projectiles[i].radius
                    &&units[u].x>projectiles[i].x-projectiles[i].radius
                    &&units[u].y<projectiles[i].y+projectiles[i].radius
                    &&units[u].y>projectiles[i].y-projectiles[i].radius)
                    {
                        if(projectiles[i].damage<units[u].shield)
                            {
                                units[u].shield-=projectiles[i].damage;
                            }
                            else
                            {
                                units[u].health-=projectiles[i].damage;
                            }
                    }
                }   
            }
            projectiles.splice(i,1);
        }

    }
}




function attack()
{
    //
    count++;
    for(var i=0;i<units.length;i++)
        {
            if(count%units[i].prate==0)
                {
                    for(var u=0;u<units.length;u++)
                        {
                            if(units[i].team!=units[u].team&&calcDist(units[i].x,units[i].y,units[u].x,units[u].y)<units[i].prange&&Math.random()<1.5)
                            {
                                var d = calcDist(units[i].x,units[i].y,units[u].x,units[u].y);
                                var sx = Math.sin(units[u].ang)*units[u].speed*(d/units[i].pspeed);
                                var sy = Math.cos(units[u].ang)*units[u].speed*(d/units[i].pspeed);
                                projectiles.push(Object.create(new projectile(units[i].ptype,units[i].pdamage,units[i].x,units[i].y,units[u].x+sx,units[u].y-sy,units[i].pspeed)));
                                break;
                                //return;
                            }
                        }
                }
        }
}


setInterval(paard,25);
setInterval(attack,1000);

Here's the download link for the game.

\$\endgroup\$
  • 1
    \$\begingroup\$ Do you have prior experience with object oriented programming? Is this your first larger programming project? Knowing this could help to write a more useful code review. \$\endgroup\$ – amon Jan 8 '14 at 17:34
  • 4
    \$\begingroup\$ Your lack of confidence in your code is concerning. CodeReview is for reviewing working code. It does not sound like your code works. Does it work? \$\endgroup\$ – rolfl Jan 8 '14 at 18:36
  • 4
    \$\begingroup\$ @rolfl I think it should be possible to provide tips for improving the code without necessarily fixing whatever bugs might be present. \$\endgroup\$ – 200_success Jan 8 '14 at 20:52
  • 3
    \$\begingroup\$ @rolfl : prior work is here : codereview.stackexchange.com/questions/37672/… \$\endgroup\$ – konijn Jan 8 '14 at 21:23
  • \$\begingroup\$ Does it work? Apparently you did not even consider reading the whole post... \$\endgroup\$ – none Jan 9 '14 at 11:54
3
\$\begingroup\$

Object Literal Notation

There are number of objects that would be easier to create

camera = new Object();
camera.x = -50;
camera.y = -50;
camera.zoom = 1;
mouse = new Object();
mouse.x=0;
mouse.y=0;
select = new Object();
select.x1=0;
select.y1=0;
select.x2=0;
select.y2=0;

could be

var camera = { x : -50 , y : -50 },
    mouse  = { x : 0 , y : 0 },
    select  = { x1 : 0 , y1 : 0 , x2 : 0 , y2 : 0  };

if you create a Point constructor you could even do

function Point( x, y ){
  this.x = x;
  this.y = y;
}

var camera = new Point( -50, -50),
    mouse = new Point( 0, 0 ),
    select = {  from : new Point( 0, 0 ) , to : new Point( 0, 0 )  }

You would still repeat ( 0, 0 ) a lot, it seems you just want to initialize the x/y variable and you would still use new a lot, that could be helped with this approach:

function Point( x, y ){
  if ( !(this instanceof Point) )
    return new Point(x,y);
  this.x = x || 0;
  this.y = y || 0;
}

var camera  = Point( -50, -50),
     mouse  = Point(),
     select = {  from : Point() , to : Point()  };

Naming

  • Constructors should start with an upper case so unit -> Unit , explosion -> Explosion.

  • paard: like kaas; we all like Dutch words, but code should only have names based on English.

  • You are a bit too Spartan with d, sx and sy.

Magical constants

  • Team colors should be properly named constants, frankly I would have liked to see ctx.fillStyle = team[i].color;
  • There are too many constants to list them all, I think you get the picture

Extracting code into functions

  • The drawing code in paard could use it's own function
  • The shooting code in attack() could use it's own function

DRY

  • The code for dealing with explosive and bullet is mostly copy pasted, consolidate that

Gameplay

if(projectiles[i].damage<units[u].shield)
  units[u].shield-=projectiles[i].damage;
else
  units[u].health-=projectiles[i].damage;

Are you sure that large damages completely bypass shield and are not at all absorbed by the shield ?

Finally, I downloaded your game. Even though limited it was fun to kill all the baddies. There is more to review in your code but you should take care of these items first.

\$\endgroup\$
0
\$\begingroup\$

When I make games in canvas, I use a grid system if it is applicable to said game. I think it could be applicable to your RTS.

A coordinate on my grid looks like this: "10x15" (X = 10, Y =15).

The grid itself, once generated, looks something like this:

var grid = ["0x0","0x1","0x2","0x3"
            "1x0","1x1","1x2","1x3"
            "2x0","2x1","2x2","2x3"];

When tracking a unit or units, you can use an array of objects like this:

var units = [
    {
        "type": "Infantry",
        "unitId": 32,
        "x": 10,
        "y": 15,
        "coords": "10x15"
    },
    {
        "type": "Artillery",
        "unitId": 45
        "x": 15,
        "y": 15,
        "coords": "15x15"
    }
];

By using an object that contains an x, y, and a combination of those points that forms a coordinate, it is much easier to move objects around and track their location for things like collision.

Hope this helps!

\$\endgroup\$

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.