// TEST
private void OnEnable()
{
if(!inventory) return;
inventory.GetGameObject().GetComponent<Inventory>().OnIndividualSlotChanged += Inventory_IndividualSlotChanged;
}
// TEST
private void OnDisable()
{
if(!inventory) return;
inventory.GetGameObject().GetComponent<Inventory>().OnIndividualSlotChanged -= Inventory_IndividualSlotChanged;
}OK I donât mean to sound like a nuisance, butâŚ:
Operator '!' cannot be applied to operand of type 'IInventorySource'
(Thatâs a syntax error the second I type that out)
This topic just wonât make it easy⌠
Anyway, when we write it like this, the syntax error is gone, but the NRE error still persists:
if (!inventory.GetGameObject().GetComponent<Inventory>()) return;
The fun of keeping 13 codebases in my head + modificationsâŚ
if(inventory==null) return;
Well⌠the heavy load of the issue has been successfully transferred to the next line of each one of these functions (the lines of the subscription and unsubscription):
// TEST
private void OnEnable()
{
if (inventory == null) return;
inventory.GetGameObject().GetComponent<Inventory>().OnIndividualSlotChanged += Inventory_IndividualSlotChanged;
}
// TEST
private void OnDestroy()
{
if (inventory == null) return;
inventory.GetGameObject().GetComponent<Inventory>().OnIndividualSlotChanged -= Inventory_IndividualSlotChanged;
}
not to mention this function is just being the biggest nuisance on planet earth right now:
public InventoryItem GetItem()
{
return inventory.GetItemInSlot(index);
}
OK umm, how about this. Iâll outline every single step I have taken so far, and maybe we can find the problem of exactly where I went wrong, although 100% of the slots flying around and acting insanely weird are coming from this one single function:
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 transform)
{
// TEST
slotPool.Release(child.GetComponent<InventorySlotUI>());
// Destroy(child.gameObject); // OBJECT POOL TEST BELOW
}
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);
}
}
Alright here it goes, and if this doesnât work, then Iâm better off leaving my bad performance inventory as is and calling it a day. On the long term, Iâll pay the price for this, but I donât know what else to do:
- in âInventory.csâ, I created this event:
// TEST
public event System.Action<int> OnIndividualSlotChanged;
and this was called everywhere âinventoryUpdated?.Invoke()â was called:
// AddToFirstEmptySlot:
/// <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(i);
}
return true;
}
// RemoveFromSlot():
/// <summary>
/// Remove a number of items from the given slot. Will never remove more
/// that there are.
/// </summary>
public void RemoveFromSlot(int slot, int number, bool refresh = true)
{
slots[slot].number -= number;
if (slots[slot].number <= 0)
{
slots[slot].number = 0;
slots[slot].item = null;
}
if (refresh)
{
inventoryUpdated?.Invoke();
OnIndividualSlotChanged?.Invoke(slot);
}
}
// AddItemToSlot():
/// <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>
public bool AddItemToSlot(int slot, InventoryItem item, int number, bool refresh = true)
{
if (slots[slot].item != null)
{
return AddToFirstEmptySlot(item, number, refresh);
}
var i = FindStack(item);
if (i >= 0)
{
slot = i;
}
slots[slot].item = item;
slots[slot].number += number;
if (refresh)
{
inventoryUpdated?.Invoke();
OnIndividualSlotChanged?.Invoke(slot);
}
return true;
}
// RemoveItem():
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();
OnIndividualSlotChanged?.Invoke(i);
}
return;
}
}
}
- I added the event in âIInventorySource.csâ as well:
/// <summary>
/// Broadcasts when an individual inventory slot has changed
/// </summary>
event Action<int> OnIndividualSlotChanged;
which means, in âPickupFinder.csâ, I did what I did in âInventory.csâ as well:
public event Action<int> OnIndividualSlotChanged;
// RemoveFromSlot():
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);
}
if (refresh)
{
inventoryUpdated?.Invoke();
OnIndividualSlotChanged?.Invoke(slot);
}
}
// RemoveItem:
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) // you found an item to pickup
{
int currentNumber = pickup.GetNumber();
if (currentNumber > number) // consume the amount of item picked up by the player (below max limit)
{
pickup.SetNumber(currentNumber - number);
if (refresh)
{
inventoryUpdated?.Invoke();
OnIndividualSlotChanged?.Invoke(currentNumber);
}
return;
}
else // max number of consumed items taken exceeded, so consume everything
{
number -= currentNumber;
Targets.Remove(pickupTarget);
Destroy(pickupTarget.gameObject);
if (refresh)
{
inventoryUpdated?.Invoke();
OnIndividualSlotChanged?.Invoke(currentNumber);
}
if (number <= 0) return;
}
}
}
}
- in âInventorySlotUI.csâ, I added a few lines of code:
// TEST
private void OnEnable()
{
if (inventory == null) return;
inventory.OnIndividualSlotChanged += Inventory_IndividualSlotChanged;
}
// TEST
private void OnDestroy()
{
if (inventory == null) return;
inventory.OnIndividualSlotChanged -= Inventory_IndividualSlotChanged;
}
// 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));
}
- in âInventoryUI.csâ, we created the pool:
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
}
// TEST
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);
}
- We tried testing the pool in âRedraw()â, but my code is failing terribly at that:
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 transform)
{
// TEST
// slotPool.Release(child.GetComponent<InventorySlotUI>());
Destroy(child.gameObject); // OBJECT POOL TEST BELOW
}
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 7. I create a âphysicalPoolâ in the hierarchy, and went to each âInventoryUI.csâ I can think of, and tried hooking them up. I can see it as it works, but the mathematics behind the job is insanely wrong
and donât forget âInventoryUI.Start()â:
private void Start() // ORIGINAL START Function:
{
foreach (Transform child in transform)
{
Destroy(child.gameObject);
}
// Redraw the PLAYER INVENTORY, JUST NOT THE BANK!
if (isPlayerInventory) Redraw();
}
These are all the steps Iâve taken, if it helps in anyway
Since you said you werenât worried about the pickups at this time, I forgot about the IInventory interfaceâŚ
Youâll need to add the event to the IInterface just like whe inventoryUpdated event.
Then you can use inventory.OnIndividualSlotChanged (and of course, youâll have to implement this method in the PickupFinder as well.
This brings up a whole new can of ugly worms, though, because the PickupFinder does not have as concrete of an idea about slot⌠as once you remove an item, the slot count will changeâŚ
So maybe this construction might be betterâŚ
if(inventory!=null && inventory.GetGameObject().TryGetComponent(out Inventory i))
{
i.OnIndividualSlotChanged += Inventory_IndividualSlotChanged;
}
And leave the Pickups to use InventoryUIâs full redraw (if you have 400 items in pickup range, thatâs a whole different can of worms).
Is that implemented in âInventorySlotUI.csâ?
the problem is⌠EVERY SINGLE SORT OF INVENTORY in the game uses that function. Do I re-write a dedicated variant in âInventoryUI.csâ, orâŚ?!
Iâll be honest though, I am sure the entire problem lies here:
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 transform)
{
// TEST
slotPool.Release(child.GetComponent<InventorySlotUI>());
// Destroy(child.gameObject); // OBJECT POOL TEST BELOW
}
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);
}
}
whatever secret sauce this function has, itâs giving me a major headache. It doesnât matter which inventory we are mentioning, this function is the âmake or breakâ of it all, and if we canât deal with this nothing else really matters tbh (everytime I reverse this function, it works perfectly regardless of what everything else is doing)
OK I know this sounds a little crazy, but Object Pooling really is being a headache, and it seems to be causing me more problems than solutions, and no matter what I did, it doesnât seem to be working for me. Letâs try the slot-based refresh instead. I donât know what kind of crazy challenges will I be dealing with, but Object Pooling is about to make me give up on this major problem tbh, and thatâs not really an option but I donât know what else to do with Object Pooling
I read about it as well from the Unity Forums, someone said itâs a huge headache because you need to keep resetting the values and what not⌠Doesnât sound worth it for this thing tbh
@Brian_Trotter OK so Iâm attempting a slot-based refresh, but this one is a bit of a headache. Hereâs what Iâve done so far:
- Get and Set a Source Slot Index. This will be the Inventory Slot UI the item is leaving:
// in 'InventorySlotUI.cs':
// variable:
private int sourceSlotIndex;
public int GetSourceSlotIndex()
{
return sourceSlotIndex;
}
public void SetSourceSlotIndex(int sourceSlotIndex)
{
this.sourceSlotIndex = sourceSlotIndex;
}
// when it leaves the UI:
// 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);
SetSourceSlotIndex(this.index);
SetTargetInventory(otherInventory); // you'll know what this does in step 3
// Setup(inventory, index); // <-- if you keep this, the new Inventory-based Pickup System will complain (because these inventory slots vanish)
}
}
- Get and Set a Destination Slot. This will be done in âInventory.AddToFirstEmptySlot()â, since this is the slot we are aiming to refresh:
/// <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;
SetDestinationSlotIndex(i);
if (refresh)
{
inventoryUpdated?.Invoke();
}
return true;
}
private int destinationSlotIndex;
public int GetDestinationSlotIndex()
{
return destinationSlotIndex;
}
public void SetDestinationSlotIndex(int destinationSlotIndex)
{
this.destinationSlotIndex = destinationSlotIndex;
}
- back in âInventorySlotUI.csâ, we need to find a way to get the inventory that we are transferring the item to, and this will also be setup in âTransferToOtherInventory()â:
private Inventory targetInventory;
public Inventory GetTargetInventory()
{
return targetInventory;
}
public void SetTargetInventory(Inventory targetInventory)
{
this.targetInventory = targetInventory;
}
// ----------------------------------------------------------------------------------------------------------------------
// 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))
{
SetTargetInventory(otherInventory);
otherInventory.AddToFirstEmptySlot(inventory.GetItemInSlot(index), number, true);
inventory.RemoveItem(item, number);
SetSourceSlotIndex(this.index);
// Setup(inventory, index); // <-- if you keep this, the new Inventory-based Pickup System will complain (because these inventory slots vanish)
}
}
- We need a way to get the last clicked UI, so we will create a variable for that in âInventoryUI.csâ and set it in âInventorySlotUI.csâ:
// in 'InventoryUI.cs':
private InventorySlotUI LastUIClicked;
public InventorySlotUI GetLastUIClicked()
{
return LastUIClicked;
}
public void SetLastUIClicked(InventorySlotUI LastUIClicked)
{
this.LastUIClicked = LastUIClicked;
}
// in 'InventorySlotUI.cs' (you'll need the 'IPointerClickInterface' Interface for this to work):
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
GetComponentInParent<InventoryUI>().SetLastUIClicked(this);
Debug.Log($"{index} was clicked once");
}
else
{
HandleDoubleClick();
}
}
- And now, I can safely write a âSlotBasedRedraw()â function:
// --------------------------------------------- Slot-based Redraw (Performance booster for Inventory Transfers) ----------------------------
private InventorySlotUI LastUIClicked;
public InventorySlotUI GetLastUIClicked()
{
return LastUIClicked;
}
public void SetLastUIClicked(InventorySlotUI LastUIClicked)
{
this.LastUIClicked = LastUIClicked;
}
private void SlotBasedRedraw()
{
// Check for null reference before proceeding
if (selectedInventory == null)
{
Debug.LogError("SelectedInventory is null!");
return;
}
var lastUIClicked = GetLastUIClicked();
if (lastUIClicked == null)
{
Debug.LogWarning("No UI clicked yet.");
return;
}
foreach (Transform child in transform)
{
var slotUI = child.GetComponent<InventorySlotUI>();
if (slotUI == null)
{
Debug.LogWarning("Child does not have InventorySlotUI component.");
continue;
}
if (slotUI.GetSlotIndex() == lastUIClicked.GetSlotIndex())
{
Destroy(child.gameObject);
}
}
var targetInventory = GetLastUIClicked().GetTargetInventory();
var destinationSlotIndex = targetInventory.GetDestinationSlotIndex();
for (int i = 0; i < targetInventory.GetSize(); i++)
{
if (i == destinationSlotIndex)
{
var itemUI = Instantiate(InventoryItemPrefab, transform);
itemUI.Setup(selectedInventory, i);
}
}
}
// ------------------------------------------------------------------------------------------------------------------------------------------
and 6. Make sure âInventoryUIâ subscriptions of âinventoryUpdatedâ are done with âSlotBasedRedrawâ instead of âRedrawâ:
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 += SlotBasedRedraw; // you need this inventory subscription
}
}
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 += SlotBasedRedraw;
Title.text = selectedInventory.GetDisplayName();
Redraw();
return true;
}
return false;
}
So, I have one question: Why is my inventory not updating correctly when picking stuff up (if I recall, slots can randomly instantiate new slots, not show icons or place icons that donât properly update their images and data in random slots), or more importantly, why is my bank not opening up correctly after this Update? (Bank shows absolutely no Inventory Slots when itâs opened up) Do you see any pitfalls here?
You may want to copy-paste that in your version of my systems, if you have one, because I also got Pickup Inventories aside from the banks, and see if you can fix it. PLEASEEE
This approach is⌠both confusing and overengineered⌠Please donât take offense, but it needs to go back to the drawing board⌠Itâs an unsimple solution that is more likely to break than it is to work properly.
A few things:
- Youâre still destroying slots, but arbitrarily in the middle of the collection of slots, meaning that items will be (at best) out of order as you rearrange slotsâŚ
- There is no need to keep track of a target and destination slot because, and Iâve said this before every transaction in Inventory is singular, either Add, or Remove. The only place that both things are happening is in the Drag scripts, in the event of a direct drop of one thing onto an existing thing, and then Drag Removes both items (each of these is an individual transaction) and puts them in their new homes (each of these is an individual transaction). The actual transactions themselves are either an add or a replace. This means that with each transaction, you only have to update ONE display
Thereâs a problem with Individual refresh when it comes to Pickups⌠because Pickups have a dynamic size⌠as you pull from the pickups, unless you pull the very last item, the indexes of the items after you pull them will change. This is because they are working off of a list of pickups in the vicinity, not an Array of pickups, and when you remove the item, it necessarily has to remove the item from the supported list. So⌠the pickups need to be approached more like the classic way.
Banks and Inventories, however, will have a fixed number of slots, unless the number of slots available changes. But⌠we donât know the inventory size until we connect to the inventory itself, so at least initially, the inventory needs a complete redraw when first starting.
If we didnât have to deal with pickups, and there was only one fixed bank in the scene, individual redraws would be fairly simple, quite literally subscribe, refresh the icon when needed. However, this is not the case, and it locks out the possibility of players buying extra inventory space, or multiple banks.
This is why an Object Pooling model works better. Yes, itâs changing the parent often, but thatâs still vastly superior to Destroying and Instantiating every change.
Nah Iâm offended, Iâll sue you for $50,000 in emotional damages 
(Iâm joking). Honestly speaking Brian, I am not taking any offence. Iâm here to learn and design, and to make this work.
Fun fact, from the concept remaining that I want to program, I believe this is as hard as it will ever get. Itâs like Iâm dealing with the final boss of a major battle, and we need to win this battle 
@bixarrio suggested this as a concept to me once. I didnât think much of it, but I might take it into consideration if I can find a spot for it on my game plan - but thatâs for another day, for now we just want to ensure that I donât run into long term performance issues. Thatâs literally all Iâve been begging for from my code, but this is quite difficult for me alone
If it improves long-term performance, and doesnât have crazy inventory slots instantiating out of nowhere and inventory slots being used here and there without an elaborate explanation, then Iâm all in on it. I honestly donât mind which approach, as long as long-term, I donât get lag spikes because weâre refreshing a huge number of inventory slots
For now let me just go erase whatever I have done if you donât recommend it, since I canât figure out how to make it work either if Iâm being honest
Iâll wait for any updates from your side if you have any recommendations of what to do next, because Iâm not sure how things are working for you, but for me itâs just⌠not, and Iâm quite adamant on not moving forward unless we fix this because Iâm afraid that if I let this go now, I might never solve it down the line, and then I have a universal complaint from all players, and thatâs not how I want things to go
As I was typing out my thesis about the destruction of all you have wrought, I came up with an idea that might solve one of the problems⌠the Pickups⌠Itâs still a two tiered system, but I have an idea that could allow both individual refresh and dynamic size, selecting between the two based on whether itâs physical or not.
Iâve got several meetings today, but Iâll work on it in between as free time permits.
Sounds amazing, Iâll be waiting to hear from you soon. All the best with your meetings today!
(Just a heads-up though, I deleted all my previous object pooling attempts just to keep my code clean⌠and recently deleted my own slot-based refresh attempts as well)
Step 1: Any place in Inventory.cs that calls InventoryUpdated should also call InventorySlotUpdated(slot) (or i, or whatever index of the slot that just changed is).
Step 2: Add the following property to IInventorySource
bool IsPhysical => true;
By including the autoimplementation of the property (true), we ensure that we donât need to actually override this in Inventory, because when there is a default implementation in an Interface, there is no need to override it to to fulfill the contract.
However, in PickupFinder, we want to override this
public overrided bool IsPhysical => false;
Now the Bank and the Inventory will be Physical, the PickupCollector wonât be.
This is because we want two different behaviours to happen based on the locationâŚ
The rest of this is handled in InventoryUI and InventorySlotUI. I pulled the ObjectPooling out, because it really wonât be saving much clock for the additional complexity.
InventoryUI.cs
using UnityEngine;
using GameDevTV.Inventories;
using TMPro;
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;
// LIFECYCLE METHODS
private void Awake()
{
if (isPlayerInventory)
{
selectedInventory = Inventory.GetPlayerInventory();
}
}
private void Start()
{
if(isPlayerInventory)
{
Redraw();
}
}
// PRIVATE
private void OnEnable()
{
Redraw();
}
private void Redraw()
{
if (selectedInventory==null) return;
foreach (Transform child in transform)
{
Destroy(child.gameObject);
}
for (int i = 0; i < selectedInventory.GetSize(); i++)
{
var itemUI = Instantiate(InventoryItemPrefab, transform);
itemUI.transform.SetParent(transform);
itemUI.Setup(selectedInventory, i);
}
}
public bool Setup(GameObject user)
{
if (selectedInventory is { IsPhysical: false })
{
selectedInventory.inventoryUpdated -= Redraw;
}
if (user.TryGetComponent(out selectedInventory))
{
if (!selectedInventory.IsPhysical)
{
selectedInventory.inventoryUpdated += Redraw;
}
Title.text = selectedInventory.GetDisplayName();
Redraw();
return true;
}
return false;
}
}
}
In Awake(), weâre first checking to see if weâre the Playerâs inventory and setting the selectedInventory appropriately. Weâre NOT subscribing to InventoryUpdated.
In Start(), we Redraw if weâre the Player Inventory, to get everything set up.
In Setup, if weâre NOT a physical inventory, then we subscribe to the inventoryUpdated, if we are a physical Inventory (bank), we donât. Then, since weâre just setting up, we force a full Redraw, because either way, when we first open the other window, we want to Redraw.
So weâve now set up updating the PickupFinder inventory when changes are made. Because it is dynamic, simply refreshing slots is a bad idea and will cause problems, so updating every time something in PickupFinder changes is the way to go.
Next, we head into InventorySlotUI.cs. Iâll just include the changed and added methods, because my version of the script does not contain the Top Ramen code that is the click handling.
public void Setup(IInventorySource inventory, int index)
{
this.inventory = inventory;
this.index = index;
Redraw();
if (inventory.IsPhysical)
{
inventory.GetGameObject().GetComponent<Inventory>().OnInventorySlotUpdated +=
Inventory_OnInventorySlotUpdated;
}
}
void OnDestroy()
{
if (inventory is { IsPhysical: true })
{
inventory.GetGameObject().GetComponent<Inventory>().OnInventorySlotUpdated -=
Inventory_OnInventorySlotUpdated;
}
}
private void Inventory_OnInventorySlotUpdated(int slot)
{
if (slot == index) Redraw();
}
private void Redraw()
{
icon.SetItem(inventory.GetItemInSlot(index), inventory.GetNumberInSlot(index));
}
So in this case, weâre checking to see if we ARE physical. In the event that we are, then we subscribe to the InventorySlotUpdated method. We factored out the Icon change to be a Redraw method, and our handler checks to see if the slot matches and Redraws if appropriate.
OnDestroy() is added, and the Inventory is checked to see if it is physical. If it is, we unsubscribe from the event.
Thank you so much Brian for the hard work youâve put into this, it really means a lot to me! - Thereâs a lot going on here, so Iâll carefully go through it and update you with any questions that I have. Hopefully this time it works
@Brian_Trotter OK so here comes the first problem. The InventorySlotUpdated(slot) will have an action of type integer event, right? I believe itâs as follows:
/// <summary>
/// Broadcasts when an inventory slot is updated
/// </summary>
public event Action<int> InventorySlotUpdated;
The problem I have is in these two functions being called:
// for the button that transfers everything (do we even need to update this one? Looks to me like the first line of the for loop took care of that)
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();
// InventorySlotUpdated?.Invoke(i);
// otherInventory.InventorySlotUpdated?.Invoke(i);
}
// and for 'TriggerUpdate()' (if I recall, this was for systems that got the player to drop all his equipment on death)
public void TriggerUpdated()
{
inventoryUpdated?.Invoke();
InventorySlotUpdated?.Invoke(slot);
}
Where do we get the slot values for these two?
Oh, and Iâm assuming the âInventorySlotUpdatedâ does not happen when restoring the save file as well, correct?
Another MAJOR problem Iâm dealing with, is that the slots are acting like I only get only one click with them and theyâll never work again beyond that, unless I close and reopen the inventory. Itâs acting like Electronic Arts right now, as in âyou only get one click, and pay $5.50 for each click beyond thatâ
Humor aside, the slots are no longer clickable after the first click, unless you close and re-open the inventory, so thatâs a new problem - this is the last problem I have with this system. Can we please fix that?
Edit: I fixed the last problem. I forgot about âisHandlingRightClickâ in my personal âTryHandleRightClick()â function, so I commented it out:
// Equipping a Weapon by clicking on it, and banking it if the bank is open:
public void TryHandleRightClick()
{
// if (isHandlingRightClick) return; // <-- this was the troublesome line
isHandlingRightClick = true;
InventoryItem item = inventory.GetItemInSlot(index);
int number = inventory.GetNumberInSlot(index);
Iâm sure thereâs a good reason this was there. I just donât remember what
and one last question, do we remove the âInventoryUpdatedâ event now?
And just a small heads-up, in âPickupFinder.csâ, you will want to refer to âIsPhysicalâ from âIInventorySource.csâ as follows:
bool IInventorySource.IsPhysical => false; // Pickup Inventory is dynamic, as the slots vanish the moment their content is consumed
if you forget âIInventorySourceâ, it wonât recognize it, and your pickup inventory wonât work (and there was a few traces of pooling in your âInventoryUI.csâ, which I ignored)
Other than that, this new system is blazingly fast, and I am incredibly happy, and I owe you a lot for just this one!
Anyway, time to move on to the next concept (I have less than 10 to go, and one of my previous ones (the NPC Fight-back logic in âEnemyStateMachine.OnTakenHitâ) needs a complete re-write because the logic is a bit of a complete mess right now, with a few unpredicted actions that Iâm struggling to hunt down happening here and there)
Iâll be honest⌠Iâm getting frustrated because your Inventory is so far off of the standard that everything keeps breaking.
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;
InventorySlotUpdated(i); //<<<-----
}
else break;
}
}
inventoryUpdated?.Invoke();
}
Just like above, youâll do InventorySlotUpdated in the middle of the for loop.
I canât blame you, I honestly did ask for a lot. I apologize for this, I truly understand that by now youâd be frustrated by me entirely, and I donât blame you, but I genuinely donât know who else to ask. Again, Iâm really sorry for all of this mess
Well after this whole inventory show, I had quite the run. In less than 24 hours. I integrated gliding, made sure that it needs to find a glider in your inventory, with an option to make this consumable, and damage from falling from high points in the game world (I also made sure you donât get damaged from jumping, by optimizing the value that determines the damage and making sure itâs not too low)
Iâm yet to fix a few bugs from my farming system though - thatâs what Iâll do next
And then I just noticed a major bug that Iâve never encountered before⌠DYNAMIC CRAFTING TABLES ARENâT SAVEABLE YET!