Merging the Third Person Controller Course with the RPG Course

Thanks a lot .
I already have object pooling for everything , I’m using a factory here is how i do it for anyone who want to know .

I really think this is an efficient solution u can just copy and paste the 2 scripts and add the first script

(objectPool script) to an object in your scene .(i jsut added it to my core since it will be in every scene )

namespace RPG.Core{
public class ObjectPool : MonoBehaviour
{//this iscript is for creating bullets at the start of the game and using them instead of constantly instantiating and destroing them this way we save performance 
   public static ObjectPool instance;//this allowes us to call this from everiwhere

   [SerializeField] private GameObject[] prefabs;//the prefab we need to instanciating the dictionary and queess
   [SerializeField] private int poolSize=200;

   
   public Dictionary<GameObject,Queue<GameObject>> poolDictionary = new Dictionary<GameObject, Queue<GameObject>>();

   private void Awake()
   {
    if (instance == null)//this makes sure we know what instance we are using
    {
        instance=this;
    }
    else
    Destroy(instance);

    InitializeDictionaryQueues();
    
    
    
   }
   public void ReturnGameObjectToPool(GameObject gameObjectToReturn)//returns gameobject to proper pool
   {
    GameObject originalPrefab=gameObjectToReturn.GetComponent<PooledGameObject>().originalPrefab;
    gameObjectToReturn.SetActive(false);
    poolDictionary[originalPrefab].Enqueue(gameObjectToReturn);
    gameObjectToReturn.transform.parent=transform;

   }

   public GameObject GetGameObject(GameObject gameObjectToSearch)//finds a specific gameobject and activates,if theres no pool for it it creates one ,if empty instantiate new
   {
    if(!poolDictionary.ContainsKey(gameObjectToSearch))
    {
        CreateDictionaryPool(gameObjectToSearch);
    }

    if(poolDictionary[gameObjectToSearch].Count==0)
    {
        CreateNewGameObject(gameObjectToSearch);

    }
        GameObject gameObjectToGet= poolDictionary[gameObjectToSearch].Dequeue();
        gameObjectToGet.SetActive(true);
        gameObjectToGet.transform.parent=null;
        

       return gameObjectToGet;
   }

   private void CreateDictionaryPool(GameObject prefab)//we are instantiating the prefab all at once so we dont have to create or destroy them later
   {
    poolDictionary[prefab]=new Queue<GameObject>();
    for (int i = 0;i<poolSize;i++)
    {
       CreateNewGameObject(prefab);

    }

   }
   private void CreateNewGameObject(GameObject prefab)//creates a copy of the prefab
   {
    GameObject newGameObject=Instantiate(prefab ,transform);
    newGameObject.AddComponent<PooledGameObject>().originalPrefab=prefab;//adds original prefab to clones so we can serach the pools with it(clone its not the same as original)

    newGameObject.gameObject.SetActive(false);
    
     poolDictionary[prefab].Enqueue(newGameObject);//adding the new instanciated bullet to the q
   }

   private void InitializeDictionaryQueues()//initialize all the pools for the prefabs
   {
    foreach (var prefab in prefabs)
    {
        CreateDictionaryPool(prefab);
    }
   }




}
}

when u use the function GetGameObject() the script will take enable a new object .if there are no object of this type available it will create one for u and set it up so it can be reused so u will never run out of objects and u will not have to create them all the time either in the script there’s also a way to instantiate prefabs on awake if u give them the prefabs b4 hand .

public class PooledGameObject: MonoBehaviour
{//this is used for the object pooling so that it can identifie the original prefab of the clone and use it as a key to search in the dictionary 
    public GameObject originalPrefab{ get;  set; }
}

so in conclusion we make this 2 scripts and we add the first to an empty object in the scene .
after that where we we were instantiating something we use this

RPG.Core.ObjectPool.instance.GetGameObject(Gameobject gameobject );

and instead of destroy we use this

RPG.Core.ObjectPool.instance.ReturnGameObjectToPool(gameObject);

the rpg.core is the namespace u probably have ur own .

heres a function u can use for timer destroy :slight_smile:

IEnumerator ReturnToPoolTimer(GameObject  gameObject,int time )//returns object to pool after a time
        {
            yield return new WaitForSeconds(time);
     
            RPG.Core.ObjectPool.instance.ReturnGameObjectToPool(gameObject);

            
        }

Im doing a topdown shooter im already pretty far into it what is your game about?

Oh let me know if u guys think theres anything i can improve here but i think thats it .

I’m working on a third person RPG :slight_smile: - it’s a little too crazy right now, xD. I’m also incredibly deep in the programming of my game

I’ll give your object pooling solution a run, although I might need your help installing it if you don’t mind (it’ll be a short while though, I’m still working on a saving bug for something I recently introduced)

Anyway, here’s my old slot based redraw attempt, this was a complete failure (just placing it here in case I need it again):

private void SlotBasedRedraw() 
        {
            int index = GetLastClickedIndex();
            InventorySlotUI slotToRedraw = null;

            foreach (Transform child in this.transform) 
            {
                // For optimization purposes, only destroy the clicked Slot, otherwise continue
                var slotUI = child.GetComponent<InventorySlotUI>();
                if (slotUI != null && slotUI.GetSlotIndex() == index)
                {
                    slotToRedraw = slotUI;
                    break;
                }
            }

            // Re-Setup the found slot, if it exists
            if (slotToRedraw != null) 
            {
                slotToRedraw.Setup(selectedInventory, index);
            }
        }

@Brian_Trotter how do we integrate this algorithm by @JohnyBranch to improve inventory performance? I tried rewriting the redraw method in ‘InventoryUI.cs’ but that didn’t go too well:

using UnityEngine;
using GameDevTV.Inventories;
using TMPro;
using UnityEngine.Events;
using System.Linq;
using RPG.Core.ObjectPooling;

namespace GameDevTV.UI.Inventories
{
    /// <summary>
    /// To be placed on the root of the inventory UI. Handles spawning all the
    /// inventory slot prefabs.
    /// </summary>
    public class InventoryUI : MonoBehaviour
    {
        // CONFIG DATA
        [SerializeField] InventorySlotUI InventoryItemPrefab = null;

        // BANK System CONFIG DATA:
        [SerializeField] bool isPlayerInventory = true;
        [SerializeField] TextMeshProUGUI Title;

        // 3 bank <-> Inventory states: 
        // 'otherInventoryUI' (bank) is open, 
        // 'otherInventoryUI' (bank) is closed, 
        // YOU are the 'otherInventoryUI' (bank)

        public bool IsOtherInventory => !isPlayerInventory;
        public IInventorySource SelectedInventory => selectedInventory;

        [SerializeField] private UnityEvent EnabledEvent;
        [SerializeField] private UnityEvent DisabledEvent;

        // CACHE
        IInventorySource selectedInventory;

        // LIFECYCLE METHODS

        private void Awake()
        {
            // if we are dealing with the player Inventory, set it up (just don't do it for the Players' bank)
            if (isPlayerInventory)
            {
                selectedInventory = Inventory.GetPlayerInventory();
                selectedInventory.inventoryUpdated += Redraw; // you need this inventory subscription
            }
        }

        private void Start() // ORIGINAL START Function:
        {
            // Redraw the PLAYER INVENTORY, JUST NOT THE BANK!
            if (isPlayerInventory) Redraw();
        }

        // ---------------------------------------- OPTIONAL (USED IN 'InventorySlotUI.cs') ---------------------------

        // INDEX
        private int LastClickedIndex;

        public int GetLastClickedIndex() 
        {
            return LastClickedIndex;
        }

        public void SetLastClickedIndex(int LastClickedIndex) 
        {
            this.LastClickedIndex = LastClickedIndex;
            Debug.Log($"Last Clicked Index is: {LastClickedIndex}");
        }

        // ------------------------------------------------------------------------------------------------------------

        // PRIVATE

        private void Redraw() // ORIGINAL REDRAW FUNCTION
        {
            // Re-creating the inventory from scratch, over each 'InventoryItemPrefab',
            // literally that's it!

            foreach (Transform child in this.transform)
            {
                Destroy(child.gameObject);
            }

            for (int i = 0; i < selectedInventory.GetSize(); i++)
            {
                var itemUI = Instantiate(InventoryItemPrefab, transform);
                itemUI.Setup(selectedInventory, i);
            }

            Debug.Log($"OnEnableRedraw Completed");
        }

        /* private void Redraw() 
        {
            // Instead of destroying gameObjects, return them to the pool
            foreach (Transform child in this.transform) 
            {
                ObjectPool.instance.ReturnGameObjectToPool(child.gameObject);
            }

            // Reuse or create new slots from the object pool
            for (int i = 0; i < selectedInventory.GetSize(); i++) 
            {
                // Get the inventory slot from the pool
                GameObject pooledObject = ObjectPool.instance.GetGameObject(InventoryItemPrefab.gameObject);

                // Ensure the pooled object is the correct type
                if (pooledObject.TryGetComponent<InventorySlotUI>(out var itemUI)) 
                {
                    // Set up the inventory slot UI
                    itemUI.Setup(selectedInventory, i);
                }
            }
        } */

        // PUBLIC

        public bool Setup(GameObject user)
        {
            Debug.Log($"InventoryUI Setup is called from {new System.Diagnostics.StackTrace().GetFrame(1).GetMethod().Name}");

            if (user.TryGetComponent(out selectedInventory))
            {
                // if the object has an inventory (our bank in this case), set the Selected Inventory Up, 
                // Subscribe to the Redraw() changes, and then redraw the inventory (to boot it up for the first time):
                selectedInventory.inventoryUpdated += Redraw; // you ALSO need this inventory subscription
                Title.text = selectedInventory.GetDisplayName();
                Redraw();
                return true;
            }
            return false;
        }

        private void OnEnable()
        {
            EnabledEvent?.Invoke();
        }

        private void OnDisable()
        {
            DisabledEvent?.Invoke();
        }

        public void TransferAllInventory()
        {
            if (selectedInventory != null)
            {
                selectedInventory.TransferAllInventory(GetOtherInventory());
            }
        }

        private Inventory GetOtherInventory()
        {

            if (IsOtherInventory) return Inventory.GetPlayerInventory();

            // return the FIRST other Inventory it finds (i.e: The bank), or null:
            var otherInventoryUI = FindObjectsOfType<InventoryUI>().FirstOrDefault(ui => ui.IsOtherInventory);

            //Check to see if it's not null (should never happen), if it's Active, and if the otherInventory's inventory is valid.
            if (otherInventoryUI != null && otherInventoryUI.gameObject.activeSelf && otherInventoryUI.SelectedInventory != null)
            {
                Inventory otherInventory = otherInventoryUI.SelectedInventory as Inventory;
                return otherInventory;
            }

            return null;

        }

    }

}

(I’m contemplating on if ‘InventoryUI.cs’ or ‘InventorySlotUI.cs’ is the right script for this job tbh)

and here’s what I wrote in ‘ObjectPool.cs’:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

namespace RPG.Core.ObjectPooling
{

public class ObjectPool : MonoBehaviour
{
    public static ObjectPool instance;

    [SerializeField] private GameObject[] prefabs; // prefabs needed to instantiate dictionary and queues
    [SerializeField] private int poolSize = 200;

    public Dictionary<GameObject, Queue<GameObject>> poolDictionary = new Dictionary<GameObject, Queue<GameObject>>();

    private void Awake() 
    {
        if (instance == null) 
        {
            // Make sure there's only one instance per game scene
            instance = this;
        }
        else 
        {
            Destroy(this.gameObject); // Destroy this instance if you found another one
        }

        InitializeDictionaryQueues();
    }

    private void InitializeDictionaryQueues() // Initialize all pools for the prefabs
    {
        foreach (var prefab in prefabs) 
        {
            CreateDictionaryPool(prefab);
        }
    }

    public void ReturnGameObjectToPool(GameObject gameObjectToReturn) // return gameObject to proper pool
    {
        GameObject originalPrefab = gameObjectToReturn.GetComponent<PooledGameObject>().OriginalPrefab;
        gameObjectToReturn.SetActive(false);
        poolDictionary[originalPrefab].Enqueue(gameObjectToReturn);
        gameObjectToReturn.transform.parent = transform;
    }

    public GameObject GetGameObject(GameObject gameObjectToSearch) // find a specific gameObject and activate it. If there's no pool for it, create one. If empty, instantiate one
    {
        if (!poolDictionary.ContainsKey(gameObjectToSearch)) 
        {
            CreateDictionaryPool(gameObjectToSearch);
        }

        if (poolDictionary[gameObjectToSearch].Count == 0) 
        {
            CreateNewGameObject(gameObjectToSearch);
        }

        GameObject gameObjectToGet = poolDictionary[gameObjectToSearch].Dequeue();
        gameObjectToGet.SetActive(true);
        gameObjectToGet.transform.SetParent(null);

        return gameObject;
    }

    private void CreateDictionaryPool(GameObject prefab) // instantiate all prefabs at once right now, so we don't have to create or destroy them later
    {
        poolDictionary[prefab] = new Queue<GameObject>();
        for (int i = 0; i < poolSize; i++) 
        {
            CreateNewGameObject(prefab);
        }
    }

    private void CreateNewGameObject(GameObject prefab) // create a copy of the Prefab
    {
        GameObject newGameObject = Instantiate(prefab, transform);
        newGameObject.AddComponent<PooledGameObject>().OriginalPrefab = prefab; // add original prefab to clones, so we can search the pools with it (clone =/= original)
        newGameObject.gameObject.SetActive(false);
        poolDictionary[prefab].Enqueue(newGameObject); // initialize all pools for the prefabs
    }

    private IEnumerator ReturnToPoolTimer(GameObject gameObject, int time) // Returns object to pool after a specific time (OPTIONAL)
    {
        yield return new WaitForSeconds(time);
        ReturnGameObjectToPool(gameObject);
    }
}

}

(Now I’m head on stuck to making this Object Pool work before moving on, because it’s been bothering me for a while tbh)

Sorry to disappoint but I’m in my holidays now so I won’t have acces to my code for around 3 weeks

1 Like

no worries man, I’m giving it a try as we speak :smiley: (you providing me somewhere to start solving this problem with is a lot by itself, and I can’t thank you enough) - enjoy your holidays!

As it turns out, Object Pooling is not really necessary for this. I’ll find a way to catch the departure and arrival inventory slots, and only refresh these in ‘InventoryUI.cs’ when there’s an object transfer.

The normal ‘Redraw()’ that destroys and re-instantiates everything shall be done only once when the game starts, and that’s it

I recall a student did do this a ways back. What I’m not sure of is how well that will work for the Pickup side of things. Actually, if you restrict pickups to only pickup, not drop in containers, then it might… the InventorySlotUI would want to subscribe to it’s Inventory.inventoryUpdated method.

Ehh let’s keep trying with the bank first and foremost. This thing has a total of over 300 slots and this will be quite essential when it comes to scaling down the line. I’ll get to pickups when I’m done with this, otherwise guess who will lose motivation to fix a bad problem? :stuck_out_tongue:

For the moment, this is what I wrote as an alternative function for ‘Redraw’, but I’m still trying to figure out how to get the data for this:

        private void SlotRedraw(InventorySlotUI departureSlot, InventorySlotUI arrivalSlot) 
        {
            Destroy(departureSlot);
            Destroy(arrivalSlot);

            var departureSlotItemUI = Instantiate(InventoryItemPrefab, transform);
            var arrivalSlotItemUI = Instantiate(InventoryItemPrefab, transform);

            departureSlotItemUI.Setup(selectedInventory, departureSlot.GetSlotIndex());
            arrivalSlotItemUI.Setup(selectedInventory, departureSlot.GetSlotIndex());
        }

which has to deal with ‘AddToFirstEmptySlot()’, and setting the clicked UI Slot

Hehe, I’m still working on the object pooling, not using Johnny’s, but using Unity’s new ObjectPool class. I think I know where it went wrong, but I spent the last week trying to figure out why the Player’s Inventory wasn’t even getting CALLED by the InventoryUI (because some idiot forgot to click on the IsPlayerInventory checkbox when I implemented the bank setup)

I asked around a bit about object pooling on the Unity forums about an hour ago. Most people told me it’s pointless for this scenario, and since I’m not a professional with design patterns just yet, I started investigating alternative solutions, but by all means whatever works works I guess :slight_smile: - as long as I stop getting those crazy high spikes in performance then we should be fine, xD

That um… won’t be faster… especially since you’re only destroying the scripts, not the GameObjects.

(I didn’t even test it yet, that was a rough draft just to try get the ball rolling :sweat_smile:)

whenever you have a chance, because I can’t find where we wrote last week’s object pooling, please share it with me again. Maybe I can find something as well (unlikely, but it won’t hurt to try :slight_smile:)

(I’m genuinely curious if I should be waiting for something tbh)

@Brian_Trotter I don’t know if this can be helpful or not, but I asked around a good friend of mine for guidance with the inventory object pooling, and here’s what he said (ignore the notifications part, this is mainly for me):


"So two things:

Notifications - I think I found the issue with it, and I think it relates onto to left sided notifications. There is an issue with the pivots on smaller resolutions which I’m looking to fix in next update.

Performance - the system itself is actually fairly small. I make heavy use of C# and Lists with it’s built in functions and try to use those rather than code specific functions say for moving things around.
To keep performance light I would aim for using the built-in functions as much as possible and try to avoid over complicating your code. Also make use for Linq expressions where possible."


and then I asked him a few more questions, and that’s what he mentioned:


So again, I’d probably focus on the code element here. If you are refreshing that many slots, you probably need to break it down as to why you are doing that and look at an alternate solution.

Object Pooling wont help you here unfortunately as its only really used for instantiated objects, or objects to be instantiated - it’s not meant for a storage/inventory system.

I would personally look at an alternate way to refresh the slots. So for example, instead of refreshing everything when swapping an item maybe just refresh the two slots that are swapping.
Or maybe if you are adding an item, find the slot that you are adding it to (perhaps at end of the list) then only refresh that one, Or if you are adding onto a stack, just refresh that one stack slot.


If you read his last paragraph, this is personally my favourite approach. Only delete and refresh the slots that will have an exchange, you don’t need to do that for every single slot, which means I will have to clone ‘Redraw()’ to something that only works with the slots that are being affected. Let’s call it ‘IndividualSlotRedraw()’


Anyway, here’s my attempt at only refreshing the Inventory Slot which transfers the items, although there are a few major issues:

  1. InventorySlotUI and InventoryUI are now in a circular dependency. Not good!
  2. It’s not working, and I’m hoping we can get this approach to work, because it will be the best one by far (@Brian_Trotter whenever possible, please help)

So here’s what I did:

  1. in ‘Inventory.cs’, I added a little bit of new code:
        private int firstPotentialSlot; // TEST (New Potential Target Slot for the item transfer, to be refreshed)

        public int GetFirstPotentialSlot() // TEST FUNCTION
        {
            return firstPotentialSlot;
        }

        public void SetFirstPotentialSlot(int firstPotentialSlot) // TEST FUNCTION
        {
            this.firstPotentialSlot = firstPotentialSlot;
        }

// in 'AddToFirstEmptySlot()', right before 'return true':

SetFirstPotentialSlot(i); // TEST

in ‘InventoryUI.cs’, I tried creating a little bit of new code:

        // ----------------------------------- TEST: INFORMATION TO ONLY GET SLOTS WHERE TRANSFER IS HAPPENING --------------------------------------

        private int sourceSlotIndex = -1;
        private int destinationSlotIndex = -1;

        public void SetSourceSlotIndex(int index) 
        {
            sourceSlotIndex = index;
        }

        public void SetDestinationSlotIndex(int index) 
        {
            destinationSlotIndex = index;
        }

        private void IndividualSlotRedraw() 
        {
            Debug.Log($"Individual Slot Redraw has been called");
            foreach (Transform child in transform) 
            {
                if (child.GetComponent<InventorySlotUI>().GetSlotIndex() == sourceSlotIndex || child.GetComponent<InventorySlotUI>().GetSlotIndex() == destinationSlotIndex) 
                {
                    Destroy(child.gameObject);
                    Debug.Log($"Child slot, of index {child.GetComponent<InventorySlotUI>().GetSlotIndex()}, has been destroyed");
                }
            }

            for (int i = 0; i < selectedInventory.GetSize(); i++) 
            {
                if (i == sourceSlotIndex || i == destinationSlotIndex) 
                {
                    var itemUI = Instantiate(InventoryItemPrefab, transform); // Create the Inventory Slot UI
                    itemUI.Setup(selectedInventory, i); // Put the item that was there, back in there
                    Debug.Log($"itemUI has been setup at slot {i}");
                }
            }
        }

        // ------------------------------------------------------------------------------------------------------------------------------------------

// all 'inventoryUpdated' event subscriptions now use 'IndividualSlotRedraw' instead of 'Redraw', which only happens now when the game or the Inventory opens/ starts up

The code above essentially is an attempt of rewriting ‘Redraw()’ but without having to redraw every single inventory slot. Instead, only Redraw the Individually affected slots, which is the final goal of this entire thing

  1. In ‘InventorySlotUI.TransferToOtherInventory’, I added two new lines of code:
                GetComponentInParent<InventoryUI>().SetSourceSlotIndex(this.index);
                otherInventory.GetComponent<InventoryUI>().SetDestinationSlotIndex(otherInventory.GetFirstPotentialSlot());

I assume you don’t have that function, so here’s what my full ‘TransferToOtherInventory’ looks like:

        // Transferring stuff from the Inventory to our Bank:
        private void TransferToOtherInventory(Inventory otherInventory, InventoryItem item, int number)
        {
            Debug.Log($"TransferToOtherInventory is called from {new System.Diagnostics.StackTrace().GetFrame(1).GetMethod().Name}");

            if (otherInventory.HasSpaceFor(item))
            {
                otherInventory.AddToFirstEmptySlot(inventory.GetItemInSlot(index), number, true);
                inventory.RemoveItem(item, number);
                // Setup(inventory, index); // <-- if you keep this, the new Inventory-based Pickup System will complain (because these inventory slots vanish)

// and here comes the tight couple that we need to get rid of...
                GetComponentInParent<InventoryUI>().SetSourceSlotIndex(this.index);
                otherInventory.GetComponent<InventoryUI>().SetDestinationSlotIndex(otherInventory.GetFirstPotentialSlot());
            }
        }

Apart from the fact that it doesn’t work, because this line is giving me NREs:

                otherInventory.GetComponent<InventoryUI>().SetDestinationSlotIndex(otherInventory.GetFirstPotentialSlot());

Now, ‘IndividualSlotRedraw’ will be called when the inventory updates are invoked, and ‘Redraw’ will be called only at the start, or when the inventory is first opened

I have no idea how to even get the proper ‘InventoryUI’ for this case, hence the NRE (I know it’s the problem, I just don’t know how to solve it)

So what’s my best way around this topic?

Actually, your friend is sort of on the right track, but it doesn’t have to be this complex…

Create a new event in Inventory

public event System.Action<int> OnIndividualSlotChanged;

Each InventorySlotUI subscribes to this IndividualSlotChanged event.

All transactions in Inventory are on single slots. And I do mean all transactions. Within Inventory, there is no Swap(a, b), and that’s a good thing. Yes, Drag has a swap function, but ultimately, it relies on individual functions within Inventory. So when InventoryUpdated?.Invoke() is called, we can call

IndividualSlotChanged?.Invoke(slot);

with the slot being whatever slot this method changed.

Then the InventorySlotUI subscribes to the IndividualSlotChanged event, and it’s handler is simply

private void Inventory_IndividualSlotChanged(int changedSlot)
{
    if(slot==changedSlot) Refresh();
}

And the refresh can update the icon, etc.

I believe I found the issue with my original object pooling, and it was the dumbest thing… I was simply deactivating the gameObjects, but they were still children of the transform… they need an arbitrary GameObject under the Canvas (so it has a RectTransform) to park the components.

Here’s my modified InventoryUI script:

using UnityEngine;
using GameDevTV.Inventories;
using TMPro;
using UnityEngine.Pool;

namespace GameDevTV.UI.Inventories
{
    /// <summary>
    /// To be placed on the root of the inventory UI. Handles spawning all the
    /// inventory slot prefabs.
    /// </summary>
    public class InventoryUI : MonoBehaviour
    {
        // CONFIG DATA
        [SerializeField] InventorySlotUI InventoryItemPrefab = null;
        [SerializeField] bool isPlayerInventory = true;
        [SerializeField] TextMeshProUGUI Title;

        [SerializeField] private Transform physicalPool;
        // CACHE
        IInventorySource selectedInventory;

        private IObjectPool<InventorySlotUI> slotPool;
        
        
        // LIFECYCLE METHODS

        private void Awake() 
        {
            if (isPlayerInventory)
            {
                selectedInventory = Inventory.GetPlayerInventory();
                selectedInventory.inventoryUpdated += Redraw; 
            }

            slotPool = new LinkedPool<InventorySlotUI>(Pool_Create, Pool_Get, Pool_Release, Pool_Destroy);
        }

        private InventorySlotUI Pool_Create()
        {
            return Instantiate(InventoryItemPrefab, transform);
        }

        private void Pool_Get(InventorySlotUI obj)
        {
            obj.gameObject.SetActive(true);
        }

        private void Pool_Release(InventorySlotUI obj)
        {
            obj.transform.SetParent(physicalPool);
            obj.gameObject.SetActive(false);
        }

        private void Pool_Destroy(InventorySlotUI obj)
        {
            Destroy(obj.gameObject);
        }


        private void Start()
        {
            foreach (Transform child in transform)
            {
                Destroy(child);
            }
            if(isPlayerInventory)
            {
                Redraw();
            }
        }

        // PRIVATE

        private void OnEnable()
        {
            Redraw();
        }

        private void Redraw()
        {
            if (selectedInventory==null) return;
            foreach (Transform child in transform)
            {
                slotPool.Release(child.GetComponent<InventorySlotUI>());
            }

            for (int i = 0; i < selectedInventory.GetSize(); i++)
            {

                var itemUI = slotPool.Get();
                itemUI.transform.SetParent(transform);
                itemUI.Setup(selectedInventory, i);
            }
        }
        
        public bool Setup(GameObject user)
        {
            if (user.TryGetComponent(out selectedInventory))
            {
                selectedInventory.inventoryUpdated += Redraw;
                Title.text = selectedInventory.GetDisplayName(); 
                Redraw();
                return true;
            }
            return false;
        }
    }
}

The physicalPool should be connected to a GameObject that is under the Canvas in the heirarchy, and it should be disabled. I actually pointed both the Bank and the Inventory to the same physicalPool.

That’s about it. It’s working for both a standard “bank” and for pickups, at least on my end.

I have a few questions:

  1. Is this done in ‘Awake’ and released on ‘OnDestroy’, or ‘OnEnable’ and ‘OnDisable’, or…?!
  1. There’s like 6 or 7 calls for this event in ‘Inventory.cs’. Do I do that for all positions where it exists? More importantly, is this an addon or a replacement?
  1. where’d “Refresh” come from? Are you talking about “Setup”, in ‘InventorySlotUI.cs’? The one below?:
        public void Setup(IInventorySource inventory, int index)
        {
            this.inventory = inventory;
            this.index = index;
            icon.SetItem(inventory.GetItemInSlot(index), inventory.GetNumberInSlot(index));
        }
  1. How do you integrate the slot event call in this scenario?:
/// <summary>
        /// Attempt to add the items to the first available slot.
        /// </summary>
        /// <param name="item">The item to add.</param>
        /// <param name="number">The number to add.</param>
        /// <returns>Whether or not the item could be added.</returns>
        public bool AddToFirstEmptySlot(InventoryItem item, int number, bool refresh = true)
        {

            // If we picked up money, automatically send it to the pouch:
            foreach (var store in GetComponents<IItemStore>()) 
            {
                number -= store.AddItems(item, number);
            }

            if (number <= 0) return true;

            // ----------------------------------------------------------

            int i = FindSlot(item);

            if (i < 0)
            {
                return false;
            }

            slots[i].item = item;
            slots[i].number += number;
            if (refresh) 
            {
                inventoryUpdated?.Invoke();
                OnIndividualSlotChanged?.Invoke(slot);
            }
            return true;
        }
  1. Same question here. How do I integrate the slot in this scenario?:
public void RemoveItem(InventoryItem item, int number, bool refresh = true) 
{

    if (item == null) return;

    for (int i = 0; i < slots.Length; i++) {

        if (object.ReferenceEquals(slots[i].item, item)) {

            slots[i].number -= number;
            if (slots[i].number <= 0) 
            {

                slots[i].item = null;
                slots[i].number = 0;

            }

            if (refresh) inventoryUpdated?.Invoke();

            return;

        }

    }

}
  1. You can take a wild guess (hint: same as 4 and 5 above) what my question will be here:
public void TransferAllInventory(Inventory otherInventory) {

    if (otherInventory == null || otherInventory == this) return;

    for (int i = 0; i < inventorySize; i++) {

        if (slots[i].item == null) continue;    // the sole responsible line for fixing my long banking times, which ensures empty inventory slots are not refreshed
        for (int j = slots[i].number; j > 0; j--) {

            if (otherInventory.HasSpaceFor(slots[i].item)) {

                otherInventory.AddToFirstEmptySlot(slots[i].item, 1, false);
                slots[i].number--;
                if (slots[i].number <= 0) slots[i].item = null;

            }

            else break;

        }

    }

    inventoryUpdated?.Invoke();
    otherInventory.inventoryUpdated?.Invoke();

}

// in 'TriggerUpdated':

        public void TriggerUpdated()
        {
            inventoryUpdated?.Invoke();
        }

  1. Safe to assume inventory updates don’t happen when the RestoreFromJToken is happening, right?

  2. I’m not fully sure of the context of this function:

Is it like this?:

private void Inventory_IndividualSlotChanged(int slotChanged) 
        {
            if (index == slotChanged) 
            {
                Setup(inventory, slotChanged);
            }
        }

Other than that, I’ll carefully integrate and give the ‘InventoryUI.cs’ a test run, and update you on the results

The results are honestly kinda wild (in a bad way :sweat_smile:)… you may want to have a look at this:

https://drive.google.com/drive/folders/1_5sewu2y16by2fgMwoQCBUVX-HAzJhiN?usp=sharing

It’s always something to do with ‘InventoryUI.Redraw’ that does the damage for some reason…:

        // PRIVATE

        private void Redraw() // ORIGINAL REDRAW FUNCTION
        {
            // Re-creating the inventory from scratch, over each 'InventoryItemPrefab',
            // literally that's it! (WILL ONLY BE DONE ONCE WHEN THE GAME STARTS)

            // TEST
            if (selectedInventory == null) return;

            foreach (Transform child in this.transform)
            {
                // Destroy(child.gameObject); // OBJECT POOL TEST BELOW

                // TEST
                slotPool.Release(child.GetComponent<InventorySlotUI>());
            }

            for (int i = 0; i < selectedInventory.GetSize(); i++)
            {
                // var itemUI = Instantiate(InventoryItemPrefab, transform); // OBJECT POOL TEST BELOW
                // itemUI.Setup(selectedInventory, i);

                // TEST
                var itemUI = slotPool.Get();
                itemUI.transform.SetParent(transform);
                itemUI.Setup(selectedInventory, i);
            }
        }

and there’s a ton of errors:

  1. an NRE for each slot when I try destroying it with the new event:
        private void Awake()
        {
            equipment = GameObject.FindWithTag("Player").GetComponent<Equipment>();
            inventory.GetGameObject().GetComponent<Inventory>().OnIndividualSlotChanged += Inventory_IndividualSlotChanged;
        }

(The error has to do with the event subscription)

  1. This no longer works:
        public InventoryItem GetItem()
        {
            return inventory.GetItemInSlot(index);
        }

This was previously working, I don’t know what recently happened

  1. This line releases an NRE too, from ‘TryHandleRightClick()’ (the Click-to-equip function, or Click-to-whatever-should-be-done):
            InventoryItem item = inventory.GetItemInSlot(index);

This was previously working as well, I don’t know what recently happened either :sweat_smile:

Awake() OnDestroy(), assuming using individual rather than pooling.

Yes, if it’s updated, it’s called.

Since the setup is already done… in other words, since the InventorySlotUI already knows it’s Inventory and Slot, what needs to be refreshed is icon.SetItem() (to update the display). I would spin that off as it’s own "Refresh() method.

Just like that
Each if(refresh) block will become:

if(refresh)
{
    inventoryUpdated?.Invoke(); //at this point, that may become entirely optional)
    OnIndividaulSlotChanged?.Invoke(slot);
}

That works, or you could have a Refresh() method that was simply the last line of Setup()

I encountered a problem with this section of code until I added code in Start() to destroy the existing child oblects.

Other than that, I’m not sure what’s going on in there. Might be Individual refreshes are the way to go

OK so I did a few changes to match with your recommendations. I don’t know how to explain this, but most of the errors are coming as an NRE from this whenever I try pick stuff up or what not:

        // TEST
        private void OnEnable() 
        {
            inventory.GetGameObject().GetComponent<Inventory>().OnIndividualSlotChanged += Inventory_IndividualSlotChanged;
        }

        // TEST
        private void OnDisable() 
        {
            inventory.GetGameObject().GetComponent<Inventory>().OnIndividualSlotChanged -= Inventory_IndividualSlotChanged;
        }

and here are some changes in ‘InventorySlotUI.cs’:

        // TEST
        private void Inventory_IndividualSlotChanged(int slotChanged) 
        {
            if (index == slotChanged) 
            {
                Refresh(inventory, slotChanged);
            }
        }

        // TEST
        public void Refresh(IInventorySource inventory, int index) 
        {
            icon.SetItem(inventory.GetItemInSlot(index), inventory.GetNumberInSlot(index));
        }

Destroying the child object directly results in another error, but destroying the child gameObject gives no errors, but the slots act in a structured, but will look random to the untrained eye, weird kinda way…

Can we at least fix the ‘OnEnable’ and ‘OnDisable’ call ? MAYBE, JUST MAYBE, they’re why the inventory acts so weird

(That error happens in ‘OnEnable’ and ‘OnDisable’ because the inventory never existed fast enough for it to begin with…)

Privacy & Terms