Hi everyone,
So I am having trouble replacing a current item, a book, with its corresponding key you get from the book. This is all done in the inventory however I can’t seem to get my head around how I should approach this. Any help would be appreciated on this.
Okay, what is the data structure you’re using?
What is book, what is key you get from the book, in terms of code?
How is inventory organized, how do you access it normally.
It’s meaningless to explain a potential solution in terms of plain English.
Otherwise I’d just tell you “put the key you get from the book into inventory”
Or whatever, though I’m assuming that by key you actually refer to some code at this point.
Yes sorry about that. Im currently using a scriptable object the holds all my items .
In the terms of the book, I meant to say was a item called book when I currently interact with it, it is placed into my inventory. Im stuck on the issue of replacing the book in my inventory with a corresponding key i.e. book 1 has key 1, book 2 has key 2 etc.
In terms of code
public class Inventory : MonoBehaviour
{
#region Singleton
public static Inventory instance;
private void Awake()
{
if (instance != null)
{
Destroy(this.gameObject);
return;
}
instance = this;
DontDestroyOnLoad(this);
}
#endregion
public delegate void OnItemChanged();
public OnItemChanged onItemChangedCallBack;
public int space = 5;
public List<Items> items = new List<Items>();
public bool AddItem(Items item)
{
if(items.Count >= space)
{
Debug.Log("Not Enough room");
return false;
}
items.Add(item);
if(onItemChangedCallBack != null)
onItemChangedCallBack.Invoke();
return true;
}
public void RemoveItem(Items item)
{
items.Remove(item);
if (onItemChangedCallBack != null)
{
onItemChangedCallBack.Invoke();
}
}
}
[CreateAssetMenu(fileName = "New Item", menuName = "Inventory/Item")]
public class Items : ScriptableObject
{
public string itemName = "New Item";
public Sprite icon = null;
public virtual void Use()
{
}
}
public class InventoryUI : MonoBehaviour
{
Inventory inventory;
public Transform itemsParent;
public GameObject inventoryUI;
InventorySlot[] slots;
private void Start()
{
//inventory = Inventory.instance;
//inventory.onItemChangedCallBack += UpdateUI;
inventory = Inventory.instance;
inventory.onItemChangedCallBack += UpdateUI;
slots = itemsParent.GetComponentsInChildren<InventorySlot>();
inventoryUI.SetActive(false);
}
private void Update()
{
if(Input.GetButtonDown("Inventory"))
{
inventoryUI.SetActive(!inventoryUI.activeSelf);
}
}
void UpdateUI()
{
Debug.Log("Updating UI");
for(int i = 0; i < slots.Length; i++)
{
if (i < inventory.items.Count)
{
slots[i].AddItem(inventory.items[i]);
}
else
slots[i].ClearSlot();
}
}
}
[CreateAssetMenu(fileName = "Inventory", menuName = "Inventory/Books")]
public class Books : Items
{
private InventorySlot inventorySlot;
//getting a new item to find
public Items currentItem;
public Items newItem;
public override void Use()
{
//Replaces the current item with a new item based on what the array is?
}
}
public class InventorySlot : MonoBehaviour
{
Items item;
public Image icon;
public Text ItemName;
public Button itemButton;
private void Start()
{
icon.enabled = false;
ItemName.text = "";
itemButton.gameObject.SetActive(false);
}
public void AddItem(Items newItem)
{
item = newItem;
icon.sprite = item.icon;
icon.enabled = true;
ItemName.text = item.itemName;
itemButton.gameObject.SetActive(true);
}
public void ClearSlot()
{
item = null;
icon.sprite = null;
icon.enabled = false;
ItemName.text = "";
itemButton.gameObject.SetActive(false);
}
public void RemoveItem()
{
Inventory.instance.RemoveItem(item);
}
}
Ok, nice.
Well, you’re lacking the functionality of finding a specific item in your inventory.
Inventories should work in a doubly linked manner. I.e. if an item knows its slot, at the same time you should be able to check the slot and learn about the item.
Currently, I can see that your item knows about the slot, but your inventory doesn’t know anything about the items. You can only add new ones, or remove existing ones, and removing is also slot-agnostic.
Let’s do this differently.
using System;
using System.Collections.Generic;
public class Inventory {
List<Item> _items;
int _count;
public int ItemCount => _count;
int _limit;
public int Size => _limit;
public Inventory(int size) {
if(size <= 1) throw new ArgumentException();
_limit = size;
initItemList();
}
public Item this[int index] {
get => _items[index];
set => StoreAt(index, value);
}
private void initItemList() {
_items = new List<Item>(_limit);
for(int i = 0; i < _limit; i++) _items[i] = null;
_count = 0;
}
public bool IsEmptySlot(int index) => _items[index] == null;
public bool Contains(Item item) => _items.IndexOf(item) >= 0;
private int getEmptySlot() => _items.IndexOf(null);
public void Store(Item item) {
if(item == null) throw new ArgumentNullException();
if(_count == _limit) throw new InvalidOperationException("Limit has been reached");
int slot = getEmptySlot();
StoreAt(slot, item);
}
public bool TryStore(Item item) {
if(_count == _limit) return false;
int slot = getEmptySlot();
StoreAt(slot, item);
return true;
}
public void Empty(int index) => StoreAt(index, null);
public void StoreAt(int index, Item item) {
if(index < 0 || index >= _limit) throw new IndexOutOfRangeException();
bool wasEmpty = IsEmptySlot(index);
bool isEmpty = (item == null);
_items[index] = value;
if(wasEmpty && !isEmpty) _count++;
else if(!wasEmpty && isEmpty) _count--;
}
public Item Remove(Item item) {
if(item == null) throw new ArgumentNullException();
var slot = _items.IndexOf(item);
if(slot >= 0) { Empty(slot); return item; }
return null;
}
public bool RemoveAt(int index) {
bool wasEmpty = IsSlotEmpty(index);
Empty(index);
return !wasEmpty;
}
public void Clear() => initItemList();
public int FindFirstOf(Item item) {
if(item == null) throw new ArgumentNullException();
return _items.IndexOf(item);
}
public int[] FindMultiplesOf(Item item) {
if(item == null) throw new ArgumentNullException();
var result = new List<int>(_limit);
int index = -1;
while(index < _limit) {
index = _items.IndexOf(item, index + 1);
if(index < 0) break;
result.Add(index);
}
return result.ToArray();
}
}
Public Interface
Inventory(int size) - Creates new inventory with a set maximum of slots. Size must be greater than 0.
int ItemCount - Returns the count of used up slots in the inventory.
int Size - Returns the total count of slots in the inventory.
Item this[int index] - Sets or gets an item from the slot at index. May return null and may be set to null.
bool IsEmptySlot(int index) - Returns true if slot was empty (null), false otherwise.
bool Contains(Item item) - Returns true if the item exists in the inventory, false otherwise. Accepts null.
void Store(Item item) - Adds item to the first available empty slot. Throws error if no empty slots. Doesn’t accept null.
bool TryStore(Item item) - Adds item to the first available empty slot. Returns true on success, false otherwise. Doesn’t accept null.
void Empty(int index) - Empties slot at index.
void StoreAt(int index, Item item) - Sets an item into slot at index. Empties slot if null is used.
Item Remove(Item item) - Removes and returns an item if it existed in the inventory. Returns null if it wasn’t found. Doesn’t accept null for an item.
bool RemoveAt(int index) - Removes an item in slot at index. Returns true if slot wasn’t empty, false otherwise.
void Clear() - Clears the inventory of all items.
int FindFirstOf(Item item) - Returns the index of the first slot where item was found. Doesn’t accept null. Returns -1 if item was not found.
int[ ] FindMultiplesOf(Item item) - Returns an array of indices where instances of item were found. Doesn’t accept null. Returns empty array if nothing was found. Obsolete (see post #8)
This is only an extensive demo, basically.
It has an overwhelming amount of functionality just to showcase the possibilities and nominal features you’d expect from an inventory system. I don’t expect you to replace your code, but try to cherry pick what you need from this and adapt it to your code accordingly.
Tell me if you need a working example of how to use it. It should be simple enough.
Now, arguably there are better ways to do this, overall, especially if you expect your inventories to regularly have more than 30-ish slots, or if you have many of them, constantly in use. This should be enough, however, for a simple RPG style of game.
Please keep in mind I haven’t tested this at all, tell me if it’s buggy or something doesn’t work.
FindMultiplesOf might hang Unity in endless loop if I made a mistake. Save your work before trying it.
(edit: FindMultiplesOf has been replaced with a better method in the post #8, below)
Pay special attention to the fact that I intentionally support iterating through the inventory slots, and that there are several ways to check for the slot state, and two ways to add/remove items to slots. StoreAt is practically the core method, almost every other manipulating method relies on it so that I don’t repeat myself in code~~, except for Remove which could be slightly more versatile tbh~~. Finally the two Find methods let you search through the list, discovering where exactly the items are located.
It’s not perfect however! You can’t use this in real production code. It cannot handle different objects of the same type as it is right now, for example (edit: it does, but doesn’t provide useful and convenient methods to work with this, this is what I meant; e.g. if you want to find all instances of type Book, and not just a specific instance of an object; you can do this from the outside though; edit2: addressed in the next post). I would also prune a lot of redundant methods in order to streamline its usage. This is really just a thorough illustration of how to build a decent collection for this application.
With this design, you don’t really need to keep slots as separate data. The class can also be made generic to support arbitrary types. If you want to do this, change class declaration to public class Inventory<T> where T : class { and every occurence of the word Item with T (lines 6, 20, 26, and all method signatures).
You would use it like this
var inv = new Inventory<Item>(16); // if it's not generic, just omit <Item>
inv[5] = myBook;
inv.Store(myPen);
Debug.Log(inv.ItemCount);
for(int i = 0; i < inv.Size; i++) {
if(!inv.IsEmptySlot(i)) {
Debug.Log($"Slot {i} contains {inv[i].ToString()}"); // assumes that Item implements ToString()
}
}
Thanks mate, I will look over this and cherry pick what seems to work with my current code
I’ve just noticed that I made a mistake regarding Remove methods. No, it can’t work like that. In my case, the underlying list is treated as fixed in terms of its size, and calling its Remove will change the number of its elements, therefore it’s a hardcore mistake.
Here’s a fix (I’ve changed the original code to reflect this change)
(I’ve also changed Remove method to return the item if it was found; a more convenient design)
(Notice that RemoveAt implementation is much faster, because it skips having to find the item first)
public Item Remove(Item item) {
if(item == null) throw new ArgumentNullException();
var slot = _items.IndexOf(item);
if(slot >= 0) { Empty(slot); return item; }
return null;
}
public bool RemoveAt(int index) {
bool wasEmpty = IsSlotEmpty(index);
Empty(index);
return !wasEmpty;
}
And btw, what you originally wanted can be implemented as
inv[inv.FindFirstItemOf(myBook)] = myGrandpasClock;
Here’s a generic method to address this issue (it requires you to specify a type that derives from Item)
It’s very similar to how FindMultiplesOf works, and in fact, renders it obsolete
public int[] FindInstancesOfType<TItem>(int startingSlot = 0) where TItem : Item {
var result = new List<int>(_limit);
int index = startingSlot;
while(index < _limit) {
index = findNext<TItem>(index);
if(index < 0) break;
result.Add(index);
index++;
}
return result.ToArray();
// local function that replaces IndexOf
int findNext<TItem>(int startingSlot) where TItem : Item {
for(int i = startingSlot; i < _limit; i++) if(_items[i] is TItem) return i;
return -1;
}
}
Obviously use cases like this make it apparent that this approach isn’t useful if you want your inventory to be large, or if you use them all over the place, because this algorithm has the worst case complexity of O(n^2) because of the underlying list. (In fact, maybe I’m exaggerating, it’s hard to judge, I think it’s less complex than that, probably it’s O(nlog(n)) which isn’t nearly as bad.)
A more robust design would be completely different, and would likely throw you off due to being completely dissimilar to you code. Just be wary of this, and be modest about its size.
var inv = new Inventory<InventoryItem>(16); // if you use a generic version of the class
inv.Store(book)
inv.Store(pieceOfEight);
inv.Store(skeletonKey);
inv.Store(quill)
inv.Store(doubloon);
var items = inv.FindInstancesOfType<Coin>();
Debug.Log(items.Length); // 2
Debug.Log(items[0]); // 1
Debug.Log(items[1]); // 4
referring to objects that are of type Coin
public class Coin : InventoryItem {
// ...
}
You can do a similar thing by using interfaces, instead of derivation (in that case, the method would have to be rewritten as unconstrained). You can also make it to expect that your items implement a certain interface, and so it could be constrained only to this interface instead, and so on. I don’t expect you to know all of this, but just for the reference.