Search Unity

  1. Welcome to the Unity Forums! Please take the time to read our Code of Conduct to familiarize yourself with the forum rules and how to post constructively.
  2. We have updated the language to the Editor Terms based on feedback from our employees and community. Learn more.
    Dismiss Notice

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

Discussion in 'Scripting' started by chance1234, Feb 6, 2018.

  1. chance1234

    chance1234

    Joined:
    Jun 7, 2016
    Posts:
    16
    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

    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4.  
    5. public class Legatus : MonoBehaviour {
    6.     public string lName;
    7.     public string lStrength;
    8.     public int lTalents;
    9.  
    10.  
    11. }
    12.  
    and then there is a worldBuilding script called at the start of the game.

    Code (CSharp):
    1. using UnityEngine;
    2.  
    3. public class WorldBuilder : MonoBehaviour {
    4.     public Transform Army;
    5.     // Use this for initialization
    6.     void Start () {
    7.         BuildWorld ();
    8.     }
    9.    
    10.     // Update is called once per frame
    11.     void Update () {
    12.        
    13.     }
    14.  
    15.     void BuildWorld ()
    16.     {
    17.  
    18.  
    19.         //add Legions
    20.         Transform Leg1 =Instantiate (Army, new Vector3 (0, 0, 4.5f), Quaternion.identity);
    21.         Transform Leg2 =Instantiate (Army, new Vector3 (1.5f, 0, 3.75f), Quaternion.identity);
    22.         Transform Leg3 =Instantiate (Army, new Vector3 (2, 0, 3.25f), Quaternion.identity);
    23.         Transform Leg4 =Instantiate (Army, new Vector3 (2, 0, 2.25f), Quaternion.identity);
    24.         Transform Leg5 =Instantiate (Army, new Vector3 (2.75f, 0, 1), Quaternion.identity);
    25.         Transform Leg6 =Instantiate (Army, new Vector3 (3.25f, 0, 0), Quaternion.identity);
    26.  
    27.         Leg1.transform.parent = GameObject.Find("LegionData").transform;
    28.         Leg2.transform.parent = GameObject.Find("LegionData").transform;
    29.         Leg3.transform.parent = GameObject.Find("LegionData").transform;
    30.         Leg4.transform.parent = GameObject.Find("LegionData").transform;
    31.         Leg5.transform.parent = GameObject.Find("LegionData").transform;
    32.         Leg6.transform.parent = GameObject.Find("LegionData").transform;
    33.  
    34.         Leg1.GetComponent<Legatus> ().lName = "TUTORIAL LEGION";
    35.         Leg2.GetComponent<Legatus> ().lName = "I GERMANICA";
    36.         Leg3.GetComponent<Legatus> ().lName = "V ALAUDAE";
    37.         Leg4.GetComponent<Legatus> ().lName = "XX VALERIA VICTRIX";
    38.         Leg5.GetComponent<Legatus> ().lName = "XXI RAPAX";
    39.         Leg6.GetComponent<Legatus> ().lName = "II AUGUSTA";
    40.  
    41.         Leg1.GetComponent<Legatus> ().lStrength = "STRONG";
    42.         Leg2.GetComponent<Legatus> ().lStrength = "STRONG";
    43.         Leg3.GetComponent<Legatus> ().lStrength = "NORMAL";
    44.         Leg4.GetComponent<Legatus> ().lStrength = "DECIMATED";
    45.         Leg5.GetComponent<Legatus> ().lStrength = "STRONG";
    46.         Leg6.GetComponent<Legatus> ().lStrength = "NORMAL";
    47.  
    48.         Leg1.GetComponent<Legatus> ().lTalents = 100;
    49.         Leg2.GetComponent<Legatus> ().lTalents = 100;
    50.         Leg3.GetComponent<Legatus> ().lTalents = 100;
    51.         Leg4.GetComponent<Legatus> ().lTalents = 75;
    52.         Leg5.GetComponent<Legatus> ().lTalents = 50;
    53.         Leg6.GetComponent<Legatus> ().lTalents = 50;
    54.  
    55.  
    56.     }
    57. }
    58.  

    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.
     
  2. jvo3dc

    jvo3dc

    Joined:
    Oct 11, 2013
    Posts:
    1,520
    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:
    Code (csharp):
    1. InstantiateLegatus(new Vector3 (0, 0, 4.5f), "TUTORIAL LEGION", "STRONG", 100);
    3. You might want to consider using an enum instead of a string for the strength value.

    4. Can you share those long nested if statements? If that's the real problem, it would be good to see it.
     
    Last edited: Feb 6, 2018
  3. johne5

    johne5

    Joined:
    Dec 4, 2011
    Posts:
    1,133
    I was thinking enums would be better then strings.
    and possibly scriptable objects.
     
    chance1234 and LiterallyJeff like this.
  4. McDev02

    McDev02

    Joined:
    Nov 22, 2010
    Posts:
    661
    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:
    Code (CSharp):
    1. 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:

    Code (CSharp):
    1. Legatus CreateLegatus(Vector3 position, //Values)
    2. {
    3.     //add Legions
    4.     var legatus = GameObject.Instantiate(LegatusPrefab, position, Quaternion.identity);
    5.  
    6.     //Assign values here
    7.  
    8.     return legatus ;
    9. }

    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<Legatus> 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.

    Code (CSharp):
    1. List<Legatus> LegatusUnits;
    2.  
    3. 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?
     
    Last edited: Feb 6, 2018
  5. LiterallyJeff

    LiterallyJeff

    Joined:
    Jan 21, 2015
    Posts:
    2,802
    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.
     
    chance1234 likes this.
  6. chance1234

    chance1234

    Joined:
    Jun 7, 2016
    Posts:
    16
    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;

    Code (CSharp):
    1. public void highlightRegions()
    2.     {
    3.         // TRIBE RGB
    4.         //        Frissi        RGB  10,10,10,255
    5.         //        Chamavi        RGB  20,20,20,255
    6.         //        Ampisvarii    RGB 30,30,30,255
    7.         //        Chauci        RGB 40 ,40,40,255
    8.         //        Angrivarri    RGB 50,50,50,255
    9.         //        Langobardi    RGB 60,60,60,255
    10.         //        Cherusci    RGB  70,70,70,255
    11.         //        Usipeces    RGB  80,80,80,255
    12.         //       Sugambri    RGB  90,90,90,255
    13.         //        Chatti        RGB 100,100,100,255
    14.         //        Tencteri    RGB  110,110,110,255
    15.         //        Mattiaci    RGB 120,120,120,255
    16.  
    17.  
    18.         switch (UIgame.sLegion.GetComponent<Legatus> ().movFrom) {
    19.  
    20.         case 0:
    21.             GameObject.Find("worldData/_chamavi").GetComponent<Renderer> ().enabled = true;
    22.             GameObject.Find("worldData/_frissi").GetComponent<Renderer> ().enabled = true;
    23.             GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
    24.             GameObject.Find("worldData/_mattiaci").GetComponent<Renderer> ().enabled = true;
    25.             GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
    26.             GameObject.Find("worldData/_tencteri").GetComponent<Renderer> ().enabled = true;
    27.             GameObject.Find("worldData/_usipeces").GetComponent<Renderer> ().enabled = true;
    28.             break;
    29.         case 10:
    30.             GameObject.Find("worldData/_chamavi").GetComponent<Renderer> ().enabled = true;
    31.             GameObject.Find("worldData/_frissi").GetComponent<Renderer> ().enabled = true;
    32.             GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
    33.             break;
    34.         case 20:
    35.             GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
    36.             GameObject.Find("worldData/_chamavi").GetComponent<Renderer> ().enabled = true;
    37.             GameObject.Find("worldData/_frissi").GetComponent<Renderer> ().enabled = true;
    38.             GameObject.Find("worldData/_usipeces").GetComponent<Renderer> ().enabled = true;
    39.             GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
    40.             break;
    41.         case 30:
    42.             GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
    43.             GameObject.Find("worldData/_chamavi").GetComponent<Renderer> ().enabled = true;
    44.             GameObject.Find("worldData/_chauci").GetComponent<Renderer> ().enabled = true;
    45.             GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
    46.             break;
    47.         case 40:
    48.             GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
    49.             GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
    50.             GameObject.Find("worldData/_chauci").GetComponent<Renderer> ().enabled = true;
    51.             GameObject.Find("worldData/_langobardi").GetComponent<Renderer> ().enabled = true;
    52.             break;
    53.         case 50:
    54.             GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
    55.             GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
    56.             GameObject.Find("worldData/_chauci").GetComponent<Renderer> ().enabled = true;
    57.             GameObject.Find("worldData/_langobardi").GetComponent<Renderer> ().enabled = true;
    58.             GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
    59.             break;
    60.         case 60:
    61.             GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
    62.             GameObject.Find("worldData/_chauci").GetComponent<Renderer> ().enabled = true;
    63.             GameObject.Find("worldData/_langobardi").GetComponent<Renderer> ().enabled = true;
    64.             GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
    65.             break;
    66.         case 70:
    67.             GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
    68.             GameObject.Find("worldData/_chatti").GetComponent<Renderer> ().enabled = true;
    69.             GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
    70.             GameObject.Find("worldData/_langobardi").GetComponent<Renderer> ().enabled = true;
    71.             GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
    72.             GameObject.Find("worldData/_usipeces").GetComponent<Renderer> ().enabled = true;
    73.             break;
    74.         case 80:
    75.             GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
    76.             GameObject.Find("worldData/_chamavi").GetComponent<Renderer> ().enabled = true;
    77.             GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
    78.             GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
    79.             GameObject.Find("worldData/_usipeces").GetComponent<Renderer> ().enabled = true;
    80.             GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
    81.             break;
    82.         case 90:
    83.             GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
    84.             GameObject.Find("worldData/_chatti").GetComponent<Renderer> ().enabled = true;
    85.             GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
    86.             GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
    87.             GameObject.Find("worldData/_tencteri").GetComponent<Renderer> ().enabled = true;
    88.             GameObject.Find("worldData/_usipeces").GetComponent<Renderer> ().enabled = true;
    89.             break;
    90.         case 100:
    91.             GameObject.Find("worldData/_cherusci").GetComponent<Renderer> ().enabled = true;
    92.             GameObject.Find("worldData/_chatti").GetComponent<Renderer> ().enabled = true;
    93.             GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
    94.             GameObject.Find("worldData/_tencteri").GetComponent<Renderer> ().enabled = true;
    95.             GameObject.Find("worldData/_mattiaci").GetComponent<Renderer> ().enabled = true;
    96.             break;
    97.         case 110:
    98.             GameObject.Find("worldData/_chatti").GetComponent<Renderer> ().enabled = true;
    99.             GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
    100.             GameObject.Find("worldData/_suganbri").GetComponent<Renderer> ().enabled = true;
    101.             GameObject.Find("worldData/_tencteri").GetComponent<Renderer> ().enabled = true;
    102.             GameObject.Find("worldData/_mattiaci").GetComponent<Renderer> ().enabled = true;
    103.             break;
    104.         case 120:
    105.             GameObject.Find("worldData/_chatti").GetComponent<Renderer> ().enabled = true;
    106.             GameObject.Find("worldData/_gaul").GetComponent<Renderer> ().enabled = true;
    107.             GameObject.Find("worldData/_mattiaci").GetComponent<Renderer> ().enabled = true;
    108.             GameObject.Find("worldData/_tencteri").GetComponent<Renderer> ().enabled = true;
    109.             break;
    110.  
    111.  
    112.         }
    113.  
    114.  
    115.     }

    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).

    Code (CSharp):
    1.   case 50:
    2.             ...
    3. GameObject.Find("worldData/_ampsivarii").GetComponent<Renderer> ().enabled = true;
    4. if (vil5.valliance <> "Hostile )
    5. {
    6.            GameObject.Find("worldData/_angrivarii").GetComponent<Renderer> ().enabled = true;
    7. }
    8. if (vil6.valliance <> "Hostile )
    9. {
    10.             GameObject.Find("worldData/_chauci").GetComponent<Renderer> ().enabled = true;
    11. }
    12.             ....
    13.  
    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.
     
  7. chance1234

    chance1234

    Joined:
    Jun 7, 2016
    Posts:
    16
    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
     
  8. jvo3dc

    jvo3dc

    Joined:
    Oct 11, 2013
    Posts:
    1,520
    Code (csharp):
    1.  
    2. case 0:
    3.     SetRendererEnabled(true, "_chamavi", "_frissi", "_gaul", "_mattiaci", "_suganbri", "_tencteri", "_usipeces");
    4.     break;
    5.  
    6. private static void SetRendererEnabled(bool enabled, params string[] names)
    7. {
    8.     foreach (string name in names)
    9.     {
    10.         GameObject.Find("worldData/" + name).GetComponent<Renderer>().enabled = enabled;
    11.     }
    12. }
    13.  
    Though I would recommend using direct links and reduce the usage of Find as much as possible.
     
  9. chance1234

    chance1234

    Joined:
    Jun 7, 2016
    Posts:
    16
    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.

    Code (CSharp):
    1. instantiateSettlement(new vector(1,0,3),"Chamavi","Hostile","Poor","Small")
    2. instantiateSettlement(new vector(1,0,2),"Usipes","Hostile","Poor","Medium")
    3. instantiateSettlement(new vector(1,0,3),"Chauci","Hostile","PRicj","Small")
    4. instantiateSettlement(new vector(1,0,3),"Angrivarii","Hostile","Poor","Small")
    5.  
    6. etc, etc
    7.  
    8. instantiateNeighbours("Chamavi","_Usipes")
    9. instantiateNeighbours("Chamavi","_Ampivarri")
    10. instantiateNeighbours("Chamavi","_Usipes")
    11.  
    12. instantiateNeighbours("Usipes","_Cherusci")
    13. instantiateNeighbours("Usipes","_Suganbri")
    14.  
    15. etc etc
     
  10. jvo3dc

    jvo3dc

    Joined:
    Oct 11, 2013
    Posts:
    1,520
    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.