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. Have a look at our Games Focus blog post series which will show what Unity is doing for all game developers – now, next year, and in the future.
    Dismiss Notice

Question Is there a better way to do this? Like/Neutral/Dislike struct with lists.

Discussion in 'General Discussion' started by Razputin, Nov 18, 2022.

  1. Razputin

    Razputin

    Joined:
    Mar 31, 2013
    Posts:
    324
    I'm trying to get rid of some coding habits where I assume better solutions exist.

    In the following scripts I have a "MonkeyInfo" script with a "Feed" method. The monkey has "FoodPreferences" which are contained in 3 lists, likedFood, neutralFood, and dislikedFood.

    Is there a better way to setup the "Liked, Neutral, and Disliked" food lists? It seems stupid of me to have to setup 3 lists, and then iterate over those 3 lists to check if the incoming "FoodType" is contained within them. Also accidents could occur where a foodtype exists in multiple lists by accident.

    Thanks,
    Adam

    Question.png

    Code (CSharp):
    1. public class Consumeable : MonoBehaviour
    2. {
    3.  
    4.     List<MonkeyAI> monkeysComingForFood = new List<MonkeyAI>();
    5.     public bool onFloor = false;
    6.     [SerializeField]
    7.     public int BitesLeft
    8.     {
    9.         get { return bitesLeft; }
    10.         private set { bitesLeft = value; }
    11.     }
    12.     public int bitesLeft = 4;
    13.     public int consumeValue = 25;
    14.  
    15.     public enum FoodType { Meat, Vegetable, Fruit, Fish, Garbage }
    16.     public FoodType foodType;
    17.  
    18.     public void TakeBite(MonkeyAI ai)
    19.     {
    20.         ai.monkeyInfo.Feed(this);
    21.         BitesLeft--;
    22.         if(BitesLeft <= 0)
    23.         {
    24.             DestroyConsumeableGameobject();
    25.         }
    26.     }
    27.  
    28.     public void DestroyConsumeableGameobject()
    29.     {
    30.         Destroy(gameObject);
    31.     }
    32. }
    Code (CSharp):
    1.     public void Feed(Consumeable c)
    2.     {
    3.         foreach(Consumeable.FoodType foodType in foodPreferance.likedFood)
    4.         {
    5.             if(foodPreferance.likedFood.Contains(foodType))
    6.             {
    7.                 SetHunger(c.consumeValue * 2);
    8.                 return;
    9.             }
    10.         }
    11.  
    12.         foreach (Consumeable.FoodType foodType in foodPreferance.neutralFood)
    13.         {
    14.             if (foodPreferance.likedFood.Contains(foodType))
    15.             {
    16.                 SetHunger(c.consumeValue);
    17.                 return;
    18.             }
    19.         }
    20.  
    21.         foreach (Consumeable.FoodType foodType in foodPreferance.dislikedFood)
    22.         {
    23.             if (foodPreferance.likedFood.Contains(foodType))
    24.             {
    25.                 SetHunger(c.consumeValue * .5f);
    26.                 return;
    27.             }
    28.         }
    29.     }
    30. }
    31.  
    32. [System.Serializable]
    33. public struct FoodPreferance
    34. {
    35.     public List<Consumeable.FoodType> likedFood;
    36.     public List<Consumeable.FoodType> neutralFood;
    37.     public List<Consumeable.FoodType> dislikedFood;
    38. }
     
  2. Murgilod

    Murgilod

    Joined:
    Nov 12, 2013
    Posts:
    8,914
    Off the top of my head, I'd probably create a FoodEnjoyment class like this:

    Code (CSharp):
    1. [System.Serializable]
    2. public class FoodTastes
    3. {
    4.    public FoodType foodType;
    5.    public float foodEnjoyment;
    6. }
    And then have the foodEnjoyment variable be the consumeValue multiplier. Create a list of that and you've simplified your code quite a bit. You could also use a Dictionary for this to prevent duplicate entries but those aren't so easily serializable in the inspector if that's important to you.
     
    angrypenguin, Kiwasi and Razputin like this.
  3. Razputin

    Razputin

    Joined:
    Mar 31, 2013
    Posts:
    324
    Yeah that helps bring it down to 1 list at least which is much more manageable, thank you!
    I ended up using an enum with a switch because I personally prefer how it looks in the inspector.

    Question.png
     
    Last edited: Nov 18, 2022
  4. Owen-Reynolds

    Owen-Reynolds

    Joined:
    Feb 15, 2012
    Posts:
    1,713
    This feels like a prime example of a textbook explaining the interface concept. Here we've got monkey-eating and want to do things such as
    if(m.isLikes("bread"));
    or
    m.setOpinion("cheese",BAD);
    . We can design those functions however -- maybe we prefer
    m.setAsBad("cheese");
    or we want both options for setting cheese to bad. Maybe we realize then that hardwiring good, neutral and bad is stupid, and clearly we want an enum "tasteLevel".

    Once we're happy with the rough idea of the interface, then we write the internals. It's not such a big deal how we write them, since the interface is already set. Maybe we have an ugly piece of garbage with 3 lists, but no one else needs to deal with it. That's a beautiful thing -- you can assume better solutions exist while putting in this crappy one in now. Your interface which gives you everything you need from the class makes it so the better solution is an easy replacement.
     
  5. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    2,795
    If this were left to me I would be making my FoodTypes scriptable objects, that ways it's trivial to add more food types in the future.
     
    Ryiah likes this.
  6. Stardog

    Stardog

    Joined:
    Jun 28, 2010
    Posts:
    1,818
    You could do it like this, also:
    Code (csharp):
    1. public Dictionary<FoodType, FoodOpinion> FoodPrefs = new()
    2. {
    3.     [FoodType.Meat] = FoodOpinion.Like,
    4.     [FoodType.Veg] = FoodOpinon.Neutral
    5.     // etc
    6. };
    7.  
    8. public Dictionary<FoodOpinion, float> FoodComsumeValues = new()
    9. {
    10.     [FoodOpinion.Like] = 2f,
    11.     [FoodOpinion.Neutral] = 1f,
    12.     [FoodOpinion.Hate] = 0.5f
    13. };
    14.  
    15. public void Feed(Consumeable c)
    16. {
    17.     SetHunger(c);
    18. }
    19.  
    20. public void SetHunger(Consumeable c)
    21. {
    22.     Hunger += FoodConsumeValues[FoodPrefs[c.foodType]];
    23. }
    The problem is you will need an asset to see dictionaries in the inspector. Although, you could use your visible Lists to build the dictionaries on start.
     
    Razputin likes this.
  7. lmbarns

    lmbarns

    Joined:
    Jul 14, 2011
    Posts:
    1,586
    Code (CSharp):
    1.  
    2. using System.Collections;
    3. using System.Collections.Generic;
    4. using UnityEngine;
    5.  
    6. public class Monkey : MonoBehaviour
    7. {
    8.  
    9.     public MonkeyInfo monkeyInfo = new MonkeyInfo();
    10.    
    11.     void Start()
    12.     {
    13.         monkeyInfo.AddItem(FoodType.banana, Enjoyment.like);
    14.         monkeyInfo.AddItem(FoodType.apple, Enjoyment.like);
    15.         monkeyInfo.AddItem(FoodType.orange, Enjoyment.neutral);
    16.         monkeyInfo.AddItem(FoodType.strawberry, Enjoyment.dislike);
    17.  
    18.         ProcessFoodInBucket(); //pump this somewhere to process food
    19.     }  
    20.  
    21.     public void ProcessFoodInBucket()
    22.     {
    23.         for (int i = 0; i < monkeyInfo.FeedBucket.Count; i++)
    24.         {
    25.             if ((int)monkeyInfo.FeedBucket[i].enjoyment < 2)
    26.             {
    27.                 if (monkeyInfo.FeedBucket[i].Consume() == 0) //none left
    28.                     monkeyInfo.FeedBucket.RemoveAt(i);
    29.                 else
    30.                     Debug.Log("Took a bite");
    31.             }
    32.             else
    33.             {
    34.                 Debug.Log("Monkey refuses to eat that");
    35.             }
    36.         }
    37.     }
    38. }
    39.  
    40. [System.Serializable]
    41. public class MonkeyInfo
    42. {
    43.     public List<AFood> FeedBucket = new List<AFood>();
    44.     public void AddItem(FoodType t, Enjoyment e)
    45.     {
    46.         FeedBucket.Add(new AFood { type = t, enjoyment = e });
    47.     }
    48. }
    49.  
    50. [System.Serializable]
    51. public class AFood
    52. {
    53.     public FoodType type;
    54.     public Enjoyment enjoyment;
    55.     public int quantity = 100;
    56.     public int Consume()
    57.     {
    58.         quantity -= 5; //bite size
    59.  
    60.         if (quantity <= 0)
    61.             quantity = 0;
    62.  
    63.         return quantity;
    64.     }
    65. }
    66.  
    67. public enum FoodType
    68. {
    69.     banana = 0,
    70.     apple,
    71.     orange,
    72.     strawberry
    73. }
    74. public enum Enjoyment
    75. {
    76.     neutral = 0,
    77.     like,
    78.     dislike
    79. }
    80.  
     
    Last edited: Nov 20, 2022 at 6:21 PM
    Razputin likes this.
  8. Kiwasi

    Kiwasi

    Joined:
    Dec 5, 2013
    Posts:
    16,847
    I would also be tempted to use a dictionary for this purpose. They are faster for searching by key (although this is marginal on small collections). But the bigger advantage is that they don't allow duplication by default, which seems to be exactly what you want.

    I would also suggest against using enums for food type, unless you are absolutely sure that you will never add more food types in the future.
     
    Razputin likes this.
  9. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    14,945
    Don't forget that collections can be sorted based on a common search key, and that we can then use that for fast lookups ourselves.

    In this case, it is. Per the above suggestions there are solutions which are simpler and get better results.

    But in some cases it could be perfectly valid to split things over multiple collections, and even to have duplicates. So don't rule those things out in principle. They just aren't good fits here.
     
    Razputin and Kiwasi like this.
  10. Murgilod

    Murgilod

    Joined:
    Nov 12, 2013
    Posts:
    8,914
    Yeah, though the issue with dictionaries is that you have to get into custom serialization stuff if you need to use them in the editor.
     
    Razputin, Kiwasi and angrypenguin like this.
  11. Kiwasi

    Kiwasi

    Joined:
    Dec 5, 2013
    Posts:
    16,847
    Yup. I tend to cheat and make a list in the editor that gets loaded into a dictionary on start up. But it is extra work you need to manage.
     
    Razputin, DragonCoder and Antypodish like this.
  12. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    14,945
    Do you also do anything to get updates to show in the Inspector, and for the Inspector to work on the actual collection at runtime?

    For anything which content designers need to work with I tend to make a list of serialization-compatible objects for this kind of thing, so that they show up and behave as expected without any other special effort. Such collections tend to be relatively small, and with searches rare enough that I'm not too concerned with performance anyway. And they can always be optimised later if needed.

    In the places where I am concerned with performance the collection tends to be large enough that Inspector display isn't especially useful in the first place, and population of the collection is done fully dynamically at runtime anyway.
     
  13. Kiwasi

    Kiwasi

    Joined:
    Dec 5, 2013
    Posts:
    16,847
    Personally I don't. But I'm mostly working by myself.

    For a real crude solution you could just keep the list updating when the inspector is active. Horrible for performance though.
     
  14. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    14,945
    I probably wouldn't be too worried about the performance of copying one collection in the context of the Inspector, since that's super heavy anyway. But wouldn't that only go one way?
     
  15. Kiwasi

    Kiwasi

    Joined:
    Dec 5, 2013
    Posts:
    16,847
    Actually yes, this would only go one way. Going both ways would require something more complex. Maybe copying the collection in one direction in Update, then the other direction in OnInspectorGUI.

    But at this point it might just be worth doing the job properly.
     
  16. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    14,945
    Which is why I stick to serializable types unless I have a really good reason.

    That being said, I'd be shocked if people hadn't already solved this and shared it on GitHub or similar.
     
    Kiwasi likes this.
  17. Murgilod

    Murgilod

    Joined:
    Nov 12, 2013
    Posts:
    8,914
    Odin Inspector has the functionality in it if you inherit from their serialized classes.
     
  18. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    2,795
    It also costs $50 USD.

    Honestly I don't see the issues with regular collections if we're dealing with only a few dozen entries. Much as I love Odin I seldom use it's serialised types as there's often no need.
     
  19. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    14,945
    $50 per person isn't a big deal if it's a significant project, and Odin seems popular enough and has a good enough reputation that I'd probably be happy to let them maintain that kind of thing for me.

    But also, we're talking about something which is a bit more than a CS101 level exercise to do ourselves. I wrote the code over lunch. No time to test it, though, which is the time consuming bit.
     
  20. Murgilod

    Murgilod

    Joined:
    Nov 12, 2013
    Posts:
    8,914
    Odin Inspector has far, far more features than just that and is an absolute godsend if you plan on working on any project even a little bit seriously. $50 is a small price to pay for dramatic workflow improvements.
     
  21. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    2,795
    Oh don't get me wrong, I love Odin and use it heavily. (Been working on some fun tools that those who use it can make use of too)

    Just when you want to add a half dozen entries into a dictionary, the answer doesn't need to be 'reach for the paid addon'. It can just be... use a regular list.
     
  22. Murgilod

    Murgilod

    Joined:
    Nov 12, 2013
    Posts:
    8,914
    Sure, but sometimes you need a robust serialization system that goes beyond what a list offers, and adding things like duplicate checking, ensuring the process doesn't break down, ensuring that the list entries update properly, getting proper key/value pairs?

    Eventually that becomes a process. If it's a mission critical component, it's going to require a lot of testing. If that process is going to take more than a couple hours? You might as well reach for the paid asset because otherwise you're undervaluing your time.
     
  23. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    2,795
    I'm not debating that Odin isn't worth it. I don't even know why we're having a back and forth here.
     
    angrypenguin likes this.
  24. Owen-Reynolds

    Owen-Reynolds

    Joined:
    Feb 15, 2012
    Posts:
    1,713
    IMHO this thread is a great example of exactly what you're worried about. Most things we write, especially for games, are going to be removed, changed, or tweaked. Here, for example, I'm guessing food preference may become a 0-10 float with like/dislike/neutral as merely a display issue. All the work on getting the best old way is wasted.

    The rest of the time, the code that we thought would grow and we spent time making it neat and expandable -- it doesn't grow. Poison is handled by crappy handful of IF's that would become unreadable as we add more types of poison. And it's fine -- we end up never adding more poison.
     
    lmbarns and Kiwasi like this.
unityunity