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\$
  • 1
    \$\begingroup\$ What line produces the error? \$\endgroup\$ – Vaillancourt Jul 24 '19 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 '19 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\$ – FlamesWillBurst Jul 24 '19 at 21:31
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);
}
| improve this answer | |
\$\endgroup\$
  • \$\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\$ – FlamesWillBurst Jul 24 '19 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 '19 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\$ – FlamesWillBurst Jul 24 '19 at 21:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.