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

RTS game improvement

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

Problem

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,spe

Solution

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.

Code Snippets

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;
var camera = { x : -50 , y : -50 },
    mouse  = { x : 0 , y : 0 },
    select  = { x1 : 0 , y1 : 0 , x2 : 0 , y2 : 0  };
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 )  }
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()  };
if(projectiles[i].damage<units[u].shield)
  units[u].shield-=projectiles[i].damage;
else
  units[u].health-=projectiles[i].damage;

Context

StackExchange Code Review Q#38842, answer score: 3

Revisions (0)

No revisions yet.