Better Way to Implement Enemy AI

Hi all,

I'm new to Unity. I've been working on a small game where the player has to navigate a maze with enemies that will start chasing the player if the player enters their line of sight. The enemy will quit chasing the player if the player can escape the enemy's line of sight for a consecutive number of seconds. If the enemy "sees" the player again after they've already escaped their line of sight, but before the countdown has finished, the timer will reset and the chase will continue. The code I have works, but it feels unruly. Is there any better way to implement this?

What I have:

Code (CSharp):

1. using UnityEngine;
2. using UnityEngine.UI;
3. using System.Collections;
4.  
5. public class EnemyAI : MonoBehaviour
6. {
7.     public GameObject target,
8.                       timerHUD;
9.     public float timerInit;
10.     public AudioClip detectedSFX,
11.                      clearSFX;
12.     private NavMeshAgent navAgent;
13.     private Ray ray;
14.     private RaycastHit hit;
15.     private AudioSource sounds;
16.     private Vector3 originPos;
17.     private Quaternion originRot;
18.     private bool playerDetected = false;
19.     private float timer;
20.  
21.     // Use this for initialization
22.     void Start()
23.     {
24.         navAgent = GetComponent<NavMeshAgent>();
25.         sounds = GetComponent<AudioSource>();
26.         originPos = transform.position;
27.         originRot = transform.rotation;
28.         timer = timerInit;
29.    
30.     }
31.    
32.     // Update is called once per frame
33.     void FixedUpdate()
34.     {
35.         ray = new Ray(transform.position, transform.forward);
36.         timerHUD.GetComponent<Text>().text = timer.ToString("0.0");
37.  
38.         if (playerDetected == false)
39.         {
40.             Undetected();
41.  
42.         }
43.  
44.         if (playerDetected == true)
45.         {
46.             Detected();
47.  
48.         }
49.  
50.     }
51.  
52.     void Detected()
53.     {
54.         navAgent.destination = target.transform.position;
55.  
56.         if (Physics.Raycast(ray, out hit, 3f))
57.         {
58.             if (hit.collider.transform.tag == target.transform.tag)
59.             {
60.                 timer = timerInit; // Start the countdown over if the player is seen again
61.  
62.             }
63.             else
64.             {
65.                 timer -= Time.deltaTime; // Countdown if the player breaks the line of sight
66.  
67.             }
68.  
69.         }
70.         else
71.         {
72.             timer -= Time.deltaTime; // Countdown if the player breaks the line of site
73.  
74.         }
75.  
76.         if (timer <= 0)
77.         {
78.             timer = 0;
79.             playerDetected = false; // Switch to undetected behaviour if the timer reaches zero
80.  
81.             sounds.PlayOneShot(clearSFX); // Play a sound to let the player know the enemy has stopped their pursuit
82.  
83.         }
84.  
85.     }
86.  
87.     void Undetected()
88.     {
89.         if (timer != timerInit)
90.         {
91.             timer = timerInit; // Reset the timer if it is less than timer Init (IE after a detection event)
92.  
93.         }
94.  
95.         if (Physics.Raycast(ray, out hit, 3f))
96.         {
97.             if (hit.collider.transform.tag == target.transform.tag)
98.             {
99.                 playerDetected = true; // Switch to detected behaviour if the raycast hits the target (player)
100.  
101.                 sounds.PlayOneShot(detectedSFX); // Play a sound to let the player know they've been detected
102.  
103.             }
104.  
105.         }
106.  
107.         if (navAgent.destination != originPos)
108.         {
109.             navAgent.SetDestination(originPos); // Reset the enemy's position
110.  
111.         }
112.  
113.         if (transform.position.x == originPos.x && transform.position.z == originPos.z)
114.         {
115.             transform.rotation = originRot; // Reset the enemy's rotation
116.            
117.         }
118.  
119.     }
120.  
121. }

Any advice would be greatly appreciated. Thanks in advance.

 

Behaviour Tree is an appropriate tool for defining such AI. It helps to keep your code neat and tidy while your overall logic stays unclutured.

Have a look at Panda BT. The package contains examples of patrol and chase behaviours from which you could get some inspiration to implement your own AIs.

 

@ericbegue way to not be helpful... except to yourself of course...

@owto. Aint nothing wrong with a script that works and serves its purpose. Nothing overly un-ruley about it either. Not terribly advanced, but no need to overcook a solution because it doesnt feel complex enough.

 

Ignore this. This is just blatant marketing of his own product. Not really helping you here. Behavior trees are something you might need for more advanced AI in the future. But not right now.

owto, your solution is fine for your needs. You are not doing any of the Unity no nos or anything really bad. It works for you in your current game. That is the most important. If your needs change later or you find it is effecting performance, then come back with specific questions then.

If you over engineer everything you will never get anything finished! Remember, even in games - KISS - Keep it simple, stupid!, is a really important philosophy to follow.

 

@JamesLeeNZ @Korno
Aside from hating my package, you guys do not really answered the OP question.

The OP has written a complex AI and feels that his code is unruly and his asking for better way to implement it. That's what I am answering.

And you guys, both combined, say nothing more than: "No, your code is fine, KISS".

That's the exact same principle that leads me to develop Panda BT. It does help to keep your code simple in a measurable way (see cyclomatic complexity): by writting a BT you essentially export the logic flow from the MonoBehaviour to the BT script, where you have a better and uncluttered view over the AI logic.

 

There's nothing wrong with your script. Infact, it actually looks nice. You could probably move your functions to a different script and call them in a "main Enemy AI" script, but it's unnecessary and you would be moving the code elsewhere.

 

Spoiler: Drama

Give it up, dude. An entire F***ing framework is not an appropriate solution to cleaning up 100 lines of code. It's got nothing to do with the quality of the tool.

@owto, the code's fine, but you're right in feeling that it's unruly. Here's the thing - if you just want a very simple AI like what you have now, there's no real reason to clean it up other than your own itch to improve it.

If you're going to add more advanced behaviour, things are going to start looking worse, and you'll want to improve stuff. If that happens, the easiest thing is to roll your own state machine. Do that until it starts hurting, and only then start looking at frameworks.

There's some general advice that I think could help you, though. From your code:

Code (csharp):

1. if (hit.collider.transform.tag == target.transform.tag)

There's different opinions about tags around - I'm in the camp that you should avoid them (and everything else that relies on strings being equal) like the plague. Other people find them useful. In this case, though, you're really beating around the bush - you want to check if the hit transform is the target's transform, so just do that:

Code (csharp):

1. if(hit.transform == target.transform)

Also note that RaycastHit's got a transform property, which is always the same as the collider's transform. It's a real pity that there's no .gameObject property, but I digress.

Your code again:

Code (csharp):

1. public GameObject timerHUD;
2. ...
3.  
4. void FixedUpdate()
5. {
6.     timerHUD.GetComponent<Text>().text = timer.ToString("0.0");
7. }

You're only using the timerHUD for it's text, so it's both more readable and (slightly) faster to assign it as a Text component:

Code (csharp):

1. public Text timerHUD;
2. ...
3.  
4. void FixedUpdate()
5. {
6.     timerHUD.text = timer.ToString("0.0");
7. }

There's also this one:

Code (csharp):

1.  
2. if (playerDetected == false)
3. {
4.     Undetected();
5. }
6. if (playerDetected == true)
7. {
8.     Detected();
9. }

It's not a problem, but I always find "myBool == false" or "myBool == true" as a sign that the programmer's not comfortable with bools. The result of "myBool == true" is exactly the same as myBool, and "myBool == false" is the same as !myBool.
This code does the same thing, but much terser:

Code (csharp):

1. if(playerDetected) {
2.     Detected();
3. }
4. else {
5.     Undetected();
6. }

You should also work on your method names. The names "Detected" and "Undetected" only makes sense if you're looking at the FixedUpdate method. This isn't a problem in this script, but in larger scripts, it's very helpful if methods have names that describe what they do. In your case, "ChasePlayer" and "Idle"? Something like that.

That's some general advice. If you want some help on how to roll a basic state machine that's manageable, just ask.

 

This part of your code can be simplified:

Code (CSharp):

1.         if (Physics.Raycast(ray, out hit, 3f))
2.         {
3.             if (hit.collider.transform.tag == target.transform.tag)
4.             {
5.                 timer = timerInit; // Start the countdown over if the player is seen again
6.             }
7.             else
8.             {
9.                 timer -= Time.deltaTime; // Countdown if the player breaks the line of sight
10.             }
11.         }
12.         else
13.         {
14.             timer -= Time.deltaTime; // Countdown if the player breaks the line of site
15.         }
16.  

You decrement timer in all cases except where you set timer to timerInit. So you can just move the decrement up front and do it every time, but then reset the timer when those conditions are met. The cost is sometimes decrementing the timer when you don't need to, but the easier to follow code is more than worth that cost imo.

Code (CSharp):

1. timer -= Time.deltaTime;
2. if (Physics.Raycast(ray, out hit, 3f))
3. {
4.     if (hit.collider.transform == target.transform)
5.     {
6.         timer = timerInit; // Start the countdown over if the player is seen again
7.     }
8. }

Note that in C# you can combine the if statements even though the first effects the second:

Code (CSharp):

1. timer -= Time.deltaTime;
2. if (Physics.Raycast(ray, out hit, 3f) && hit.collider.transform == target.transform)
3. {
4.     timer = timerInit; // Start the countdown over if the player is seen again
5. }

You can do this because the order of operations for boolean operators (&& in this case, and for C# not necessarily all languages) are guaranteed to evaluate from left to right to support short-circuit boolean evaluation.

Spoiler: As to the behavior tree thing

@ericbegue You've reached the point where even if you were to recommend your asset in a thread where it actually makes sense you're going to get push back on it. You're doing yourself and your product a disservice by continuing to bring it up in every single thread where someone wants to talk about logic and program flow. You can be helpful without promoting so heavily.

 

@owto
Ignore @Dave Carlile and @Baste, they are just haters. It's not the first time they are gratuitously criticizing Panda BT. I've already ask them what's wrong about this framework, but they have not give a single constructive answer back (yet).

But as an answer, I would like to detail and demonstrate how Panda BT would simplify your AI by refactoring your script into a behaviour tree.

@Dave Carlile and @Baste, since I feel you've never gave a single look at this framework, please read carefully, I might offer you a glimps of what this framework is about. Maybe it would turn your hate into love.

The first step to build a Behaviour Tree consists of defining tasks. Tasks are a simple actions that you use as a building blocks to compose your behaviour tree. Tasks are implemented as functions in a MonoBehaviour. From your code, I have extracted the following tasks:

Code (CSharp):

1. using UnityEngine;
2. using UnityEngine.UI;
3. using System.Collections;
4. using Panda;
5.  
6. public class EnemyAI : MonoBehaviour
7. {
8.     public GameObject target,
9.                       timerHUD;
10.     public float timerInit;
11.     public AudioClip detectedSFX,
12.                      clearSFX;
13.     private NavMeshAgent navAgent;
14.     private AudioSource sounds;
15.     private Vector3 originPos;
16.     private Quaternion originRot;
17.     private float timer;
18.     private Vector3 destination;
19.  
20.     // Use this for initialization
21.     void Start()
22.     {
23.         navAgent = GetComponent<NavMeshAgent>();
24.         sounds = GetComponent<AudioSource>();
25.         originPos = transform.position;
26.         originRot = transform.rotation;
27.         timer = timerInit;
28.     }
29.  
30.     [Task]
31.     bool IsPlayerVisible()
32.     {
33.         bool isVisible = false;
34.         Ray ray = new Ray(transform.position, transform.forward);
35.         RaycastHit hit;
36.         if (Physics.Raycast(ray, out hit, 3f))
37.             isVisible = hit.collider.transform.tag == target.transform.tag;
38.         return isVisible;
39.     }
40.  
41.     [Task]
42.     void SetDestination_Player()
43.     {
44.         destination = target.transform.position;
45.         Task.current.Succeed();
46.     }
47.  
48.     [Task]
49.     void SetDestination_Origin()
50.     {
51.         destination = originPos;
52.         Task.current.Succeed();
53.     }
54.  
55.     [Task]
56.     void MoveToDestination()
57.     {
58.         if (Task.current.isStarting)
59.             navAgent.SetDestination(destination);    
60.  
61.         float distance = Vector3.Distance(this.transform.position, destination);
62.         if ( distance < navAgent.stoppingDistance )
63.             Task.current.Succeed();
64.     }
65.  
66.     [Task]
67.     void PlayDetectedSound()
68.     {
69.         sounds.PlayOneShot(clearSFX); // Play a sound to let the player know the enemy has stopped their pursuit
70.         Task.current.Succeed();
71.     }
72.  
73.     [Task]
74.     void ResetRotation()
75.     {
76.         transform.rotation = originRot; // Reset the enemy's rotation
77.         Task.current.Succeed();
78.     }
79. }

The MonoBehaviour is now mainly a list of 6 tasks, which are:

• IsPlayerVisible: Returns whether or not the player is visible
• SetDestination_Player: Set player position as the NavMesh destination
• SetDestination_Origin: Set the start position as the NavMesh destination
• MoveToDestination: Move to the destination point and succeed when arrive
• PlayDetectedSound: Play the "player have been spotted" sound
• ResetRotation: Set the rotation to default

Notice how these tasks are simple and straight forward: they consists of a few lines of code and contains sometime one or two if-statements (that's how the cyclomatic complexity is decreased). Furthermore, they are self contained and does not depend on each other. Their simplicity and their independence is a great factor in reducing bugs.

The second step is to use these tasks to build up a behaviour tree, and that's the real meat and potatoes of this framework. Since Panda BT is a tool oriented toward programmers a behaviour tree is defined by a BT script, which look like this:

Code (CSharp):

1. tree("Root")
2.     fallback // Chase the player or get back to origin
3.         tree("ChasePlayer")
4.         tree("ReturnToOrigin")
5.  
6. tree("ChasePlayer")
7.     fallback
8.         while IsPlayerVisible
9.             sequence
10.                 PlayDetectedSound
11.                 SetDestination_Player
12.                 MoveToDestination
13.  
14.         while not IsPlayerVisible // pursue the player for 3 sec when player becomes non visible.
15.             sequence
16.                 Wait(3.0)
17.  
18. tree("ReturnToOrigin")
19.     while not IsPlayerVisible
20.         sequence
21.             SetDestination_Origin
22.             MoveToDestination
23.             ResetRotation
24.  

If you've not familiar with Behaviour Tree, this might look a bit exotic to you. But I'm sure that even if you don't understand in details what's going on in this script, you could make some sens out of it, and it will make a lot of sens once you've learned some more about Behaviour Tree.

My goal of posting the complete behaviour tree of your AI (it really is complete), is to illustrate how simple the full logic can be: the overall behaviour consist of 24 lines and it stands on one screen. Furthermore, you can visualized the execution of the script as it is running. That visualization give you exact information about what your AI is currently doing and why.

@Dave Carlile and @Baste. I am really open to critics. I am begging you this time to give contructive critics (instead of dodging my requests again) about how Panda BT is not usefull to simplify AI behaviour?

 

I'm all for cleverness where its needed, but you don't fire up a 6 burner bbq to cook an egg.

Using your plugin requires a bunch of extra code that needs support for a relatively simple task, it also requires user to learn and understand your behavior tree, which while it doesnt look overly complex, is more than needed for a simple task like this. It would be better for a user to write their own code while learning, then using something like your kit later when getting the basics right isn't a challenge.

If you don't want to be criticized, don't open with 'use my stuff'. At least offer something constructive before you start the self promotion.

 

I don't recall seeing anyone hate on your asset. I don't doubt that it's at least a fully adequate behavior tree library. In fact it could be downright brilliant - I admittedly have never looked at it. The problems some of us have had is that you promote it as the solution for every single logic question someone has.

I love behavior trees. In fact I even use them in my project - here is some sample code from the behavior tree library I developed for it...

Code (csharp):

1. MineResourceAction {
2.   Sequence {
3.     ReserveTask(Task = TargetTask)
4.  
5.     Selector {    
6.       IsCarryingAsset(Asset = Pick)
7.       Sequence(Name = "Find Tool") {
8.         FindZone(Asset = Pick, Output = TargetZone, FindMode = Reserve)
9.         MoveToTarget(Target = TargetZone, PathMode = TouchPosition)
10.         PickUpAsset(Target = TargetZone, Asset = Pick, Quantity = 1, TransferToInventory = true)
11.         ReleaseZone(Node = TargetZone)  
12.       }
13.     }
14.    
15.     MoveToTarget(Target = TargetPosition, PathMode = TouchPosition)
16.     MineAsset(Target = TargetPosition)
17.     ReleaseTask(Task = TargetTask, Delay = 15)
18.  
19.     Selector {
20.       FindAdjacent(Asset = @AgentHeldAsset, Output = TargetNode, Tag = ContainerHasSpace)
21.       FindAdjacent(Output = TargetNode, Tag = ContainerEmpty)
22.     }
23.  
24.     DropAsset(Target = TargetNode)
25.   }
26. }

They are a beautiful and easy to understand method for simplifying complex AI, and they're especially powerful for serializing logic that occurs across frames. But to beat a dead horse, they are *not* the proper tool for a simple 20 line piece of code that someone is trying to learn from. And *that* is the so-called hate your perceiving.

 

@JamesLeeNZ
Thank you for answering.

You are saying that my plugin is overkill for tackling simple logic such as the OP AI. I agree that this AI is really simple: the OP described it clearly using 4 english sentences.
However, the part that causes complications is the implementation of such simple behaviour. The OP implementation is pretty decent, and I doubt that it could be further simplified with the constraint of using only the Unity native tools (C#, MonoBehaviour or coroutine). But this type of implementation is nonetheless complicated. Complicated in the sens that it requires great effort to analyse what the code actually do, since the essential logic is spread all over the script (you need scroll up and down while reading). And it would require even more effort to modify this script to make a more advanced AI, and that without introducing bugs and keeping the code readable and understandable.

I also agree that my framework requires to know about Behaviour Tree (which is not a technique I've created, I'm only an extreme enthusiast of this technique). But BT is not that hard to assimilate. The concepts involved are rather intuitive: you organize your logic into actions and sub-actions, into sequences, fallbacks (alternatives), or in parallel, ... nothing really inacessible even for a beginner. It is indeed something extra to learn, but the investment pays back.

I don't think Panda BT (or Behaviour Tree in general) is an overpowered tool for this simple AI. It is simply an other way to implement it. As FSM would have been another way (which would not be my first option btw).

I want to be critized. And my constructive contribution is to suggest using BT. I believe BT is an underrated tool and I am convinced that it could help to simplify the authoring of complex (or simple) AI or logic. And by promoting my tool, I'm also promoting BT. And sincerely, I don't know any other BT tool more appropriate to be demonstrated on a forum (because script/text based).

 

Thank you all for your advice.

@JamesLeeNZ and @Korno I'm all for keeping it simple, but my major concern with this project is getting as solid of a grip on the basics as possible. That way when I move on to more difficult things in the future I'll have an easier time of it. KISS is a good philosophy to have though.

@Baste That seems like a lot of really sound advice for making the code more elegant that I'll be sure to implement. I'll be sure to look into state machines as well.

@Dave Carlile That section of code was actually one of my main concerns. I knew there had to be a more elegant way of designing it that wasn't so redundant, but after staring at it for so long my brain just couldn't quite see it. Thanks for pointing this out.

@ericbegue Your tool seems like it may be useful in the future, but I don't think it's warranted here. This is also just a small starter project and I'm not interested in investing too much money into it. I'll keep it in mind for the future though.

 

You don't have to spend money, there is a free edition. I'm sure it will help you in the future, particularly if you intend to make more complex AIs. You are welcome to contact me again if you have more questions.

I've noticed a recurrent idea that I disagree with. I have selected of few quotes on this thread, that are essentially saying that Panda BT is not appropritate for simple logics. But imho, that's not justified. So far, no one has provided a technical explaination why this framework would not appropriate.

Why not for simple AI and why not right now?

Why not? You don't have to use all the 6 burners though, 1 burner and a small fry pan is enough to cook an egg. What can do more, can do less, can't it?

It is! It is appropriate to cleaning up 121 lines (to be precise). Consider these:

• That entire fluffy framework is 131 KB (without the examples).
• The OP code listing originally contains exactly 121 lines.
• The refactoring into a BT brings the line count to 79 and reduces the MonoBehaviour into 6 overly simple functions.
• On top of it, the BT script is only 24 lines and it contains the whole logic of the AI.

How is that not a cleaning/simplification? @Baste, this time, are going to give me sound argument why is that not appropriate, or are you going to continue to ignore my questions and report me as a spammer again? Please convince me with technical arguments. If you succeed, I promise, I will give it up, dude.

I don't propose it for every single logic quesion. Only where I'm sure it is appropriate, that is with timely logics (spanning several frames).

What bunch of extra code exactly?

Not 20 but 121 lines. But even if it was 20 lines, why not? In fact, I'm going to demonstrate how to beat a dead horse: the simple logic of a blinking light (see below).

@All

I really want you guys to tell why do you think this framework is not simple and not appropriate.

I've implemented the logic of a blinking light (tada!) using different common techniques:

• A bare bone implementation (using no particular technique)
• Finite State Machine
• Coroutine
• BT (of course)

So you guys have something concrete to compare BT with and some bases to build up your arguments.
@owto these implementations might be interesting for you to compare different AI techniques and draw your own conclusion.

In order to not derail this thread further, I've made a new thread where you guys are invited to give me your answers:
http://forum.unity3d.com/threads/panda-bt-shakedown-testing.414069/

 

I doubt your behavior trees run themselves

 

Its more so that your first and lasting impression was as a shill. The first post was basically "check out my product" in a section that isn't for advertising. Something less aggravating might've been explaining what fsms and behavior trees are and listing a couple of them instead of just your own.

 

damit the problem isn't with your BT, it is that you spam ads for your product everywhere. I have even seen you advertise it when all the user wanted was a timer using invoke or Time.deltaTime and a update loop. There are locations for advertising, but this is not one of them.

 

You also want to be careful of floating point error when comparing vectors. Checking if one vector is equal to another often leads to trouble. As a better method check if the distance between the vectors is less then a certain threshold.

On the overall structure, one way to clean it up would be a coroutine FSM. Note that this structure is not universally popular, but I like it. One of the advantages is code only gets called when you enter the state, no need to do checks every frame.

 

@ericbegue This is why people are hating on you.

Instead of starting with helping the OP by looking at his code and commenting on it, you just jumped into advertising mode. You made it seem like your solution is the only way to go when it clearly is not. A couroutine based FSM is possible; his code actually works so just keeping that, with a few modifications and cleaning it up, is possible; and a third choice would be a BT.

To put it simply - You offered no constructive criticism of the OPs efforts. Ignored his question. You just tried to sell your product.

Are we clear now?

 

Do you really mean the code used for the framework implementation?
That code is completely transparent for the user. Do you have the same doubt about Unity running itself or the C#/Mono platform running itself? In order to offer simplicity you need extra code that runs under the hood doing some extra work for you. Don't you agree?

I am not advertising nor spamming. My answers are genuine and specific to each question. I did suggest BT for logic that could be as well implemented using invoke, Time.deltaTime. So what? On this current thread, suggesting a BT for the OP question is perfectly appropriate. The OP AI is typically the kind of AI where a BT is an appropriate solution.

I've read this as "I have a working implementation but it feel messy. How could I restructure it better?"
And instead of doing micro cleaning on a line by line basis, I've suggested a fundamental restructuring based on BT.
But I admit my first answer was a bit bold. Sorry about that. I should have indeed started by motivating why and how a BT refactoring would make the code cleaner and more structured. Though I've done that in my following answers on this thread.

I don't say it is the only solution possible, there are obviously other possible implementations. But imho the OP AI is best implemented as a BT.

A working code doing the job is not enough. Ideally, the code should be also as readable and understandable as possible, mainly for maintenance reasons (unreadable code consumes time to decipher and are likely to contain bug). About implementing this AI as a coroutine based FSM, please show us some code so we can compare whether or not it would result into simpler and more structured code.

I don't think I've ignored his question. But I could have been first more critical on his code indeed. I'm not just trying to sell, there is a free edition (how do you sell free?).

Why suggesting an asset, purposely developped to solve the same problems as the OP, should not be considered as helping?