Switching Targets after actor is destroyed

[Counterpart0]

I'm trying to make a very basic turret that fires at an enemy as they approach. I got it to attack the first enemy that approaches, but when it destroys the target it continues to fire in the same direction.

What am I doing wrong?

AwesomeTurret.uc

class AwesomeTurret extends AwesomeActor;

var actor Enemy;
var float AttackDistance;
var bool bAttacking;

function GetEnemyAI()
{
    local AwesomeEnemy AE;

    foreach DynamicActors(class'AwesomeEnemy', AE)
        {
            if(AE != none)
            Enemy = AE;
        }
}

auto state Seeking
{
    function Tick(float DeltaTime)
    {
        if(Enemy == none)
              `log("Turret is Seeking!!!!!!!!!!!!!!!");
               GetEnemyAI();

        if(Enemy != none)
                GoToState('Attacking');
        else
                GoToState('Seeking');
    }
}

state Attacking
{

}

defaultproperties
{
    AttackDistance=45.0

    Begin Object Class=DynamicLightEnvironmentComponent Name=MyLightEnvironment
        bEnabled=True
    End Object
    Components.Add(MyLightEnvironment)

     Begin Object Class=StaticMeshComponent Name=TurretMesh
        StaticMesh=StaticMesh'Pickups.Health_Large.Mesh.S_Pickups_Base_Health_Large'
        LightEnvironment=MylightEnvironment
    End Object
    Components.Add(TurretMesh)

    Begin Object Class=CylinderComponent Name=CollisionCylinder
        CollisionRadius=15.0
        CollisionHeight=15.0
        BlockNonZeroExtent=True
        BlockZeroExtent=True
        BlockActors=True
        CollideActors=True
    End Object
    CollisionComponent=CollisionCylinder
    Components.Add(CollisionCylinder)
}

AwesomeTurret_Rocket.uc

class AwesomeTurret_Rocket extends AwesomeTurret
        placeable;

var actor CurrentEnemy;

auto state Seeking
{
    function Seeking()
    {
        CurrentEnemy = Enemy;
    }
}

state Attacking
{
    function BeginState(name PreviousStateName)
    {
        SetTimer(1, false, 'Attack');

    }
        function Attack()
        {
            local UTProj_Rocket MyRocket;

            if(CurrentEnemy == none)
               GetEnemyAI();

            if(CurrentEnemy != none)
                bAttacking=True;
                MyRocket = spawn(class'UTProj_Rocket', self,, Location);
                MyRocket.Init(normal(Enemy.Location - Location));
                SetTimer(2, false, 'Seeking');
        }
}

defaultproperties
{
    AttackDistance=45.0
}

[Spoof]

There's no mechanism to release the reference to Enemy.

Enemy is set to the last enemy it finds in DynamicActors, and from that point on Enemy is always valid even when DynamicActors is empty. When the final target is destroyed it gets removed from the world - but it can't be garbage collected because Enemy still holds a reference to it, so it remains in memory and the code still accesses the old Enemy.Location

Simple solution is to set Enemy = None before iterating over DynamicActors.

function GetEnemyAI()
{
    local AwesomeEnemy AE;

    Enemy = None;

    foreach DynamicActors(class'AwesomeEnemy', AE)
    {
        if(AE != none)
            Enemy = AE;
    }
}

Edit: wait... (sips his tea, still waking up)... just noticed you only call GetEnemyAI from the subclass. You need to remove the conditional check for (CurrentEnemy == None) from the Attack() function and poll GetEnemyAI() regardless.

[Spoof]

It's also unnecessary to validate AE in the loop, the inner codeblock will only execute if it is valid.

foreach DynamicActors(class'AwesomeEnemy', AE)
{
    Enemy = AE;
}

[Arthr702]

Spoof are right. Set them to none is the one site. You should make a break; in your foreach-loop. For every DynamicActor the Enemy will set a reference to it. When the loop is finished you will get the last Actor of type DynamicActor each time. Don't know if you want that.

Next thing: In your Turret class, you have to check if the Enemy is alive. For now there are an endless loop. Try te following code in your state:

state Attacking:

if (Enemy == none)
{
     GoToState('Seeking');
}

[Spoof]

I think you'd be better off consolidating Enemy and CurrentEnemy into one property, or you may encounter complications as the code grows. You can rewrite the GetEnemyAI function to work with local variables and return it's result:

function AwesomeEnemy GetEnemyAI()
{
    local AwesomeEnemy AE, target;

    foreach DynamicActors(class'AwesomeEnemy', AE)
    {
        // Later on you can perform additional checks to pick the best
        // target while iterating over all potential enemies.
        target = AE;
        break; // for now, just accept the first one found and stop iterating
    }
    return target;
}

In your attack function...

Enemy = GetEnemyAI();

... which will automatically give you a None reference when there's no target.

[Counterpart0]

Well that makes a lot of sense. No need to check if the iterator found one, just set it to a var when it does find one. Also, didn't realize that Enemy was keeping a reference to the last dynamicactor regardless of whether it had been destroyed or not. CurrentEnemy was a desperate attempt to isolate the actor that the foreach iterator had found...I was hoping to clear out the Enemy var some how, but couldn't figure out how to do it. Setting Enemy to none within the iterator should do nicely.

If, in the Attack state, I set the code to:

Enemy = GetEnemyAI()

and in GetEnemyAI() I set Enemy=none at the beginning of the loop then the turret should continuously find new targets, will it wait to see if the target has been destroyed before moving on to the next target? At the moment none of the enemies have any health, the TakeDamage event simply calls Destroy() when damage is caused. If I set the enemy with a health system, say Health=1 to start, can I then check the Enemy Health from with the AwesomeTurret_Rocket subclass to see if it is destroyed?

[VendorX]

function AwesomeEnemy GetEnemyAI()
{
	local AwesomeEnemy AE;

	foreach DynamicActors(class'AwesomeEnemy', AE)
		// Skip if we can't see him and/or is dead...
		if(Pawn.LineOfSightTo(AE) && AE.IsAliveAndWell())
			return AE;

	return None;
}

Don't bother about setting Enemy to None - GetEnemyAI will take care of that.

	// Call iterator only if Enemy is None or dead...
	if(Enemy == None || !Enemy.IsAliveAndWell())
		Enemy = GetEnemyAI();

Last thing:

auto state Seeking
{
    function Seeking()
    {
        CurrentEnemy = Enemy;
    }
}

...rename state or function...

[Spoof]

There are different ways to do it.

One way is to place all the functionality into GetEnemyAI(), and then whatever it returns is the currently valid target or None. You then simply call GetEnemyAI() each time the turret needs to shoot. However, it may be a little nieve and will likely result in the turret switching targets erratically if there are similar candidates. It's also less optimal because it searches for targets each time it shoots.

A better way is to lock onto a target. When the turret is 'seeking' you periodically call GetEnemyAI() until it returns something other than None. Then you set the turret's Target property (CurrentEnemy, Enemy, or whatever you want to call it) and raise the state to Attacking. In the attacking state you need to check if the target is still alive, still in range, still in line of sight, etc each time it fires. If the lock breaks for any of those reasons you clear CurrentTarget (None) and drop back into Seeking state.

Either way, it's good practice to have functions like GetEnemyAI perform their task internally, without storing information in global properties. You then get a very simple, robust pattern such as:

Target = GetEnemyCandidate();

if( Target != None )
{
    // valid target, go to work...
    GotoState('Attacking');
}
else
{
    // ho hum, keep spinning around looking for trouble...
}

It may even be useful to have a dedicated function for validating the current target...

function bool IsValidTarget( AwesomeEnemy AE )
{
    if( AE != None && AE.Health > 0 && VSizeSQ(Location - AE.Location) < MaxRangeSQ )
        return true;
    return false;
}

... but you could just as easily inline that into the state code too.

Also, not a good idea to name functions the same as state names. It's just confusing.