fix(read_console): stop the Console window's filters from hiding entries - #1342
fix(read_console): stop the Console window's filters from hiding entries#1342KamilDev wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthrough
ChangesConsole filter handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to read_console now retrieves Console entries independently of UI severity and search filters while restoring the user's original Console state. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ReadConsole
participant UnityConsole
participant LogEntries
ReadConsole->>UnityConsole: Save filter state
ReadConsole->>UnityConsole: Enable severity flags and clear search text
ReadConsole->>LogEntries: Iterate console entries
LogEntries-->>ReadConsole: Return entries
ReadConsole->>UnityConsole: Restore saved filter state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description follows the repository template, explains the bug and implementation, identifies compatibility details, testing status, documentation impact, and linked issue. It also clearly states that the new tests were not run as an NUnit fixture. Full details: Linked Issues checkExplanation The changes satisfy issue
✨ 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 |
…hiding entries
|
Hi there just got time to review this. Can you rebase this so I could take further look? Thanks! |
LogEntries filtering state is global and shared with the Console window. StartGettingEntries() honors both the toolbar's Log/Warning/Error severity toggles (ConsoleFlags bits 1<<7..1<<9) and the search box (SetFilteringText), so a toggle switched off or a leftover search query silently starves read_console of entries. The tool still reports success, so an agent reads "Retrieved 0 log entries" as a clean console -- worst case right after a compile, where it proceeds on broken state. Snapshot both, neutralize them for the duration of the read, and restore them in the finally block once the iteration session is closed, so the user's view is left exactly as they had it. The tool applies its own types and filterText arguments, which is what callers actually asked for. The search query is only cleared when GetFilteringText is also available, so a user's search is never discarded without a way to put it back. All four reflected members exist in 2021.3, 2022.3, 6000.0 and 6000.3, but they are reflected optionally: if a future Unity renames them, console reads degrade to the previous behavior instead of failing outright. Fixes CoplayDev#1239
17495d4 to
c026cb4
Compare
|
Rebased onto The only conflict was in Re-verified against a live Editor (6000.3.14f1) afterwards, since
As before, those two test bodies were executed against the Editor rather than run as an NUnit fixture — that still needs a licensed CI run. |
2 participants
Description
read_consoleinherits the Console window's UI filters, so entries the user has hidden in the Editor are invisible to the agent — and the tool still reportssuccess: true, so "Retrieved 0 log entries" is indistinguishable from a clean console. Worst case is right after a compile, where an agent proceeds on broken state.LogEntriesfiltering state is global and shared with the Console window.StartGettingEntries()honors both:ConsoleFlags.LogLevelLog|Warning|Error, bits1<<7,1<<8,1<<9LogEntries.SetFilteringTextThe tool's own
filterTextargument is applied client-side afterwards, so it cannot compensate for entries the native call never returned.GetConsoleEntriesnow snapshots both, neutralizes them for the duration of the read, and restores them in thefinallyblock once the iteration session is closed — so the user's Console view is left exactly as they had it. The tool's owntypesandfilterTextarguments still do the filtering, which is what the caller actually asked for.Type of Change
Changes Made
MCPForUnity/Editor/Tools/ReadConsole.csLogEntries.consoleFlags,SetFilteringTextandGetFilteringText.TryForceLogLevelFlags/RestoreConsoleFlags— OR the three severity bits on for the read, restore afterwards. Idempotent: when the bits are already set the property is not written at all.TryClearFilteringText/RestoreFilteringText— blank the search query for the read, restore afterwards.EndGettingEntries, so it never runs inside an open iteration session, and it runs even if the iteration throws.TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ReadConsoleTests.csHandleCommand_Get_IgnoresConsoleSearchFilter— logs a probe, sets a non-matching search query, asserts the probe is still returned and that the query is left as the user set it.HandleCommand_Get_IgnoresConsoleSeverityToggles— logs a probe, switches the Log/Warning bits off, asserts the probe is still returned and that the flags are restored.Notes on the approach
Collapse(1<<0) also changes whatStartGettingEntriesreturns, but forcing it off would flood a page with duplicates of a spammed message and push distinct entries past thecount/pageSizelimit — that hides output rather than revealing it. Left alone deliberately.GetFilteringTextis also available, so a user's search is never discarded without a way to put it back.HandleCommandreflection guard is deliberately unchanged — these members are not required for a console read to work at all, andcleardoes not depend on them.Compatibility / Package Source
file:checkoutPackages/packages-lock.json: n/a (file:source)Testing/Screenshots/Recordings
cd Server && uv run pytest tests/ -v)Compile
tools/compile-check.shrun against local Hub installs of 2022.3.27f1 and 6000.3.14f1 —MCPForUnity.RuntimeandMCPForUnity.Editorcompile clean on win, osx and linux for both. The EditMode test assembly was compiled the same way (same reference set plus the built package DLLs) to confirm the two new tests build; the only diagnostics are pre-existingCS0618/CS0649warnings from unrelated test files.Runtime, in a live 6000.3.14f1 Editor
Verified against a project consuming this branch through a
file:package reference, confirming first that the loadedMCPForUnity.Editorassembly actually contained the new methods.The bug, reproduced: with the Log and Warning toggles switched off, a raw
LogEntries.StartGettingEntries()returns 0 — with a freshly logged entry sitting in the console. That is the tool's old view.The fix, against that same state:
read_consolereturns the probe entry, andconsoleFlagsis left byte-identical afterwards.Search-box case (the reported one): with the search box set to a query matching nothing,
read_consolereturns the probe, andGetFilteringText()afterwards still returns the user's query unchanged.Idempotence confirmed incidentally: in that project the three severity bits were already on, so
TryForceLogLevelFlagscorrectly declined to write the property at all andconsoleFlagsnever changed.On the two new tests
They have not been run as an NUnit fixture.
TestProjects/UnityMCPTestsis pinned to 2021.3.45f2 and I have no 2021 Editor installed; opening it in a newer one would silently upgrade the trackedProjectVersion.txt. Note that theTest in editmodecheck on this PR passes in ~5s without a license, so it has not run them either — they need a licensed Editor run.What I could do instead: execute both test bodies verbatim — same NUnit
Assertcalls, same setup and teardown — against the live Editor. Both pass. That exercises the assertion logic, but not the fixture plumbing ([Test]discovery, test-runner domain state), so a maintainer's licensed run is still the real gate.Python tests were not run — this change touches no Python.
Manual repro, for anyone verifying by hand
Debug.Log("PROBE").read_consolewithaction: "get",types: ["log"].{"success": true, "message": "Retrieved 0 log entries"}. After: the probe is returned, and the search box / toggles are left untouched.Documentation Updates
No tool or resource signature changed — the response shape and every argument are identical, so
website/docs/reference/is unaffected.Related Issues
Fixes #1239
Additional Notes
Supersedes #1252, which fixed the same issue and was closed unmerged by its author. Differences from that patch, all of them the review points it collected:
GetFilteringTextis not guaranteed to exist", but it does exist on every version this package supports, and losing a user's search on every console read is avoidable.try/finallyas the iteration, so a throw restores both.HandleCommandreflection guard is left alone, soclearstays available regardless of these optional members.Credit for the original diagnosis goes to @beast-ofcourse in #1239.
Summary by CodeRabbit