[C#] Problems with my Inventory System script.

Hello guys!
I decided to make an inventory system that has stacking and what not. I have currently started to implement an AddItem() function that automatically finds the first free slot or the one that has the same item and still has place for the quantity that you add and places the item there. Everything is working except I can't figure out how to divide stacks into smaller pieces.

For example -
I have the function AddItem(int itemID, int quantity).

I do AddItem(1, 7). That adds 7 items with the ID number of 1.

The system finds the child of the game object ItemDatabase that is named after the itemID number and gets the <Item> component of the child. There we find out that the maxStack for the item with the itemID number 1 is 8.

The system finds that we already have that item in our inventory. Let's say that we already have 7.

What would I need to write for the system to fill up the slot that already has the item and then put the rest in the next slot and if it fills up the next one then just continue filling up slots untill all items are added?

This is my Inventoy.cs script -

Code (CSharp):

1. using UnityEngine;
2. using UnityEngine.UI;
3. using System.Collections;
4.  
5. public class Inventory : MonoBehaviour {
6.  
7.     public bool isOpen = false;
8.     public Transform inventory;
9.     public Transform[] slots;
10.  
11.     public Transform itemDatabase;
12.  
13.     void Update() {
14.         if (Input.GetKeyDown (KeyCode.I)) {
15.             if (isOpen == false) {
16.                 OpenInventory();
17.             } else if (isOpen == true) {
18.                 CloseInventory();
19.             }
20.         }
21.  
22.         if (Input.GetKeyDown (KeyCode.C)) {
23.             AddItem(1, 1);
24.         }
25.         if (Input.GetKeyDown (KeyCode.V)) {
26.             AddItem(1, 7);
27.         }
28.     }
29.  
30.     void OpenInventory() {
31.         inventory.gameObject.SetActive(true);
32.         isOpen = true;
33.     }
34.  
35.     void CloseInventory() {
36.         inventory.gameObject.SetActive(false);
37.         isOpen = false;
38.     }
39.  
40.     void AddItem(int itemID, int quantity) {
41.         for(int i = 0; i < slots.Length; i++) {
42.             if (itemDatabase.FindChild (itemID.ToString ()).GetComponent<Item> ().maxStack == 1) {
43.                 if (quantity == 1) {
44.                     if (slots [i].GetComponent<Slot> ().hasItem == false) {
45.                         slots [i].GetComponent<Slot> ().hasItem = true;
46.                         slots [i].GetComponent<Slot> ().itemID = itemID;
47.                         slots [i].GetComponent<Slot> ().quantity = quantity;
48.                         slots [i].FindChild ("ItemIcon").gameObject.SetActive (true);
49.                         slots [i].FindChild ("ItemIcon").GetComponent<Image> ().sprite = itemDatabase.FindChild (itemID.ToString ()).GetComponent<Item> ().itemIcon;
50.                         break;
51.                     }
52.                 } else if (quantity > itemDatabase.FindChild (itemID.ToString ()).GetComponent<Item> ().maxStack) {
53.                     for (int x = 0; x < quantity; x++) {
54.                         if (x != quantity) {
55.                             AddItem (itemID, 1);
56.                         } else {
57.                             break;
58.                         }
59.                     }
60.                     break;
61.                 }
62.             } else {
63.                 if (slots [i].GetComponent<Slot> ().itemID == itemID) {
64.                     if ((slots [i].GetComponent<Slot> ().quantity + quantity) <= itemDatabase.FindChild (itemID.ToString ()).GetComponent<Item> ().maxStack) {
65.                         slots [i].GetComponent<Slot> ().quantity = slots [i].GetComponent<Slot> ().quantity + quantity;
66.                         break;
67.                     } else {
68.                         // THIS IS WHERE IT'S A NICE PLACE TO CONTINUE THE CODE.
69.                     }
70.                 } else {
71.                     slots [i].GetComponent<Slot> ().hasItem = true;
72.                     slots [i].GetComponent<Slot> ().itemID = itemID;
73.                     slots [i].GetComponent<Slot> ().quantity = quantity;
74.                     slots [i].FindChild ("ItemIcon").gameObject.SetActive (true);
75.                     slots [i].FindChild ("ItemIcon").GetComponent<Image> ().sprite = itemDatabase.FindChild (itemID.ToString ()).GetComponent<Item> ().itemIcon;
76.                     break;
77.                 }
78.             }
79.         }
80.     }
81. }
82.  

This is my Slot.cs script -

Code (CSharp):

1. using UnityEngine;
2. using UnityEngine.UI;
3. using System.Collections;
4.  
5. public class Slot : MonoBehaviour {
6.  
7.     public bool hasItem = false;
8.  
9.     public int itemID = 0;
10.     public int quantity = 0;
11.  
12.     public Text QuantityText;
13.  
14.  
15.     void Start() {
16.         QuantityText = transform.FindChild ("QuantityText").GetComponent<Text> ();
17.     }
18.  
19.     void Update() {
20.         if (quantity != 0) {
21.             QuantityText.text = "x" + quantity;
22.         }
23.     }
24. }
25.  

This is my Item.cs script -

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3.  
4. public class Item : MonoBehaviour {
5.  
6.     [Header("Item Information Settings")]
7.     public Sprite itemIcon;
8.     public int itemID = 1;
9.  
10.     public string name = "Default Name";
11.     public string description = "This is the default description.";
12.  
13.     public int maxStack = 1;
14.  
15.     public enum itemTypes {
16.         Unusable,
17.         Weapon,
18.         Tool,
19.         Food,
20.         Buildable
21.     };
22.  
23.     public itemTypes itemType;
24.  
25. }
26.  

P.S Keep in mind that I'm just a beginner programmer so the code might be inefficient and dumb.

I really hope you can help me because I've been struggling with this for days.

- Daniel

 

Code (CSharp):

1. else {
2.                 if (slots [i].GetComponent<Slot> ().itemID == itemID) {
3.                     if ((slots [i].GetComponent<Slot> ().quantity + quantity) <= itemDatabase.FindChild (itemID.ToString ()).GetComponent<Item> ().maxStack) {
4.                         slots [i].GetComponent<Slot> ().quantity = slots [i].GetComponent<Slot> ().quantity + quantity;
5.                         break;
6.                     } else {
7.                         // THIS IS WHERE IT'S A NICE PLACE TO CONTINUE THE CODE.
8.                     }

I'd think you could do something real, real simple here. Simply doing a quick subtraction to get the leftover amount and assigning that amount to quantity after adding it to the slot's quantity.

Code (CSharp):

1. else {
2.                 if (slots [i].GetComponent<Slot> ().itemID == itemID) {
3.                     if ((slots [i].GetComponent<Slot> ().quantity + quantity) <= itemDatabase.FindChild (itemID.ToString ()).GetComponent<Item> ().maxStack) {
4.                         slots [i].GetComponent<Slot> ().quantity = slots [i].GetComponent<Slot> ().quantity + quantity;
5.                         break;
6.                     }
7.  
8. else {
9.                         // THIS IS WHERE IT'S A NICE PLACE TO CONTINUE THE CODE.
10.     int leftover = 0;
11.  
12.     leftover =  slots [i].GetComponent<Slot> ().quantity + quantity - itemDatabase.FindChild (itemID.ToString ()).GetComponent<Item> ().maxStack);
13.  
14.  
15.     slots [i].GetComponent<Slot> ().quantity = slots [i].GetComponent<Slot> ().quantity = itemDatabase.FindChild (itemID.ToString ()).GetComponent<Item> ().maxStack);
16.  
17.     quantity = leftover;
18.                     }

Temp variable to hold the remainder after maxing the first stack, then feed that back into quantity, and feed that back into the loop for the next iteration. That's quick and dirty and sloppy looking, but you get the code idea.

 

Daniel,

First off, I do think the code is inefficient, but it might not be for the reason you think - you're extending every part of your inventory system out of the MonoBehaviour class, and you're doing a lot of calls to functions like "GetComponent" and "Find", which not only are expensive, but also make the intent behind your code harder to read.

That said, I think you're moving in the right direction, but that you're probably getting a little lost in the complexity of your own implementation No worries, if you're just a beginner then you're already doing pretty well!

So, right now you're using a big bunch of if / else statements wrapped in for loops, wrapped in more if/else statements. While it looks like it's hitting the logic, it's definitely complicating things for you.

What I'd suggest is starting with something simpler, like this:

Code (CSharp):

1. // assume these variables were set above
2. // itemsToAdd <-- the number of items you want to add
3. // itemId <-- the id of the item you want to add
4. // itemsMax <-- the max number of this item you can add to a slot
5.  
6. foreach (Slot slot in slots)
7. {
8.     if ( slot.contains( itemID ) || slot.isEmpty() )
9.     {
10.         while ( slot.quantity() < itemsMax && itemsToAdd > 0 )
11.         {
12.             slot.addItem( itemId );
13.             itemsToAdd--;
14.         }
15.     }  
16. }
17.  
18. if ( itemsToAdd > 0 )
19. {
20.     // we have left over items - take care of them //
21. }
22.  

It's not perfect, but it should get you started. Of course - you'll have to rewrite it so that it works in your implementation, but hopefully it gives you a hint in the right direction. Good luck!

 

Thank you for the quick reply! I'll start to recode the whole thing tomorrow. If I'll have any questions I'll make sure to continue posting in this forum. Thank you.

 

apsdsm is definitely right, in that your code could be cut way, way, way down. I was just too lazy to go about trying to type up fresh code on the forum

You should definitely try and recode and trim it down to a single, simple loop like the one he shows above. His example should be a great start for you to work with.

 

I recoded it all and it works like a charm. Thank you too. At least you tried to help and that counts too.