Inventory Redraw() Framerate hiccups with high slot count

[JamesD]

Hello, after going through Section 3 of the Inventory course and playing around with the player inventory slot count, I noticed that once you add more than a few dozen inventory slots, the game will noticeably pause while it destroys and instantiates the UI Slots 2 or 4 times (empty vs swapping). Sam mentioned that he first codes something very straightforward to make sure the system works, which makes a lot of sense. However, is this something that will be revisited later in the course?

I rewrote the code to use a similar setup as the InventoryItem cache, but was wondering what better approaches there might be. I feel like my solution to every problem is a Dictionary, so it’d be interesting to learn about other ways of accomplishing this.

Here’s the additional code:

Summary

    // CACHE
    Inventory playerInventory;
    int playerInventorySize;
    Dictionary<int, InventorySlotUI> inventorySlotCache;
    
    private void Redraw()
    {
        // Get inventory size
        int sizeCheck = playerInventory.GetSize();

        // Check if Inv cache is null or different size
        if(inventorySlotCache == null || playerInventorySize != sizeCheck)
        {
            // Destroy all old slots
            foreach (Transform child in transform)
            {
                Destroy(child.gameObject);
            }

            inventorySlotCache = new Dictionary<int, InventorySlotUI>();
            playerInventorySize = sizeCheck;
            for (int i = 0; i < playerInventorySize; i++)
            {
                var itemUI = Instantiate(InventoryItemPrefab, transform);
                inventorySlotCache[i] = itemUI;
                itemUI.Setup(playerInventory, i);
            }

            // Set content window to show top row
            transform.localPosition = new Vector3(transform.localPosition.x, 0, transform.localPosition.z);
        }
        foreach (KeyValuePair<int, InventorySlotUI> inventorySlot in inventorySlotCache)
        {
            var itemUI = inventorySlot.Value;
            itemUI.Setup(playerInventory, inventorySlot.Key);
        }
    }

I found that the playerInventory doesn’t change size during runtime currently, but even with 200+ slots, the Redraw() isn’t producing any stutter.

[Brian_Trotter]

This is actually quite nice. The other solution is to create a Factory to get new ItemUI, and instead of destroying them, you return them to the Factory. The advantage this solution has is that unless the number of inventory slots changes, you won’t have to return them to the Factory for “refurbishing”, you can just refresh the data on the fly. @sampattuzzi, you might want to take a glance at this.

[sampattuzzi]

This is really cool. I probably won’t be revisiting in the course but both of these solutions work great.

[JamesD]

I haven’t heard of a Factory before, googling mentions the Factory Method Design Pattern, would that be what to look into? It really looks useful to learn, but I’m not sure I follow how one would refresh the data in each itemUI.

Once I get through the item drop section, I’ll see if I can tinker around with this method with different item types and understand it a bit better.

[Brian_Trotter]

Yes, the Factory design pattern is what I’m talking about…
Here’s how it works:
First, you create a Factory for the prefab you wish to instantiate. In this case, the itemUI.

Whenever a new item is needed, instead of Instantiate(prefab), you make a call to the Factory and request an itemUI. The Factory maintains a queue of recycled itemUI… if it finds an itemUI in the queue, it pops the item from the queue, activates it, and returns it to the slot manager. If it doesn’t find any, it Instantiates a new one. When the slot manager would normally Destroy the itemUIs under it, instead, it returns them to the Factory where they are dutifully placed in the queue (after being deactivated).
When the itemUI is received from the factory, simply call the itemUI.Setup.

In cases where you don’t know how many itemUI you will need, this is actually quite peformant, because one area where Unity does NOT shine is in it’s garbage collection which could run at any time and cause significant freezes in the game… were you to say… destroy 200 itemUI and reinstantiate them. When the number of items is fixed, however, your solution is better because there is no need to detach the itemUI elements

[JamesD]

Well, I made an attempt, but fell short in a few places. Going by this and this guide, I tried to create a Factory that has ReturnSlots() and GetSlots() and operate the queue from there. The main class would be abstract and let different ProductCreators override, but I wasn’t able to figure out how to declare the main class as abstract, since Unity then wouldn’t let me attach it. In the end, I think this ended up being a short spawning class more than anything else.

Video of performance, works pretty well until you go to high slot values:

Inventory Factory Attempt

Changes to the Inventory.cs:

Summary

public int GetSize()
{
    // If inventory size has changed, remake the inventory array
    if(slots.Length != inventorySize)
    {
        print(slots.Length + " slots with inv size " + inventorySize);
        InventoryItem[] recalcSlots = new InventoryItem[inventorySize];
        int upperBound = Math.Min(slots.Length, inventorySize);

        for (int i = 0; i < upperBound; i++)
        {
            recalcSlots[i] = slots[i];
        }

        slots = recalcSlots;
    }

    return slots.Length;
}

New UIFactory script, not sure which namespace this should belong to and which object it should live on in the scene:

Summary

public class UIFactory : MonoBehaviour
{
    [SerializeField] InventorySlotUI inventoryItemPrefab = null;
    public Queue<InventorySlotUI> SlotsInQueue = new Queue<InventorySlotUI>();

    public void ReturnSlots(GameObject objSlotUI)
    {
        var slotUI = objSlotUI.GetComponent<InventorySlotUI>();
        if (slotUI != null)
        {
            objSlotUI.transform.SetParent(transform, true);
            objSlotUI.SetActive(false);
            SlotsInQueue.Enqueue(slotUI);
        }
        else
        {
            Destroy(objSlotUI);
        }        
    }

    public InventorySlotUI GetSlotUI()
    {
        if (SlotsInQueue.Count > 0)
        {
            InventorySlotUI slotUI = SlotsInQueue.Dequeue();
            slotUI.gameObject.SetActive(true);
            return slotUI;
        }
        
        InventorySlotUI newSlotUI = Instantiate(inventoryItemPrefab, transform);
        return newSlotUI;
    }
}

InventoryUI.cs changes:

Summary

// CONFIG DATA
// [SerializeField] InventorySlotUI InventoryItemPrefab = null;
[SerializeField] UIFactory factory = null;

private void Redraw()
{
    // Stepping backwards to make sure none are missed...
    int children = transform.childCount;
    for (int i = children - 1; i > -1; i--)
    {
        factory.ReturnSlots(transform.GetChild(i).gameObject);
    }

    // wasn't able to find a better way to get the slots in the correct order for some reason
    for (int i = playerInventory.GetSize() - 1; i > -1; i--)
    {
        var itemUI = factory.GetSlotUI();
        itemUI.transform.SetParent(transform, true);
        itemUI.transform.SetAsFirstSibling();
        itemUI.Setup(playerInventory, i);
    }
}

The inventoryUI.cs ended up causing the biggest challenge. When using foreach(Transform child in transform), it was skipping children when detaching the gameobjects back to the factory. And then to get the slots to mostly be in the right order, I had to step backward through the playerInventory, as well. Somewhere along the line, I created a duping bug when you swap and change inventory sizes, not sure what causes that.

This was interesting to try out, even though I don’t think I made a true factory pattern. I’ll definitely be on the lookout for good tutorials of where to use this when I have object types that share a lot of attributes. For now, I think my game will have a fixed inventory size.

Edit: I see where I went wrong with the abstract classes. I don’t need any overlap with MonoBehaviour in the Factory base class or the subclasses of it. Now the question is where those classes and subclasses should go.

Edit 2: Got it working as an abstract class within InventoryUI.cs, but now I’m running into a couple of different issues with inheriting.

1. If the abstract class does not inherit MonoBehaviour, Instantiate() is not available. So, the class must (?) inherit from MonoBehaviour.
2. If it does inherit, then it’s not able to get created with the “new” keyword while on InventoryUI.cs. The effect of this is that it doesn’t have a transform of its own and the DragItem.cs Awake functions don’t cache properly, which makes the dragging not layer correctly.

This seems closer than my last attempt in terms of the format, but I’m obviously still missing some important pieces to get it functioning as intended.

Summary

namespace GameDevTV.UI.Inventories
{
    public class InventoryUI : MonoBehaviour
    {
        // CONFIG DATA
        [SerializeField] InventorySlotUI InventoryItemPrefab = null;
        [SerializeField] UIFactory factory = null;

        // CACHE
        Inventory playerInventory;

        // LIFECYCLE METHODS

        private void Awake() 
        {
            playerInventory = Inventory.GetPlayerInventory();
            playerInventory.inventoryUpdated += Redraw;
            factory = new InvUIFactory(InventoryItemPrefab);
        }

        private void Start()
        {
            Redraw();
        }

        // PRIVATE
        private void Redraw()
        {
            int children = transform.childCount;
            for (int i = children - 1; i > -1; i--)
            {
                factory.ReturnSlots(transform.GetChild(i).gameObject);
            }

            for (int i = playerInventory.GetSize() - 1; i > -1; i--)
            {
                var itemUI = factory.GetSlotUI();
                itemUI.transform.SetParent(transform, true);
                itemUI.transform.SetAsFirstSibling();
                itemUI.transform.localScale = Vector3.one;
                itemUI.Setup(playerInventory, i);
            }
        }
    }

    public abstract class UIFactory : MonoBehaviour
    {
        public abstract void ReturnSlots(GameObject obj);
        public abstract InventorySlotUI GetSlotUI();
    }

    public class InvUIFactory : UIFactory
    {
        [SerializeField] InventorySlotUI inventoryItemPrefab = null;
        public Queue<InventorySlotUI> SlotsInQueue = new Queue<InventorySlotUI>();

        public InvUIFactory(InventorySlotUI prefab)
        {
            inventoryItemPrefab = prefab;
        }

        public override void ReturnSlots(GameObject objSlotUI)
        {
            var slotUI = objSlotUI.GetComponent<InventorySlotUI>();
            if (slotUI != null)
            {
                // this factory has no transform
                // objSlotUI.transform.SetParent(transform, true);
                objSlotUI.SetActive(false);
                SlotsInQueue.Enqueue(slotUI);
            }
            else
            {
                Destroy(objSlotUI);
            }
        }
        public override InventorySlotUI GetSlotUI()
        {
            if (SlotsInQueue.Count > 0)
            {
                InventorySlotUI slotUI = SlotsInQueue.Dequeue();
                slotUI.gameObject.SetActive(true);
                return slotUI;
            }

            InventorySlotUI newSlotUI = Instantiate(inventoryItemPrefab);
            return newSlotUI;
        }
    }
}