Visual boardgame

The visual board game is currently a game in unity, where I’ve created a map and a grid. The visualization is like this:

Making this code obviously needed nested for loops and the like, but developing this I mainly had the attitude “just go with it”, which means I now have rather confusing code.

I have created a class which currently contains all my code, and I believe I need to do some cleanup on my variables, and maybe move some methods to other classes?

using UnityEngine;
using System.Collections;

public class gridInit : MonoBehaviour {

    static int mapWidth = 33;
    static int mapHeight = -27;
    static float tileSizeUnits = 0.333333333333333333333f; this float is cancer, I'm sorry
    public GameObject tile;
    // Use this for initialization
    void Start () {
        GenerateObjects();
        GenerateTileMap();
        Screen.fullScreen = true;  

    }

    void Awake()
    {

    }
    // Update is called once per frame
    void Update () {

    }

    bool walkable = false;
    private void GenerateTileMap()
    {
        int tileNumber = 0;
        for (float y = 0; y > mapHeight; y--)
        {
            for (float x = 0; x < mapWidth; x++)
            {
                tile.GetComponent<Renderer>().enabled = false;
                GameObject go = Instantiate(tile, new Vector3(x/3, y/3, -2), Quaternion.identity) as GameObject;
                tileNumber++;
                go.name = "tile" + tileNumber;
                go.tag = "grid";

                RaycastHit2D hit;
                Vector3 fwd = go.transform.TransformDirection(Vector3.forward);
                Debug.DrawRay(go.transform.position, fwd * 50, Color.green);
                hit = Physics2D.Raycast(go.transform.position, fwd, 200);
                if (hit)
                {
                    go.tag = "walkableGrid";
                    Debug.Log("works");
                }         
            }
        }
    }

    public GameObject center;
    public GameObject stair;
    public GameObject corner;
    public GameObject straight;
    public GameObject empty;
    public GameObject stair90;
    public GameObject stair180;
    public GameObject stair270;
    public GameObject corner90;
    public GameObject corner180;
    public GameObject corner270;
    public GameObject straight90;
    public GameObject mainCenter;

    private void GenerateObjects()
    { 
        GameObject[,] map = new GameObject[9, 11] { { empty, center, corner180, stair180, stair180, stair180, stair180, stair180, corner270, center, empty }, { center, center, straight, center, stair270, stair180, stair90, center, straight, center, center }, { straight, corner270, center, corner180, center, center, center, corner270, center, corner180, straight }, { corner, center, straight, center, center, stair180, center, center, straight, center, corner90 }, { stair270, center, center, center, stair270, mainCenter, stair90, center, center, center, stair90 }, { corner270, center, straight, center, center, stair, center, center, straight, center, corner180 }, { straight, corner, center, corner90, center, center, center, corner, center, corner90, straight }, { center, center, straight, center, stair270, stair, stair90, center, straight, center, center }, { empty, center, corner90, stair, stair, stair, stair, stair, corner, center, empty } };
        int tileNumber = 0;

        for(int i = 0; i < 9; i++)
        {
            for (int j = 0; j < 11; j++) 
            {
                GameObject go = Instantiate(map[i,j], new Vector3(j + tileSizeUnits, (i * -1) - tileSizeUnits, 0), map[i,j].transform.rotation) as GameObject;
                tileNumber++;
                go.name = "object" + tileNumber;
                go.tag = "path";
            }   
        }
    }
}

It’s not a mandatory rule but camelCase for classes is little bit weird in C#. gridInit should be GridInit. Also you may want to pick a better name for that. Init is clear but it doesn’t help to understand what will be initialized (and then responsibilities of this class). Class isn’t inherited then it should be sealed too.

Consistency. Sometimes you use private to denote private methods and sometimes you don’t (because it’s default). One way or the other but do not mix otherwise reader will have to understand if he is missing something…

With static int mapWidth = 33; you declare a read/write public fields. Few things here. You should not have public writeable fields. Public fields should be PascalCase. You do not need static but const. This is true for all of them.

Isn’t easier to declare tileSizeUnits as const float TileSizeUnits = 1.0f / 3;?

When you want to expose object outside your class you should use read-only properties: public GameObject Tile { get; private set; }. I strongly doubt you need all those public objects (aren’t they just implementation details? Is this a mere factory class?) but I don’t see all surrounding code then I’m just guessing. What I think is worse is that you’re holding a lot of objects and this class just handles some initialization code. Not enough to do for a class and too much for a single method. I think if you merge it into calling code you will have easier to read code…

GenerateTileMap() has some magic numbers. Make constants from them and reader won’t need to understand (guess) what they are.

In GenerateObjects() I don’t see the need for map object. 2D array isn’t the easiest thing to read (especially when initialized in this way, at least don’t make it single-line). A single for (with integer division and modulus to calculate row/column where required) is much easier to read (IMO). The problem (I think) is that here you’re receiving a bunch of objects but that logic is somewhere else.

@Adriano Repetti has listed so many issues that I have only one short comment…

GameObject[,]

If you don’t have to, you shouldn’t use multidimensional arrays. Linq cannot work with them. It’s better to use jagged arrays GameObject[][]