Search Unity

Lists, random elements, and defining elements

Discussion in 'Scripting' started by zlong1, Dec 12, 2018.

  1. zlong1

    zlong1

    Joined:
    Sep 7, 2018
    Posts:
    4
    My objective is to create a list that will, upon a collision, call a random element. Each of these elements will be a powerup that will temporarily alter the player in various ways.

    I've been following the Unity tutorial on lists and dictionaries...




    ... and have tried to tailor it to my needs, but I'm afraid all I have done is make a mess. I've been banging my head against this problem for nearly a month and am convinced that I don't know what I'm doing when it comes to making lists.

    I've left comments on a few lines that stat my understanding of what the code is doing. Could anyone please tell me if I'm identifying its purpose correctly and where I'm getting it wrong?



    Code (CSharp):
    1. using System.Collections.Generic;
    2. using UnityEngine;
    3.  
    4. public class powerUpList : MonoBehaviour
    5. {
    6.  
    7.  
    8.     void Start()
    9.     {
    10.         List<powerUps> powerUpsList = new List<powerUps>(); // List is called powerUpsList
    11.  
    12.         powerUpsList.Add(new powerUps("fire")); // A new element is added to the list, its name is "fire"
    13.         powerUpsList.Add(new powerUps("static"));
    14.         powerUpsList.Add(new powerUps("shield"));
    15.         powerUpsList.Add(new powerUps("repel"));
    16.         powerUpsList.Add(new powerUps("repairKit"));
    17.         powerUpsList.Add(new powerUps("thorns"));
    18.         powerUpsList.Add(new powerUps("bearBooster"));
    19.  
    20.         powerUps RandomPowerup = powerUpsList[Random.Range(0, powerUpsList.Count)]; // A random element from the list is selected.
    21.  
    22.         Debug.Log(powerUps); // Says what element is selected.
    23.  
    24.     }
    25. }
    26.  
    Code (CSharp):
    1. using System.Collections;
    2. using UnityEngine;
    3. using System;
    4.  
    5. public class powerUps : IComparable<powerUps>
    6.  
    7. {
    8.     public string name;
    9.  
    10.  
    11.     public powerUps(string newName)
    12.     {
    13.         name = newName;;
    14.     }
    15. }
     
  2. tonemcbride

    tonemcbride

    Joined:
    Sep 7, 2010
    Posts:
    1,089
    That looks ok to me apart from a few points:

    1) In your first class you declare the list inside the Start function:

    List<powerUps> powerUpsList = new List<powerUps>(); // List is called powerUpsList

    That's fine if you only want to use it within the Start function but I suspect you might want to use it in other functions. To do that you'll need to move that list declaration outside of the function.

    2) You have a debug log that looks like this:

    Debug.Log(powerUps); // Says what element is selected.

    That won't work as you're logging the actual class itself rather than your instance of it. It should read:

    Debug.Log(RandomPowerup.name);
     
  3. Hosnkobf

    Hosnkobf

    Joined:
    Aug 23, 2016
    Posts:
    1,096
    I agree with what @tonemcbride says plus the following:

    I don't like your names:
    - Usually you use uppercase names for classes in C# (PowerUpList instead of powerUpList; PowerUps instead of powerUps)
    - Usually you use lowecase names for local variables (randomPowerup instead of RandomPowerup)
    - You should name everything as descriptive and correct as possible. Instead of describing what the class contains, you should describe what it is for in the name (I would rename powerUpList to something like PowerUpSelector)
    - A class usually describes one single thing. So I would rename powerUps to PowerUp (singular)

    Also, you might want to set the power ups inside the unity editor rather than in the code itself. To do so, you need to make the list a member variable (as @tonemcbride already suggested) and mark it that it is serialized (either make it public or better add a SerializeFieldAttribute to it). The PowerUp class needs to be marked as Serializable as well to work.

    Applying all this you would end up in something like this:
    Code (CSharp):
    1. using System.Collections.Generic;
    2. using UnityEngine;
    3.  
    4. public class PowerUpSelector : MonoBehaviour
    5. {
    6.     [SerializeField]
    7.     List<PowerUp> powerUps = new List<PowerUp>(); // filled via unity inspector
    8.  
    9.     void Start()
    10.     {
    11.         PowerUp randomPowerUp = PickRandom();
    12.         if(randomPowerUp != null)
    13.         {
    14.             Debug.Log(randomPowerUp.name);
    15.         }
    16.     }
    17.  
    18.     public PowerUp PickRandom()
    19.     {
    20.         if(powerUps.Count == 0)
    21.         {
    22.             Debug.LogError("Cannot pick power up: There is no power up specified.");
    23.             return null;
    24.         }
    25.  
    26.         return powerUps[Random.Range(0, powerUps.Count)];
    27.     }
    28. }
    29.  
    Code (CSharp):
    1. using System.Collections;
    2. using UnityEngine;
    3. using System;
    4.  
    5. [Serializable]
    6. public class PowerUp : IComparable<PowerUp>
    7. {
    8.     public string name;
    9.  
    10.     public PowerUp(string name)
    11.     {
    12.         this.name = name;
    13.     }
    14.  
    15.     public int CompareTo(PowerUp other)
    16.     {
    17.         return 0; // do your comparison logic here instead... BTW: for what do you need it?
    18.     }
    19. }