Added DangerousAreaScanner #51
Conversation
Added DangerousAreaScanner, a script to scan a potentially dangerous area for dangerous obstacles upon entering.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a DangerousAreaScanner script that allows an agent to detect potentially dangerous areas and obstacles while navigating a scene. It also includes a new TestAgentScript for basic navigation and enhances obstacle collection with ObstacleListSingleton.
- Introduces DangerousAreaScanner with scanning logic and delay handling.
- Provides ObstacleListSingleton for grouping obstacles by tag.
- Adds TestAgentScript for demonstrating NavMeshAgent destination setting.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Assets/Wizards Code/MaxArchxd/Scripts/TestAgentScript.cs | New script to set the destination for a NavMeshAgent. |
| Assets/Wizards Code/MaxArchxd/Scripts/ObstacleListSingleton.cs | Singleton for aggregating obstacles and filtering them by tag. |
| Assets/Wizards Code/MaxArchxd/Scripts/DangerousAreaScanner.cs | Main logic for scanning dangerous areas and processing obstacles. |
| Other *.meta and scene files | Updated metadata to support the added scripts and scene configurations. |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Thank you for your contribution. This is an interesting idea. The implementation is a little confusing, with many one-line methods. I'm sure that this can be resolved easily enough.
The coding style doesn't match the rest of the project. Again, easily resolvable.
There are a number of recommendations for improvement inline.
| void Update() | ||
| { | ||
| if (waiting) return; | ||
| timeSinceLastScan += Time.deltaTime; |
There was a problem hiding this comment.
Should be running as a Coroutine rather than an update as this will only run every 1s. This will also remove the need to track timeSinceLastScan.
| return false; | ||
| } | ||
|
|
||
| private List<NavMeshObstacle> getRelevantObstacles() |
There was a problem hiding this comment.
Should probably call this in Start() so as not to cause a CPU spike in a single frame on first test.
|
|
||
| private List<NavMeshObstacle> getRelevantObstacles() | ||
| { | ||
| if (dangerousObstacles.Count == 0) return new List<NavMeshObstacle>(ObstacleListSingleton.Instance.ObstaclesInScene); |
There was a problem hiding this comment.
What if an obstacle is added to or removed from the scene?
| List<NavMeshObstacle> obstacles = getRelevantObstacles(); | ||
| foreach(NavMeshObstacle obstacle in obstacles) | ||
| { | ||
| if(Vector3.Distance(obstacle.transform.position, transform.position) <= detectionRadius) return true; |
There was a problem hiding this comment.
This is an expensive operation if there are a reasonable number of obstacles. Using SqeMagnituted would help a little but spreading this over multiple frames would help even more.
| using WizardsCode.BackgroundAI; | ||
|
|
||
| [RequireComponent(typeof(NavMeshAgent))] | ||
| public class DangerousAreaScanner : AbstractSingleton<DangerousAreaScanner> |
There was a problem hiding this comment.
I feel that this would be better as a Sense (See LineOfSight as example) rather than a monobehaviour. Though this needs a little more thought before being sure about that.
There was a problem hiding this comment.
Also, should this really be a singleton? Doesn't that mean that it is impossible to have different collections of dangerous obstacles for different characters?
2 participants
Added DangerousAreaScanner, a script to scan a potentially dangerous area for dangerous obstacles upon entering.