So I have a Script where After I Select a unit Which is within a List named “unSelectedUnitList”, move to another List named “SelectedUnitList”.
but I am getting an Error:
InvalidOperationException: Collection was modified; enumeration operation may not execute. System.ThrowHelper.ThrowInvalidOperationException (System.ExceptionResource resource) (at <599589bf4ce248909b8a14cbe4a2034e>:0) System.Collections.Generic.List1+Enumerator[T].MoveNextRare () (at <599589bf4ce248909b8a14cbe4a2034e>:0)* *System.Collections.Generic.List1+Enumerator[T].MoveNext () (at <599589bf4ce248909b8a14cbe4a2034e>:0) UnitSelectorDynamic.CheckAndSelectUnSelectedUnits () (at Assets/Scripts/UnitSelectorDynamic.cs:96) UnitSelectorDynamic.Update () (at Assets/Scripts/UnitSelectorDynamic.cs:71) My Code:--------------- ```csharp
*using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class Unit : MonoBehaviour
{
public GameObject selectorIndicator; // This is actually the game Object Circle Indicator that is placed within the script, so we are not actually turning off the script
void Start()
{
SetSelector(false); // This Turns the Selection circle off when game starts
SelectionManager.unitList.Add(this);
SelectionManager.unSelectedUnitList.Add(this);
}
public void SetSelector(bool on) // This is the switch function bool
{
selectorIndicator.SetActive(on);
moveToSelectedList();
}
public void moveToSelectedList()
{
SelectionManager.unSelectedUnitList.Remove(this);
SelectionManager.SelectedUnitList.Add(this);
}
}* * *-----------------------------------* *Removing the following removes the error:* *csharp SelectionManager.unSelectedUnitList.Remove(this); ``` Thank you In advance!
It’s not safe to modify a list while you are iterating over it in a foreach loop.
The typical way to work around this issue is to have a secondary list of “stuff you want to change”. While iterating the main list, you don’t actually make any changes, you just make a list of the stuff that you want to change. Then when you’re done, iterate over your list of stuff you want to change, and use that information to make the actual changes to the main list.
You also might think about reorganizing your data structures to sidestep the problem entirely. Do you really need a list of unselected units? Maybe you’d be better off with a list of ALL units plus a list (or a HashSet) of selected units, with any unit NOT in the “selected” collection implicitly being unselected. Or maybe even just put a boolean variable on each unit that says whether it’s selected or not. Then it would be safe to modify whether a unit is selected or not while you are iterating the list of all units.
@Antistone , Thank you for reply, The thing is, my script was working perfectly, Drag selecting units + Single Selecting Units, even Deselecting all units by clicking on space. But the Only thing I wanted to do, is to De-Select A single unit “assuming I already have a group of units that Have been selected” by Shift+clicking on it.
I tried like 10 different ways but nothing worked Primarily, because the way I single Select Units by "if(Input.GetMouseButtonDown(0)) " , always returns true and thus “else” Statement doesn’t work. Thats why, I am creating 2 lists and thus my "if(Input.GetMouseButtonDown(0)) " will effect the units Differently Depending on which List they are because I have the same script I linked above affecting 2 different Lists. (Second Script I have not worked on yet due to this error).
This is the Function that Always Returns true:
if (dist < 45)
{
myUnSelectedUnits.SetSelector(true);
}
It should basically iterate through a copy of the list. But you can modify the actual list. Depends how performant you want it to be. If you’ve got a big list, you probably don’t want to be copying it.
If you want it to be more efficient, I think you will need to remove items with their index number, not their value. You don’t want the program to have to go find the item once you’ve already found it.
And yeah I agree with Antistone, I’m not sure what you’re trying to do, but juggling two lists like this is probably a lot messier and and less performant than it needs to be.
Yes it will be a big list. Basically every Unit in this RTS game will be in the list. I am going to have to read what you said maybe 100x until I understand it haha. I am still in my learning process, Lists and stuff like this I really struggle with.
You can apparently get away from the issue of the program complaining about you changing the list length/order while looping through it, by looping in reverse.
You can probably do something like this, RemoveAt(index) is supposedly performant compared to other methods, but you need to know the index.
//Reverse For Loop
for (int x = SelectionManager.unSelectedUnitList.Count - 1; x >= 0; x--)
....
//Call function via item index, and tell the SetSelector the index of the item
SelectionManager.unSelectedUnitList[x].SetSelector(true, x);
....
public void SetSelector(bool on, int index)
{
selectorIndicator.SetActive(on);
//Pass the index to the function that removes the item
moveToSelectedList(index);
}
public void moveToSelectedList(int index)
{
//Remove item by index, not value
SelectionManager.unSelectedUnitList.RemoveAt(index);
SelectionManager.SelectedUnitList.Add(this);
}
The system won’t complain about it as long as you are using a for loop (not foreach), whether you go forward or in reverse–but that doesn’t necessarily mean that you’re safe, just that the compiler can’t tell if you’re doing something wrong. If you do this carelessly, you can end up skipping items or iterating the same item multiple times.
It is also possible to do it safely, but you need to reason through how exactly your list is changing and what that’s going to do to the indices of its contents.
@Antistone , But TBH, I still want to achieve this the way you explained it. So I am going to keep trying otherwise I am not learning something that I think is probably very important. Btw Despite how old my account is, I think I have cumulatively only been programming for 1 month. But I am trying to really learn everything as of 2 weeks ago. I am a healthcare professional myself but as a result of spine Injury that has made me bed bound, this seems like the way to go for me!
and I honestly Appreciate everyone who is helping me. I will give full credit to everyone if I am to start making videos on youtube about my learning journey.
It you only have a few dozen units, Lists are fine. But, Lists are arrays and removing from one is a loop – everything past the removed item needs to slide down to fill the gap. Removing 10 things from a size 100 list is roughly 500 steps (which is not that much, even through it’s bigger than it needs to be).
The fix is using a LinkedList. Removes from them aren’t loops. They work great and any Sophomore in Computer Science can easily explain how to loop through using a node pointer. But don’t even look at them until there’s a way-too-long pause when someone selects units (or if you just now slapped your head saying “gah! I can’t believe I didn’t realize C# had linked-lists!”)
@Owen-Reynolds , Thank you so much for this info, First time I hear of it. tbh I Dont feel any lag, but maybe thats coz my PC is made for processing and rendering. Would Love to learn what your talking about!!
That said, this is a RTS game, its very possible withing 10 seconds 1000-2000 units will get added and removed from the list considering every time a Unit gets selected it will get added and removed if deselected.