A better Pickup System for the Third Person Variant of the RPG Course

OK so… One of my biggest complaints since merging my RPG and the Third Person, following Brian’s excellent tutorial, is the pickups

Now, whilst for small numbers of items, the pickup system works seamlessly, I have a short time span to collect items in my game (to make things exciting), and this means that players must be able to pick stuff up really quickly, and choosing what you pick up is really not an option with the current approach

So I did a bit of brainstorming, and decided that similar to this tutorial (the one regarding making a bank), instead of playing an animation and picking stuff up (which works perfectly by the way, but is limited by nature), why don’t we just open a secondary inventory UI, and display all the stuff within the radius of the player, and allow him to select what to transfer to his inventory? And each item can vanish after a timer based on the ‘Destroy After’ timer on the pickups themselves from that inventory UI

I personally think this will not only be significantly faster and more user-friendly with the players, but also give him the choice of what he wants to pick up much quicker than if he has to keep trying until he eventually gets what he wants, which I personally find VERY frustrating (and if the timer to pick your stuff up runs out, and you didn’t catch what you value the most, I won’t be surprised if you destroy your keyboard as a result)

Now that I’ve convinced you that I got brilliant ideas, what’s the best way to go around this? (I’ll update you all as I go anyway :smiley:)

And… I’m stuck. Quite badly tbh:

After finally getting the title to show up (check step 5 below. You’ll see the debugs to see what I went through to try and get that title right… it was a disaster :sweat_smile:), I got 2 problems now (and one of them has been going on for a while, it’s hard to ignore now). I’ll name them “a” and “b”

a. (This is the bad one) - unless I save and quit my game, the visible UI will be mixed between my bank, and the pickup UI I’m trying to create. The information is correct, but the bank will, for some reason, try to open up my ‘PickupInventory’ UI instead of my ‘Bank’ UI, ever since I introduced this system today (unless I save and quit, and then return to the game, that is).

I have no idea why any of this is happening, but it’s also why my shop sometimes needs to be re-opened to work, why my money won’t always show the accurate value, why my quiver won’t show the arrows unless 50% of the time, etc… I’ll return to fixing this bug as soon as this topic is over. It has to be addressed!

b. The UI for the Pickup Inventory I’m trying to setup won’t properly refresh for some reason, and that’s what this entire comment will be all about. It’s about the steps I took so far to reach where I currently am, and to help me brainstorm this problem and get it fixed :slight_smile:

Let’s start by addressing the second problem first, to stay on topic. Here’s a rundown of exactly what I did (mind you, you need to follow @Brian_Trotter 's merge course, along with his Inventory Containers from his tips and tricks, to understand what’s going on here)

  1. In ‘PlayerFreeLookState.cs’, I went ahead and updated my Pickup to open the UI properly:
        private void InputReader_HandlePickupEvent()
        {
            // if (stateMachine.PickupFinder.GetNearestPickup() != null)
            if (stateMachine.PickupFinder.FindNearestTarget())
            {
                if (AnimalMountManager.isOnAnimalMount)
                {
                    // No pickup handling from this script when the player is mounting the animal is allowed.
                    // It will be handled elsewhere, or maybe even not handled at all
                    return;
                }

                // stateMachine.SwitchState(new PlayerPickupState(stateMachine, stateMachine.PickupFinder.CurrentTarget));

                // I'm replacing the current Pickup System above, with a brand new Inventory-based Pickup System.
                // Player-wise, this will be significantly easier to deal with, and faster for the players to use as well, 
                // so it's a win-win situation (Check below)

                // There is only one component in the entire game that's allowed to have a tag of 'PlayerPickupInventory', so this approach is pretty safe
                stateMachine.SwitchState(new PlayerPickupInventoryState(stateMachine, GameObject.FindGameObjectWithTag("PlayerPickupInventory").GetComponent<OtherInventory>()));
            }
        }
  1. I created the required state, and called it ‘PlayerPickupInventoryState.cs’, a solution to open up the Pickup UI that I created:
using System.Collections;
using System.Collections.Generic;
using GameDevTV.Inventories;
using RPG.States.Player;
using UnityEngine;

public class PlayerPickupInventoryState : PlayerBaseState
{
    private OtherInventory pickupInventory;

    public PlayerPickupInventoryState(PlayerStateMachine stateMachine, OtherInventory pickupInventory) : base(stateMachine)
    {
        this.pickupInventory = pickupInventory;
    }

    public override void Enter()
    {
        Debug.Log($"Player entered 'PlayerPickupInventoryState'");
        pickupInventory.OpenPickupInventory(stateMachine);
    }

    public override void Tick(float deltaTime) {}

    public override void Exit()
    {
        Debug.Log($"Player exited 'PlayerPickupInventoryState");
    }
}

as you can see, it’s quite simple. Opening it is handled here, and closing it will be handled at the ‘Quit button’ in my game inspector

  1. in ‘OtherInventory.cs’ (It’s an old creation of mine, to compensate for having a bank in my game. Again, follow this tutorial to get the idea), I created this function, specifically to open up the bank:
        public void OpenPickupInventory(PlayerStateMachine player) 
        {
            player.GetComponent<PickupAction>().StartPickupAction
            (
                () => 
                {
                    // Unlike the banking action, you don't need a transform here. You just need to open the inventory, and that's it! 
                    // (whether there's a Pickup Item or not is handled in 'PlayerFreeLookState.cs', where we originally access this part of the code from)
                    pickupInventoryThirdPersonShowHideUI.ShowPickupInventory(GameObject.FindWithTag("PlayerPickupInventory").GetComponent<OtherInventory>().gameObject);
                }
            );
        }

As you can tell, this is attached to something called ‘PickupInventory’ (check the before-the-last image of this comment to see what I’m talking about), with a script called ‘ThirdPersonShowHideUI’ attached to it, that has a connection to a ‘PlayerPickupInventory’-tagged GameObject on the hierarchy, which… has an ‘OtherInventory’ on it, so we can tell it how many slots the ‘PickupInventory’ UI should have

  1. Here is the ‘ShowPickupInventory’ from my ‘ThirdPersonShowHideUI.cs’ script:
    public void ShowPickupInventory(GameObject pickupInventory)
    {
        // Displaying the Pickup Inventory, which slightly differs from 'ShowOtherInventory', because this isn't a bank anymore
        UIContainer.SetActive(true);
        otherInventoryContainer.SetActive(true);
        otherInventoryUI.SetupPickupInventory(pickupInventory);
    }

Here, we are setting up the inventory, but also where my problem is most likely at, unless I missed out on something. Moving on to step 5:

  1. [THIS STEP IS A COMPLETE DISASTER. I SUGGEST YOU HOLD BACK ON IT FOR NOW] ‘OtherInventoryUI’ is essentially the script called ‘InventoryUI.cs’, and here is where I try to setup the UI appropriately, using a ‘SetupPickupInventory()’ function:
        public bool SetupPickupInventory(GameObject pickupInventoryUser)
        {
            if (pickupInventoryUser.TryGetComponent(out selectedInventory)) // Confirms that it is indeed the player calling this UI
            {
                selectedInventory.inventoryUpdated += PickupRedraw; // Refresh the Player Inventory
                
                if (PickupTitle != null) 
                {
                    var textMeshproComponent = PickupTitle.GetComponent<TextMeshProUGUI>();
                    if (textMeshproComponent != null) 
                    {
                        PickupTitle.text = textMeshproComponent.text;
                    }
                    else 
                    {
                        Debug.Log($"TextMeshProUGUI component is missing on PickupTitle GameObject");
                    }
                }
                else 
                {
                    Debug.Log($"PickupTitle is not assigned in the inspector");
                }

                Redraw();
                return true;
            }
            
            else return false;
        }

and a ‘PickupRedraw()’ function:

        private void PickupRedraw() 
        {
            // Redrawing the Inventory from scratch, when the pickup happens
            foreach (Transform child in this.transform) 
            {
                Destroy(child.gameObject);
            }

            // Find items within the Pickup Radius
            Collider[] hitColliders = Physics.OverlapSphere(selectedInventory.transform.position, pickupRadius);
            for (int i = 0; i < hitColliders.Length; i++) 
            {
                var pickupItem = hitColliders[i].GetComponent<Pickup>();
                if (pickupItem != null) 
                {
                    var itemUI = Instantiate(InventoryItemPrefab, transform);
                    itemUI.Setup(selectedInventory, i);
                }
            }
        }

However, I’m not extremely familiar with what exactly is going on in step 5, hence why I’m here seeking help

  1. I also created this script, called ‘PickupAction.cs’, so that the ‘IAction.cs’ Interface can use it, and actually get the player to open up the UI, as follows:
using System.Collections;
using System.Collections.Generic;
using RPG.Core;
using UnityEngine;

namespace RPG.Pickups {

public class PickupAction : MonoBehaviour, IAction
{
    System.Action callback;
    bool isPickingUp;

    public void StartPickupAction(System.Action callback) 
    {
        this.callback = callback;
        GetComponent<ActionSchedular>().StartAction(this);
        isPickingUp = true;
    }

    public void Update()
    {
        if (isPickingUp)
        {
            callback?.Invoke(); // This is how you open up the Pickup Inventory
            Debug.Log($"Player Pickup Inventory opened through update");
            isPickingUp = false;
            return; // You don't want to forget this line, unless you want your computer to explode...! (THIS IS AN UPDATE YOU SICK BASTARD!)
        }
        else return; // Explosion-proof code
    }

    public void Cancel() 
    {
        ThirdPersonShowHideUI.CloseAllWindows();
        isPickingUp = false;
    }
}

}
  1. If I forgot to mention this earlier, the closing of that UI is handled through the Quit button, so we don’t need to worry about that for now (if you need it, just ask)

As for the Unity Inspector itself, I created these to help me out:

7a. Place ‘PickupAction.cs’, the script from step 6, on my player

7b. Created this ‘PickupInventory’ (a copy of my functional bank right above it), which is what the Pickup Inventory UI should open up to, and set it up as follows:

7c. I also created this ‘PlayerPickupInventory’, which holds my ‘OtherInventory’ (and no saving system on this one. I’m not saving the pickups if they can be accumulated off the ground. Let it be as is), so the UI in step 7b knows how many slots there are, and how to work with it:

I think that’s about all the modifications done. I tried not to do anything crazy to my original code, out of fear of mistakes

But my question now is, regarding problem ‘b’ at the top of this specific comment, why can’t I get the UI to place the items around the player, when he pressed the ‘P’ key, to show up in the UI Slots?

The goal is to get them to show up in the form of items in an inventory, where you can only press to take them from that inventory to yours, and no way around, and then destroy them from the game world when they’re taken

I’m sorry if that sounds like a crazy concept, or it’s too much to take in, but it’s essential to make my game pickup system easier :slight_smile:

The goal is to replace the current pickup system, mainly to deal with a few frustrations that I personally have with it

I have a feeling that something in the logic is flawed somewhere, and more likely than not, I did something (or things) wrong, and I need a set of fresh eyes to help me. Please help me out, I’m not extremely familiar with UI-related coding or work in general (just trying to use what I have so far) :sweat_smile:

and… when I return to the original system after all these crazy changes are done, there’s a duplication bug :slight_smile: (I did something terribly wrong I suppose)

A little bit of an extra update, the next day:

Get rid of Step 5 above, this one will cause unwanted bugs and will be an absolute disaster. Everything else mentioned above is correct, but with a few addons:

  1. The first thing you want to do, is go to ‘OtherInventory.cs’ and introduce a boolean called ‘IsPickupInventory’, as follows:
        public bool isPickupInventory;
        public bool IsPickupInventory => isPickupInventory;
  1. next, you will notice that for this kind of inventory, you will need some special things. For example, you can’t have the items stored in there like a bank. When this inventory is closed, everything has to go as well, otherwise you’ll have an item duplication bug

For that, I created this special function:

        public void ClosePickupInventory(Inventory pickupInventory)
        {
            // This inventory is a special case, because when you close it, you want to destroy all of its contents as well
            // If you fail to do that, you will have a duplication bug, and this will just make your life miserable
            for (int i = 0; i < pickupInventory.GetSize(); i++)
            {
                pickupInventory.RemoveFromSlot(i, pickupInventory.GetNumberInSlot(i), false);
            }

            // Once we have destroyed everything in the Pickup Inventory, now we can safely
            // shut it down like any other Inventory
            ThirdPersonShowHideUI.CloseAllWindows();
            PlayerStateMachine player = GameObject.FindGameObjectWithTag("Player").GetComponent<PlayerStateMachine>();
            player.SwitchState(player.Targeter.CurrentTarget ? new PlayerTargetingState(player) : new PlayerFreeLookState(player));
        }

This goes on the ‘Quit Button’ of your PickupInventory on your game hierarchy

(Pickups, is the storage field, which we introduced above. I’ll also leave a screenshot below if you need it):

  1. The most important part, which you’ve been waiting for, is a solution to show your inventory contents. In ‘ThirdPersonShowHideUI.cs’, we’ll use the boolean we introduced in step 1 to classify the Pickup Inventory from the rest, and update our ‘ShowOtherInventory’ to give specific operations for the pickup inventory, which classify it from the bank:
    public void ShowOtherInventory(GameObject otherInventory)
    {
        if (otherInventory != null && otherInventory.GetComponent<OtherInventory>().IsPickupInventory) // Pickups Inventory
        {
            // For the 'PickupInventory' system
            Debug.Log($"Pickup Inventory Unlocked");

            // ACTIVATION
            UIContainer.SetActive(true); // activate the player inventory
            otherInventoryContainer.SetActive(true); // activate the player bank

            // TRANSFER THE NEARBY PICKUPS TO THE PICKUP INVENTORY, BEFORE SETTING IT UP
            TransferNearbyPickupsToPickupInventory(otherInventory.GetComponent<Inventory>());

            // Set up the other inventory UI for the pickup Inventory
            otherInventoryUI.Setup(otherInventory);
        }

        else { // Bank, and other types of Inventories (there's only a 'Pickup' and a 'Bank' inventory only in the game anyway)
        Debug.Log($"ThirdPersonShowHideUI called from {new System.Diagnostics.StackTrace().GetFrame(1).GetMethod().Name}");
        // Displaying the bank, when this function is called (mainly through a 'System.Action Callback', in 'OtherInventory.cs'):
        UIContainer.SetActive(true);
        otherInventoryContainer.SetActive(true);
        otherInventoryUI.Setup(otherInventory);
        }
    }

and of course, you’ll need a ‘TransferNearbyPickupsToPickupInventory’:

    public void TransferNearbyPickupsToPickupInventory(Inventory otherInventory)
    {
        Collider[] hitColliders = Physics.OverlapSphere(player.transform.position, pickupRadius);
        foreach (Collider hitCollider in hitColliders)
        {
            var pickup = hitCollider.GetComponent<Pickup>();
            if (pickup != null)
            {
                var item = pickup.GetItem();
                var number = pickup.GetNumber();

                if (otherInventory.HasSpaceFor(item))
                {
                    otherInventory.AddToFirstEmptySlot(item, number);
                }
            }
        }
    }

Now, you’re set to actually have nearby pickups show up around your player

The next step, which I’m still working on, are three solutions:

  1. [SOLVED] Find a way to stop the player from placing pickups back in the pickup inventory (Which means it’s a one-way transfer, otherwise say hello to Unwanted bugs :slight_smile:)

Edit: For the first problem, I found exactly where the transfer between inventories is happening, and then I placed this line of code:

                // Block any transfers back from the Player's Inventory to the Pickup Inventory 
                // (otherwise you'll have a severe bug with in-game pickups). 
                // Once it's taken, it can be destroyed from the game world:
                if (otherInventoryUI.GetComponentInParent<ThirdPersonShowHideUI>().CompareTag("PickupInventory")) return;

For my case, this was in ‘InventorySlotUI.TryHandleRightClick()’. Remember, the parent of your slots must be tagged’ PickupTarget’ for this to work. I’ll get to the drag and drop equivalent down the line

For simplicity’s sake, this is what my code looks like right now, after introducing the new line:

var otherInventoryUI = FindObjectsOfType<InventoryUI>().FirstOrDefault(ui => ui.IsOtherInventory);  // returns other inventory, or null

            if (otherInventoryUI != null && otherInventoryUI.gameObject.activeSelf && otherInventoryUI.SelectedInventory != null) {

                // Block any transfers back from the Player's Inventory to the Pickup Inventory 
                // (otherwise you'll have a severe bug with in-game pickups). 
                // Once it's taken, it can be destroyed from the game world:
                if (otherInventoryUI.GetComponentInParent<ThirdPersonShowHideUI>().CompareTag("PickupInventory")) return;

                Inventory otherInventory = otherInventoryUI.SelectedInventory;
                TransferToOtherInventory(otherInventory, item, 1);
                return;

            }

Before I move on to the next problem, I have to make sure Drag and Drop has the same constraint. I’ll go figure it out now :slight_smile:

[EDIT 2: I MADE A MISTAKE IN ‘OtherInventory.cs’, WHERE I WAS GETTING THE WRONG ‘thirdPersonShowHideUI’ VARIABLE IN AWAKE, AND THIS WAS MAKING MY FIRST SOLUTION A NIGHTMARE. LONG STORY SHORT, MAKE SURE YOUR VARIABLES ARE FOUND THROUGH A TAG SEARCH AS FOLLOWS:

            bankInventoryThirdPersonShowHideUI = GameObject.FindGameObjectWithTag("BankInventory").GetComponent<ThirdPersonShowHideUI>();

IF YOU USE A ‘FirstOrDefault’ LINQ Approach, you’ll have a huge headache, because this solution won’t work then

  1. Destroy that pickup in the game world once it’s been consumed by the player. This one will be quite complex, because I’ll need a man in the middle list to destroy them when the X button is clicked

Not to mention for stackable items, you’ll need to check for the quantity left, and only destroy it in the game world if the quantity left is zero

  1. [SOLVED] Find a way to make sure that they don’t use the same dimensions for the bank and the pickup, because I find the bank UI size to be a little too large for a pickup inventory. Just play around with the dimensions, and you’ll be just fine

I Solved problem 3 when I solved the “Tag Search” Problem I mentioned in Problem 1. Now I’m just working on Problem 2

Once I have any updates about these, I’ll post it here, and this topic will be done

For step 2, here’s what I’m trying to do so far:

  1. Introduce a ‘WorldID’. Similar to how the enemies in our game world have their unique IDs, I added another layer called ‘WorldID’ to help us identify the item we just picked up in the game world, so we can essentially destroy it

This will be done in ‘Pickup.cs’:

        // TEST - 2/7/2024 - USED IN 'ThirdPersonShowHideUI' 
        // TO PRECISELY CLASSIFY IN-GAME WORLD ITEMS APART,
        // ENSURING WE ONLY PICKUP WHAT WE FIND
        public string WorldID;

        // SOLUTION TO ENSURE WE DON'T HAVE OVERLAPPING IDs
        private void OnValidate() 
        {
            if (string.IsNullOrEmpty(WorldID) || JSONSaveableEntity.usedIdentifiers.Contains(WorldID))
            {
                WorldID = Guid.NewGuid().ToString();
            }
        }
  1. in ‘ThirdPersonShowHideUI.cs’, we will create a list of ‘transferredItemIDs’, which will contain the IDs of the items that are nearby the player:
    // UNIQUE WORLD PICKUPS ID LIST (SWITCH TO PRIVATE AFTER TESTING)
    public List<string> transferredPickupIDs = new List<string>();

and when we pickup an item, we will add it to the list of IDs:

    public void TransferNearbyPickupsToPickupInventory(Inventory pickupInventory)
    {
        Collider[] hitColliders = Physics.OverlapSphere(player.transform.position, pickupRadius);
        foreach (Collider hitCollider in hitColliders)
        {
            var pickup = hitCollider.GetComponent<Pickup>();
            if (pickup != null)
            {
                var item = pickup.GetItem();
                var number = pickup.GetNumber();

                if (pickupInventory.HasSpaceFor(item))
                {
                    pickupInventory.AddToFirstEmptySlot(item, number);
                    transferredPickupIDs.Add(pickup.WorldID); // (TEST) GET THE WORLD ID, SO WE CAN DESTROY THAT ITEM IN THE GAME WORLD
                }
            }
        }
    }
  1. And… of course, when we close the Pickup Inventory Down, in ‘OtherInventory.cs’, we want to clean up this list:
        public void ClosePickupInventory(Inventory pickupInventory)
        {
            // This inventory is a special case, because when you close it, you want to destroy all of its contents as well
            // If you fail to do that, you will have a duplication bug, and this will just make your life miserable
            for (int i = 0; i < pickupInventory.GetSize(); i++)
            {
                pickupInventory.RemoveFromSlot(i, pickupInventory.GetNumberInSlot(i), false);

                // CLEAR WORLD IDs OF ALL ITEMS
                bankInventoryThirdPersonShowHideUI.transferredPickupIDs.Clear();
                pickupInventoryThirdPersonShowHideUI.transferredPickupIDs.Clear();
            }

            // Once we have destroyed everything in the Pickup Inventory, now we can safely
            // shut it down like any other Inventory
            ThirdPersonShowHideUI.CloseAllWindows();
            PlayerStateMachine player = GameObject.FindGameObjectWithTag("Player").GetComponent<PlayerStateMachine>();
            player.SwitchState(player.Targeter.CurrentTarget ? new PlayerTargetingState(player) : new PlayerFreeLookState(player));
        }

  1. The next step will be to find a way to actually delete the item from the game world. I’m still trying to figure this one out (if I don’t respond in a few hours, I’m probably asleep. Wait till the morning for that). I also need to consider the dynamic random drops from the NPCs for that thing too, along with the drag and drop system, and a bit of bug fixing… (There is a known bug for me currently, where whilst I do successfully block the transfer from the inventory to the pickup, I accidentally also blocked the transfer from the inventory to the bank somehow)

This is one lengthy system, and I was not expecting any of that at all… :sweat_smile:

This is insane… I’ve read through it twice, and I’m still not sure where the issues lie…

I’m going to simplify things a bit… you already have a repository of all pickups within range of the player in the PickupFinder… So that’s your source…
What we need is to adapt the bank window to use the PickupFinder as the source of the inventory…
This likely means a NEW bank window, dedicated to the PickupFinder, with new versions of the InventoryItemUI that interface with that PickupFinder…
OR…
We could use a little abstraction…

Let’s start with an IInventorySource which takes all public methods associated with an Inventory:

IInventorySource.cs
using System;

namespace GameDevTV.Inventories
{
    public interface IInventorySource
    {
        /// <summary>
        /// Broadcasts when the items in the slots are added/removed.
        /// </summary>
        event Action inventoryUpdated;

        string GetDisplayName(); //Edit:  Added because we'll need it later

        /// <summary>
        /// Could this item fit anywhere in the inventory?
        /// </summary>
        bool HasSpaceFor(InventoryItem item);

        int FreeSlots();

        /// <summary>
        /// How many slots are in the inventory?
        /// </summary>
        int GetSize();

        /// <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>
        bool AddToFirstEmptySlot(InventoryItem item, int number);

        /// <summary>
        /// Is there an instance of the item in the inventory?
        /// </summary>
        bool HasItem(InventoryItem item);

        /// <summary>
        /// Return the item type in the given slot.
        /// </summary>
        InventoryItem GetItemInSlot(int slot);

        /// <summary>
        /// Get the number of items in the given slot.
        /// </summary>
        int GetNumberInSlot(int slot);

        /// <summary>
        /// Remove a number of items from the given slot. Will never remove more
        /// that there are.
        /// </summary>
        void RemoveFromSlot(int slot, int number);

        /// <summary>
        /// Will add an item to the given slot if possible. If there is already
        /// a stack of this type, it will add to the existing stack. Otherwise,
        /// it will be added to the first empty slot.
        /// </summary>
        /// <param name="slot">The slot to attempt to add to.</param>
        /// <param name="item">The item type to add.</param>
        /// <param name="number">The number of items to add.</param>
        /// <returns>True if the item was added anywhere in the inventory.</returns>
        bool AddItemToSlot(int slot, InventoryItem item, int number);
    }
}

These are all the public methods that are used within our InventorySlotUI and InventoryUI…

We make Inventory.cs an IInventorySource just by slapping the interface on the end of the list…
And because of this, we only have to implement GetDisplayName() in Inventory.cs, because the rest is already done.

public string GetDisplayName() => name;

Next, we need to go into our InventoryUI.cs and change the type of the PlayerInventory from Inventory to IInventorySource

IInventorySource PlayerInventory;

And of course, the compiler will immediately throw a warning that InventorySlotUI.Setup can’t take an IInventorySource, so we’ll head there and make a teeny tiny change

        IInventorySource inventory;

        // PUBLIC

        public void Setup(IInventorySource inventory, int index)

Now is where it gets tricky on my end, because I haven’t set up banks within the Third Person conversion (I started with a clean version of the course repo). My next comment will start after I have added this banking feature (at least enough of it to get started on what needs to happen with the PickupFinder to make it into a “bank”
@Bahaa Don’t reply until I get that part done, which I’m working on now.

Ok, I added enough of the system to allow me to get the TargetFinder code written…

We’re going to make TargetFinder an IInventorySource. Set the range of your TargetFinder to be enough to pickup nearby objects, but not too crazy… I’d say larger than the manual kneel down and pickup radius, maybe 10?

Then with your InventoryUI.Setup() (from the Banking Tutorial), pass the GameObject of the PickupFinder in your PlayerPickupState (or whatever you call it) to the Setup.

I’ve modified the PickupFinder to act as an IInventorySource which is withdrawal only. No Swaps, no deposits, no returns. You pick it up, it’s yours.

PickupFinder.cs
using System;
using GameDevTV.Inventories;
using RPG.Core;

namespace RPG.Inventories
{
    public class PickupFinder : RangeFinder<PickupTarget>, IInventorySource
    {
        
        protected override void AddTarget(PickupTarget target)
        {
            base.AddTarget(target);
            target.OnPickedUp += RemoveTarget;
        }

        protected override void RemoveTarget(PickupTarget target)
        {
            base.RemoveTarget(target);
            target.OnPickedUp -= RemoveTarget;
        }

        public event Action inventoryUpdated;
        public string GetDisplayName()
        {
            return "Nearby Objects";
        }

        public bool HasSpaceFor(InventoryItem item)
        {
            return false;  //Pickup only, no deposit, no return
        }

        public int FreeSlots()
        {
            return 0;
        }

        public int GetSize()
        {
            return Targets.Count;
        }

        public bool AddToFirstEmptySlot(InventoryItem item, int number)
        {
            return false;
        }

        public bool HasItem(InventoryItem item)
        {
            foreach (PickupTarget pickupTarget in Targets)
            {
                if (pickupTarget.GetComponent<Pickup>().GetItem() == item) return true;
            }
            return false;
        }

        public InventoryItem GetItemInSlot(int slot)
        {
            return Targets[slot].GetComponent<Pickup>().GetItem();
        }

        public int GetNumberInSlot(int slot)
        {
            return Targets[slot].GetComponent<Pickup>().GetNumber();
        }

        public void RemoveFromSlot(int slot, int number)
        {
            int remain = GetNumberInSlot(slot) - number;
            if (remain > 0)
            {
                Targets[slot].GetComponent<Pickup>().SetNumber(remain);
            }
            else
            {
                var item = Targets[slot];
                Targets.Remove(item);
                Destroy(item.gameObject);
            }
            inventoryUpdated?.Invoke();
        }

        public bool AddItemToSlot(int slot, InventoryItem item, int number)
        {
            return false;
        }
    }
}

I’ll leave hooking it up to you. This, at the very least, should give you a strong start, and should be significantly simpler than… (see posts 1-4)

I was asleep :sweat_smile:

wait, do I have to delete everything I did and do that all over again, or where am I supposed to proceed from? I’m a little confused here, apologies :slight_smile:

(Ahh nevermind, I’ll reverse to a backup before my chaos started.

Side question though. Does the solution you provided above consider for destroying the stuff in the game world once it’s picked up? It’s what I’m currently trying to implement)

for my case, because I was trying to improve inventory transfer performance at one point, three of the functions here needed a ‘bool refresh = true’ optional parameter in the end. I added that in the interface itself (it’s optional, so it probably won’t be hurting anyone)

I am 99% sure you meant ‘InventorySlotUI.cs’. InventoryUI.cs does not have any of that code :sweat_smile:

and my ‘PlayerInventory’ is called ‘Inventory’ in ‘InventorySlotUI.cs’ (I’m assuming that’s what you meant earlier):

        Inventory inventory;

but changing the type of that isn’t exactly “teeny tiny”… it gives me around 13 problems :sweat_smile: (and all of them are related to changing the type from ‘Inventory’ to ‘IInventorySource’. Some functions are not in the interface, so the compiler is confused)

If it helps to clear out my current issues. Here’s what my current ‘InventorySlotUI.cs’ looks like:

using UnityEngine;
using System.Linq;
using GameDevTV.Inventories;
using GameDevTV.Core.UI.Dragging;
using UnityEngine.EventSystems;
using RPG.Inventories;
using RPG.Combat;
using RPG.Skills;

namespace GameDevTV.UI.Inventories
{
    public class InventorySlotUI : MonoBehaviour, IItemHolder, IDragContainer<InventoryItem>, IPointerClickHandler
    {

        // CONFIG DATA
        [SerializeField] InventoryItemIcon icon = null;

        // PRIVATE
        private Equipment equipment;

        private void Awake()
        {
            equipment = GameObject.FindWithTag("Player").GetComponent<Equipment>();
        }

        // STATE
        int index;
        InventoryItem item;
        Inventory inventory;

        // PUBLIC

        public void Setup(Inventory inventory, int index)
        {
            this.inventory = inventory;
            this.index = index;
            icon.SetItem(inventory.GetItemInSlot(index), inventory.GetNumberInSlot(index));
        }

        public int MaxAcceptable(InventoryItem item)
        {
            if (inventory.HasSpaceFor(item))
            {
                return int.MaxValue;
            }
            return 0;
        }

        public void AddItems(InventoryItem item, int number)
        {
            inventory.AddItemToSlot(index, item, number);
        }

        public InventoryItem GetItem()
        {
            return inventory.GetItemInSlot(index);
        }

        public int GetNumber()
        {
            return inventory.GetNumberInSlot(index);
        }

        public void RemoveItems(int number)
        {
            inventory.RemoveFromSlot(index, number);
        }

        private bool isHandlingRightClick;

        SkillStore skillStore;

        // Equipping a Weapon by clicking on it, and banking it if the bank is open:
        public void TryHandleRightClick() {

            if (isHandlingRightClick) return;
            isHandlingRightClick = true;

            InventoryItem item = inventory.GetItemInSlot(index);
            int number = inventory.GetNumberInSlot(index);
            
            if (item == null || number < 1) {
                isHandlingRightClick = false;
                return; // inventory/bank slot is empty, so do nothing
            }

            // If you're pressing on an item inside the Bank's Inventory Slot UIs, send it to the players' inventory:
            if (!inventory.gameObject.CompareTag("Player")) {

                TransferToOtherInventory(Inventory.GetPlayerInventory(), item, 1);
                return;

            }

            var otherInventoryUI = FindObjectsOfType<InventoryUI>().FirstOrDefault(ui => ui.IsOtherInventory);  // returns other inventory, or null

            if (otherInventoryUI != null && otherInventoryUI.gameObject.activeSelf && otherInventoryUI.SelectedInventory != null) {

                Inventory otherInventory = otherInventoryUI.SelectedInventory;
                TransferToOtherInventory(otherInventory, item, 1);
                return;

            }

            if (item is EquipableItem equipableItem)
            {

                if (!equipableItem.CanEquip(equipableItem.GetAllowedEquipLocation(), equipment)) return;    // The line solely responsible for checking for Predicate conditions, prior to being able to wield a weapon (if you're not high enough of a level, you can't wield that)

                // Test: Check 'Equipment.MaxAcceptable()' to confirm if a weapon is 2-handed or not:
                // if (equipment.MaxAcceptable(equipableItem.GetAllowedEquipLocation(), equipableItem) == 0) return;

                if (equipableItem is WeaponConfig weapon)
                {
                    TryEquipWeapon(weapon);
                    return;
                }

                if (equipableItem is ShieldConfig shield)
                {
                    TryEquipShield(shield);
                    return;
                }

                /* // TEST (Delete if failed): Equipping dual hand weapons into the shield slot:
                if (equipableItem is WeaponConfig dualHandWeapon) 
                {
                    TryEquipDualHandWeapon(dualHandWeapon);
                    return;
                } */

                // if equipableItem is WeaponConfig dualHandWeapon, then TryEquipDualHandWeapon here

                // Equipment equipment = Inventory.GetPlayerInventory().GetComponent<Equipment>();
                EquipableItem equippedItem = equipment.GetItemInSlot(equipableItem.GetAllowedEquipLocation());

                equipment.RemoveItem(equipableItem.GetAllowedEquipLocation());
                equipment.AddItem(equipableItem.GetAllowedEquipLocation(), equipableItem);
                RemoveItems(1);

                if (equippedItem != null)
                {
                    AddItems(equippedItem, 1);
                }

            }

            else if (item is ActionItem actionItem) {

                ActionStore actionStore = inventory.GetComponent<ActionStore>();
                int slot = actionStore.GetFirstEmptySlot();

                if (slot > -1) {

                    actionStore.AddAction(actionItem, slot, GetNumber());
                    RemoveItems(GetNumber());

                }

            }

            else if (item is AmmunitionItem ammunitionItem && inventory.TryGetComponent(out Quiver quiver))
            {

                // If there is a "Quiver" Equipment slot, and what we clicked on is a Rangers' arrow,
                // then we shall transfer the arrow to the dedicated slot

                Debug.Log($"{ammunitionItem} is ammo, transferring to {quiver}");
                TransferToQuiver(ammunitionItem, quiver, number);

            }

        }

            // BELOW LIES THE CODE THAT ENSURES WE CAN CLICK ON STUFF IN OUR INVENTORY TO BANK IT:

            // Static variable. Each time InventorySlotUI is clicked, this value is updated:
            private static InventorySlotUI LastUIClicked;

            // The 'OnPointerClick' method below checks to see if the Last UI Clicked Slot is 'this' inventory slot.
            // if it isn't, set 'Last UI Clicked' to this, and a 'TimesUp()' timer (a timer that checks to see if we're the LastUIClicked. If so, clear 'LastUIClicked') 
            // is invoked

            public void OnPointerClick(PointerEventData eventData) {

                if (LastUIClicked != this) {

                    LastUIClicked = this;
                    Invoke(nameof(TimesUp), .001f); // a very small timer means spam clicks are impossible, eliminating potential bugs coming out of that
                    Debug.Log($"{index} was clicked once");

                }

                else {
                    HandleDoubleClick();
                }
            }

            // 'TimesUp' checks if we're the last UI clicked (if true, clear 'LastUIClicked')
            private void TimesUp() {
                if (LastUIClicked == this) LastUIClicked = null;
            }

            // HandleDoubleClicked() function deals with 3 states:

            // 1. I am the player, and 'OtherInventory' (Bank) is Open
            // 2. I am the bank, and 'OtherInventory' (Player) is Closed
            // 3. I am the 'OtherInventory' (bank)

            private void HandleDoubleClick() {

                TimesUp();  // avoids triple clicking from starting another 'HandleDoubleClick'
                InventoryItem item = inventory.GetItemInSlot(index);
                int number = inventory.GetNumberInSlot(index);

                if (item == null || number < 1) return;

                if (inventory.gameObject.CompareTag("Player")) {

                    var otherInventoryUI = FindObjectsOfType<InventoryUI>().FirstOrDefault(ui => ui.IsOtherInventory);

                    if (otherInventoryUI != null && otherInventoryUI.gameObject.activeSelf && otherInventoryUI.SelectedInventory != null) {

                        Inventory otherInventory = otherInventoryUI.SelectedInventory;
                        TransferToOtherInventory(otherInventory, item, 1);

                    }

                    else if (item is EquipableItem equipableItem && inventory.TryGetComponent(out Equipment equipment)) {
                    EquipItem(equipableItem, equipment, number);
                    }

                }

                else {
                    TransferToOtherInventory(Inventory.GetPlayerInventory(), item, 1);
                }

            }

        // Transferring stuff from our Inventory to the Quiver:
        private void TransferToQuiver(AmmunitionItem ammunitionItem, Quiver quiver, int number) {

            // 1. Get the type of old arrows that are in the quiver, to be replaced
            AmmunitionItem item = quiver.GetItem();

            // 2. Get how many of the old arrows are currently in the quiver
            int amount = quiver.GetAmount();

            // 3. Remove the new arrows from the inventory
            inventory.RemoveFromSlot(index, number);

            // 4. Check if the new arrows to the quiver and current arrows are the same or not (if so, add up. If not, replace)
            quiver.AddAmmunition(ammunitionItem, ammunitionItem == item ? number + amount : number);

            // 5. If your quiver had something in it, and that item is not the same as the item coming in, replace it 
            // (i.e: place the old item into the inventory slot)
            if (item != null && ammunitionItem != item) inventory.AddItemToSlot(index, item, amount);

        }

        // Transferring stuff from the Inventory to our Bank:
        private void TransferToOtherInventory(Inventory otherInventory, InventoryItem item, int number) {

                    if (otherInventory.HasSpaceFor(item)) {

                        otherInventory.AddToFirstEmptySlot(inventory.GetItemInSlot(index), number);
                        inventory.RemoveItem(item, number);
                        Setup(inventory, index); // RECHECK Second argument, if it's 'item' or 'index'

                    }

                }

                // Equip item, if it's Equipable:
                private void EquipItem(EquipableItem equipableItem, Equipment equipment, int number) {

                    if (equipableItem.CanEquip(equipableItem.GetAllowedEquipLocation(), equipment)) {

                        EquipableItem otherEquipableItem = equipment.GetItemInSlot(equipableItem.GetAllowedEquipLocation());
                        equipment.RemoveItem(equipableItem.GetAllowedEquipLocation());
                        inventory.RemoveFromSlot(index, number);
                        equipment.AddItem(equipableItem.GetAllowedEquipLocation(), equipableItem);

                        if (otherEquipableItem != null) inventory.AddItemToSlot(index, otherEquipableItem, 1);

                        Setup(inventory, index);

                    }

                }

                public void UpdateUI() {
                    icon.SetItem(GetItem(), GetNumber());
                }

        // ------------------------------------- OUT OF COURSE CONTENT: Trying to Equip 2-Handed weapons, and dealing with the consequences ---------------------------------------

        private void TryEquipWeapon(WeaponConfig weapon)
        {
            RemoveItems(1);
            WeaponConfig otherWeapon = (WeaponConfig)equipment.GetItemInSlot(EquipLocation.Weapon);
            ShieldConfig otherShield = (ShieldConfig)equipment.GetItemInSlot(EquipLocation.Shield);
            if (weapon.IsTwoHanded)
            {
                if (!inventory.HasSpaceFor(equipment.GetWeaponAndShieldAsIEnumerable()))
                {
                    AddItems(weapon, 1);
                    return;
                }
                equipment.RemoveItem(EquipLocation.Shield);
                equipment.RemoveItem(EquipLocation.Weapon);
                if (otherWeapon) AddItems(otherWeapon, 1);
                if (otherShield) AddItems(otherShield, 1); //If you try to AddItems and an item exist, it goes to first empty slot
                equipment.AddItem(EquipLocation.Weapon, weapon);
                return;
            }
            if (weapon.IsLeftHanded)
            {
                if (otherShield)
                {
                    equipment.RemoveItem(EquipLocation.Shield);
                    AddItems(otherShield, 1);
                }
            }
            //regardless of weapon hand, otherWeapon must be removed
            if (otherWeapon)
            {
                equipment.RemoveItem(EquipLocation.Weapon);
                AddItems(otherWeapon, 1);
            }
            equipment.AddItem(EquipLocation.Weapon, weapon);
        }

        private void TryEquipShield(ShieldConfig shield)
        {
            RemoveItems(1);
            WeaponConfig otherWeapon = (WeaponConfig)equipment.GetItemInSlot(EquipLocation.Weapon);
            ShieldConfig otherShield = (ShieldConfig)equipment.GetItemInSlot(EquipLocation.Shield);
            if (otherWeapon != null && (otherWeapon.IsLeftHanded || otherWeapon.IsTwoHanded)) //by definition, if this is true then we have space and can swap
            {
                equipment.RemoveItem(EquipLocation.Weapon);
                AddItems(otherWeapon, 1);
            }
            else if (otherShield)
            {
                equipment.RemoveItem(EquipLocation.Shield);
                AddItems(otherShield, 1);
            }
            equipment.AddItem(EquipLocation.Shield, shield);
        }

        // ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    
        // TEST - Delete if failed: Getting the slot index:
        public int GetSlotIndex() 
        {
            return index;
        }

        public InventorySlotUI GetLastUIClicked() 
        {
            return LastUIClicked;
        }
    
    }

}

and my ‘InventoryUI.cs’:

using UnityEngine;
using GameDevTV.Inventories;
using TMPro;
using UnityEngine.Events;
using System.Linq;

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 Inventory SelectedInventory => selectedInventory;

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

        // CACHE
        Inventory 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;
            }
        }

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

        // PRIVATE

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

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

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

        // PUBLIC

        public bool Setup(GameObject user) {
            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;
                Title.text = selectedInventory.name;
                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;
                return otherInventory;
            }

            return null;
        
        }

    }

}

Yes, when you RemoveItems, it either deducts the number you took (in the case of stacks) or removes the Pickup altogether. An ItemDropper’s list of items is simply its child objects.

Change SelectedInventory and selectedInventory to IInventorySource (InventoryUI.cs)

Change Inventory to IInventorySource in both instances.
Any methods that didn’t get added to my IInventorySource interface, add them to the interface and implement them in the PickupFinder.

Assuming you mean change the inventory reference in ‘InventorySlotUI.cs’ and the one in the ‘InventorySlotUI.Setup()’, this does produce a lot of errors because of the wild number of inventory references in ‘InventorySlotUI.cs’ (13 errors, in total, because of that simple change :sweat_smile:)

Is there a better approach than changing the main ‘Inventory inventory’ in ‘InventorySlotUI.cs’?

I’m still trying to work out the rest. Sorry if this sounds like a bit of work :sweat_smile: (I’ll try fix them manually step by step, but I still struggle miserably with the advanced concepts like dictionaries, interfaces and all the advanced concepts, so please bear with me :slight_smile:)


(Right now, changing the inventories in ‘InventoryUI.cs’ means changing A LOT of stuff in ‘InventorySlotUI.cs’ as well. They’re linked to each other, and most of them are context-based syntax errors :sweat_smile:). Here’s what I mean:

So this, unlocks a dark portal into the world of Syntax errors in ‘InventorySlotUI.cs’. I’ll summarize them here:

in ‘TryHandleRightClick()’:

// If you're pressing on an item inside the Bank's Inventory Slot UIs, send it to the players' inventory:

// SYNTAX ERROR IN THE IF-STATEMENT LINE BELOW:
if (!inventory.gameObject.CompareTag("Player")) {

                TransferToOtherInventory(Inventory.GetPlayerInventory(), item, 1);
                return;

            }

Also in ‘TryHandleRightClick()’:

            if (otherInventoryUI != null && otherInventoryUI.gameObject.activeSelf && otherInventoryUI.SelectedInventory != null) {

                Inventory otherInventory = otherInventoryUI.SelectedInventory;
                TransferToOtherInventory(otherInventory, item, 1); // SYNTAX ERROR IN THIS LINE
                return;

            }

When getting the Action Store:

            else if (item is ActionItem actionItem) {

                ActionStore actionStore = inventory.GetComponent<ActionStore>(); // SYNTAX ERROR IN THIS LINE
                int slot = actionStore.GetFirstEmptySlot();

                if (slot > -1) {

                    actionStore.AddAction(actionItem, slot, GetNumber());
                    RemoveItems(GetNumber());

                }

            }
            else if (item is AmmunitionItem ammunitionItem && inventory.TryGetComponent(out Quiver quiver))

// THE INTERFACE DOESN'T KNOW WHAT 'TryGetComponent' IS... SO, IT'S A SYNTAX ERROR (ALONG WITH THE DEBUG.LOG BENEATH IT)

in ‘HandleDoubleClick()’ (Does this function even work anymore…?! :sweat_smile:):

// SYNTAX ERROR IN THE '.gameObject' BELOW
if (inventory.gameObject.CompareTag("Player")) {

                    var otherInventoryUI = FindObjectsOfType<InventoryUI>().FirstOrDefault(ui => ui.IsOtherInventory);

                    if (otherInventoryUI != null && otherInventoryUI.gameObject.activeSelf && otherInventoryUI.SelectedInventory != null) {

                        Inventory otherInventory = otherInventoryUI.SelectedInventory; // SYNTAX ERROR HERE AS WELL
                        TransferToOtherInventory(otherInventory, item, 1);

                    }

// SYNTAX ERROR IN 'TRYGETCOMPONENT' BELOW AS WELL, ALONG WITH EQUIPPING THE ITEMS
                    else if (item is EquipableItem equipableItem && inventory.TryGetComponent(out Equipment equipment)) {
                    EquipItem(equipableItem, equipment, number);
                    }

                }

TransferToOtherInventory:

        // Transferring stuff from the Inventory to our Bank:
        private void TransferToOtherInventory(Inventory otherInventory, InventoryItem item, int number) {

                    if (otherInventory.HasSpaceFor(item)) {

                        otherInventory.AddToFirstEmptySlot(inventory.GetItemInSlot(index), number);
// INVENTORY.REMOVEITEM BELOW HAS A SYNTAX ERROR
                        inventory.RemoveItem(item, number);
                        Setup(inventory, index); // RECHECK Second argument, if it's 'item' or 'index'

                    }

                }

AND FINALLY, IN ‘TRYEQUIPWEAPON’:

// SYNTAX ERROR IN 'equipment.GetWeaponAndShieldAsIEnumerable()'
if (!inventory.HasSpaceFor(equipment.GetWeaponAndShieldAsIEnumerable()))
                {
                    AddItems(weapon, 1);
                    return;
                }

SOO… Any idea how to fix these? I’ll try until you return, but I’ll need help in the end :sweat_smile:

It only would if you have methods in Inventory.cs that aren’t in the IInventory interface. Simply add them to the interface and implement methods in the PickupFinder.

Dictionaries and Interfaces are extremely important concepts. Interfaces are literally one of the planks of the SOLID archetecture. That’s why I went with an Interface approach here. (And pay attention, because my Inventory rewrite will be using this specific interface and more).

Add this to the IInventorySource interface:

GameObject GetGameObject();

And implement it in both IInventorySource classes as

public GameObject GetGameObject() => gameObject;

Then you’ll use

if (!inventory.GetGameObject().CompareTag("Player")) 
{ //grrr
ActionStore actionStore = inventory.GetGameObject().GetComponent<ActionStore>();

Everywhere you find a GetComponent, TryGetComponent, etc that are base MonoBehaviour functions, insert that GetGameObject().

Add RemoveItem(item, Number) to the interface and implement it in the PickupFinder.

I wasn’t thinkiing we’d need the IEnumerable. Once again, add this method to the Interface and implement it within the TargetFinder.

Alright surprisingly, I fixed all 13 bugs. Time to go implement that in ‘PickupFinder.cs’ (a new set of challenges on the way)

I used them from time to time over recent concepts (if you recall, I once had to split the XP in multiple ways, after I introduced shared XP with NPCs who are involved in the fight. To fix that, I used a dictionary to first split how much total XP the player gets, and then a second dictionary to split that XP based on the skills used in that fight

As for interfaces, I think it had something to do with projectiles recently, and that whole ‘Don’t call animals from the sea’ mathematical problem that I recently had)

But I don’t fully understand when and how they’re used on a generic level, so I’m trying my best to use them only when absolutely necessary, otherwise I’d be dead confused :sweat_smile:

OK so… A little update. After cleaning up my Interface, here’s what I ended up with in ‘PickupFinder.cs’:

using System;
using System.Collections.Generic;
using System.Linq;
using GameDevTV.Inventories;
using RPG.Core;
using UnityEngine;

namespace RPG.Inventories 
{
    public class PickupFinder : RangeFinder<PickupTarget>, IInventorySource
    {
        protected override void AddTarget(PickupTarget target)
        {
            base.AddTarget(target);
            target.OnPickedUp += RemoveTarget;
            Debug.Log($"Pickup Finder: Adding {target.name}");
        }

        protected override void RemoveTarget(PickupTarget target)
        {
            base.RemoveTarget(target);
            target.OnPickedUp -= RemoveTarget;
            Debug.Log($"Pickup Finder: Removing {target.name}");
        }

        public PickupTarget GetNearestPickup()
        {
            CurrentTarget = Targets.OrderBy(t => Vector3.Distance(transform.position, t.transform.position)).FirstOrDefault();
            return CurrentTarget;
        }

        // Most of the Functions down here are for 'IInventorySource.cs' to work. We don't need them all,
        // but Interfaces demand everything to be used, so most will be empty
        // (it's either this, or a performance-intense solution of programming a second inventory, which
        // can be a bit of a messy solution)

        public event Action inventoryUpdated;

        public bool AddItemToSlot(int slot, InventoryItem item, int number, bool refresh = true)
        {
            return false; // No returns. Only takes
        }

        public bool AddToFirstEmptySlot(InventoryItem item, int number, bool refresh = true)
        {
            return false; // No returns. Only takes
        }

        public string GetDisplayName()
        {
            return "Nearby Pickups";
        }

        public GameObject GetGameObject()
        {
            return gameObject;
        }

        public InventoryItem GetItemInSlot(int slot)
        {
            return Targets[slot].GetComponent<Pickup>().GetItem();
        }

        public int GetNumberInSlot(int slot)
        {
            return Targets[slot].GetComponent<Pickup>().GetNumber();
        }

        public int GetSize()
        {
            return Targets.Count; // How many total Pickups are around you...?
        }

        public bool HasItem(InventoryItem item)
        {
            foreach (PickupTarget pickupTarget in Targets) 
            {
                if (pickupTarget.GetComponent<Pickup>().GetItem() == item) return true; // if you got this far, it means you found the item
            }
            return false; // can't find it nearby? Then it's not there
        }

        public bool HasSpaceFor(InventoryItem item)
        {
            return false; // No returns. Only takes
        }

        public bool HasSpaceFor(IEnumerable<InventoryItem> items)
        {
            return false; // No returns. Only takes
        }

        public void RemoveFromSlot(int slot, int number, bool refresh = true)
        {
            int remainder = GetNumberInSlot(slot) - number;
            if (remainder > 0) 
            {
                Targets[slot].GetComponent<Pickup>().SetNumber(remainder);
            }
            else 
            {
                var item = Targets[slot];
                Targets.Remove(item);
                Destroy(item.gameObject);
            }
            inventoryUpdated?.Invoke();
        }

        public void RemoveItem(InventoryItem item, int number, bool refresh = true)
        {
            if (item == null) return;
        }

        public void TransferAllInventory(Inventory otherInventory)
        {
            // For now, this is empty. It's just to get 'InventoryUI' to be quiet about whole inventory
            // transfers, that it made it in the 'IInventorySource' Interface (but we don't really need it for now, 
           // unless I put that transfer button back in)
        }

    }

}

Please skim through it whenever possible, and see if anything looks suspicious in here

AND…:

  1. What do I write in ‘RemoveItem’? I don’t have access to the inventory slots, so I figured copy-pasting it from ‘Inventory.cs’ would be a little confusing

Edit: Regarding ‘RemoveItem’, I gave it a go (and asked ChatGPT for a little bit of help along the way), and here’s what we came up with:

public void RemoveItem(InventoryItem item, int number, bool refresh = true)
        {
            if (item == null) return; // No item, no pickup

            foreach (PickupTarget pickupTarget in Targets) 
            {
                var pickup = pickupTarget.GetComponent<Pickup>();
                if (pickup.GetItem() == item) 
                {
                    int currentNumber = pickup.GetNumber();
                    if (currentNumber > number) // consume a fraction of the item
                    {
                        pickup.SetNumber(currentNumber - number);
                        inventoryUpdated?.Invoke();
                        return;
                    }
                    else // you took more than the item has, so consume everything
                    {
                        number -= currentNumber;
                        Targets.Remove(pickupTarget);
                        Destroy(pickupTarget.gameObject);
                        inventoryUpdated?.Invoke();
                        if (number <= 0) return;
                    }
                }
            }
        }

Now, without knowing how to hook this all up together, I have no way of telling if this code is accurate or not, which leads me to my next problem below:

  1. Assuming all else is fine, what’s the best way to hook this up? I was looking forward for using the whole switching states to ‘PickupInventory’ and then using ‘OtherInventory’ and what not to open it (from posts 1-4 above), similar to my bank attempt

But now that I’m using an architecture that I’m not fully familiar with, this is a little challenging :sweat_smile: (Please help)

The RemoveItem looks right except that you aren’t respecting the bool refresh. If you’ll recall, we added that refresh=true as an optional parameter to speed things up and not create so many new inventory copies when getting all from the bank… For that, you just need to wrap your inventoryUpdated?.Invoke(); inside of an if statement if(refresh)

Hooking it up… In the original Bank tutorial, the IRaycastable opened the window…

This Hookup turned out to be more complicated than I thought…
I set aside the PlayerPickupState because it’s focused on the animation with the pickup at the end.

Instead I created a PlayerCollectionState()

using GameDevTV.Inventories;

namespace RPG.States.Player
{
    public class PlayerCollectionState : PlayerBaseState
    {
        public PlayerCollectionState(PlayerStateMachine stateMachine) : base(stateMachine)
        {
        }

        public static event System.Action<IInventorySource, System.Action> OnCollectionStateOpened;
        
        public override void Enter()
        {
            OnCollectionStateOpened?.Invoke(stateMachine.PickupFinder, CollectionFinished);
        }

        void CollectionFinished()
        {
            stateMachine.SwitchState(new PlayerFreeLookState(stateMachine));
        }
        
        public override void Tick(float deltaTime)
        {
            Move(deltaTime);
        }

        public override void Exit()
        {
            
        }
    }
}

Note I’m making use of a static event that the window can subscribe to…
This will pass the PickupFinder along with a callback to instructions to switch back to PlayerFreeLookState when we’re done.

The next script is the PickupWindow script, which will go on our Bank GameObject. It’s important for this to work that the Bank object (the copy of the Inventory Object) NOT be under the Hiding Panel.

using System;
using GameDevTV.Inventories;
using GameDevTV.UI.Inventories;
using RPG.States.Player;
using UnityEngine;

namespace RPG.UI.Inventories
{
    public class PickupWindow : WindowController
    {
        [SerializeField] private InventoryWindow inventoryWindow;


        protected override void Subscribe()
        {
            PlayerCollectionState.OnCollectionStateOpened += ToggleBankWindow;
            gameObject.SetActive(false);
        }

        protected override void Unsubscribe()
        {
            PlayerCollectionState.OnCollectionStateOpened -= ToggleBankWindow;
        }

        private System.Action callback;
        
        private void ToggleBankWindow(IInventorySource source, Action finishedCallback)
        {
            callback = finishedCallback;
            inventoryWindow.gameObject.SetActive(true);
            GetComponentInChildren<InventoryUI>().Setup(source.GetGameObject());
            gameObject.SetActive(true);
        }

        protected override void OnDisable()
        {
            base.OnDisable();
            inventoryWindow.gameObject.SetActive(false);
            callback?.Invoke();
        }
    }
}

Put this on your Bank, and link the InventoryWindow (which should be on the HidingPanel GameObject) to it.

So… in both ‘RemoveItem’ and ‘RemoveFromSlot’, I’ll turn this line:

inventoryUpdated?.Invoke();

to this:

if (refresh) {inventoryUpdated?.Invoke();}

I was wondering if we can paste this on a duplicate of the bank UI? Mainly because I was hoping to make this a little smaller in size, as the current size of the bank for a pickup inventory is a little absurd :sweat_smile:

And I can also remove that from under the hiding panel, as per your instructions

How you make it is entirely up to you. When I wrote the tutorial, I went simple and duplicated the Inventory window.

Amazing. I’ll duplicate it and keep you updated on how it goes then :slight_smile:

OK this works 99% perfectly as expected (apart from a need to rewire the quit button for that one. I can’t be using the same button as ‘CloseBank’. When it comes to re-reading my game from the future, this will be a disaster), thanks again a ton Brian for saving me again. This would’ve honestly been significantly harder for me (if not impossible) to figure out without your help

There’s just ONE TINY LITTLE CAVEAT…

Why do the inventory slots disappear the moment I pick the item up? Can we eliminate that little section? I just want a constant number of slots to work with (it’s just personal taste)

Other than that, although I’m really confused to what’s going on here (this was a nuke bomb of new information that’ll take me some time to understand), this whole thing works perfectly, which can be identified in ‘Inventory’, like the bank

As soon as I eliminate that Slot Removal Mechanic, I’ll write a summary below (so I know what to do next time I come back to this topic) whilst it’s still fresh in my mind. This will be essential for me to understand what’s going on in the future I believe

They dissappear because the item is no longer there to support the slot. To craft a solution that kept the slots “empty” would require… quite a bit more coding.

[THERE’S A LOT GOING ON BELOW. PLEASE, JUST SKIP TO THE FINAL COMMENT. THERE’S 4 REMAINING BULLET-POINT STYLE PROBLEMS AT THE BOTTOM FOR ME TO SOLVE :sweat_smile:]

if it’s a nuisance, we can skip it. If not, it would be a nice little addon to keep things looking clean :slight_smile:

At the very least, what would be the idea behind it? At least I’d like to give it a try

(Just like the bank, give it an inventory size, and fill them up accordingly. When the items are taken, keep the slot empty without permanently deleting that slot from the game world. It’s just there to keep the UI clean and simple for users, xD)

Privacy & Terms