[HELP]I would like advice as to if and or how I can write this code more eloquently.

Unity Engine
1

Hey Guys and Gals I was wondering if I could get some advice on my code? I was hoping that someone could give me some pointers on how to write something like this more eloquently - I feel like too much of this is hard coded in - which seems to make this less extensible - This was three solid days of head scratching and hair pulling - Any advice would be greatly appreciated!! :smile:

using UnityEngine;
using System.Collections.Generic;

public class TileMap : MonoBehaviour {
 
    //Reference to cube prefabs scale 1, 0.25, 1
    public GameObject sPrefab;
    public GameObject iPrefab;
    public GameObject dPrefab;
 
    //Width x and height y of grid
    public float gridX = 8f;
    public float gridY = 8f;
 
    //Variable to store the traslations of our tiles
    Vector3 subs; 
    Vector3 dntwn;
    Vector3 inCit;
 
    void Start() {
        //Place the districts
        createSubs (0, 0);
        createIC ((Mathf.FloorToInt(subs.x + 1)), 0); //farthest x coord of previous district
        createDT ((Mathf.FloorToInt(subs.x + 1)), (Mathf.FloorToInt(inCit.z + 1))); //farthest x coord and farthest y coord of last district
        createIC ((Mathf.FloorToInt(subs.x + 1)), (Mathf.FloorToInt(dntwn.z + 1))); //farthest x coord and farthest y coord of last district
        createSubs ((Mathf.FloorToInt(inCit.x + 1)), 0); //farthest x coord of previous district
     
        //Move the camera ~ halfway up and across the map + up half the distance of x + y
        int cameraHeight = Mathf.FloorToInt ((gridX + gridY) / 2);
        Camera.main.transform.position = new Vector3(Mathf.FloorToInt((gridX/2)), cameraHeight, Mathf.FloorToInt((gridY/2)));
     
     
    }//End of Start
 
    //set the Suburbs to be a third of the width of the map and the whole height
    public void createSubs(int i, int k){
        for (int y = 0; y < (Mathf.FloorToInt((gridY/3)) * 3); y++) {
            for(int x = 0; x < Mathf.FloorToInt((gridX/3)); x++){
                subs = new Vector3(i + x, 0 ,k + y);
                sPrefab.renderer.material.color = Color.green;
                Instantiate(sPrefab, subs, Quaternion.identity);
            }
        }
    }
 
    //set te Inner City to be a third of the width of the map and a third of the height
    public void createIC(int i, int k){
        for (int y = 0; y < Mathf.FloorToInt((gridY/3)); y++) {
            for(int x = 0; x < Mathf.FloorToInt((gridX/3)); x++){
                inCit = new Vector3(i + x, 0 ,k + y);
                iPrefab.renderer.material.color = Color.blue;
                Instantiate(iPrefab, inCit, Quaternion.identity);
            }
        }
    }
 
    //set te Downtown to be a third of the width of the map and a third of the height
    public void createDT(int i, int k){
        for (int y = 0; y < Mathf.FloorToInt((gridY/3)); y++) {
            for(int x = 0; x < Mathf.FloorToInt((gridX/3)); x++){
                dntwn = new Vector3(i + x, 0 ,k + y);
                dPrefab.renderer.material.color = Color.cyan;
                Instantiate(dPrefab, dntwn, Quaternion.identity);
            }
        }
    }
 
} //End of class
2

Use code tags for a start.

Like so:

using UnityEngine;
using System.Collections.Generic;

public class TileMap : MonoBehaviour
{

    #region Fields

    //Reference to cube prefabs scale 1, 0.25, 1
    public GameObject sPrefab;
    public GameObject iPrefab;
    public GameObject dPrefab;
    //Width x and height y of grid
    public float gridX = 8f;
    public float gridY = 8f;
    //Variable to store the traslations of our tiles
    Vector3 subs;
    Vector3 dntwn;
    Vector3 inCit;
    //Store the farthest x and farthest y
    int farthestX;
    int farthestY;
   
    #endregion
   
    #region CONSTRUCTOR
   
    void Start() {
        //Place the districts
        createSubs (0, 0);
        createIC ((Mathf.FloorToInt(subs.x + 1)), 0);
        createDT ((Mathf.FloorToInt(subs.x + 1)), (Mathf.FloorToInt(inCit.z + 1)));
        createIC ((Mathf.FloorToInt(subs.x + 1)), (Mathf.FloorToInt(dntwn.z + 1)));
        createSubs ((Mathf.FloorToInt(inCit.x + 1)), 0);
        //Move the camera ~ halfway up and across the map + up half the distance of x + y
        int cameraHeight = Mathf.FloorToInt ((gridX + gridY) / 2);
        Camera.main.transform.position = new Vector3(Mathf.FloorToInt((gridX/2)), cameraHeight, Mathf.FloorToInt((gridY/2)));
    }
   
    #endregion
   
    #region Methods
   
    //set the Suburbs to be a third of the width of the map and the whole height
    public void createSubs(int i, int k)
    {
        for (int y = 0; y < (Mathf.FloorToInt((gridY/3)) * 3); y++) {
            for(int x = 0; x < Mathf.FloorToInt((gridX/3)); x++){
                subs = new Vector3(i + x, 0 ,k + y);
                sPrefab.renderer.material.color = Color.green;
                Instantiate(sPrefab, subs, Quaternion.identity);
                farthestX = (i+x);
                farthestY = (k+y);
            }
        }
    }
    //set te Inner City to be a third of the width of the map and a third of the height
    public void createIC(int i, int k)
    {
        for (int y = 0; y < Mathf.FloorToInt((gridY/3)); y++) {
            for(int x = 0; x < Mathf.FloorToInt((gridX/3)); x++){
                inCit = new Vector3(i + x, 0 ,k + y);
                iPrefab.renderer.material.color = Color.blue;
                Instantiate(iPrefab, inCit, Quaternion.identity);
                farthestX = (i+x);
                farthestY = (k+y);
            }
        }
    }
    //set te Downtown to be a third of the width of the map and a third of the height
    public void createDT(int i, int k)
    {
        for (int y = 0; y < Mathf.FloorToInt((gridY/3)); y++) {
            for(int x = 0; x < Mathf.FloorToInt((gridX/3)); x++){
                dntwn = new Vector3(i + x, 0 ,k + y);
                dPrefab.renderer.material.color = Color.cyan;
                Instantiate(dPrefab, dntwn, Quaternion.identity);
                farthestX = (i+x);
                farthestY = (k+y);
            }
        }
    }
   
    #endregion
   
}
1 Like
3

Thanks :smile:

4

From what I’ve read, you’re doing just fine. But if you really want to go extreme, you might want to design some form of a proper grid editor that exports and imports the data which defines the what to instantiate and where.