Inventory stacking stops after first time

Hi, I've been making an inventory system and it seems to be working well other than the item stacking. I am able to place a stackable item in, then add one to that stack but after that it stops working.

Here is my inventory code, it is line 138 which is causing me problems, when the item stack in the inventory list is 2 it stops incrementing the item count. what confuses me is that it runs the code for that section (137 - 140) but just doesn't seem to add to the inventoryList.currentStack. I'm sure it will probably be something daft that I missed but I can't for the life of me see it.

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3. using System.Collections.Generic;
4.  
5. [System.Serializable]
6.  
7. public class InventoryManager : MonoBehaviour {
8.  
9.     public List<Item> InventoryList = new List<Item>();
10.     ItemDatabase itemDatabase;
11.  
12.     public int inventorySize;
13.     // Use this for initialization
14.     void Start ()
15.     {
16.         inventorySize = 24;
17.  
18.         itemDatabase = GameObject.FindGameObjectWithTag("ItemDatabase").GetComponent<ItemDatabase>();
19.  
20.         for (int i = 0; i < inventorySize; i++)
21.         {
22.             InventoryList.Add(new Item());  
23.         }
24.  
25.     }
26.  
27.     // Update is called once per frame
28.     void Update()
29.     {
30.         if(Input.GetMouseButtonDown(0))
31.         {
32.             AddItem(1, 1);
33.         }
34.         if(Input.GetMouseButtonDown(1))
35.         {
36.             Debug.Log(itemDatabase.items[1].currentStack);
37.         }
38.  
39.         if(Input.GetKeyDown(KeyCode.A))
40.         {
41.             Debug.Log(InventoryList[0].itemName + " " + InventoryList[0].currentStack);
42.         }
43.         if (Input.GetKeyDown(KeyCode.S))
44.         {
45.             Debug.Log(InventoryList[1].itemName + " " + InventoryList[1].currentStack);
46.         }
47.         if (Input.GetKeyDown(KeyCode.D))
48.         {
49.             Debug.Log(InventoryList[2].itemName + " " + InventoryList[2].currentStack);
50.         }
51.         if (Input.GetKeyDown(KeyCode.F))
52.         {
53.             Debug.Log(InventoryList[3].itemName + " " + InventoryList[3].currentStack);
54.         }
55.     }
56.  
57.  
58.     void AddItem(int id, int amount)
59.     {
60.        
61.         for (int i = 0; i < itemDatabase.items.Count; i++)
62.         {
63.             if(itemDatabase.items[i].itemID == id)
64.             {
65.                 Item item = itemDatabase.items[i];
66.                 if(item.maxStack != -1)
67.                 {
68.                     item.currentStack = amount;
69.                 }
70.                 AddItemAtCorrectSlot(item);
71.                 break;
72.             }
73.         }
74.     }
75.    
76.  
77.     void AddItemAtCorrectSlot(Item newItem)
78.     {
79.         //Check If stackable
80.         if(newItem.maxStack == -1) //Not Stackable
81.         {
82.             for(int i = 0; i < InventoryList.Count; i++)
83.             {
84.                 if(InventoryList[i].itemName == null)
85.                 {
86.                     InventoryList[i] = newItem;
87.                     break;
88.                 }
89.             }
90.         }
91.         else if(newItem.maxStack != -1) //Stackable
92.         {
93.             bool isInInv = false;
94.             for(int i = 0; i < InventoryList.Count; i++)
95.             {
96.                 if(InventoryList[i].itemID == newItem.itemID)
97.                 {
98.                     isInInv = true;
99.                 }
100.             }
101.             if(isInInv == false) //Isn't in inventory
102.             {
103.                 Debug.Log("NotInInv");
104.                 for (int i = 0; i < InventoryList.Count; i++)
105.                 {
106.                    
107.                     if (InventoryList[i].itemName == null)
108.                     {
109.                        
110.                         InventoryList[i] = newItem;
111.                         break;
112.                     }
113.                 }
114.             }
115.             else //Is in inventory
116.             {
117.                 Debug.Log("IsInInv");
118.                 AddToStack(newItem);
119.                 return;
120.             }
121.         }
122.     }
123.  
124.     void AddToStack(Item stackItem)
125.     {
126.         for (int i = 0; i < InventoryList.Count; i++)
127.         {
128.             if (InventoryList[i].itemID == stackItem.itemID)
129.             {
130.                 if (InventoryList[i].currentStack != InventoryList[i].maxStack)
131.                 {
132.  
133.                     int space = InventoryList[i].maxStack - InventoryList[i].currentStack;
134.  
135.                     if (stackItem.currentStack <= space)
136.                     {
137.                         Debug.Log("Current stack added " + stackItem.currentStack);
138.                         InventoryList[i].currentStack += stackItem.currentStack;
139.                         Debug.Log("Current Stack" + InventoryList[i].currentStack);
140.                         break;
141.                     }
142.                     else
143.                     {
144.                         InventoryList[i].currentStack = InventoryList[i].maxStack;
145.                         stackItem.currentStack = stackItem.currentStack - space;
146.                         AddItem(stackItem.itemID, stackItem.currentStack);
147.                         break;
148.                     }
149.                 }
150.  
151.             }
152.         }
153.     }
154.  
155. }
156.  

Any help would be greatly appreciated.

 

Are you sure you want to `break;` there? That ends the loop. Maybe you mean `continue;` which goes to the next element in the loop, although you dont need it here at all.

 

Yeah, break is used when you want to break out of the loop. You have a lot of nested if's, so it might be hard to notice, but your break will go all the way up to your for loop and break out of it.

 

Thanks for the replies guys, I took the breaks out of it but unfortunately I'm still having the same problem. Might have to see if I can work out a simpler method for these checks as it is pretty messy and try just rewriting it.

 

You have several other breaks throughout your code. Are you sure these are all intended breaks?

 

You should use some LinQ syntax, this is helpfull for clean code and for performance :

Code (CSharp):

1.             Item targetItem = InventoryList.Where(oneItem => oneItem.itemID == stackItem.itemID).First();
2.             //First if you are sure the element is in the list, if not : exception
3.             //you can use FirstOrDefault, if the element is not in the list then return null
4.  
5.             if(targetItem.currentStack < targetItem.maxstack)
6.             {
7.                 int space = targetItem.maxStack - targetItem.currentStack;
8.                 ...
9.                 ...
10.  
11.             }

 

Yeah, the other breaks in the AddItemAtCorrectSlot() prevent it from filling the inventory with the one item as it stops once the slot has been filled.

I haven't come across this yet, I will take a look!

 

This current code creates the following problem: say your currentStack is at 4, and your max stack is 6. Your space will be at 2. So now, currentStack > space, and you will not add an item to your stack. Basically just do something like this:

Code (CSharp):

1.  
2.                 if (InventoryList[i].currentStack < InventoryList[i].maxStack)
3.                 {                  
4.                         Debug.Log("Current stack added " + stackItem.currentStack);
5.                         InventoryList[i].currentStack += stackItem.currentStack;
6.                         Debug.Log("Current Stack" + InventoryList[i].currentStack);
7.                         break;
8.                 }
9.  

 

I personally don't like List.Where(linq).First
I'd rather do
if(List.FindIndex(linq) >= 0)
It effectively does the same thing, but gives you the index of the entry rather than the entry itself

 

Cheers for the advice guys, I used the list.FindIndex method to help cut down my code to a 50 line function that covers all the possibilities it needs. Fixed the problem and works beautifully now, as well as being easy to read!