How to remove items from a target list on a target object's defeat.

I’m trying to get some functionality out of a system I’ve made and I would like to know where I am going wrong. Essentially, an enemy has a list of targets which it checks for. I want to ensure that said enemies don’t target something that is dead.
The 2 modules that handle the target list are here:

	public void CheckTargets ()
	{
		m_TargetList.AddRange(GameObject.FindGameObjectsWithTag("Enemy"));
		if(!m_TargetList.Contains(GameObject.FindWithTag("Player")))
		{
			if (!GameObject.FindWithTag("Player").GetComponent<PlayerStatistics>().m_IsDefeated)
			{
				m_TargetList.Add(GameObject.FindWithTag("Player"));
			}
		}
		if(m_TargetList.Count > 0)
		{
			for (int i=0;i<m_TargetList.Count;++i)
			{
				if (m_TargetList *== this.gameObject)*
  •  		{*
    

m_TargetList.Remove(m_TargetList*);
_
}*_

if (m_TargetList == GameObject.FindWithTag(“Player”))
* {*
if(m_TargetList*.gameObject.GetComponent().m_IsDefeated)
_ {_
m_TargetList.Remove(m_TargetList);
_ }
}
}
}
}
----------
public GameObject ClosestTarget(List targetlist)
{
GameObject closest = null;
float distance = Mathf.Infinity;
Vector3 position = transform.position;
foreach (GameObject target in targetlist)
{
Vector3 diff = target.transform.position - position;
float currentDistance = diff.sqrMagnitude;
if (currentDistance < distance)
{
closest = target;
distance = currentDistance;
}
}
return closest;
}*_

@CobbledGames, I don’t see where your problem is, but I can generally see why you may have problems with this approach. I don’t mean that you;re doing the wrong thing, because there is no right way of doing things - just a lot of different ways. But if it were me I would simplify the approach. To preface these comments, I don’t have your full context.

First, it seems that you are building a target list for each enemy. Does one enemy have a list of targets that is different than the other? Based on the code, I don’t see how. If not, It would be better to have one target list shared by all enemies

Second, it seem you build the target list when you CheckEnemy. I don’t know exactly when that runs, but seems to me that it would be more efficient to build a list of targets at the start of your game, and simply maintain that list as more enemies or players appear or are removed.

Third, you build the list and then go through and remove gameobjects you don’t want. That is costly.

Lastly, from the code above it seems like you are pulling a lot of horses with all those GameObject.FindGameObjectsWithTag calls. Those more costly in terms of processor time than other methods of tracking instantiated gameobject

(side note: no need to take the square root of the magnitude to compare distances.)

If it were me, to simplify and make things efficient:

  1. Put all game objects, enemies and players, “into” the same list when they are instantiated. If you aren’t using prefabs and instantiating them, you should be! This will be your enemy targets list.

  2. Separate attributes that both enemies and players share, such as m_IsDefeated and put them into a new script (e.g. ‘EntityAttributes’) that is added to both enemies and game objects. That way, when you run through the list you don’t have to check if something is a player or not to access a different script

2.a. Add an ‘int enemyType’ to that EntityAttributes so you can distinguish between player and enemy, or even different types of enemies by comparing integers. You will set this when you instance the gameobject and add it to the list. You can create a Constant for each enemy and player type. LIke: ‘const enemy = 0’ and ‘const player = 1’ and use those identifiers in your code.

2.a.1 In this way, it is easy check, for example, if something needs to be skipped in a loop, like this.gameobject because the EntityAttributes script on this.gameobject contains it’s enemyType value, you just skip that value rather than checking if list == gameobject. It’s more efficient to compare enemyType == player.

2.b. Add ‘string ID’ attribute to EntityAttributes that holds a unique identifier for each attribute. Assign this you instance the attrubute, and also set each gameobject name to to the same identifier. You can keep a running counter to add to a base string, so that when you instance the gameobject you set the gameObjetInstance.name = “enemy_” + counter++, and set the ID in the EntityAttributes script to the same string.

2.b.1 that way its easy if you need to distinguish an individual list item in a loop, or where ever.

In sum, because you have one list shared by all enemies and players, you don’t constantly rebuild it. You don’t ever have to go through and remove objects when you want to pass the enemy list to the ClosestTarget. It’s easier to check if something is defeated.

This is a simplified version of an approach I’ve used several times, so I hope it helps!