patternjavascriptMinor
Xonix game in JavaScript/Canvas: detection of conquered regions
Viewed 0 times
conqueredxonixjavascriptgamedetectionregionscanvas
Problem
I've written a Xonix clone in Javascript/Canvas. It seems to work fine. However I would like to refactor the code of detecting conquered regions which seems to be bulky (see methods
Below is the relevant code. The full code is available on GitHub.
```
// cell attributes:
var CA_CLEAR = 1 = 0 && x = 0 && y = 0; i--) {
fillCellArea.apply(null, [cfgMain.colorFill].concat(this.aTrailRects[i]));
}
this.aTrailRects = [];
}
},
isPosValid: function(x, y) {
return x >= -2 && x = -2 && y = 0? this.aCells[i] : 0;
},
set: function(x, y, v) {
var i = this.index(x,y);
if (i >= 0) this.aCells[i] = v;
return i;
},
setOn: function(x, y, v) {
var i = this.index(x,y);
if (i >= 0) this.aCells[i] |= v;
return i;
},
setOff: function(x, y, v) {
var i = this.index(x,y);
if (i >= 0) this.aCells[i] &= ~v;
return i;
},
applyRelDirs: function(x, y, dir, aDeltas) {
var ret = [];
for (var n = aDeltas.length, i = 0; i 1)
this.aTrailNodes.push(this.aTrail[nTrail-1]);
var aConqRects = this._conquer() || this._buildTrailRects();
this.aTrail = []; this.aTrailNodes = [];
if (!aConqRects || !aConqRects.length) return;
for (var n = aConqRects.length, i = 0; i = 4 &&
((delta = Math.abs(this.aTrailNodes[0] - this.aTrailNodes[nNodes-1])) == 1 || delta == this.nWx);
if (bClosedTrail) { // if the cursor trail is self-closed
aOutlineset.push([this.aTrailNodes, 1]);
}
var bAddTrailRects = false;
var posPre = this.pos(this.cellPreTrail), posCr = cursor.pos();
var aDeltas = [-90, 90];
for (var side = 0; side = 0) {
var pos2 = l 3) {
iLastNode = ret[3];
l = ret[4];
bEndAtN
conquer, _conquer, _findOutline and _buildConquerRects).Below is the relevant code. The full code is available on GitHub.
```
// cell attributes:
var CA_CLEAR = 1 = 0 && x = 0 && y = 0; i--) {
fillCellArea.apply(null, [cfgMain.colorFill].concat(this.aTrailRects[i]));
}
this.aTrailRects = [];
}
},
isPosValid: function(x, y) {
return x >= -2 && x = -2 && y = 0? this.aCells[i] : 0;
},
set: function(x, y, v) {
var i = this.index(x,y);
if (i >= 0) this.aCells[i] = v;
return i;
},
setOn: function(x, y, v) {
var i = this.index(x,y);
if (i >= 0) this.aCells[i] |= v;
return i;
},
setOff: function(x, y, v) {
var i = this.index(x,y);
if (i >= 0) this.aCells[i] &= ~v;
return i;
},
applyRelDirs: function(x, y, dir, aDeltas) {
var ret = [];
for (var n = aDeltas.length, i = 0; i 1)
this.aTrailNodes.push(this.aTrail[nTrail-1]);
var aConqRects = this._conquer() || this._buildTrailRects();
this.aTrail = []; this.aTrailNodes = [];
if (!aConqRects || !aConqRects.length) return;
for (var n = aConqRects.length, i = 0; i = 4 &&
((delta = Math.abs(this.aTrailNodes[0] - this.aTrailNodes[nNodes-1])) == 1 || delta == this.nWx);
if (bClosedTrail) { // if the cursor trail is self-closed
aOutlineset.push([this.aTrailNodes, 1]);
}
var bAddTrailRects = false;
var posPre = this.pos(this.cellPreTrail), posCr = cursor.pos();
var aDeltas = [-90, 90];
for (var side = 0; side = 0) {
var pos2 = l 3) {
iLastNode = ret[3];
l = ret[4];
bEndAtN
Solution
I haven't had a chance to go through the whole code yet, but I noticed that you have an if statement that isn't really needed here
if you try to run the for loop on an empty loop the condition will not be met and the loop will not loop. you don't need the if statement.
and the last statement inside the if statement would return an empty array anyway, so you would have something like this
Right Here inside the for loop
you might as well combine these into one if statement like this
You shouldn't double up variables and statements like this
it makes it hard to see what is going on, especially with as much code as you have here, all with out comments. it should look like this
and it doesn't look like you actually want
render: function() {
if (this.aTrailRects.length) {
for (var i = this.aTrailRects.length-1; i >= 0; i--) {
fillCellArea.apply(null, [cfgMain.colorFill].concat(this.aTrailRects[i]));
}
this.aTrailRects = [];
}
},if you try to run the for loop on an empty loop the condition will not be met and the loop will not loop. you don't need the if statement.
and the last statement inside the if statement would return an empty array anyway, so you would have something like this
render: function() {
for (var i = this.aTrailRects.length-1; i >= 0; i--) {
fillCellArea.apply(null, [cfgMain.colorFill].concat(this.aTrailRects[i]));
}
this.aTrailRects = [];
},Right Here inside the for loop
// find outline of conquered region (polygon)
// from given cell position (x0, y0) and starting direction (dir)
// as well as indices of starting cell and last node cell in the trail:
_findOutline: function(x0, y0, dir, iStartCell, iLastNode) {
....
if (aCurrUsed.indexOf(d) >= 0) continue;
if (isClear(aPosOpts[i])) continue;you might as well combine these into one if statement like this
if (aCurrUsed.indexOf(d) >= 0 || isClear(aPosOpts[i]))
continue;You shouldn't double up variables and statements like this
var min = false, idx = false;
for (i = 0; i < nPass; i++) {
if (!i || aPassWeight[i] < min) {
min = aPassWeight[i]; idx = aPassIdx[i];
}
}it makes it hard to see what is going on, especially with as much code as you have here, all with out comments. it should look like this
var min = false
var idx = false;
for (i = 0; i < nPass; i++) {
if (!i || aPassWeight[i] < min) {
min = aPassWeight[i];
idx = aPassIdx[i];
}
}and it doesn't look like you actually want
min and idx to be false, that you actually want them to be a number. so to make this a little more readable you should assign min and idx before you start the loop and just start the incrementation variable at 1 instead of 0. check it outvar min = aPassWeight[0];
var idx = aPassIdx[0];
for (i = 1; i < nPass; i++) {
if (aPassWeight[i] < min) {
min = aPassWeight[i];
idx = aPassIdx[i];
}
}Code Snippets
render: function() {
if (this.aTrailRects.length) {
for (var i = this.aTrailRects.length-1; i >= 0; i--) {
fillCellArea.apply(null, [cfgMain.colorFill].concat(this.aTrailRects[i]));
}
this.aTrailRects = [];
}
},render: function() {
for (var i = this.aTrailRects.length-1; i >= 0; i--) {
fillCellArea.apply(null, [cfgMain.colorFill].concat(this.aTrailRects[i]));
}
this.aTrailRects = [];
},// find outline of conquered region (polygon)
// from given cell position (x0, y0) and starting direction (dir)
// as well as indices of starting cell and last node cell in the trail:
_findOutline: function(x0, y0, dir, iStartCell, iLastNode) {
....
if (aCurrUsed.indexOf(d) >= 0) continue;
if (isClear(aPosOpts[i])) continue;if (aCurrUsed.indexOf(d) >= 0 || isClear(aPosOpts[i]))
continue;var min = false, idx = false;
for (i = 0; i < nPass; i++) {
if (!i || aPassWeight[i] < min) {
min = aPassWeight[i]; idx = aPassIdx[i];
}
}Context
StackExchange Code Review Q#100802, answer score: 4
Revisions (0)
No revisions yet.