[Logging] Add destroyed agent events #204
Conversation
|
🧱 Stack PR · Base of stack Stack Structure:
|
tryuan99
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces INTERCEPTOR_DESTROYED and THREAT_DESTROYED events to provide more explicit logging for when agents are destroyed before reaching their targets. Correspondingly, the THREAT_MISS event has been removed. These changes have been consistently implemented across the C# simulation code, Python analysis tools, and documentation. The refactoring to support these new events is well-executed. I have one suggestion to improve maintainability by replacing a hardcoded string used for ground collision detection with a more robust solution like a Unity Tag.
📝 WalkthroughWalkthroughRenames AgentBase.CheckFloorCollision to CheckGroundCollision. Replaces multiple interceptor/threat-specific delegate types with unified InterceptorEventHandler and ThreatEventHandler; removes OnMiss events and adds OnDestroyed events on interceptors and threats. Updates InterceptorBase and ThreatBase to invoke OnDestroyed where appropriate, reorganizes interceptor miss/destroy handling via RegisterMiss/RegisterDestroyed/RequestTargetReassignment, and adjusts collision flows to call CheckGroundCollision. SimManager and SimMonitor wiring updated to subscribe to OnDestroyed and record destroyed events. Adds EventType constants for INTERCEPTOR_DESTROYED and THREAT_DESTROYED, a new metric NumMissileInterceptorsDestroyed, and corresponding logging/visualization/docs updates. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/Simulation_Logging.md (1)
200-208:⚠️ Potential issue | 🟡 MinorUpdate the example output to include the new destroyed metric.
The
process_run.pyscript now includesNumMissileInterceptorsDestroyed()inSCALAR_METRICS, but this example output doesn't reflect that. Consider adding a line for the destroyed metric to keep documentation consistent with actual output.📝 Suggested addition
Aggregating the stats for 50 runs found in the log directory. Number of missile interceptors: mean: 150.360000, std: 13.805448. Number of missile interceptor hits: mean: 99.840000, std: 0.611882. Number of missile interceptor misses: mean: 36.360000, std: 14.784803. + Number of missile interceptors destroyed: mean: X.XXXXXX, std: X.XXXXXX. Missile interceptor hit rate: mean: 0.741564, std: 0.077913. Missile interceptor efficiency: mean: 0.669430, std: 0.059498. Minimum intercept distance: mean: 6154.423782, std: 1283.849098.Assets/Scripts/Monitors/SimMonitor.cs (1)
209-218:⚠️ Potential issue | 🟡 MinorAdd a null guard before filtering on
IsTerminated.The per-agent null check is good, but the LINQ filter can still throw if a destroyed Unity object remains in the list. Guard nulls before
IsTerminatedto avoidMissingReferenceException.🛠️ Proposed fix
- List<IAgent> agents = SimManager.Instance.Agents.Where(agent => !agent.IsTerminated).ToList(); + List<IAgent> agents = SimManager.Instance.Agents + .Where(agent => agent != null && !agent.IsTerminated) + .ToList();
🤖 Fix all issues with AI agents
In `@Assets/Scripts/Interceptors/IInterceptor.cs`:
- Around line 10-13: Fix the typo in the comment for the OnMiss event: update
the comment text near the event declaration for OnMiss in IInterceptor (the
event named OnMiss) to correct "is misses its target" to "misses its target" or
similar concise grammar ("called when the interceptor misses its target but is
not destroyed.").
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Tools/visualize_log.py (1)
64-82: 🧹 Nitpick | 🔵 TrivialConsider adding statistics for the new
INTERCEPTOR_DESTROYEDevent type.The
log_event_summaryfunction logs detailed statistics forINTERCEPTOR_HITandINTERCEPTOR_MISSevents but doesn't include the newly addedINTERCEPTOR_DESTROYEDevent type. For completeness, you may want to add similar logging for destroyed interceptors.Proposed addition for destroyed event statistics
logging.info("Number of interceptor misses recorded: %d.", len(interceptor_misses)) if not interceptor_misses.empty: first_miss_time = interceptor_misses[Column.TIME].min() last_miss_time = interceptor_misses[Column.TIME].max() logging.info(" First miss at %.2f, last miss at %.2f.", first_miss_time, last_miss_time) + + # Determine the times of interceptor destroyed events. + interceptor_destroyed = ( + event_df[event_df[Column.EVENT] == EventType.INTERCEPTOR_DESTROYED]) + logging.info("Number of interceptors destroyed recorded: %d.", + len(interceptor_destroyed)) + if not interceptor_destroyed.empty: + first_destroyed_time = interceptor_destroyed[Column.TIME].min() + last_destroyed_time = interceptor_destroyed[Column.TIME].max() + logging.info(" First destroyed at %.2f, last destroyed at %.2f.", + first_destroyed_time, last_destroyed_time)
🤖 Fix all issues with AI agents
In `@Tools/visualize_log.py`:
- Around line 27-38: EVENT_TYPE_TO_MARKER is missing a mapping for the new
EventType.THREAT_DESTROYED so destroyed threats are not visualized; add an entry
to EVENT_TYPE_TO_MARKER mapping EventType.THREAT_DESTROYED to an appropriate
EventMarker (e.g., symbol, color, and label) similar to
EventType.INTERCEPTOR_DESTROYED. Update the dict where EVENT_TYPE_TO_MARKER is
defined and use the EventMarker class to provide a distinct marker (for example
"v", "black", "Threat Destroyed" or whatever fits the visual scheme) so
SimMonitor.cs logged THREAT_DESTROYED events appear in the visualization.
To be more explicit for the interceptor and threat events, this PR adds a new event called
INTERCEPTOR_DESTROYEDandTHREAT_DESTROYED, which occur when agents are destroyed prior to reaching their target.INTERCEPTOR_HITdenotes when an interceptor successfully hits its target.INTERCEPTOR_MISSdenotes exclusively when an interceptor misses its target due to the kill probability. This does not destroy the interceptor.INTERCEPTOR_DESTROYEDdenotes when an interceptor is destroyed prior to reaching its target, e.g., through a ground collision.THREAT_HITdenotes when a threat hits its target (the asset).THREAT_DESTROYEDdenotes when a threat is destroyed prior to reaching the asset, e.g., by being intercepted or colliding with the ground.THREAT_MISShas been removed because threats cannot miss their targets.