Isometric Chunk Game

Question

I'm trying to create an Isometric Game in JavaScript. I think the code is pretty good, but I'd like to know if there is something to improve.

        var CameraX = 0,
            CameraY = 0;

        for(var yChunks = 0; yChunks < NumYChunks; yChunks++)
        {
            var ChunkY = yChunks * HalfChunk,
                ChunkCameraY = yChunks * 4294967296;

            for(var xChunks = 0; xChunks < NumXChunks; xChunks++)
            {

                var FinalyChunk = ChunkY + CameraY;

                var FinalxChunk = xChunks * SizeChunk,
                    ChunkCameraX = xChunks + NumXChunks + CameraX;

                var index = new Number(ChunkCameraY + ChunkCameraX);

                ctx.fillStyle = "black";
                ctx.font = "bold 13px Arial";
                ctx.fillText("ID: " + index, FinalxChunk + 10, FinalyChunk + 20);

                for(var CeldY = 0; CeldY < CeldsYChunk; CeldY++)
                {
                    var yCeld = CeldY * 40;
                    for(var CeldX = 0; CeldX < CeldsXChunk; CeldX++)
                    {
                        var NewCeldX = HalfChunk + FinalxChunk + (CeldX * 40) - yCeld - 40 ,
                            NewCeldY = (CeldX * 20) + (yCeld / 2) + (FinalyChunk) - 10 ;
                        ctx.beginPath();
                        ctx.rect(NewCeldX, NewCeldY, 80, 40);
                        ctx.stroke();
                        ctx.closePath();
                    }
                }
            }
        }

Answer 1

Someone posted an answer which had some great points, but they deleted it. I am only going to mention a couple of points that they made as well as some new ones, but they should definitely put their answer back up!

I am no Javascript expert but I have used it before. Despite this, I am having great difficulty understanding what your code does. You say you think the code is pretty good, so I'll have to take your word for it that it does what you are expecting it to do. It is very hard to understand, though!

I'll just go through and point out things as I see them.

    var CameraX = 0,
        CameraY = 0;

Personally for this I would just say var CameraX; and var CameraY; on separate lines. I don't see what you are really gaining by omitting the second var. I know that it is legal to do it your way in Javascript, so it's not that it's wrong, but in my opinion it just causes a potential reader of your code to have to pause and reread it to fully understand.

for(var yChunks = 0; yChunks < NumYChunks; yChunks++)

You are iterating based on yChunks, but the fact that you have it plural just makes it confusing. I would suggest simply yChunk, but I will talk about that more in the next paragraph. The name NumYChunks is also a bit confusing. Is it the max number of y chunks? If so, I would call it MaxYChunks instead.

To elaborate on my earlier point, your naming is all over the place and completely inconsistent. You say things like ChunkY, Finalychunk, yChunks, NumYChunks, all in different places and they all mean different things. This makes the logical flow of your program very hard to follow. At the very least, all of these things need to have a similar format, so if you are going to have Y capitalized and at the end of the variable, always do that. So for example FinalyChunk would be FinalChunkY. But an even better approach is to minimize the number of similar names, so try to come up with better, more descriptive names whenever possible.

A related point, I would suggest that you follow the variable naming conventions in whatever language you are writing in, so for Javascript that would be camelCase.

You use many magic numbers in the code, such as

ChunkCameraY = yChunks * 4294967296;
ctx.fillText("ID: " + index, FinalxChunk + 10, FinalyChunk + 20);
ctx.rect(NewCeldX, NewCeldY, 80, 40);

What is 4294967296? What do 10 and 20 represent? What are 80 and 40? By declaring these as variables somewhere in the code, such as rectangleXSize = 80; rectangleYSize = 40; you will be accomplishing two things. First, it will be much easier to read and understand your code. Second, you will have a much easier time trying to modify the code at a later time, because at the top of the function you will see those values declared. Otherwise you will have to search through the code to find the numbers you want to change.

The main logic of the function is where things get the most confusing, at least for me. You have a four level deep for-loop structure, which is a sign that the design needs to be reevaluated. Any time you have to have that many for loops to accomplish something, there is usually a better approach. Additionally, I would break as much as possible out of the for loops and into smaller functions. This will improve the readability of your code and make the logic easier to follow and easier to change later. If you use descriptive function names, it will also be self documenting.

Finally I will echo the other (now deleted) answer and say that you should have more comments in your code. You might even be forgiven for a deeply nested for loop if you have comments that explain what is happening at each stage. At the very least, it will be a lot easier for you to understand in the future if you ever try to look at the code again!