Inventory System using enums [issue]

Hi there! I've been attempting to create an inventory system that allows the player to pick up objects from the ground and places them in a Hotbar, that aspect was easy enough, the issues I'm having now are getting items of the same type to stack when picked up. But if an object of a different type is picked up, it goes into a different slot. I've been struggling to get an enum loop to work, but I think using enums is the way to go. I'd love some help with this. With this current code, I get an "Index was outside the bounds of the array" error.

This is the code attached to any object that is supposed to be picked up by the player:

Code (CSharp):

1. using System.Collections;
2. using System.Collections.Generic;
3. using UnityEngine;
4.  
5. public class Pickup : MonoBehaviour
6. {
7.     public ItemType itemType;
8.  
9.  
10.  
11.  
12.     private Inventory inventory;
13.     public GameObject itemButton;
14.  
15.    
16.  
17.  
18.     private void Start()
19.     {
20.        
21.         inventory = GameObject.FindGameObjectWithTag("Player").GetComponent<Inventory>();
22.      
23.     }
24.    
25.     private void OnTriggerEnter2D(Collider2D other)
26.     {
27.      
28.  
29.         if (other.CompareTag("Player"))
30.         {
31.  
32.             for (int i = 0; i < inventory.slots.Length; i++)
33.             {
34.  
35.                 if (inventory.isFull[i] == false)
36.                 {
37.                         inventory.itemType[i] = this.itemType;
38.                         if (inventory.slots.Length > inventory.maxStack)
39.                         {
40.                             inventory.isFull[i] = true;
41.                         }
42.                    
43.                
44.                     Instantiate(itemButton, inventory.slots[i].transform, false);
45.                     Destroy(gameObject);
46.                     break;
47.  
48.                 }
49.             }
50.         }
51.     }
52.  
53.  
54. }
55.  
56. public enum ItemType
57. {
58.     empty = 0,
59.     Leather = 1,
60.     Paper = 2,
61.     Iron = 3,
62.     Gold = 4,
63.     Diamond = 5
64.  
65.  
66. }

And this is the code of the slots in the hotbar:

Code (CSharp):

1. using System.Collections;
2. using System.Collections.Generic;
3. using UnityEngine;
4.  
5. public class Slot : MonoBehaviour
6. {
7.     private Pickup pu;
8.     private Inventory inventory;
9.     public int i;
10.  
11.     private void Start()
12.     {
13.         inventory = GameObject.FindGameObjectWithTag("Player").GetComponent<Inventory>();
14.         pu = FindObjectOfType<Pickup>();
15.     }
16.     private void Update()
17.     {
18.         if(transform.childCount < inventory.maxStack)
19.         {
20.             inventory.isFull[i] = false;
21.         }
22.     }
23.  
24.     public void DropItem()
25.     {
26.         foreach (Transform child in transform)
27.         {
28.             child.GetComponent<Spawn>().SpawnDroppedItem();
29.             GameObject.Destroy(child.gameObject);
30.          
31.         }
32.     }
33.  
34. }
35.  

Annnd this is the code for the inventory:

Code (CSharp):

1. using System.Collections;
2. using System.Collections.Generic;
3. using UnityEngine;
4.  
5. public class Inventory : MonoBehaviour
6. {
7.     public ItemType[] itemType;
8.     public bool[] isFull;
9.     public GameObject[] slots;
10.     public int maxStack = 3;
11.  
12.  
13.  
14.  
15.  
16.  
17. }
18.  

Any tips are helpful! Thanks!

 

First steps, let's simplify and compartmentalize your logic. We can start by making Inventory a singleton, so that it can be easily accessed anywhere without having to worry about FindWithTag and all that. In Inventory, add:

Code (csharp):

1. public Inventory instance;
2. void Awake() {
3. instance = this;
4. }

Now, anywhere else, you can just call Inventory.instance.SomeFunction() or whatever.

Ideally, your OnTriggerEnter logic shouldn't need to know a thing about the inventory's inner workings. It really shouldn't be anything more than something like:

Code (csharp):

1. private void OnTriggerEnter2D(Collider2D other)
2.     {
3.         if (other.CompareTag("Player"))
4.         {
5.              Inventory.instance.PickupItem(this.itemType);
6.              Destroy(gameObject);
7.         }
8.     }

And then allow the Inventory script to do with that what it will. This way all the code that deals with that will be in one place.

So now, your inventory script needs all the logic to add items. So let's talk about what that logic should look like. Let's start with the data structure. I think confusion about the data structure is your fundamental problem; for example, Pickup line 38 appears to have confused "number of inventory slots" with "max number of stacked items in a slot", which is an issue. We're gonna scrap that logic though so don't worry about fixing it.

Anytime you have parallel arrays, you should probably think about making them into a class instead. In this case, you've already got a class (Slot), and you should just be storing this information there instead.

Code (csharp):

1. public class Slot : MonoBehaviour {
2. public ItemType itemType;
3. public int maxStack = 3;
4. public int currentStack = 0;
5. public bool isFull {
6.    get {
7.       return currentStack >= maxStack;
8.    }
9. }
10. }

You'll have an array public Slot[] slots; replacing your 3 arrays in your Inventory script, which you can set up in the inspector.

Now let's implement Inventory.PickupItem. This basically needs to go through each slot until it finds an appropriate one, then add to that count.

Code (csharp):

1. (public void PickupItem(ItemType thisType) {
2. for (int i=0; i < slots.Length; i++) {
3. if (slots[i].itemType == ItemType.empty) {
4. slots[i].itemType = thisType;
5. slots[i].currentStack = 1;
6. return;
7. }
8. if (slots[i].itemType == thisType && !slots[i].isFull) {
9. slots[i].currentStack++;
10. return;
11. }
12. }

Now, the Slot class will need to know how to update itself, including if it's empty.

This should get you started, and hopefully with a more clearly laid out data structure you should be able to figure out the other stuff you need. Feel free to come back with questions

 

Hey, thanks for the help! So I've done all I can see you said to do, but I'm still not sure how to tell the pickup which item is being collected. I rearranged the structure as you recommended, and now, when I collide with an object set to 'Paper' or 'empty' I get an error saying: "Object reference not set to an instance of an object" and it says the issue is in .cs23 which is this line:

Code (CSharp):

1.             if (slots[i].itemType == ItemType.empty)

Any idea as to why this is happening? Thanks!

 

I don't think that's changed from your code - it's still the itemType drop down in the pickup's inspector window. Unless I misunderstand your question?

If you get that it means your slots array has some empty slots. If you're not sure what that means, can you post a screenshot of what your "Inventory" inspector looks like?

 

My bad, the inspector was missing references. Okay, so here's what is happening now. The restructuring helped a ton. My only issue is when I pick up an object with a tag of 'Paper' it goes to an empty slot, but when I grab an object with a tag of "Leather" it goes into the same slot as the paper. I need a way for the slots to only allow an item of the same type to be inserted. Any ideas?

 

How does it go into the same slot? Do you mean it changes the itemType of the slot to leather and then thinks you have 2 leather rather than 1 paper and 1 leather?

Are you sure the itemType on the pickups was correctly set?

 

Yes, I've changed the item type on the pickup to Paper and one to Leather and they get sorted into the same slot.
Here's the revised pickup code:

Code (CSharp):

1. using System.Collections;
2. using System.Collections.Generic;
3. using UnityEngine;
4.  
5. public class Pickup : MonoBehaviour
6. {
7.     public ItemType itemType;
8.  
9.  
10.  
11.     public Inventory inventory;
12.  
13.     public GameObject itemButton;
14.    // public GameObject[] Items = new GameObject[(int)ItemType.maxValue];
15.  
16.     private void Start()
17.     {
18.  
19.  
20.  
21.     }
22.  
23.  
24.     private void OnTriggerEnter2D(Collider2D other)
25.     {
26.         if (other.CompareTag("Player"))
27.         {
28.             for (int i = 0; i < inventory.slots.Length; i++)
29.             {
30.  
31.                 Instantiate(itemButton, inventory.slots[i].transform, false);
32.  
33.                 inventory.instance.PickupItem(this.itemType);
34.                 Destroy(gameObject);
35.                 break;
36.             }
37.         }
38.     }
39.  
40. }
41.  
42.  
43.  
44.  
45. public enum ItemType
46. {
47.     empty = 0,
48.     Leather = 1,
49.     Paper = 2,
50.     Iron = 3,
51.     Gold = 4,
52.     Diamond = 5,
53.     maxValue = 2
54.  
55.  
56. }
57.  

Hold on, maybe they don't go into the same one, perhaps just the object instantiates into the same slot. I need the item to be visible in the hotbar, that's what the itemButton is, but I think the loop is sorting the item button to the closest slot. Odd.

 

What's that Instantiate call doing there? The item buttons should all be created (and set to empty) before you ever pick up any items.

 

I've fixed the issue, thanks for all the help! Sorry for being so slow at figuring it out.