I'm trying to create an inventory system. I have an Item class, and I want to change the visual appearance of the slot I create based on the data in this class, but it's not working. Where am I making a mistake?
This is my Inventory class, where I create a list and try to place the values from Item into the corresponding slots in the inventory. By the way, the inventory slots are prefabs.
public class Inventory : MonoBehaviour
{
public List<Item> InventoryList = new List<Item>();
[Header("InventoryUI")]
public GameObject InventorySlotPrefab;
public void Update()
{
if (Input.GetKeyDown(KeyCode.E))
{
DisplayItemsOnSlots();
}
}
public void AddItem(int _itemId,string _itemName, int _itemAmount, Sprite _sprite)
{
Item newItem = Item.CreateInstance<Item>();
newItem.SetItemDetails(_itemId,_itemName,_itemAmount, _sprite);
InventoryList.Add(newItem);
}
public void DisplayItemsOnSlots()
{
for (int i = 0; i < InventoryList.Count; i++)
{
Image itemImage = InventorySlotPrefab.GetComponentInChildren<Image>();
itemImage.sprite = InventoryList[i].sprite;
}
}
}
This is my item class, which extends ScriptableObject.
[CreateAssetMenu(fileName = "Item", menuName ="ItemSystem/BasicItem")]
public class Item : ScriptableObject
{
public int id;
public string name;
public int amount;
public Sprite sprite;
public void SetItemDetails(int _id, string _name, int _amount, Sprite _sprite)
{
this.id = _id;
this.name = _name;
this.amount = _amount;
this.sprite = _sprite;
}
}
In your loop
for (int i = 0; i < InventoryList.Count; i++)
{
Image itemImage = InventorySlotPrefab.GetComponentInChildren<Image>();
itemImage.sprite = InventoryList[i].sprite;
}
You are
sprite of the prefab,not of actual instances from the sceneInventoryList[i].sprite to the very same Image instance and thereby overwriting it -> if anything it would always end up with the last iteration's valueAs I mentioned on one of your previous questions I would rather do something like e.g.
[CreateAssetMenu(fileName = "Item", menuName ="ItemSystem/BasicItem")]
public class Item : ScriptableObject
{
public Sprite sprite;
// as this is an asset it already has a built-in "name" anyway ;)
}
Then in your component you can interact with
public class PickableItem : MonoBehaviour
{
// reference the according asset
public Item item;
// and configure the amount via Inpsector
[Min(1)] public int amount = 1;
}
then in your Inventory you store items and their amount. Either in a list
public class InventoryItem
{
public Item item;
public int amount;
}
public class Inventory : MonoBehaviour
{
public List<InventoryItem> InventoryList = new ();
// And then accordingly for adding new items
private void AddItem(Item item, int amount)
{
// try find existing inventory item which is based on given item
InventoryItem inventoryItem = null;
foreach(var i in InventoryList)
{
if(i.item == item)
{
inventoryItem = i;
break;
}
}
// Or the same using System.Linq;
//var inventoryItem = InventoryList.FirstOrDefault(i => i.item == item);
// or if none was found create a new one
if(inventoryItem == null)
{
inventoryItem = new InventoryItem();
InventoryList.Add(inventoryItem);
}
// then simply apply the amount
inventoryItem.amount += amount;
RefreshItemsOnSlots();
}
...
}
or you could actually also just go by a Dictionary and do
public class Inventory : MonoBehaviour
{
public Dictionary<Item, int> InventoryList = new ();
private void AddItem(Item item, int amount)
{
if(!InventoryList.ConainsKey(item))
{
InventoryList[item] = amount;
}
else
{
InventoryList[item] += amount;
}
RefreshItemsOnSlots();
}
...
}
and then for displaying the UI accordingly I would probably go for object pooling (meaning when you remove items you only display the UI) and event driven display (meaning update the UI everytime something changed). First I'd use a dedicated component on the slots like e.g.
public class InventorySlot : MonoBehaviour
{
public Image image;
public TMP_Text amountText;
}
public class Inventory : MonoBehaviour
{
... // the stuff above ;)
public RectTransform slotParent; // object to spawn slots under
public InventorySlot InventorySlotPrefab;
private readonly List<InventorySlot> slotInstances = new();
private void RefreshItemsOnSlots()
{
for (int i = 0; i < Mathf.Max(InventoryList.Count, slotInstances.Count); i++)
{
if(i < InventoryList.Count) // for the items in inventory
{
InventorySlot currentSlot;
if(i >= slotInstances.Count) // spawn required slot UI
{
currentSlot = Instatiate(InventorySlotPrefab, slotParent);
slotInstances.Add(currentSlot);
}
else // or re-use existing slot UI if enough available
{
currentSlot = slotInstances[i];
}
currentSlot.gameObject.SetActive(true);
currentSlot.image.sprite = InventoryList[i].item.sprite;
currentSlot.amountText = InventoryList[i].amount.ToString();
}
else
{
// hide remaining slot UI instances
// (could destroy as well - usually pooling is better in performance if done often)
slotInstances[i].gameObject.SetActive(false);
}
}
}
}
or for the dictionary approach can't use indexer so use separate loops for initialize and de-initialize
private void RefreshItemsOnSlots()
{
// for the items in inventory
for (int i = 0; i < InventoryList.Count; i++)
{
if(i >= slotInstances.Count) // spawn required slot UI
{
currentSlot = Instatiate(InventorySlotPrefab, slotParent);
slotInstances.Add(currentSlot);
}
else // or re-use existing slot UI if enough available
{
currentSlot = slotInstances[i];
}
currentSlot.gameObject.SetActive(true);
currentSlot.image.sprite = InventoryList[i].item.sprite;
currentSlot.amountText = InventoryList[i].amount.ToString();
}
// for remaining UI slots
for(var i = InventoryList.Count; i < slotInstances.Count; i++)
{
currentSlot = slotInstances[i];
currentSlot.gameObject.SetActive(false);
}
}
and then a last bit would be to also use/remove items from the inventory
public bool RemoveItem(Item item, int amount)
{
// try find existing inventory item which is based on given item
InventoryItem inventoryItem = null;
foreach(var i in InventoryList)
{
if(i.item == item)
{
inventoryItem = i;
break;
}
}
// Or the same using System.Linq;
//var inventoryItem = InventoryList.FirstOrDefault(i => i.item == item);
if(inventoryItem == null)
{
// don't have this item at all
return false;
}
if(inventoryItem.amount < amount)
{
// do not have enough items
return false;
}
inventoryItem.amount -= amount;
if(inventoryItem.amount <= 0)
{
InventoryList.Remove(inventoryItem);
}
RefreshItemsOnSlots();
}
and again same for he dictionary approach
public bool RemoveItem(Item item, int amount)
{
if(!InventoryList.ContainsKey(item))
{
// don't have that item at all
return false;
}
if(InventoryList[item] < amount)
{
// do not have enough items
return false;
}
InventoryList[item] -= amount;
if(InventoryList[item] <= 0)
{
InventoryList.Remove(item);
}
RefreshItemsOnSlots();
}
and finally to show/hide your inventory UI you would simply toggle the slotParent accordingly:
private void Update()
{
if(Input.GetKeyDown.KeyCode.E)
{
slotParent.SetActive(!slotParent.activeSelf);
}
}