0
\$\begingroup\$

I have been working on my this aspect of my project since last night and have finally got this code to work exactly as I want it to.

However if the first target is removed before the second one, I get an out of range exception. I would like to know if there is a better way to fix my problem.

void LateUpdate()
{
    if (targets.Count == 0)
        return;
    Move();
    Zoom();
}

void Move()
{
    GetBoundsData();
    Vector3 newPosition = new Vector3(centerPoint.x + offset.x, transform.position.y, centerPoint.z + offset.z);
    transform.position = Vector3.SmoothDamp(transform.position, newPosition, ref velocity, smoothTime);
}

[SerializeField] Bounds bounds;
[SerializeField] Vector3 centerPoint;
public Vector3 maxBoundsPoints;

void GetBoundsData()
{
    if (targets[0] == null)
    {
        targets.RemoveAt(0);
    }
    if (targets.Count == 1)
        centerPoint = targets[0].position;
    SetUpBounds();
    centerPoint = bounds.center;
    maxBoundsPoints = bounds.max;

}

void SetUpBounds()
{
    bounds = new Bounds(targets[0].position, Vector3.zero);
    for (int i = 0; i < targets.Count; i++)
    {
        if (targets[i] == null)
        {
            targets.RemoveAt(i);
        }
        bounds.Encapsulate(targets[i].position);
    }
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ What line produces the error? \$\endgroup\$
    – Vaillancourt
    Jul 24, 2019 at 15:29
  • \$\begingroup\$ I don't know much about Unity Programming, but if it's the same as in Java an array[0] throws an out-of-range (out-of-bound in Java) if the array is completly empty (so no element at position 0, first position in the array). And there are many such calls in the given code, where is the problem exactly? \$\endgroup\$
    – Serverfrog
    Jul 24, 2019 at 15:37
  • \$\begingroup\$ Bounds bounds = new Bounds(lastPos, Vector3.zero); this line produces an error though i think may have a good solution now thanks to your well-done explanation look in the comment on the answer by DMGregory \$\endgroup\$ Jul 24, 2019 at 21:31

1 Answer 1

1
\$\begingroup\$

You have a problem in this loop:

for (int i = 0; i < targets.Count; i++)
{
    if (targets[i] == null)
    {
        targets.RemoveAt(i);
    }
    bounds.Encapsulate(targets[i].position);
}

Imagine that you have 5 items in your list, and the last one is null. At i = 4 we check our loop condition i < targets.Count, and get 4 < 5, which is true, so we enter the loop body. Then our if check detects that targets[i] is null, so we remove it and then continue executing the loop body.

Now our targets.Count is 4 (we removed the last entry), so when we call bounds.Encapsulate(targets[4].position) we're accessing past the end of the list.

This code also has errors if you have two consecutive nulls in your list, where you check for the first null and remove it, then run headlong into a null reference exception trying to access .position on the next entry which is also null.

So in general, you want to avoid removing items from a collection that you're in the middle of iterating. Instead, prune the bad data out first, then iterate only the good data:

// Remove all nulls from the list in one pass.
targets.RemoveAll(item => item == null);

// Now you have a list without nulls you can iterate without extra checks.
foreach(var item in targets) {
    bounds.Encapsulate(item.position);
}
\$\endgroup\$
3
  • \$\begingroup\$ Thank you for your answer to work around my problem I actually created a for loop that removes null entries but now I have something better to work with. though there is still a problem when the list is empty and the bounds are reinitialised it still throws an error though to by bypass this I used and is the statement. which check is if(targets.Count!=0) { lastPos=targets[0].position; } Bounds bounds=new Bounds(lastPos,Vector3.zero); this seems to have solved my problem but i am still not statisfied with my code \$\endgroup\$ Jul 24, 2019 at 21:29
  • \$\begingroup\$ If your code works but you'd like to solicit suggestions to improve it, we have a Code Review StackExchange for that very purpose. :) \$\endgroup\$
    – DMGregory
    Jul 24, 2019 at 21:34
  • \$\begingroup\$ Thank you I Will look into that.though I will work on it for some time before I post anything there. \$\endgroup\$ Jul 24, 2019 at 21:37

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .