Thoughts on best approach to instantiate gameobjects with different property values in script ?

Hi,

Firstly I am looking for a bit of a discussion here, not “a please write my code for me”.

I am working on a strategy game, and pretty much everything is working in terms of logic. However, my code is on the verge of becoming one large plate of spaghetti and I want to start pulling it back and tidying it up.

The game is a strategy game, there are Roman legions, forts, fleets and barbarian settlements. This is how I am currently creating my legions.

Each legion prefab has a script called Legatus, which (cut down) looks like this

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class Legatus : MonoBehaviour {
    public string lName;
    public string lStrength;
    public int lTalents;


}

and then there is a worldBuilding script called at the start of the game.

using UnityEngine;

public class WorldBuilder : MonoBehaviour {
    public Transform Army;
    // Use this for initialization
    void Start () {
        BuildWorld ();
    }
   
    // Update is called once per frame
    void Update () {
       
    }

    void BuildWorld ()
    {


        //add Legions
        Transform Leg1 =Instantiate (Army, new Vector3 (0, 0, 4.5f), Quaternion.identity);
        Transform Leg2 =Instantiate (Army, new Vector3 (1.5f, 0, 3.75f), Quaternion.identity);
        Transform Leg3 =Instantiate (Army, new Vector3 (2, 0, 3.25f), Quaternion.identity);
        Transform Leg4 =Instantiate (Army, new Vector3 (2, 0, 2.25f), Quaternion.identity);
        Transform Leg5 =Instantiate (Army, new Vector3 (2.75f, 0, 1), Quaternion.identity);
        Transform Leg6 =Instantiate (Army, new Vector3 (3.25f, 0, 0), Quaternion.identity);

        Leg1.transform.parent = GameObject.Find("LegionData").transform;
        Leg2.transform.parent = GameObject.Find("LegionData").transform;
        Leg3.transform.parent = GameObject.Find("LegionData").transform;
        Leg4.transform.parent = GameObject.Find("LegionData").transform;
        Leg5.transform.parent = GameObject.Find("LegionData").transform;
        Leg6.transform.parent = GameObject.Find("LegionData").transform;

        Leg1.GetComponent<Legatus> ().lName = "TUTORIAL LEGION";
        Leg2.GetComponent<Legatus> ().lName = "I GERMANICA";
        Leg3.GetComponent<Legatus> ().lName = "V ALAUDAE";
        Leg4.GetComponent<Legatus> ().lName = "XX VALERIA VICTRIX";
        Leg5.GetComponent<Legatus> ().lName = "XXI RAPAX";
        Leg6.GetComponent<Legatus> ().lName = "II AUGUSTA";

        Leg1.GetComponent<Legatus> ().lStrength = "STRONG";
        Leg2.GetComponent<Legatus> ().lStrength = "STRONG";
        Leg3.GetComponent<Legatus> ().lStrength = "NORMAL";
        Leg4.GetComponent<Legatus> ().lStrength = "DECIMATED";
        Leg5.GetComponent<Legatus> ().lStrength = "STRONG";
        Leg6.GetComponent<Legatus> ().lStrength = "NORMAL";

        Leg1.GetComponent<Legatus> ().lTalents = 100;
        Leg2.GetComponent<Legatus> ().lTalents = 100;
        Leg3.GetComponent<Legatus> ().lTalents = 100;
        Leg4.GetComponent<Legatus> ().lTalents = 75;
        Leg5.GetComponent<Legatus> ().lTalents = 50;
        Leg6.GetComponent<Legatus> ().lTalents = 50;


    }
}

And it works. Later on, in my game there are all kind of other scripts, that react on click events, change the properties, show data in the UI etc, etc.

Where it all is going scu-wiffy, is some of my routines are becoming massive long nested if statements. This isn’t so much a problem with my legion scripts, but my settlements have tons of information, such as population size, loyalty, number of druids, trade agreements, strengths, etc

Now , from what I have been reading up on so far,

I could potentially …

i) Use 2d Arrays
ii) Use Dictionarys

Or there are some other options, such as hash tables which seem to rely on outside libraries?

I am unsure what approach I should be pursuing, so any thoughts welcomed.

  1. A Dictionary essentially is a hash table, so no need to rely on outside libraries for that. The purpose is really to map a key to a value, so you can quickly look up the value based on the key. I’m not sure how that would specifically help here.

  2. The WorldBuilder code above could easily be shortened into 6 lines like this:

InstantiateLegatus(new Vector3 (0, 0, 4.5f), "TUTORIAL LEGION", "STRONG", 100);
  1. You might want to consider using an enum instead of a string for the strength value.

  2. Can you share those long nested if statements? If that’s the real problem, it would be good to see it.

I was thinking enums would be better then strings.
and possibly scriptable objects.

2 Likes

You should make it more structured and generalized. As pointed out already make a method called CreateLegatus or so. This is the concept of Factories. You can even write a FactoryClass for your Units. This will help you especially once you have different units and other objects.
What such a class does is storing links to prefabs and it manages creation as well as destruction, and maybe pooling, of a specific set of objects.

Instead of using transform you can directly assign a Legatus object as your prefab. So you might change Army into:

public Legatus Army;  //LegatusPrefab

Then when you instantiate Army you get Legatus object already, no need to call GetComponent();.
The method cnn look like this:

Legatus CreateLegatus(Vector3 position, //Values)
{
    //add Legions
    var legatus = GameObject.Instantiate(LegatusPrefab, position, Quaternion.identity);

    //Assign values here

    return legatus ;
}

Then you have to store your unis in a list. If you need a dictionary or not depends on how you want to access them but generally a List is just fine. Why 2D array? you mean because of position? That can make sense but depends on the type of game. But then this would be a seperate array like a Spatial Partition.

List<Legatus> LegatusUnits;

LegatusUnits.Add(CreateLegatus(new Vector3 (0, 0, 4.5f),"TUTORIAL LEGION",EPower.Strong, 100))

As you can see I also recommend using an enum for strength e.g. (Weak, normal, Strong) or simply numbers.

Also why do you use “l” as a prefix for your members?

I second the use of Scriptable Objects for any varying data sets that are configurable at edit-time. Refrain from magic numbers in your classes. Create a set of properties as a scriptable object, create numerous instances of that object for all the variations of your data. Then give your script references to those scriptable objects for spawning, hot-swapping, etc. You can create new instances of them at runtime if necessary.

I too encourage the use of Enum for types.

1 Like

thanks for all the replies,

In regards to the long nested if statements; this code is just above the level before it gets really messy,

If you can see, in the top right hand corner I have selected my Roman legion and the map has highlighted the availiable regions I can move too. I am doing this with a raycast and then passing the result like so;

public void highlightRegions()
    {
        // TRIBE RGB
        //        Frissi        RGB  10,10,10,255
        //        Chamavi        RGB  20,20,20,255
        //        Ampisvarii    RGB 30,30,30,255
        //        Chauci        RGB 40 ,40,40,255
        //        Angrivarri    RGB 50,50,50,255
        //        Langobardi    RGB 60,60,60,255
        //        Cherusci    RGB  70,70,70,255
        //        Usipeces    RGB  80,80,80,255
        //       Sugambri    RGB  90,90,90,255
        //        Chatti        RGB 100,100,100,255
        //        Tencteri    RGB  110,110,110,255
        //        Mattiaci    RGB 120,120,120,255


        switch (UIgame.sLegion.GetComponent<Legatus> ().movFrom) {

        case 0:
            GameObject.Find("worldData/_chamavi").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_frissi").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_mattiaci").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_tencteri").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_usipeces").GetComponent<Renderer> ().enabled = true;
            break;
        case 10:
            GameObject.Find("worldData/_chamavi").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_frissi").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
            break;
        case 20:
            GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_chamavi").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_frissi").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_usipeces").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
            break;
        case 30:
            GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_chamavi").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_chauci").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
            break;
        case 40:
            GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_chauci").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_langobardi").GetComponent<Renderer> ().enabled = true;
            break;
        case 50:
            GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_chauci").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_langobardi").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
            break;
        case 60:
            GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_chauci").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_langobardi").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
            break;
        case 70:
            GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_chatti").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_langobardi").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_usipeces").GetComponent<Renderer> ().enabled = true;
            break;
        case 80:
            GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_chamavi").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_usipeces").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
            break;
        case 90:
            GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_chatti").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_tencteri").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_usipeces").GetComponent<Renderer> ().enabled = true;
            break;
        case 100:
            GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_chatti").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_tencteri").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_mattiaci").GetComponent<Renderer> ().enabled = true;
            break;
        case 110:
            GameObject.Find("worldData/_chatti").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_tencteri").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_mattiaci").GetComponent<Renderer> ().enabled = true;
            break;
        case 120:
            GameObject.Find("worldData/_chatti").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_mattiaci").GetComponent<Renderer> ().enabled = true;
            GameObject.Find("worldData/_tencteri").GetComponent<Renderer> ().enabled = true;
            break;


        }


    }

That works fine and dandy, but this is where my code is becoming spaghetti like. A legion can not always move into a region. This depends on the tribes status. so imagine the above, for each case with an if statement to check each region (vil5 and vil6 refer to instantiate settlements like the legion above).

  case 50:
            ...
GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
if (vil5.valliance <> "Hostile )
{
            GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
}
if (vil6.valliance <> "Hostile )
{
            GameObject.Find("worldData/_chauci").GetComponent<Renderer> ().enabled = true;
}
            ....

I then have another level to go down where i have to look at factors in the neighbouring settlements to decide the outcome of the legions actions in the current. I won’t post it here as you can imagine with the above code and the if approach I am using it is very messy.

Very old habit, back from the days when I used to program(boring business software) I always used to avoid the word name and quite a few others due to being a reserved keyword in something or other we were using…

thanks for your explanation as well, most useful

case 0:
    SetRendererEnabled(true, "_chamavi", "_frissi", "_gaul", "_mattiaci", "_suganbri", "_tencteri", "_usipeces");
    break;

private static void SetRendererEnabled(bool enabled, params string[] names)
{
    foreach (string name in names)
    {
        GameObject.Find("worldData/" + name).GetComponent<Renderer>().enabled = enabled;
    }
}

Though I would recommend using direct links and reduce the usage of Find as much as possible.

this was kind of why I mentioned arrays in my original post, I was thinking of incorporating info like this into an array, for each settlement and then reference back to another array with the settlement full details in it. Which i am guessing now can also be done with lists ? so pseudo code would look like.

instantiateSettlement(new vector(1,0,3),"Chamavi","Hostile","Poor","Small")
instantiateSettlement(new vector(1,0,2),"Usipes","Hostile","Poor","Medium")
instantiateSettlement(new vector(1,0,3),"Chauci","Hostile","PRicj","Small")
instantiateSettlement(new vector(1,0,3),"Angrivarii","Hostile","Poor","Small")

etc, etc

instantiateNeighbours("Chamavi","_Usipes")
instantiateNeighbours("Chamavi","_Ampivarri")
instantiateNeighbours("Chamavi","_Usipes")

instantiateNeighbours("Usipes","_Cherusci")
instantiateNeighbours("Usipes","_Suganbri")

etc etc

Arrays, sure, and then probably in a ScriptableObject like mentioned before. But if done completely from code, you can circumvent the need to define a new array by using params instead.