-
Notifications
You must be signed in to change notification settings - Fork 462
Fix: ReadConsole tool: stable log severity classification and reliable console reads across Unity versions #202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…y versions - Classify severity via stacktrace/message first (LogError/LogWarning/Exception/Assertion), with safe fallback to mode-bit mapping - Fix error/warning/log mapping; treat Exception/Assert as errors for filtering - Return the current console buffer reliably and remove debug spam - No changes outside ReadConsole behavior
…llback when stacktrace shows Debug:Log - Detect explicit Debug.Log in stacktrace (UnityEngine.Debug:Log) - Do not downgrade/upgrade to Warning via mode bits for editor-originated logs - Keeps informational setup lines (e.g., MCP registration, bridge start) as Log
…eserve Debug.Log as Log without mode fallback, add explicit Debug.Log detection helper
WalkthroughRefactors console log classification to infer severity from message content with mode-based fallback, refines filtering and output to use inferred unityType, adds stack-trace extraction, ensures StartGettingEntries/EndGettingEntries are always paired, warns that sinceTimestamp is parsed but unimplemented, and simplifies reflection/error handling. No public APIs changed. Changes
Sequence Diagram(s)sequenceDiagram
actor Caller
participant ReadConsole
participant UnityConsole as Unity Console Backend
Caller->>ReadConsole: GetConsoleEntries(sinceTimestamp, filters)
Note over ReadConsole: Parse sinceTimestamp (warn: not implemented)
ReadConsole->>UnityConsole: StartGettingEntries()
loop For each raw entry
ReadConsole->>ReadConsole: InferTypeFromMessage(message)
alt Message not explicit debug/log
ReadConsole->>ReadConsole: GetLogTypeFromMode(mode) (fallback)
end
ReadConsole->>ReadConsole: GetRemappedTypeForFiltering(unityType)
ReadConsole->>ReadConsole: ExtractStackTrace(message)
ReadConsole-->>Caller: Emit entry { type: unityType, stackTrace? }
end
ReadConsole->>UnityConsole: EndGettingEntries() (finally)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–25 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
UnityMcpBridge/Editor/Tools/ReadConsole.cs (4)
46-48
: Remove empty lines for better code consistency.These blank lines don't add value and create unnecessary whitespace.
if (logEntriesType == null) throw new Exception("Could not find internal type UnityEditor.LogEntries"); - - -
107-110
: Remove empty calibration comment block.This empty comment block should be removed as part of the calibration cleanup.
if (_instanceIdField == null) throw new Exception("Failed to reflect LogEntry.instanceID"); - - // (Calibration removed) -
261-264
: Inconsistent brace placement.The opening brace should be on the same line as the if statement to match the codebase style.
- if (string.IsNullOrEmpty(message)) - { + if (string.IsNullOrEmpty(message)) { continue; // Skip empty messages - } + }
454-476
: Unused method - consider removal.The
GetRemappedTypeForFiltering
method is defined but never used in the codebase. This appears to be leftover from the previous implementation.#!/bin/bash # Description: Verify that GetRemappedTypeForFiltering is indeed unused # Expected: Should only find the method definition rg "GetRemappedTypeForFiltering"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
UnityMcpBridge/Editor/Tools/ReadConsole.cs
(7 hunks)
🔇 Additional comments (9)
UnityMcpBridge/Editor/Tools/ReadConsole.cs (9)
19-20
: LGTM!Good cleanup - removing calibration code simplifies the codebase and reduces maintenance burden.
268-275
: Excellent improvement to log classification reliability.The hybrid approach using message content analysis with mode-based fallback is a significant improvement over pure mode-based classification. This addresses the core issue of Unity's mode bitfield variations across versions.
333-333
: Output format uses reliable classification.Good change - using
unityType.ToString()
instead of mode-derived types ensures consistent output formatting that matches the improved classification logic.
389-409
: Simplified and clearer mode-to-type mapping.The direct bit checking approach is much cleaner and easier to understand than the previous multi-branch logic. The comment about version variations is helpful context.
413-444
: Robust message-based classification implementation.The
InferTypeFromMessage
method provides excellent cross-version compatibility by analyzing actual message content. The order of checks is logical - Debug API calls first, then compiler diagnostics, then exceptions/assertions.
446-452
: Good helper for explicit debug detection.This method correctly identifies explicit Debug.Log calls, which is essential for the hybrid classification approach.
484-538
: Comprehensive stack trace extraction logic.The
ExtractStackTrace
method is well-implemented with multiple heuristics for detecting stack trace patterns. The performance note aboutTrimStart
is helpful, and the logic handles various Unity stack trace formats effectively.
366-378
: Proper resource cleanup with finally block.Excellent improvement - ensuring
EndGettingEntries
is always called in the finally block prevents resource leaks and console corruption, especially after domain reloads.
180-186
: Good warning for unimplemented feature.The warning about unimplemented timestamp filtering is appropriate and helps manage user expectations.
…directly from unityType (Exception/Assert treated as errors)
Summary
Short: Improves read_console severity classification (CSxxxx warnings/errors; Debug.Log stays Log) and ensures consistent results across domain reloads. No API/schema changes.
Addresses #199 & #125
What changed
UnityMcpBridge/Editor/Tools/ReadConsole.cs
(no API/schema changes)Why
Unity’s internal
LogEntry.mode
bitfield can differ between versions, which caused misclassification (e.g., errors showing as warnings). Stacktrace‑first classification is stable and version‑resilient.How to verify
Debug.Log
,Debug.LogWarning
,Debug.LogError
(and optionally an exception/assert)read_console
tool with type filters (log
,warning
,error
) and confirm correct classificationread_console
continues to return the current buffer accuratelySummary by CodeRabbit
New Features
Bug Fixes
Chores