Add and remove Tiles during pan event for 4 directions

Question

This function accepts a String (either "right", "left","up", or "down") to do 4 similar but different things. I'd like a more elegant solution to this problem. I sense that there is one, but I can't think of it. Maybe by using a a few smaller functions? Maybe by using a dictionary?

private function addAndRemoveTiles(pan:String):void
{
    var lenRows:int = onScreenRows;
    var lenCols:int = onScreenCols;
    if (pan == "right"){
        onScreenLeftCol++;              
        // for each row
        for (var i:int = 0; i < lenRows; i++){

        //////////////////
        // remove tile from Left
        //////////////////

            // t = Tile to be removed from left of row
            var t:Tile = onScreenArray[i].shift();
            // tileNum = Tile Type
            var tileNum:int = tileType(t);
            // add tile from onScreenArray to arrGround
            arrGround[tileNum].push(t);

            // put tile off screen
            t.x = -tileWidth;
            t.y = -tileHeight;

        /////////////
        // add tile to Right
        /////////////
            // c = right tile col
            var c:int = onScreenCols + onScreenLeftCol-1;
            // cType = right tile Type
            var cType:int = complexArray[i+onScreenTopRow][c];
            // t = Tile to take from arrGround
            var t:Tile = arrGround[cType][arrGround[cType].length - 1];
            // farRightTile = Tile on far right of screen
            var farRightTile:Tile = onScreenArray[i][onScreenArray[i].length-1];

            // add tile from arrGround to onScreeArray
            onScreenArray[i].push(t);
            // remove tile from arrGround to onScreenArray
            arrGround[cType].pop();

            // place tile on screen
            t.x = farRightTile.x + tileWidth;
            t.y = farRightTile.y;
        }               
    } else if (pan == "left"){
        onScreenLeftCol--;
        for (var i:int = 0; i < lenRows; i++){

            // remove tile from Right
            var t:Tile = onScreenArray[i].pop();
            var tileNum:int = tileType(t);
            arrGround[tileNum].push(t);

            // place tile off screen
            t.x = -tileWidth;
            t.y = -tileHeight;

            // add tile to Left
            var c:int = onScreenLeftCol;
            var cType:int = complexArray[i+onScreenTopRow][c];
            var t:Tile = arrGround[cType][arrGround[cType].length - 1];
            var farLeftTile:Tile = onScreenArray[i][0];
            onScreenArray[i].unshift(t);
            arrGround[cType].pop();

            // place tile on Screen
            t.x = farLeftTile.x - tileWidth;
            t.y = farLeftTile.y;
        }
    } else if (pan == "down"){
        onScreenTopRow++;   

        //////////////////
        // remove tile from Top
        //////////////////
        var removedArray:Array = onScreenArray.shift();
        for (var i:int = removedArray.length; i > 0; i--){
            // t = Tile removed from top of column
            var t:Tile = removedArray[removedArray.length-1];
            // tileNum = Tile Type
            var tileNum:int = tileType(t);
            // add tile from onScreenArray to arrGround
            arrGround[tileNum].push(t);

            // add Tile from top of column                                  
            t.x = -tileWidth;
            t.y = -tileHeight;
            removedArray.pop();
        }       

        //add new Bottom Row
        onScreenArray.push([]);

        // for each col
        for (var i:int = 0; i < lenCols; i++){

        /////////////
        // add tile to Bottom
        /////////////

            // botRow = bottom tile row
            var botRow:int = onScreenRows-1 + onScreenTopRow;
            // tType = bottom tile Type
            var tType:int = complexArray[botRow][i+onScreenLeftCol];
            // t = Tile to take from arrGround
            var t:Tile = arrGround[tType][arrGround[tType].length - 1];
            // tile on bottom of pertinent column
            var bottomTile:Tile = onScreenArray[onScreenArray.length-2][i];

            // add tile from arrGround to onScreeArray
            onScreenArray[onScreenArray.length-1].push(t);
            // remove tile from arrGround to onScreenArray
            arrGround[tType].pop();

            // place tile on screen
            t.y = bottomTile.y + tileWidth;
            t.x = bottomTile.x;
        }
    } else if (pan == "up"){
        onScreenTopRow--;   

        //////////////////
        // remove tile from Bottom
        //////////////////
        var removedArray:Array = onScreenArray.pop();
        for (var i:int = removedArray.length; i > 0; i--){
            // t = Tile removed from bottom of column
            var t:Tile = removedArray[removedArray.length-1];
            // tileNum = Tile Type
            var tileNum:int = tileType(t);
            // add tile from onScreenArray to arrGround
            arrGround[tileNum].push(t);

            // add Tile from top of column                  
            t.x = -tileWidth;
            t.y = -tileHeight;
            removedArray.pop();
        }       

        //add new Top Row
        onScreenArray.unshift([]);

        // for each col
        for (var i:int = 0; i < lenCols; i++){

        /////////////
        // add tile to Top
        /////////////

            // topRow = Top tile row
            var topRow:int = 0;
            // tType = bottom tile Type
            var tType:int = complexArray[onScreenTopRow][i+onScreenLeftCol];
            // t = Tile to take from arrGround
            var t:Tile = arrGround[tType][arrGround[tType].length - 1];
            // tile on bottom of pertinent column
            var topTile:Tile = onScreenArray[topRow+1][i];

            // add tile from arrGround to onScreeArray
            onScreenArray[topRow].push(t);
            // remove tile from arrGround to onScreenArray
            arrGround[tType].pop();

            // place tile on screen
            t.y = topTile.y - tileWidth;
            t.x = topTile.x;
        }
    }
    // put border back on top
    addChild(blockingBorder);
}

The basic organization is:

if (right){
     remove tile from left
     add tile to right
}
if (left){
     remove tile from right
     add tile to left
}
if (up){
     remove tile from bottom
     add tile to top
}
if (down){
     remove tile from top
     add tile to bottom
}

but the complication in simplifying this is in the fact that adding or removing tiles to the bottom and top is implicitly different than adding them to the sides because in the former I'm adding and removing whole arrays in a 2D array, and in the latter, I'm removing or adding just one element in the 2nd level array.

edit

arrGround is all tiles needed for the whole map

OnScreenArray is the tiles only visible in camera.

complexArray is the array of index numbers that tell us where from the tile map bitmap to draw the bitmap data (i.e., 1 is ground... But what type of ground is dependent on what is surrounding it, so the complex array may turn a 1 (ground) into a 13, which is a cliff edge)

The purpose of pushing and popping tiles endlessly in and out of onScreenArray was so that I can loop through that small array for pertinent functions instead of the potentially huge array of total tiles.

Answer 1

I'm not sure I understand your code entirely. Say for example I have no idea what complexArray is. I was about to shy away from writing a review because of that. But thinking about it, not understanding code might be the best reason to review it.

but the complication in simplifying this is in the fact that adding or removing tiles to the bottom and top is implicitly different than adding them to the sides because in the former I'm adding and removing whole arrays in a 2D array, and in the latter, I'm removing or adding just one element in the 2nd level array.

And this leads to the question why you add and remove elements this way. I cannot think of a reason. You have a lot of code that is busy taking elements out of some 2D array and putting it into another.

// add tile from arrGround to onScreeArray
onScreenArray[topRow].push(t);
// remove tile from arrGround to onScreenArray
arrGround[tType].pop();

Why? Especially the part the places elements back into the ground array is not plausible. Why put things back at all? Why not just leave them in the ground array? It looks like you modelled your data structures so that they are mutually exclusive: a Tile is either on screen or on the "ground" which. It's never both. I think this is where the problem is at, because it appears to be an unnecessary overcomplication.

You are dealing with Objects which are referenced. Having several arrays that each reference an object is not much of an overhead. It's not like you are duplicating data to fill up memory.

think top down, not bottom up

You implemented the functionality to move in all 4 directions. That's good, because you can combine those to create any other more complex motion, right? Right. But it's somewhat clumsy. If you want to go 5 tiles to the right or to some arbitrary position, your code doesn't work too well.

Instead, approach the problem more abstractly. The general problem you are trying to solve is this:

• You have some 2D array which stores all tile objects. This is probably what arrGround is in your current code. (again, I'm not 100% sure how your code works, so I might be wrong on that) This array is constant. For my answer, I will call this the Map
• There's a number of Tile objects that you want to display. The number of them is constant and determined by the width and height of what you use for display. This is probably what onScreenArray is in your code. This is always a subset of Map.
• You want to be able to change what Tiles are in the aforementioned subset, so that you can display other parts of the Map.

The above boils down to something usually called a camera.

public class Map
{
    private const tileGrid:Array = [];
    private const camera:Rectangle = new Rectangle(0, 0, 10, 7); // display 10 by 7 tiles

    public function Map()
    {
        // build 2D array tileGrid here from whatever
    }

    public function setCameraPosition(x:Number, y:Number):void
    {
        camera.x = x;
        camera.y = y;
    }

    public function copySubset(target:Array):Array
    {
        // use camera object as boundaries to decide what's part of the subset and what's not

        for(int x:int = camera.left; x < camera.right; ++x)
            for(int y:int = camera.top; y < camera.bottom; ++y)
                target[x][y] = tileGrid[x][y];
    }
}

Treat the above as pseudo code as I haven't tested it. As you can see, the idea is to have some Rectangle object that acts as a camera that defines some subset of the entire map.

You can now pass your onScreen 2D array of appropriate size to the copySubset() method and it will be filled with whatever is visible by the camera. When you want to pan around, just pass other variables to setCameraPosition(), then copy again.

This code is far from complete. No values are sanity checked. This should be several classes Camera, Map, etc. You could build connections via observer pattern (EventDispatcher) to update propagate changes of values throughout all objects interested in them automatically. But the basic idea should be clear.

tl, dr;

The problem of your existing code is that you have manoeuvred yourself into some niche of use cases. On first sight, it looks like a good idea to build the specific cases, but by doing that it's overlooked that the general solution is far simpler.

edit

complexArray is the array of index numbers that tell us where from the tile map bitmap to draw the bitmap data (i.e., 1 is ground... But what type of ground is dependent on what is surrounding it, so the complex array may turn a 1 (ground) into a 13, which is a cliff edge)

This functionality should be executed before your code runs, likely even before the program is compiled.

Don't just leave a hint on every Tile object how to find some clue in some other data structure how to render it. Figure that out as soon as possible.

• If the map is fixed and never changes during execution, the status of each tile should be calculated prior to compilation. Create some kind of map editor or parser or whatever program to calculate the exact type of each Tile. Run this program before you hit compile.
• If the map changes at runtime, you'd have to include that logic into your program. Execute it when the map changes and only then.

The problem with your existing code is that it distributes data of Tile objects across several data structures. That's bad. You have to fiddle around with all those arrays just to look up some number that might have a fixed value that was know even before the program existed. There's no need to look it up across several arrays every time you try to render a tile.

As your code merely deals with deciding what's displayed and what's not, this shouldn't be of interest at all. Maybe there's a property .type in the Tile class that holds a value to represent its type. Maybe this value has to be recalculated because the map changes. But the code you posted is not the place to do that.

The purpose of pushing and popping tiles endlessly in and out of onScreenArray was so that I can loop through that small array for pertinent functions instead of the potentially huge array of total tiles.

Then onScreen might be redundant. Similarly to copying a subset of the entire Map, you can execute functions on a subset of the map, without storing all Tile objects that belong to this subset in another data structure.

more edits

I think I grasp the idea of displaying only relevant tiles from the array (a subset) and that's fine. I would display those in a parent display object and move that object to simulate panning left, right, up, down, and then when the bottom row (for instance) pans out of view, recalculate which tiles to display, and simultaneously adjust the parent display object. Does that sound like that would "fit" within the architecture you are advocating?

Yes. This should all be internal behaviour. To actually pan around, all you do is modify the camera position. Based on that change, the relevant DisplayObjects should be added/removed and the container should be moved accordingly.