[RESOLVED] Help: How come this for-loop won't finish sometimes?
Hello Community
I'm currently trying my skill at javascripting for the first time in coding a Civilization Revolution clone. Perhaps not the easiest choice of task, I know. I've only gotten started and I'm already running into problems with the map generation. Thus, I would much appreciate your knowledgeable advice.
Below is the code I came up with. The problem that keeps occurring when running the code is this: Sometimes, the map generation will just stop short. (Infrequently, the map will generate completely.) As you will see upon inspecting my code, I'm using a div (with id = 'map') for displaying the map, appending other divs (with className = 'col') to it to make the map's columns and appending these with yet more divs (with className = 'tile') to make the actual tiles of the map.
When tweaking the code I found I could increase the chance of the map generating completely by decreasing the number of rows (stored in variable 'Rows'). In fact, it seems that once the first column has generated with all its tiles, so will the rest of the columns and the map will generate fully. That is to say: The abortion of the map generation only seems to occur whenever there's something wrong with the FIRST column.
I'm relying on a randomised variable (called 't') for the allocation of different terrains to the tiles. Perhaps that's what causes the bug?
Again: I would much appreciate your help in this matter. As of now, the map generation isn't all that fancy. I would really love to improve on it to come up with some nifty maps once this bug is squashed.
function checkAdjacentTilesForLand (x, y) {
if (map[x-1][y-1].terrain !== "sea") return true;
if (map[x] [y-1].terrain !== "sea") return true;
if (map[x+1][y-1].terrain !== "sea") return true;
if (map[x-1][y] .terrain !== "sea") return true;
if (map[x+1][y] .terrain !== "sea") return true;
if (map[x-1][y+1].terrain !== "sea") return true;
if (map[x] [y+1].terrain !== "sea") return true;
if (map[x+1][y+1].terrain !== "sea") return true;
return false;
}
// Set Map Dimensions
// ******************
var Cols = 80; // Set map width
var Rows = 45; // Set map height
// Set Partioning of Terrains
// **************************
var Sea = 40; // Set % of sea-tiles
var Plains = 10; // Set % of plain-tiles
var Grasslands = 10; // Set % of grassland-tiles
var Forests = 15; // Set % of forest-tiles
var Hills = 10; // Set % of hill-tiles
var Mountains = 10; // Set % of mountain-tiles
var Deserts = 5; // Set % of desert-tiles
My suspicion is an error in the following logic although I have not investigated all of your code at this time.
Note the addition of the following range checks for the passed x and y parameters.
I'm not sure what you expect to happen if x=0 and then you look at element x[-1]
but I'm pretty sure you will not get what you expect unless it has been initialized.
Same thing for the y parameter.
I don't know if the return should be true or false so you might want to re-evaluate you logic about this function
Code:
function checkAdjacentTilesForLand (x, y) {
if ((x-1) <0) { return true; } // ???
if ((y-1) <0) { return true; } // ???
if ((x+1) >= map.length) { return true; } // ???
if ((y+1) >= map[x].length) { return true; } // ???
if (map[x-1][y-1].terrain !== "sea") return true;
if (map[x] [y-1].terrain !== "sea") return true;
if (map[x+1][y-1].terrain !== "sea") return true;
if (map[x-1][y] .terrain !== "sea") return true;
if (map[x+1][y] .terrain !== "sea") return true;
if (map[x-1][y+1].terrain !== "sea") return true;
if (map[x] [y+1].terrain !== "sea") return true;
if (map[x+1][y+1].terrain !== "sea") return true;
return false;
}
Also note that your following code, the "if ... else" statements are not correct syntax
The ';' character indicates the end of a statement, so the else portion of your first "if" will not get executed.
Code:
/ Set Terrain
// ***********
var t = Math.floor(Math.random()*100)+1;
var terrain;
if (t < Sea) terrain = "sea";
else if (t < Sea + Plains) terrain = "plain";
else if (t < Sea + Plains + Grasslands) terrain = "grassland";
else if (t < Sea + Plains + Grasslands + Forests) terrain = "forest";
else if (t < Sea + Plains + Grasslands + Forests + Hills) terrain = "hill";
else if (t < Sea + Plains + Grasslands + Forests + Hills + Mountains) terrain = "mountain";
else if (t < Sea + Plains + Grasslands + Forests + Hills + Mountains + Deserts) terrain = "desert";
And if the value of 'Sea' is 'sea' and the value of "Plains" is 'plain'
then the test of Sea+Plains will become 'seaplain' which is probably not what you expect.
BTW: You should enclose your code between [ code] and [ /code] tags (without the spaces)
to make it easier to read, copy, test and evaluate your problem.
Thanks so much JMRKER! You are indeed a SUPER Moderator!!!
Turns out the bug did lie within the checkAdjacentTilesForLand-function.
I commented it out and - tadaa! - the map will generate completely every time.
Strange though: My incorrect syntax within the if-else-statements with the surplus semicolons doesn't seem to do any harm. Thanks for pointing it out, anyway. I deleted them. And thanks for letting me know about enclosing code in the appropriate tags. I'm sure I'll make use of that aplenty in the future.
If leaving in the ';' or removing them made no difference, then I suspect they are not need, nor are the 'else' portions
Code:
// Set Terrain
// ***********
var t = Math.floor(Math.random()*100)+1;
var terrain;
if (t < Sea) terrain = "sea";
if (t < Sea + Plains) terrain = "plain";
if (t < Sea + Plains + Grasslands) terrain = "grassland";
if (t < Sea + Plains + Grasslands + Forests) terrain = "forest";
if (t < Sea + Plains + Grasslands + Forests + Hills) terrain = "hill";
if (t < Sea + Plains + Grasslands + Forests + Hills + Mountains) terrain = "mountain";
if (t < Sea + Plains + Grasslands + Forests + Hills + Mountains + Deserts) terrain = "desert";
// ...
Because 't' is randomly assigned values from 1-100,
I will make the assumption that Sea = (a number), Plaines = (a number), etc.
Since I don't know what your (number) assignments are, this is only a recommendation.
Consider numbering as this...
Code:
<script>
var Terrain = ['sea','plain','grassland','forest','hill','mountain','desert'];
// Set Terrain
// ***********
var t = Math.floor(Math.random()*Terrain.length);
var terrain = Terrain[t];
alert('Current terrain is: '+terrain);
</script>
Bookmarks