Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

How to eliminate: InvalidOperationException: Collection was modified...

Discussion in 'Scripting' started by MadboyJames, Nov 14, 2017.

  1. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    Hi, so I'm getting an "InvalidOperationException: Collection was modified; enumeration operation may not execute." Error when I run my code. I think I know why I'm getting this: I'm modifying my list as I try and call it. But I don't know how to modify my code to remove it.

    Code (csharp):
    1.  
    2. using System.Collections;
    3. using System.Collections.Generic;
    4. using UnityEngine;
    5.  
    6. public class Room : MonoBehaviour
    7. {
    8.  
    9.     public static List<Room> rooms = new List<Room>();
    10.     public static Room currentRoom = null;
    11.  
    12.     void OnEnable() { rooms.Add(this); }
    13.     void OnDisable() { rooms.Remove(this); }
    14.  
    15.     private void Update()
    16.     {
    17.         CurrentRoomCheck();
    18.     }
    19.  
    20.     private void OnTriggerEnter2D(Collider2D collision)
    21.     {
    22.         Activate();
    23.         currentRoom = this;
    24.  
    25.         // turn off old rooms:
    26.         foreach (Room room in rooms)
    27.         {
    28.             if (room != this)
    29.                 room.Deactivate();
    30.         }
    31.     }
    32.  
    33.     private void OnTriggerExit2D(Collider2D collision)
    34.     {
    35.         // only turn off if we're the active room:
    36.         if (currentRoom == this)
    37.         {
    38.             Deactivate();
    39.             currentRoom = null;
    40.         }
    41.     }
    42.  
    43.  
    44.     // disabling the script
    45.     public void Activate()
    46.     {
    47.         enabled = true;
    48.     }
    49.     public void Deactivate()
    50.     {
    51.         enabled = false;
    52.     }
    53.  
    54.     //removes the item if it's not the most recent item
    55.     public void CurrentRoomCheck()
    56.     {
    57.         foreach (Room room in rooms)
    58.             if (room != rooms[rooms.Count-1] && rooms.Count>1)
    59.              rooms.Remove(this);
    60.     }
    61. }
    62.  
    I have been playing around with this code for about a day (generously donated by those more experienced than I) and quite frankly I'm a little embarrassed it took me as long as it did to figure out how to use it. At any rate I did get it to work, I just want to clean up the errors. Any help would be appreciated!
     
  2. whileBreak

    whileBreak

    Joined:
    Aug 28, 2014
    Posts:
    289
    Hi you could use a buffer list like this:
    Code (CSharp):
    1. List<Room> toDelete = new List<Room>(0);
    2.  
    3. foreach (Room room in rooms)
    4.             if (room != rooms[rooms.Count-1] && rooms.Count>1)
    5.              toDelete.Add( room);
    6.  
    7. foreach (Room room in toDelete)
    8.             rooms.Remove(room);
    Still i don't know if this
    Code (CSharp):
    1. rooms.Remove(this);
    was an error of yours (because i cant seem to find a logic to that), or if it was intentional.

    If it was intentional then you only need to add this to your code, since it would only remove one room each time the foreach loop is run
    Code (CSharp):
    1.  foreach (Room room in rooms)
    2.          if (room != rooms[rooms.Count-1] && rooms.Count>1)
    3.          {
    4.               rooms.Remove(this);
    5.               break;
    6.           }
    7.            
    The break just tells the loop to stop.
     
  3. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    Hi, Thank you! So the rooms.Remove(this) at the bottom was indeed intentional, but my logic could be fallacious. I want the room the player just entered to be the only room in the list, even if there is significant overlap between rooms, hence rooms.Remove(this); the break function does not stop the errors. I'm currently implementing the buffer list now.
     
  4. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    Would I then compare toDelete and rooms to see
    Code (csharp):
    1.  
    2. if (!toDelete.Contains(this))
    3. rooms.remove(this)
    4.  
    I think there is something I'm missing in my current logic/ thought process with this, I think that code is just a roundabout way of doing exactly what I was already doing?
     
  5. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    Or is that the kind of solution where it "just works"?
     
  6. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    Oh actually scratch both of those posts. I just put 2 and 2 together and answered my own questions. It still takes me a bit of time reading over code to actually "see" it (I look, but I do not see).
     
  7. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    Well, the buffer list reduces the amount of errors I get by half. This System.Collections.Generic.List`1+Enumerator[Room].VerifyState () still turns up after the InvalidOperationException: Collection was modified; enumeration operation may not execute. Maybe I'll be able to suss out how to VerifyState.
     
  8. GroZZleR

    GroZZleR

    Joined:
    Feb 1, 2015
    Posts:
    3,201
    You can't modify a collection you're iterating over in a foreach. You'll need to use a for loop instead but why are you maintaining a list at all if your intention is just to delete all but one every frame anyways?

    What's your *goal*?
     
  9. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    Well, my immediate goal is for the list to contain the Room the player last entered. After that I want to make each instance of the GameObject that Room is attached to have different specifications to send to the Main Camera. (e.g. a smaller room will have a smaller camera scale than the last room). I want some of these Rooms to overlap, hence my attention to figuring out which one was hit first on the list, or the "current" room.
     
  10. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    I want to delete all but one item so the camera knows which room to get information from.
     
  11. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    although, upon reflection I could probably just get the info from room[(rooms.count)-1] or something...
     
  12. GroZZleR

    GroZZleR

    Joined:
    Feb 1, 2015
    Posts:
    3,201
    Well the list maintains a collection of all rooms, while currentRoom tells you what the last room entered was, as the last room in the list isn't necessarily the last room entered. Surely that's enough information without having to needlessly modify the collection?
     
  13. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    erm, yeah. that's exactly right. Needless is indeed the correct word. So with that realization I used
    Code (csharp):
    1.  Room curRoom = Room.rooms[Room.rooms.Count - 1]; // gets whatever room we most recently hit
    to do what I've been trying to do for the last few days. And it works perfectly with no errors or anything.
     
  14. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    So I think my problem from the start is that I wasn't thinking like a programmer. Is there a word that means that? If there isn't there should be.
     
  15. MadboyJames

    MadboyJames

    Joined:
    Oct 28, 2017
    Posts:
    262
    At any rate I believe I am finally past this hurdle, and I am immensely grateful for your help.
     
  16. whileBreak

    whileBreak

    Joined:
    Aug 28, 2014
    Posts:
    289