Why the loop is not removing from the List all the items that are not starting with "CM"?

user_847881 0
Viewed 54
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using Cinemachine;
using System.Linq;

public class CamerasInfo : MonoBehaviour
{
    public List<CinemachineFreeLook> FreeLook;
    public List<CinemachineVirtualCamera> Virtual;
    public List<Camera> AllCameras;
    public List<string> currentActiveCameras;

    // Start is called before the first frame update
    void Start()
    {
        Cameras();
    }

    // Update is called once per frame
    void Update()
    {
        
    }

    public void Cameras()
    {
        FreeLook = FindObjectsOfType<CinemachineFreeLook>().ToList();
        Virtual = FindObjectsOfType<CinemachineVirtualCamera>().ToList();
        for(int i = 0; i < Virtual.Count; i++)
        {
            if (!Virtual[i].name.StartsWith("CM"))
                Virtual.RemoveAt(i);
        }

        AllCameras = Camera.allCameras.ToList();

        foreach (CinemachineFreeLook freelook in FreeLook)
        {
            if (freelook.isActiveAndEnabled)
                currentActiveCameras.Add(freelook.Name);
        }

        foreach (CinemachineVirtualCamera vir in Virtual)
        {
            if (vir.isActiveAndEnabled)
                currentActiveCameras.Add(vir.Name);
        }

        for(int i = 0; i < AllCameras.Count; i++)
        {
            if (AllCameras[i].isActiveAndEnabled)
                currentActiveCameras.Add(AllCameras[i].name);
        }
    }
}

At this part in this case there are 8 cameras listen in virtual :

for(int i = 0; i < Virtual.Count; i++)
            {
                if (!Virtual[i].name.StartsWith("CM"))
                    Virtual.RemoveAt(i);
            }

Cameras

But it's removing only 3 cameras instead 6. In the end I should left with only two cameras that starts with CM but in the editor there are 5 cameras :

It's not removing the Bottom Top and Middle :

Cameras

unity3d c#
3 Answers

The problem is that you're iterating forward through the list, while also removing items from it.

Let's take an example

 0       1       2
[TopRig, TopRig, CM]

We enter the for loop, i = 0. We look at element 0, which is TopRig, so we call .RemoveAt(0). Our list is now:

 0       1
[TopRig, CM]

We now enter the next iteration of the for loop, and i is increment to 1. We look at element 1 of the (modified) list, and see that it's CM.

See the problem? We skipped right over that second TopRig, because we removed an element from the list (causing all subsequent elements to be shifted backwards), and also increment i. We need to make sure that we only increment i if we don't remove an item from the list:

for (int i = 0; i < Virtual.Count; /* no increment */)
{
    if (!Virtual[i].name.StartsWith("CM"))
        Virtual.RemoveAt(i);
    else
        i++;
}

or account for the fact that it will be incremented:

for (int i = 0; i < Virtual.Count; i++)
{
    if (!Virtual[i].name.StartsWith("CM"))
    {
        Virtual.RemoveAt(i);
        i--;
    }
}

You can also loop through the list backwards, which sidesteps the issue:

for (int i = Virtual.Count - 1; i >= 0; i--)
{
    if (!Virtual[i].name.StartsWith("CM"))
        Virtual.RemoveAt(i);
}
user_10861210

Suppose you have 4 elements in a list. The first two start with "CM". You remove the first item and move to the second. But which item is "second" now? You have removed the first item, so everything shifts up one slot, and you "skip" the original second item.

One trick to removing items in a loop it to iterate backwards:

for(int i = Virtual.Count-1; i >= 0 ; i--)
{
    if (!Virtual[i].name.StartsWith("CM"))
        Virtual.RemoveAt(i);
}

Or you can find all of the items using Linq, then remove each item (instead of removing based on the index), but that's much less performant since the list must be iterated multiple times (once to find the items and once for each item to be removed).

user_10818970

The issue was already described in other answers: You remove items while iterating the list so you don't iterate through all the elements.

You could simply go the other way round and only add the items to the list that fulfill your condition:

var temp = FindObjectsOfType<CinemachineVirtualCamera>();
Virtual = new List<CinemachineVirtualCamera>();
foreach(var item in temp)
{
    if(item.name.StartsWith("CM"))
    {
        Virtual.Add(item);
    }
}

Using Linq Where you can filter your collection in one easy line (and O(n))

using System.Linq;

...

Virtual = FindObjectsOfType<CinemachineVirtualCamera>().Where(item => item.name.StartsWith("CM")).ToList();

This removes all items for which item.name.StartsWith("CM") returns true or better said, it only keeps those for which it returns false. So basically it dies exactly the same as the loop above.

user_71115610
Related