Search Unity

Feedback Question about optimization

Discussion in 'Scripting' started by XavrosRA, Jan 1, 2023.

  1. XavrosRA

    XavrosRA

    Joined:
    Sep 3, 2022
    Posts:
    3
    Hello, I'm implementing a crafting system and was wondering if this is a good way to do so. It uses a fair bit of dictionaries and wasn't sure if there was a more optimized approach. Honestly, if anywhere needs improvement or I'm using something incorrectly, please let me know.
    I have two scriptable objects, one for all items in the game and another for recipes for making said item. This could probably be combined into one, but I felt it was a bit cluttered and don't believe all items will be craftable further down the road.

    Inventory items:
    Code (CSharp):
    1. public class InventoryItemData : ScriptableObject
    2. {
    3.     public int ID = -1;
    4.     public int MaxStackSize;
    5.     public int GoldValue;
    6.    
    7.     [TextArea(4, 4)]
    8.     public string Description;
    9.     public string DisplayName;
    10.     public Sprite Icon;
    11.  
    12.     public GameObject ItemPrefab;
    13.     public CraftingRecipe RequiredItems;
    14. }
    Recipes:
    Code (CSharp):
    1. [Serializable]
    2. public struct RecipeIngredients
    3. {
    4.     public InventoryItemData RecipeItemNeeded;
    5.     public int RecipeItemAmountNeeded;
    6. }
    7.  
    8. [CreateAssetMenu(menuName = "Crafting System/Crafting Recipes")]
    9. public class CraftingRecipe : ScriptableObject, IEnumerable
    10. {
    11.     public string ItemName;
    12.     public InventoryItemData ItemToMake;
    13.     public List<RecipeIngredients> CraftingIngredients = new List<RecipeIngredients>();
    14.     public Dictionary<InventoryItemData, List<RecipeIngredients>> CraftingItemRecipe = new Dictionary<InventoryItemData, List<RecipeIngredients>>();
    15.  
    16.     private void OnEnable()
    17.     {
    18.         CraftingItemRecipe.Add(ItemToMake, CraftingIngredients);
    19.     }
    20.  
    21.     public List<RecipeIngredients> GetIngredientsList(InventoryItemData item)
    22.     {
    23.         return CraftingItemRecipe[item];
    24.     }
    25. }
    Finally, where I check if the player's inventory has the required ingredients:

    Code (CSharp):
    1. public bool TryBuild(InventoryItemData itemToBuild)
    2.     {
    3.         // Get the ingredients needed for the selected item.
    4.         var ingredientsNeeded = itemToBuild.RequiredItems.GetIngredientsList(itemToBuild);
    5.  
    6.         // Get all the items in the player's inventory.
    7.         var inventoryIngredients = playerInventory.GetAllItemsHeld();
    8.        
    9.         // Declare a dictionary of items and bools to determine which the player has and has enough of.
    10.         var ingredientsInPlayerInventory = new Dictionary<InventoryItemData, bool>();
    11.        
    12.         // For every ingredient in the item we're trying to build.
    13.         foreach(var itemNeeded in ingredientsNeeded)
    14.         {
    15.             Debug.Log("Item needed from recipe: " + itemNeeded.RecipeItemNeeded + " Amount needed from recipe: " + itemNeeded.RecipeItemAmountNeeded);
    16.            
    17.             // For every item in the player's inventory that's on the ingredients list.
    18.             if(!inventoryIngredients.ContainsKey(itemNeeded.RecipeItemNeeded))
    19.             {
    20.                 Debug.Log("Boo, item NOT in player inventory.");
    21.                 ingredientsInPlayerInventory.TryAdd(itemNeeded.RecipeItemNeeded, false);
    22.             }
    23.             else
    24.             {
    25.                 Debug.Log("Woo, item IS in player inventory.");
    26.                 ingredientsInPlayerInventory.TryAdd(itemNeeded.RecipeItemNeeded, true);
    27.             }
    28.  
    29.             // If player has greater amount than required.
    30.             if(inventoryIngredients.ContainsKey(itemNeeded.RecipeItemNeeded) && (inventoryIngredients[itemNeeded.RecipeItemNeeded] >= itemNeeded.RecipeItemAmountNeeded))
    31.             {
    32.                 Debug.Log("Amount left over in player's inventory: " );
    33.                 Debug.Log(inventoryIngredients[itemNeeded.RecipeItemNeeded] - itemNeeded.RecipeItemAmountNeeded);
    34.                 Debug.Log("Player has enough " + itemNeeded.RecipeItemNeeded + " in their inventory.");
    35.                    
    36.             }
    37.             else
    38.             {
    39.                 // Else, player lacks ingredients.
    40.                 Debug.Log("Player does not have enough " + itemNeeded.RecipeItemNeeded + " in their inventory.");
    41.                 ingredientsInPlayerInventory.TryAdd(itemNeeded.RecipeItemNeeded, false);
    42.             }
    43.         }
    44.  
    45.         // If our Dictionary doesn't contain a false, we can craft the item.
    46.         if(!ingredientsInPlayerInventory.ContainsValue(false))
    47.         {
    48.             playerInventory.AddToInventory(itemToBuild, 1);
    49.             return true;
    50.         }
    51.         return false;
    52.     }
    Feedback is greatly appreciated!
     
  2. Number-3434

    Number-3434

    Joined:
    Dec 31, 2022
    Posts:
    13
    It seems fine. Just use the Awake() method instead of OnEnable()
     
  3. XavrosRA

    XavrosRA

    Joined:
    Sep 3, 2022
    Posts:
    3
    Number-3434 likes this.
  4. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,697
    If it works, it works. Move on.

    Just so you know...

    These things (inventory, shop systems, character customization, crafting, etc) are fairly tricky hairy beasts, definitely deep in advanced coding territory.

    They contain elements of:

    - a database of items that you may possibly possess / equip
    - a database of the items that you actually possess / equip currently
    - perhaps another database of your "storage" area at home base?
    - persistence of this information to storage between game runs
    - presentation of the inventory to the user (may have to scale and grow, overlay parts, clothing, etc)
    - interaction with items in the inventory or on the character or in the home base storage area
    - interaction with the world to get items in and out
    - dependence on asset definition (images, etc.) for presentation

    Just the design choices of an inventory system can have a lot of complicating confounding issues, such as:

    - can you have multiple items? Is there a limit?
    - if there is an item limit, what is it? Total count? Weight? Size? Something else?
    - are those items shown individually or do they stack?
    - are coins / gems stacked but other stuff isn't stacked?
    - do items have detailed data shown (durability, rarity, damage, etc.)?
    - can users combine items to make new items? How? Limits? Results? Messages of success/failure?
    - can users substantially modify items with other things like spells, gems, sockets, etc.?
    - does a worn-out item (shovel) become something else (like a stick) when the item wears out fully?
    - etc.

    Your best bet is probably to write down exactly what you want feature-wise. It may be useful to get very familiar with an existing game so you have an actual example of each feature in action.

    Once you have decided a baseline design, fully work through two or three different inventory tutorials on Youtube, perhaps even for the game example you have chosen above.

    Breaking down a large problem such as inventory:

    https://forum.unity.com/threads/weapon-inventory-and-how-to-script-weapons.1046236/#post-6769558

    If you want to see most of the steps involved, make a "micro inventory" in your game, something whereby the player can have (or not have) a single item, and display that item in the UI, and let the user select that item and do things with it (take, drop, use, wear, eat, sell, buy, etc.).

    Everything you learn doing that "micro inventory" of one item will apply when you have any larger more complex inventory, and it will give you a feel for what you are dealing with.

    As for optimization...

    DO NOT OPTIMIZE "JUST BECAUSE..." If you don't have a problem, DO NOT OPTIMIZE!

    If you DO have a problem, there is only ONE way to find out. Always start by using the profiler:

    Window -> Analysis -> Profiler

    Failure to use the profiler first means you're just guessing, making a mess of your code for no good reason.

    Not only that but performance on platform A will likely be completely different than platform B. Test on the platform(s) that you care about, and test to the extent that it is worth your effort, and no more.

    https://forum.unity.com/threads/is-...ng-square-roots-in-2021.1111063/#post-7148770

    Remember that optimized code is ALWAYS harder to work with and more brittle, making subsequent feature development difficult or impossible, or incurring massive technical debt on future development.

    Notes on optimizing UnityEngine.UI setups:

    https://forum.unity.com/threads/how...form-data-into-an-array.1134520/#post-7289413

    At a minimum you want to clearly understand what performance issues you are having:

    - running too slowly?
    - loading too slowly?
    - using too much runtime memory?
    - final bundle too large?
    - too much network traffic?
    - something else?

    If you are unable to engage the profiler, then your next solution is gross guessing changes, such as "reimport all textures as 32x32 tiny textures" or "replace some complex 3D objects with cubes/capsules" to try and figure out what is bogging you down.

    Each experiment you do may give you intel about what is causing the performance issue that you identified. More importantly let you eliminate candidates for optimization. For instance if you swap out your biggest textures with 32x32 stamps and you STILL have a problem, you may be able to eliminate textures as an issue and move onto something else.

    This sort of speculative optimization assumes you're properly using source control so it takes one click to revert to the way your project was before if there is no improvement, while carefully making notes about what you have tried and more importantly what results it has had.
     
    XavrosRA likes this.
  5. XavrosRA

    XavrosRA

    Joined:
    Sep 3, 2022
    Posts:
    3
    Thank you very much Kurt. I really appreciate the resources and you've given me a ton to research and think over. I'm fairly new to unity, about 4 months in, but not new to programming. I suppose I'm concerned about scalability and would like to nail it the first time around I suppose. I greatly appreciate your response, there's a lot of good advice here. I certainly need to take a step back and better document / plan. Thank you again!
     
  6. chemicalcrux

    chemicalcrux

    Joined:
    Mar 16, 2017
    Posts:
    720
    When thinking about performance, you should consider two factors:

    • How much does it cost to do X?
    • How often do I do X?

    You should only be worried if the answers to both of these questions are "a lot".

    The player probably isn't trying to craft hundreds of items per second, so you'd need one slow crafting function for there to be any issues.

    On the other hand, if you were to test if every crafting recipe is possible to use every single update, you might need to think harder about performance -- only after the Profiler shows that there's a problem, of course.