patternjavascriptMinor
RTS game improvement
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
```
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
could be
if you create a Point constructor you could even do
You would still repeat ( 0, 0 ) a lot, it seems you just want to initialize the x/y variable and you would still use
Naming
-
Constructors should start with an upper case so
-
-
You are a bit too Spartan with
Magical constants
Extracting code into functions
DRY
Gameplay
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.
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
paardcould use it's own function
- The shooting code in
attack()could use it's own function
DRY
- The code for dealing with
explosiveandbulletis 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.