Javascript scripting error

Question

After reading up on Field of View algorithms, I decided to create one myself for a game I'm working on. After a couple of hours, I came up with the following script:

function CalculateFOV() {
    ClearFOV();

    map[player.x][player.y].light = 100;

    var open = new Array();
    var closed = new Array();

    sourceNeighbors = map[player.x][player.y].neighbors;

    for(var i = 0;i<sourceNeighbors.length;i++) {
        open.push(sourceNeighbors[i]);
    }

    while(open.length > 0) {
        var curTile = open[0];

        var highestLightValue = 0;

        for(j in curTile.neighbors) {
            if(curTile.neighbors[j].light > highestLightValue) {
                highestLightValue = neighbors[j];               
            }
        }

        curTile.light = highestLightValue-10;

        if(curTile.light > 10) {
            for(var j = 0;j<curTile.neighbors.length;j++) {
                var addCell = true;

                if(FindValue(closed, open[0].neighbors[j])) addCell = false;

                if(addCell) {
                    open.push(neighbors[j]);
                }
            }
        }

        closed.push(curTile);
        open.shift();
    }
}

function ClearFOV() {
    for(var x = 0;x<mapSizeX;x++) {
        for(var y = 0;y<mapSizeY;y++) {
            map[x][y].lightValue = 0;
        }
    }
}

function FindValue(list, value) {
    for(var i = 0;i<list.length;i++) {
        if(list[i] == value) {
            return true;
        }
    }

    return false;
}

It's supposed to spread a light-value outward from a source, decreasing as it goes. It uses a Closed list of tiles which have already been given a light value in order to avoid hitting a single cell multiple times. This should theoretically increase the efficiency a lot.

Unfortunately, there seems to be an error, perhaps more than one, with it. My tired brain is failing to locate it (or them), so I would really appreciate some help with this. Does it even make sense?

Also, in case you might need it, here's the Tile class:

function Tile(x,y,character, thisMap, blocked, blockSight) {
    this.x = x;
    this.y = y;
    this.character = character;
    this.blocked = blocked;
    this.blockSight = blockSight;
    this.lightValue = 25;

    this.neighbors = new Array();
}

Tile.prototype = {
    create: function(blocked, blockSight, character) {
        this.blocked = blocked;
        this.blockSight = blockSight;
        this.character = character;

        var rockTile = RandomRange(1,4);
        var rockTileStr = "rock"+rockTile;
    },

    draw: function() {
        var id = '#t' + this.x + '_' + this.y;

        var lightColor = this.lightValue/100;

        $(id).css("opacity", lightColor); 
    },

    assign: function() {
        var north = this.x-1;
        var south = this.x+1;
        var east = this.y+1;
        var west = this.y-1;

        if(north >= 0 && south <= mapSizeX && west >= 0 && east <= mapSizeY) {

            //1st: we add the northwest neighbor.
            // *|/
            // -#-
            // /|\
            this.neighbors[0] = map[north][west];

            //2nd: we add the north neighbor.
            // \*/
            // -#-
            // /|\
            this.neighbors[1] = map[north][this.y];

            //3rd: we add the northeast neighbor.
            // \|*
            // -#-
            // /|\
            this.neighbors[2] = map[north][east];

            //4th: we add the east neighbor.
            // \|/
            // -#*
            // /|\
            this.neighbors[3] = map[this.x][east];

            //5th: we add the southeast neighbor.
            // \|/
            // -#-
            // /|*
            this.neighbors[4] = map[south][east];

            //6th: we add the south neighbor.
            // \|/
            // -#-
            // /*\
            this.neighbors[5] = map[south][this.y];

            //7th: we add the southwest neighbor.
            // \|/
            // -#-
            // *|\
            this.neighbors[6] = map[south][west];

            //8th: we add the west neighbor.
            // \|/
            // *#-
            // /|\
            this.neighbors[7] = map[this.x][west];
        }
    },
}

Thanks, and apologies for posting such a long question; I just don't know where else to turn at the moment.

A demonstration would be much more helpful than tons of code, can you provide any? :) — Dr.Molle, Apr 15 '11 at 01:33
A demonstration? If I could provide you with a demonstration I wouldn't be here. :) — Elliot Bonneville, Apr 15 '11 at 01:37
A demonstration of the error would be good too, otherwise we have to ask a crystal ball about map, mapSizeX and mapSizeY — Dr.Molle, Apr 15 '11 at 01:40
Good point. Map is an 2d array filled with Tile objects, and mapSizeX and mapSizeY define the length of the first and second dimensions of map. Fairly intuitive names, I suppose. — Elliot Bonneville, Apr 15 '11 at 01:46
@Elliot Bonneville you could try to make a 'fiddle' @ http://jsfiddle.net for demonstration purposes — KooiInc, Apr 15 '11 at 05:04

score 1 · Accepted Answer · answered Apr 15 '11 at 01:52

I can see the following typo:

highestLightValue = neighbors[j];

should be

highestLightValue = neighbors[j].light;

Furthermore, if you are concerned about efficiency, you might want to redesign how you store the set of closed elements. With the array of closed tiles you need O(n) time to execute FindValue (in fact, with current implementations of javascript arrays, O(n*log(n))). If you use an array of true/false indexed by a tile id, you get O(1) (or O(log(n)) in the real javascript world). Since this is in the inner loop of the calculation, the speedup may be noticeable.

Ah, thanks. And good point about the closed elements array, I will definitely implement that. — Elliot Bonneville, Apr 15 '11 at 01:54

Javascript scripting error

1 Answers1