Bug in code, narrowed down to 'list.add'

I am simply trying to keep track of player positions and load relevant cells, however when I run the code it hangs/infLoops, and I can’t for the life of me figure out why. When I run the code with a Debug.Log it runs fine, but as soon as I re-enable the list.add() it freezes up. Any idea what could be the issue?

void UpdateRequiredCells()
    {
        AllRequiredCells = ActiveCells;
        for(int c = 0; c < ActiveCells.Count; c++)
        {
            for(int x = ActiveCells[c].x - SpawnRange; x < ActiveCells[c].x + SpawnRange; x++)
            {
                for(int y = ActiveCells[c].y - SpawnRange; y < ActiveCells[c].y + SpawnRange; y++)
                {
                    if( !AllRequiredCells.Contains( new Vector2Int(x, y) ) )
                    {
                        Debug.Log($"{x}, {y}");
                        //AllRequiredCells.Add(new Vector2Int(x, y));
                    }
                }
            }
        }
    }

The console displays all the relevant cell ID’s when run, but the AllRequiredCells.Add(xy) is causing the engine to hang/freeze. I tried adding a bool that activates to prevent a new update from starting it over and clearing it out prematurely, but it had no effect. Not really sure what’s the problem here.

Line 3 just makes AllRequiredCells point at the ActiveCells list. It isn’t a copy, it’s a reference.

Then you just keep adding to it, which keeps requiring the loop on line 4 to keep growing longer and longer: it’s the same list.

Line 3 can be made to take a copy with:

var AllRequiredCells = List<Vector2Int>( ActiveCells);
1 Like

Since activecells == allrequiredcells, you continually add new vectors to the same list you are iterating all elements.

1 Like

Aaaaand Kurt beat me to it. Again.

:slight_smile:

2 Likes

If you reverse the direction of the outer loop, it would end after a set number of iterations, yes. But can you prove that your logic is still correct? Kurt’s approach is safer and batter programming pratice: create a copy of the list (it’s quite small, the new list only copies references to the items in the original, it does notz also create copies of the elements), process the copy. Garbage collection in unity will take care of releasing the memory afterwards when you no longer need the temporary list.