Enemy patrol script using NavMeshAgent I'm piecing together an Enemy script to attach to my characters. This is the portion that related the a game objects ability to patrol to given locations. Basically, patrolPoisitions are exactly what they say; they are defined in the editor. This is only the pertinent portion of the code, so there might be some straggler variables. I think I deleted them all though.

I am still very new to game development as well as Unity, so are there any suggestions on how to improve this code?

public float BaseAcceleration = 10.0f;
public float BaseSpeed = 5.0f;
public float IncreasedAccelerationFactor = 10.0f;
public float IncreasedSpeedFactor = 5.0f;
public float AlertCoolDownTime = 5.0f;
public float AlertDistance = 15.0f;
public float AttackDistance = 10.0f;
public float StoppingDistance = 0.0f;

private NavMeshAgent navMeshAgent = null;
public List&lt;Vector3&gt; patrolPositions = new List&lt;Vector3&gt;();
public float PatrolHoldTime = 0.0f;
private Vector3 StartPosition;
private int patrolPositionIndex = 0;
private bool isAtPatrolPosition = false;
private Vector3 CurrentDestination;

void Start () {
            navMeshAgent = GetComponent&lt;NavMeshAgent&gt; ();
            StartPosition = transform.position;
    }

void Update () {
            if (CurrentTarget) {
                    navMeshAgent.SetDestination (CurrentTarget.position);
            } else {
                    Patrol ();
            }

            SetNavMeshProperties ();
    }

#region Patrol Methods
void Patrol() {
            Vector3 Position = transform.position;
            if (patrolPositions != null &amp;&amp; patrolPositions.Count &gt; 1) {
                    Vector3 Destination = patrolPositions [patrolPositionIndex];
                    if (Destination != Position &amp;&amp; CurrentDestination != Destination) {
                            Debug.Log (String.Format ("Patrol plotting route to {0}", Destination.ToString ()));
                            navMeshAgent.SetDestination (Destination);
                            CurrentDestination = Destination;
                            isAtPatrolPosition = false;
                    } else if (Destination == Position &amp;&amp; !isAtPatrolPosition) {
                            isAtPatrolPosition = true;
                            Debug.Log (String.Format ("Patrol waiting {0} seconds", PatrolHoldTime));
                            StartCoroutine (Wait (PatrolHoldTime));
                    }
            } else if (Position != StartPosition) {
                    navMeshAgent.SetDestination (StartPosition);
            }
    }
void SetNavMeshProperties() {
            navMeshAgent.acceleration = BaseAcceleration;
            navMeshAgent.speed = BaseSpeed;
            navMeshAgent.stoppingDistance = StoppingDistance;
    }


IEnumerator Wait(float duration) {
            yield return new WaitForSeconds (duration);
            if (patrolPositionIndex &lt; patrolPositions.Count - 1) {
                    patrolPositionIndex++;
            } else {
                    patrolPositionIndex = 0;
            }
            isAtPatrolPosition = false;
            Debug.Log (String.Format ("Patrol position index {0}", patrolPositionIndex.ToString ()));
    }
#endregion
Your indentation and Java-style braces make it very hard to follow. Compare this:


void Patrol() {
            Vector3 Position = transform.position;
            if (patrolPositions != null &amp;&amp; patrolPositions.Count &gt; 1) {
                    Vector3 Destination = patrolPositions [patrolPositionIndex];
                    if (Destination != Position &amp;&amp; CurrentDestination != Destination) {
                            Debug.Log (String.Format ("Patrol plotting route to {0}", Destination.ToString ()));
                            navMeshAgent.SetDestination (Destination);
                            CurrentDestination = Destination;
                            isAtPatrolPosition = false;
                    } else if (Destination == Position &amp;&amp; !isAtPatrolPosition) {
                            isAtPatrolPosition = true;
                            Debug.Log (String.Format ("Patrol waiting {0} seconds", PatrolHoldTime));
                            StartCoroutine (Wait (PatrolHoldTime));
                    }
            } else if (Position != StartPosition) {
                    navMeshAgent.SetDestination (StartPosition);
            }
    }



to that:

void Patrol() 
{
    Vector3 Position = transform.position;
    if (patrolPositions != null &amp;&amp; patrolPositions.Count &gt; 1) 
    {
        Vector3 Destination = patrolPositions[patrolPositionIndex];
        if (Destination != Position &amp;&amp; CurrentDestination != Destination) 
        {
             Debug.Log(String.Format("Patrol plotting route to {0}", Destination.ToString()));
             navMeshAgent.SetDestination(Destination);
             CurrentDestination = Destination;
             isAtPatrolPosition = false;
        } 
        else if (Destination == Position &amp;&amp; !isAtPatrolPosition) 
        {
             isAtPatrolPosition = true;
             Debug.Log(String.Format("Patrol waiting {0} seconds", PatrolHoldTime));
             StartCoroutine(Wait(PatrolHoldTime));
        }
    } 
    else if (Position != StartPosition) 
    {
        navMeshAgent.SetDestination(StartPosition);
    }
}




I'd remove this comment:

// Use this for initialization


And I'd avoid #region blocks - you're not showing your entire class, but regions are typically a sign that too many things are going on, and can often be extracted into their own class (or method, when the region is defined within a method's body).



I don't understand why a method called Wait would return an IEnumerator, and it seems a bit counter-intuitive that Wait would have the side-effect of changing the patrolPositionIndex - could the method have a more descriptive name?



These initializations are redundant, since they're initializing to the default value for their type:

private NavMeshAgent navMeshAgent = null;




public float StoppingDistance = 0.0f;




public float PatrolHoldTime = 0.0f;




private int patrolPositionIndex = 0;




private bool isAtPatrolPosition = false;