Avoiding/Removing the InvalidOperationException Error.

I've written a chunk of code in a foreach statement, so that I can add and remove items to a List, and whilst they are on the list they will move at a consistent rate and will remove themselves from the list when they reach their destination. I've encountered a problem, however, where Unity is telling me off and pausing every time the list becomes empty, with the following error:



I will attach the code below (although it's fairly long and probably a bit laborious to read through), but even by putting an if-statement around the entire foreach loop (ensuring that it only tries to run whilst there are GameObjects in the list), it still kicks off with the error every time the list becomes empty..

Any suggestions or workarounds for this problem would be much appreciated. Code attached below:

Code (CSharp):

1. using System.Collections;
2. using System.Collections.Generic;
3. using UnityEngine;
4.  
5. public class GameManager : MonoBehaviour {
6.  
7.     public static List<int> cardStack1 = new List<int>();
8.     public static List<int> cardStack2 = new List<int>();
9.     public static List<int> cardStack3 = new List<int>();
10.     public static List<int> cardStack4 = new List<int>();
11.  
12.     public GameObject[] cardPosons;
13.     public static GameObject[] cardPosns;
14.  
15.     //each script can choose speed. Most(if not all) will pull this value.
16.     public static float cardSpeed = 5;
17.  
18.  
19.     private static List<GameObject> movingCards = new List<GameObject>();
20.     private static List<float> movingCardSpeed = new List<float>();
21.     private static List<Vector3> movingCardGoal = new List<Vector3>();
22.     private static float speedMultiplier = 0.1f;
23.  
24.  
25.     void Start() {
26.         cardPosns = cardPosons;
27.     }
28.  
29.     void Update() {
30.         StepMovement();
31.     }
32.  
33.     private static void StepMovement() {
34.         int counter = -1;
35.         if (movingCards != null) {
36.             foreach (GameObject g in movingCards) {
37.                 counter++;
38.                 Transform currentLocal = movingCards[counter].transform;
39.                 float speed = movingCardSpeed[counter] * speedMultiplier;
40.                 Vector3 remainingDistance = movingCardGoal[counter] - movingCards[counter].transform.position;
41.  
42.                 // Individualize remainingDistance's floats, turn all positive.
43.                 float remX = remainingDistance.x; if (remX < 0) { remX = -remX; }
44.                 float remY = remainingDistance.y; if (remY < 0) { remY = -remY; }
45.                 float remZ = remainingDistance.z; if (remZ < 0) { remZ = -remZ; }
46.  
47.                 float maxAxisLength = 0;
48.                 if (remX >= remY && remX >= remZ) { maxAxisLength = remX; }
49.                 if (remY >= remX && remY >= remZ) { maxAxisLength = remY; }
50.                 if (remZ >= remX && remZ >= remY) { maxAxisLength = remZ; }
51.  
52.                 if (maxAxisLength == 0) { print("Failure lines 41-44, remainingDistance: " + remainingDistance); }
53.  
54.                 if (speed > maxAxisLength) { speed = maxAxisLength; }
55.  
56.                 float xResult = speed * (remainingDistance.x / maxAxisLength);
57.                 float yResult = speed * (remainingDistance.y / maxAxisLength);
58.                 float zResult = speed * (remainingDistance.z / maxAxisLength);
59.  
60.                 currentLocal.position += new Vector3(xResult, yResult, zResult);
61.                 movingCards[counter].transform.position = currentLocal.position;
62.  
63.                 if (currentLocal.position == movingCardGoal[counter]) {
64.                     // Attempt to remove gameobject, speed and goal from respective lists on completion.
65.                     movingCards.Remove(movingCards[counter]);
66.                     movingCardSpeed.Remove(movingCardSpeed[counter]);
67.                     movingCardGoal.Remove(movingCardGoal[counter]);
68.                 }
69.             }
70.         }
71.     }
72.  
73.     public static void MoveCard(GameObject card, int goal, float speed) {
74.         Transform cardStartPosition = card.transform;
75.         Transform goalLocation = cardPosns[goal].transform;
76.         float posOfset = card.GetComponent<Card>().CardHeightOffset();
77.  
78.         Vector3 ofsetPosition = new Vector3(goalLocation.position.x, goalLocation.position.y + +posOfset, goalLocation.position.z);
79.  
80.         if (speed == 0) {
81.             card.transform.position = ofsetPosition;
82.             card.transform.rotation = goalLocation.rotation;
83.         }
84.         else {
85.             movingCards.Add(card);
86.             movingCardSpeed.Add(speed);
87.             movingCardGoal.Add(ofsetPosition);
88.         }
89.     }
90. }

 

It's against the rules to modify a collection if you're iterating through it with an enumerator.

 

You can either switch to a for-loop, since you use a counter anyway. You'd then have to adjust the index for the iteration whenever you insert or remove elements previous to the iteration index, so that you prevent the loop from skipping elements, running out of bounds and handling an element twice.

Or simply create a shallow copy of the list and manipulate the original while you iterate the copied list.

I would also recommend to create a seperate type for the lists that you're trying to keep in sync. That would simplify your code as well as eliminate a little of overhead, as you'd only manipulate a list of the new type, not N lists that you're trying to keep in sync.

 

Found a nice solution, by just replacing the foreach loop with a for-loop, yet not needing to index the lists:

On line 36,
replaced: foreach (GameObject g in movingCards) {
with this: for (int i = 0; i < movingCards.Count; i++) {

depressingly was that simple. Thanks for the help though!