Repository navigation
[Escape] Add escape detectors - #147
Conversation
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
Assets/Scripts/Escape/SpeedEscapeDetector.cs (2)
25-28: Division by zero risk indistanceTimeConstantcalculation.If
DragCoefficientorCrossSectionalAreaare unset/null, the null-coalescing to0causes a zero denominator, resulting inInfinityorNaN.Apply this diff to guard against division by zero:
float distanceTimeConstant = 2 * (Agent.StaticConfig.BodyConfig?.Mass ?? 0) / - (Constants.CalculateAirDensityAtAltitude(Agent.Position.y) * - (Agent.StaticConfig.LiftDragConfig?.DragCoefficient ?? 0) * - (Agent.StaticConfig.BodyConfig?.CrossSectionalArea ?? 0)); + Mathf.Max(Constants.CalculateAirDensityAtAltitude(Agent.Position.y) * + (Agent.StaticConfig.LiftDragConfig?.DragCoefficient ?? 0) * + (Agent.StaticConfig.BodyConfig?.CrossSectionalArea ?? 0), 1e-6f);
31-31: Division by zero risk inminTurningRadiuscalculation.If
Agent.MaxNormalAcceleration()returns 0, this producesInfinity. WhenangleToTargetis 0, the expressionangleToTarget * minTurningRadiusbecomes0 * Infinity = NaN, corrupting subsequent calculations.Apply this diff to handle the zero acceleration case:
- float minTurningRadius = Agent.Velocity.sqrMagnitude / Agent.MaxNormalAcceleration(); + float maxNormalAccel = Agent.MaxNormalAcceleration(); + float minTurningRadius = maxNormalAccel > 1e-6f + ? Agent.Velocity.sqrMagnitude / maxNormalAccel + : float.MaxValue;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (18)
Assets/Scripts/Escape.meta(1 hunks)Assets/Scripts/Escape/EscapeDetectorBase.cs(1 hunks)Assets/Scripts/Escape/EscapeDetectorBase.cs.meta(1 hunks)Assets/Scripts/Escape/GeometricEscapeDetector.cs(1 hunks)Assets/Scripts/Escape/GeometricEscapeDetector.cs.meta(1 hunks)Assets/Scripts/Escape/IEscapeDetector.cs(1 hunks)Assets/Scripts/Escape/IEscapeDetector.cs.meta(1 hunks)Assets/Scripts/Escape/SpeedEscapeDetector.cs(1 hunks)Assets/Scripts/Escape/SpeedEscapeDetector.cs.meta(1 hunks)Assets/Scripts/Escape/TimeEscapeDetector.cs(1 hunks)Assets/Scripts/Escape/TimeEscapeDetector.cs.meta(1 hunks)Assets/Scripts/Hierarchical/FixedHierarchical.cs(1 hunks)Assets/Tests/EditMode/GeometricEscapeDetectorTests.cs(1 hunks)Assets/Tests/EditMode/GeometricEscapeDetectorTests.cs.meta(1 hunks)Assets/Tests/EditMode/SpeedEscapeDetectorTests.cs(1 hunks)Assets/Tests/EditMode/SpeedEscapeDetectorTests.cs.meta(1 hunks)Assets/Tests/EditMode/TimeEscapeDetectorTests.cs(1 hunks)Assets/Tests/EditMode/TimeEscapeDetectorTests.cs.meta(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-17T05:39:03.474Z
Learnt from: tryuan99
Repo: PisterLab/micromissiles-unity PR: 110
File: Assets/Scripts/Algorithms/Clustering/Cluster.cs:123-126
Timestamp: 2025-10-17T05:39:03.474Z
Learning: In `Assets/Scripts/Algorithms/Clustering/Cluster.cs`, the `IsFullyTerminated()` method intentionally treats null agents as terminated via `?? true`, which differs from the null-handling in `Centroid()` and `Velocity()` methods that skip null agents entirely. This semantic difference is by design.
Applied to files:
Assets/Scripts/Escape/GeometricEscapeDetector.csAssets/Scripts/Escape/EscapeDetectorBase.cs
🧬 Code graph analysis (8)
Assets/Scripts/Escape/IEscapeDetector.cs (4)
Assets/Scripts/Escape/TimeEscapeDetector.cs (1)
IsEscaping(13-41)Assets/Scripts/Escape/GeometricEscapeDetector.cs (1)
IsEscaping(13-34)Assets/Scripts/Escape/EscapeDetectorBase.cs (1)
IsEscaping(13-13)Assets/Scripts/Escape/SpeedEscapeDetector.cs (1)
IsEscaping(13-20)
Assets/Tests/EditMode/GeometricEscapeDetectorTests.cs (4)
Assets/Scripts/Escape/GeometricEscapeDetector.cs (3)
GeometricEscapeDetector(6-35)GeometricEscapeDetector(11-11)IsEscaping(13-34)Assets/Scripts/Escape/EscapeDetectorBase.cs (1)
IsEscaping(13-13)Assets/Scripts/Escape/IEscapeDetector.cs (1)
IsEscaping(12-12)Assets/Scripts/Hierarchical/FixedHierarchical.cs (5)
FixedHierarchical(5-24)FixedHierarchical(14-14)FixedHierarchical(15-16)FixedHierarchical(17-18)FixedHierarchical(19-23)
Assets/Scripts/Escape/GeometricEscapeDetector.cs (4)
Assets/Scripts/Escape/EscapeDetectorBase.cs (3)
EscapeDetectorBase(4-14)EscapeDetectorBase(8-10)IsEscaping(13-13)Assets/Scripts/Escape/TimeEscapeDetector.cs (1)
IsEscaping(13-41)Assets/Scripts/Escape/IEscapeDetector.cs (1)
IsEscaping(12-12)Assets/Scripts/Escape/SpeedEscapeDetector.cs (1)
IsEscaping(13-20)
Assets/Tests/EditMode/TimeEscapeDetectorTests.cs (7)
Assets/Tests/TestBase.cs (2)
TestBase(6-81)InvokePrivateMethod(22-25)Assets/Scripts/Escape/TimeEscapeDetector.cs (3)
TimeEscapeDetector(7-42)TimeEscapeDetector(11-11)IsEscaping(13-41)Assets/Scripts/Escape/GeometricEscapeDetector.cs (1)
IsEscaping(13-34)Assets/Scripts/Escape/EscapeDetectorBase.cs (1)
IsEscaping(13-13)Assets/Scripts/Escape/IEscapeDetector.cs (1)
IsEscaping(12-12)Assets/Scripts/Escape/SpeedEscapeDetector.cs (1)
IsEscaping(13-20)Assets/Scripts/Hierarchical/FixedHierarchical.cs (5)
FixedHierarchical(5-24)FixedHierarchical(14-14)FixedHierarchical(15-16)FixedHierarchical(17-18)FixedHierarchical(19-23)
Assets/Scripts/Escape/TimeEscapeDetector.cs (3)
Assets/Scripts/Escape/EscapeDetectorBase.cs (3)
EscapeDetectorBase(4-14)EscapeDetectorBase(8-10)IsEscaping(13-13)Assets/Scripts/Escape/IEscapeDetector.cs (1)
IsEscaping(12-12)Assets/Scripts/Escape/SpeedEscapeDetector.cs (1)
IsEscaping(13-20)
Assets/Scripts/Escape/SpeedEscapeDetector.cs (5)
Assets/Scripts/Escape/EscapeDetectorBase.cs (3)
EscapeDetectorBase(4-14)EscapeDetectorBase(8-10)IsEscaping(13-13)Assets/Scripts/Escape/TimeEscapeDetector.cs (1)
IsEscaping(13-41)Assets/Scripts/Escape/GeometricEscapeDetector.cs (1)
IsEscaping(13-34)Assets/Scripts/Escape/IEscapeDetector.cs (1)
IsEscaping(12-12)Assets/Scripts/Utils/Constants.cs (2)
Constants(5-18)CalculateAirDensityAtAltitude(11-13)
Assets/Scripts/Escape/EscapeDetectorBase.cs (4)
Assets/Scripts/Escape/TimeEscapeDetector.cs (1)
IsEscaping(13-41)Assets/Scripts/Escape/GeometricEscapeDetector.cs (1)
IsEscaping(13-34)Assets/Scripts/Escape/IEscapeDetector.cs (1)
IsEscaping(12-12)Assets/Scripts/Escape/SpeedEscapeDetector.cs (1)
IsEscaping(13-20)
Assets/Tests/EditMode/SpeedEscapeDetectorTests.cs (7)
Assets/Scripts/Escape/SpeedEscapeDetector.cs (3)
SpeedEscapeDetector(7-45)SpeedEscapeDetector(11-11)IsEscaping(13-20)Assets/Scripts/Utils/Constants.cs (1)
Constants(5-18)Assets/Scripts/Escape/TimeEscapeDetector.cs (1)
IsEscaping(13-41)Assets/Scripts/Escape/GeometricEscapeDetector.cs (1)
IsEscaping(13-34)Assets/Scripts/Escape/EscapeDetectorBase.cs (1)
IsEscaping(13-13)Assets/Scripts/Escape/IEscapeDetector.cs (1)
IsEscaping(12-12)Assets/Scripts/Hierarchical/FixedHierarchical.cs (5)
FixedHierarchical(5-24)FixedHierarchical(14-14)FixedHierarchical(15-16)FixedHierarchical(17-18)FixedHierarchical(19-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test plugins on windows-latest
- GitHub Check: Test plugins on ubuntu-latest
- GitHub Check: Test plugins on macos-latest
- GitHub Check: Build plugins for windows-latest
🔇 Additional comments (10)
Assets/Scripts/Hierarchical/FixedHierarchical.cs (1)
6-7: Documentation clarifies shadowing rationale.This comment directly addresses the prior reviewer concern and clearly explains why shadowing is necessary: the parent class properties are read-only, while
FixedHierarchicalneeds settable properties for testing purposes, andSpeedmust shadow the parent's derived property to use the localVelocityproperty.Assets/Tests/EditMode/GeometricEscapeDetectorTests.cs.meta (1)
1-2: LGTM!Standard Unity meta file with valid GUID for asset tracking.
Assets/Scripts/Escape/IEscapeDetector.cs (1)
8-13: Clean interface design.The interface is well-defined with appropriate documentation. The init-only setter for
Agentensures immutability after construction, which is a good pattern for this use case.Assets/Tests/EditMode/GeometricEscapeDetectorTests.cs (1)
16-91: Good test coverage for GeometricEscapeDetector.The tests comprehensively cover the main logic branches: null target handling, fallback range-rate behavior (negative/positive/zero), and the nested-target scenarios (close/far, ahead/behind/orthogonal). The test naming convention clearly indicates the scenario and expected outcome.
Assets/Scripts/Escape/EscapeDetectorBase.cs (1)
4-14: Clean abstract base class implementation.The base class correctly implements
IEscapeDetector, provides constructor-based initialization for theAgentproperty, and defines the abstractIsEscapingmethod for subclasses to implement.Assets/Tests/EditMode/SpeedEscapeDetectorTests.cs (1)
34-68: Tests cover the core functionality.The tests verify null handling and key scenarios for the speed-based escape detection. The test setup properly configures the aerodynamic parameters needed for
CalculatePredictedAgentSpeed.Assets/Tests/EditMode/TimeEscapeDetectorTests.cs (1)
16-128: Comprehensive test coverage for TimeEscapeDetector.The test suite thoroughly covers all logic branches: null target handling, fallback range-rate behavior, head-on approach with time-to-hit comparisons (slow/fast target, equal times), and tail-chasing scenarios with speed comparisons. Test naming is clear and descriptive.
Assets/Scripts/Escape/TimeEscapeDetector.cs (2)
30-32: Potential division by zero whenAgent.Speedis zero.While
target.Speedis guarded by_minimumSpeed,Agent.Speedon line 31 is not. IfAgent.Speedis zero,agentTimeToHitbecomesInfinity, which may produce correct behavior (agent never reaches target, so target escapes), but this implicit handling differs from the explicit guard fortarget.Speed.If agent speed of zero is a valid scenario, consider adding an explicit guard or documenting this as an invariant:
if (target.Speed < _minimumSpeed) { return false; } + if (Agent.Speed < _minimumSpeed) { + return true; // Agent cannot reach target. + } float targetTimeToHit = targetRelativePositionToTarget.magnitude / target.Speed; float agentTimeToHit = agentToTargetTransformation.Position.Range / Agent.Speed;
13-41: Well-structured escape detection logic.The implementation correctly handles: null targets, head-on vs tail-chasing scenarios, time-to-hit comparisons, and fallback to relative velocity. The documentation comments clearly explain each branch's purpose.
Assets/Scripts/Escape/GeometricEscapeDetector.cs (1)
13-34: LGTM!The escape detection logic is well-structured:
- Proper null handling
- The
_rangeBufferFactoris now correctly applied to the distance comparison- The geometric check using
Vector3.Dotcorrectly determines if the agent is behind the target relative to the target's destination- Sensible fallback to relative velocity when the target has no active target
[All] Implement fully hierarchical defense strategy * [Agent] Add agent-related interfaces and base implementations (#95) * [Interceptor/Threat] Add interceptor and threat interfaces (#96) * [Algorithm] Add algorithm interfaces and base implementations (#97) * [Movements] Implement all aerial and ground-based movements (#99) * [Gemini] Disable Gemini pull request summary (#142) * [Sensor] Add sensor interface and ideal sensor implementation (#100) * [Chore] Clean up private properties (#101) * [Evasion] Implement evasion behaviors (#102) * [Agent] Add agent-related interfaces and base implementations (#95) * [Interceptor/Threat] Add interceptor and threat interfaces (#96) * [Algorithm] Add algorithm interfaces and base implementations (#97) * [Movements] Implement all aerial and ground-based movements (#99) * [Sensor] Add sensor interface and ideal sensor implementation (#100) * [Chore] Clean up private properties (#101) * [Evasion] Implement evasion behaviors (#102) * [Attack Behavior] Implement direct attack behavior (#103) * [Controller] Add controller interface and implementations (#105) * [Predictor/Planner] Clean up predictor and planner classes (#108) * [Clustering] Change clustering algorithms to act on hierarchical objects (#111) * [Utils] Clean up coordinates classes and tests (#112) * [Assignment] Refactor assignment algorithms (#113) * [Accessor] Use init accessor for read-only fields (#116) * [Interceptors/Threats] Add Legacy prefix to existing interceptor and threat class names (#121) * [Interceptors/Threats] Implement all interceptor and threat types (#122) * [Proto/Configs] Add launcher types to proto definition and simulation configs (#123) * [IADS] Clean up IADS and simulation manager (#124) * [UI] Remove swarm status dialog and tactical panel (#125) * [All] Remove all legacy code (#127) * [Prefabs] Add prefabs for launchers (#128) * [Release] Adjust minimum distance in proximity release (#129) * [Configs] Add simulation configuration with 2 launchers (#130) * [Simulation] Destroy terminated game objects (#132) * [UI] Draw gizmos for agents (#133) * [UI] Fix summary text label sizes (#134) * [Clustering] Implement recursive clustering within hierarchy (#139) * [UI] Clean up UI scripts (#140) * [Reassignment] Assign new targets to excess interceptors (#141) * [Evasion] Fix threat evasion of interceptors (#143) * [Reassignment] Use capacity remaining to reassign interceptors (#144) * [Agent] Add IsPursuable property (#145) * [Reassignment] Implement threat reassignment (#146) * [Escape] Add escape detectors (#147) * [Escape] Check for escaping targets (#148) * [Movement] Fix missile movement to avoid the ground (#149) * [Reassignment] Fix relaunching multiple carrier interceptors (#150) * [Unity] Fix serializable fields and references (#152) * [Docs] Update docs with hierarchical strategy (#153) * [Utils] Sample from normal distribution (#154) * [Proto] Add run configuration proto (#155) * [Visualization] Fix visualizing simulation monitor logs (#157) * [Tools] Build standalone player (#158) * [Manager] Add run manager to handle run configurations (#159) * [Tools] Refactor common simulation-related constants and utilities (#160) * [Tools] Add metric and aggregator classes (#161) * [Tools] Process the aggregated logs of a simulation run (#162) * [Docs] Update documentation with run configuration and logging (#163) * [Tests] Remove unused Rigidbody variables from unit tests (#166) * [Agent] Add target model API in Agent interface (#167) * [Logging] Emit more detailed events (#168) * [Metrics] Consider only missile interceptors in metrics (#169) * [Logging] Remove logging from CP-SAT solver (#171) * [Tools] Add script to animate telemetry log (#172) * [Docs] Remove unnecessary docs image files (#174) * [Interceptor] Rename IsPursuable to IsPursuer (#175) * [UI] Reimplement tactical panel (#176) * [UI] Add camera to follow and rotate around interceptors or threats (#177) * [Configs] Add configuration for 5 swarms and 1000 UCAVs (#179) * [Agent] Cache the agent transform (#180) * [Agent] Clean up finding relative transformations (#181) * [Configs] Exclude 5_swarms_1000_ucav configuration from CI (#182) * [Agents] Fix collisions with terminated agents (#184) * [Controls] Implement vector proportional navigation (#186) * [Reassignment] Find subhierarchicals with at least one active target (#187)
This PR implements the escape detector interface and its subclass implementations. The escape detectors determine heuristically whether a target is escaping its pursuer. This PR implements three different escape detectors:
Unit tests are provided for all three escape detectors.