Objects continue to be added to the List

Question

I have 2 Targets in the world that I want to add to a list when the Player comes close to them.

I first use the Physics OverlapBox method to return an array of colliders. After this, I run a for loop in which the 2 targets should get added to the list. Only 2 target objects are in the scene but the list gets occupied with hundreds of copies of those objects.

Code Down below

private void TrySelectTarget(bool switchInput)
{
    targetArray = Physics.OverlapBox(transform.position, range, Quaternion.identity, targetLayer, QueryTriggerInteraction.Ignore);
    for (int i = 0; i < targetArray.Length ; i++)
    {
        if (targetArray[i].TryGetComponent<Target>(out Target target))
        {
            availableTargets.Add(target);
            
        }
    }      
}

I did a deblug.Log on targetarray.Length and it returned 2, so I don't understand why so many objects are being added to the availableTargets List. I am calling the TrySelectTarget() method in Update().

I am new to c# and programming, so apologies if I am making a stupid mistake.

Thank you for the help.

I would guess your function is running every game tick - after you do this, are you UN-overlapping those objects? — Steve, Jan 16 '23 at 23:42
I am not doing anything after this. and I have no idea what un-overpping means. Could you please elaborate on that. — ImposterDev, Jan 16 '23 at 23:46
Ok, so ... your two things moved into an overlap condition. You check for this in your method, and if they are overlapping, you add something to the list. If this method is being called by Unity every frame, *are they still overlapping in the subsequent frame*? If they are, then you'll add another to the list. And again on the next frame - until your overlap condition is gone. — Steve, Jan 16 '23 at 23:48
Oh yea. They are still in range and hence are overlapping. I assumed that since the for loop would only run 2 times (since there are 2 targets), nothing more would be added to the list. I know I can use .Contains to check if the object is already in the list and so not to add it but I have read that it is not advisable to use .Find() / .Contains() in actual projects since they are very expensive. Is there an alternative method I could try?Thanks — ImposterDev, Jan 16 '23 at 23:54
Contains should not be expensive with such a small number of objects. — 500 - Internal Server Error, Jan 16 '23 at 23:59
You could change your `List` to a `HashSet` to ensure objects can't exist twice in your collection. I don't think `HashSet` is unity-serializable, though, if that matters. [Don't worry about performance if there aren't many objects though](https://stackoverflow.com/a/10762995/6572277). — , Jan 17 '23 at 00:06

score 1 · Answer 1 · answered Jan 17 '23 at 08:55

With your code everytime Physics.OverlapBox returns hits you add the same objects that have already references stored in the list, again. To simply solve the issue of not having duplicates being stored you should check if an object is already referenced (has an entry in the list). Do that by doing:

if (targetArray[i].TryGetComponent<Target>(out Target target))
{
    if (!availableTargets.Contains(target)) availableTargets.Add(target);
        
}

That will not solve the issue of targets not being removed when not in range anymore though. If that is needed then you should change your code so that the list gets cleared before any new references are being added. You could do:

availableTargets.Clear();
targetArray = Physics.OverlapBox(transform.position, range, Quaternion.identity, targetLayer, QueryTriggerInteraction.Ignore);
for (int i = 0; i < targetArray.Length ; i++)
...

The better solution to solve for this problem in general is to make use of OnTriggerEnter() and OnTriggerExit() messages provided through a Rigidbody component. Those Methods only get invoked if at least one of the interacting objects has a Rigidbody component. Add a Rigidbody to your player object and a collider with the size of the detection range/zone size and set this collider to be IsTrigger. If you dont want that physics affects the object just check the IsKinematic option on the Rigidbody component. In a script on the player then do:

private void OnTriggerEnter (Collider other)
{
    if (other.TryGetComponent<Target>(out Target target))
    {
        // check if not present already to be 100% sure not to get duplicates
        // even though it generally shouldn't be happening, better safe than sorry
        if (!availableTargets.Contains(target))
        {
            availableTargets.Add(target);
        }

    }
}

private void OnTriggerExit (Collider other)
{
    if (other.TryGetComponent<Target>(out Target target))
    {
        availableTargets.Remove(target);
    }
}

This works perfectly. Thank you. I would like to ask do you know a way to achieve the same functionality without using .Contain() as I have read it is very expensive. I have made my character controller using the the unity's built in one so using Rigidbody is a no go for me. I am not looking for the functionality it provides. — ImposterDev, Jan 18 '23 at 04:44
Things like that start to be a problem when you are handling collections with hundreds of thousands of items in them, at which point a list is not first choice anyway. Your list will definitely not be large enough for `.Contains()` to be a noticable factor, so don't worry about that. When you're using a CharacterController you can still add a rigidbody, this works perfectly fine as long as the rigidbody is set to be kinematic. I've done that many times just to get the unity messages that only come with the Rigidbody. I'm doing it the same way in the game I'm currently working on, no issue. — CBX_MG, Jan 19 '23 at 00:21

score 0 · Answer 2 · answered Jan 17 '23 at 07:25

I think you either want to

 availableTargets.Clear();

first so you start with an empty list every time, as anyway you seem to only be interested in the targets overlapping in that instance.

Or you could use Link and do e.g.

using System.Linq;

...

private void TrySelectTarget(bool switchInput)
{
    availableTargets = Physics.OverlapBox(transform.position, range, Quaternion.identity, targetLayer, QueryTriggerInteraction.Ignore)
        .Select(col => col.GetComponent<Target>())
        .Where(target => target)
        .ToList();
}

Objects continue to be added to the List

2 Answers2