why isn't my lock working

Question

hey all, hope ya'll r well today ihave been looking through documentation and others answers but i cant make to seem any sence out if them or well not a case specific answer but what i have come up with is for my foreach loop is placing a lock on it so i can remove items so far i have come up with this for my dictionary removal but i am getting an error and from what i am understanding after reading alot of peoples questions and solutions that i am placing the lock in there correctly but.... Aparently not

     public void Remove()
       {
          List<SelectedInfo> selectedObjects = playerController.GetSelected();
          
          foreach (SelectedInfo unit in selectedObjects)
          {
             if (unit.isSelected)
             {
                lock (groupLookupCache)
                {
                   foreach (var item in groupLookupCache)
                   {
                      for (int i = 0; i < item.Key.Length; i++)
                      {
                         this.groupLookupCache.Remove(item.Key);
                      }
                   }
                }
             }
          }
       }

this is the error i am reciving InvalidOperationException: Collection was modified; enumeration operation may not execute. System.Collections.Generic.Dictionary`2+Enumerator[TKey,TValue].MoveNext () (at :0) RPG.Inventories.UnitSelectionGroup.Remove () (at Assets/Scripts/Inventories/UnitSelectionGroup.cs:95) RPG.Inventories.UnitSelectionGroup.Update () (at Assets/Scripts/Inventories/UnitSelectionGroup.cs:42)

would someone be able to point me in the right direction maybe some good videos of locks or removal of keys while itereating

any help would be appreciated i am making a case specific question because im having trouble understanding some of the finer details that others are talking about (my key is also a component)

Answer 1

Best Answer

Answer by AlirezaSH2004 · May 17, 2020 at 02:47 AM

Hello again. Foreach loop is not going to be able to continue it's work when it's modified, meaning that something has been removed, added, etc. I recommend you instantiate an identical dictionary and manipulate that and then apply the changes to the actual dictionary of your preference. So:

       foreach (SelectedInfo unit in selectedObjects)
       {
          if (unit.isSelected)
          {
                 var tc = new Dictionary<YOURDATATYPE, YOURDATATYPE>(groupLookupCache);
                 for (i = 0; i < tc.Count; i++)
                 {
                          for (j = 0; j < tc[i].Key.Length; j++)
                          {
                                   groupLookupCache.Remove(item.Key);
                          }
                 }
           }
     }

You could also go with a foreach loop.

Add comment · Show 7 · Share

scradsleep · May 17, 2020 at 04:09 AM 0

Share

. hello again you have been a tremedous help!

im getting an error on line 8 tc[i] 'i' being the error because my key is an array in the dictionary declaration? and then another one on line 10 with item because item was only used because of the foreach loop and hasn't got anyuse elsewhere

alt text

i understand what you where saying about the loop removing the object and it not being able to work when it is being modified but i thought that the lock was to stop that because that's what i thought the lock was doing was preserving a state so as to that error to not occur because it holds the for each loop like it would if we where to instantiate another dictionary.

alt text

im not sure if what i am doing with the dictionary here is bad practise or not im guessing it is because im getting a lot of problems, im so so new to coding only done a few tutorials and hacking away at someones base code and looking at unity answers and stack overflow to learn i think i had to make the dictionary component an array is because of the for loop in the other question you helped me out on i had it as a for each loop but couldn't get it to work and now i have it as an array with the for loop, that we had to ad the item and the capital k (3days if i new that capital k by the way cos i was trying item.)

i gotta go out for a bit... but ill try change it back to the foreach loop and remove the array and nut out how to add this temp dict to the main dict im cluless as to that too but im sure ill be able to find something somewhere, would you $$anonymous$$d showing me as the forEach loop as you said i could go with i tried adding a for each loop in and it was just getting the same error again cos it was manipulating the temporary dictionary

sorry im a total noob at all this i realy appreciate your time

capture.png (12.6 kB)

an-bg.png (14.1 kB)

scradsleep scradsleep · May 17, 2020 at 07:17 AM 0

Share

hello me again now have this (thanks to your help)

public void Remove() { List selectedObjects = playerController.GetSelected();

          foreach (SelectedInfo unit in selectedObjects)
          {
             var items = new Dictionary<Component[], Component>(groupLookupCache);
             {
                foreach (var item in items)
                {
                   if (groupLookupCache.TryGetValue(item.Key, out Component value))
                   {
                      for (int j = 0; j < item.Key.Length; j++)
                      {
                         if (unit.isSelected == true)
                         {
                            groupLookupCache.Remove(item.Key);
                         }
                      }
                   }
                }
             }
          }
       }

only problem is that if statement on the end aint working and its removing the whole group.... could be a feature for now i dont $$anonymous$$d at leaset i can now group and degroup the units, might come back to that as a polish thing so that only what you have selected gets removed. no biggie

thankyou for everything my friend ill immortalise you as an npc im thinking a robot with the name like @AlirezaSH2004 "bleep bloop" tyvm!!!

AlirezaSH2004 scradsleep · May 17, 2020 at 12:13 PM 0

Share

Hello there. I’m happy that I’ve helped. Thanks bro, my username says 2004, cause I’ve been born in 2004, that’s not some arbitrary stuff (Grinning Face With Sweat). Good luck with the rest of your project.

Show more comments

AlirezaSH2004 · May 17, 2020 at 06:04 AM 1

Share

As you’re comfortable with foreach statement:

 foreach (SelectedInfo unit in selectedObjects)
           {
              if (unit.isSelected)
              {
                    var tc = new Dictionary<Component[], Component>(groupLookupCache);
                    foreach (var item in tc)
                    {
                       for (int i = 0; i < item.Key.Length; i++)
                       {
                          this.groupLookupCache.Remove(item.Key);
                       }
                    }
                 }
              }

why isn't my lock working

1 Reply

Your answer

Follow this Question

Related Questions