Commit Graph

3 Commits (8eb684ddc237bf3f65d7b639d5272d00bc788d76)

Author SHA1 Message Date
dsarno 8eb684ddc2
fix: comprehensive performance optimizations, claude code config, and stability improvements (issue #577) (#595)
* fix: reduce per-frame GC allocations causing editor hitches (issue #577)

Eliminate memory allocations that occurred every frame, which triggered
garbage collection spikes (~28ms) approximately every second.

Changes:
- EditorStateCache: Skip BuildSnapshot() entirely when state unchanged
  (check BEFORE building). Increased poll interval from 0.25s to 1.0s.
  Cache DeepClone() results to avoid allocations on GetSnapshot().

- TransportCommandDispatcher: Early exit before lock/list allocation
  when Pending.Count == 0, eliminating per-frame allocations when idle.

- StdioBridgeHost: Same early exit pattern for commandQueue.

- MCPForUnityEditorWindow: Throttle OnEditorUpdate to 2-second intervals
  instead of every frame, preventing expensive socket checks 60+/sec.

Fixes GitHub issue #577: High performance impact even when MCP server is off

* fix: prevent multiple domain reloads when calling refresh_unity (issue #577)

Root Cause:
- send_command() had a hardcoded retry loop (min 6 attempts) on connection errors
- Each retry resent the refresh_unity command, causing Unity to reload 6 times
- retry_on_reload=False only controlled reload-state retries, not connection retries

The Fix:
1. Unity C# (MCPForUnity/Editor):
   - Added --reinstall flag to uvx commands in dev mode
   - Ensures local development changes are picked up by uvx/Claude Code
   - Applies to all client configurators (Claude Code, Codex, etc.)

2. Python Server (Server/src):
   - Added max_attempts parameter to send_command()
   - Pass max_attempts=0 when retry_on_reload=False
   - Fixed type handling in refresh_unity.py (handle MCPResponse objects)
   - Added timeout to connection error recovery conditions
   - Recovery logic now returns success instead of error to prevent client retries

Changes:
- MCPForUnity/Editor: Added --reinstall to dev mode uvx commands
- Server/refresh_unity.py: Fixed type handling, improved error recovery
- Server/unity_connection.py: Added max_attempts param, disable retries when retry_on_reload=False

Result: refresh_unity with compile=request now triggers only 1 domain reload instead of 6

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: UI and server stability improvements

Unity Editor (C#):
- Fix "Resuming..." stuck state when manually clicking End Session
  Clear ResumeStdioAfterReload and ResumeHttpAfterReload flags in
  OnConnectionToggleClicked and EndOrphanedSessionAsync to prevent
  UI from getting stuck showing "Resuming..." with disabled button
- Remove unsupported --reinstall flag from all uvx command builders
  uvx does not support --reinstall and shows warning when used
  Use --no-cache --refresh instead for dev mode cache busting

Python Server:
- Add "aborted" to connection error patterns in refresh_unity
  Handle WinError 10053 (connection aborted) gracefully during
  Unity domain reload, treating it as expected behavior
- Add WindowsSafeRotatingFileHandler to suppress log rotation errors
  Windows file locking prevents log rotation when file is open by
  another process; catch PermissionError to avoid noisy stack traces
- Fix packaging: add py-modules = ["main"] to pyproject.toml
  setuptools.packages.find only discovers packages (directories with
  __init__.py), must explicitly list standalone module files

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* docs: improve refresh_unity connection loss handling documentation

Add detailed comments and logging to clarify why connection loss during
compile is treated as success (expected domain reload behavior, not failure).
This addresses PR feedback about potentially masking real connection errors.

The logic is intentional and correct:
- Connection loss only treated as success when compile='request'
- Domain reload causing disconnect is expected Unity behavior
- Subsequent wait_for_ready loop validates Unity becomes ready
- Prevents multiple domain reload loops (issue #577)

Added logging for observability:
- Info log when expected disconnect detected
- Warning log for non-recoverable errors

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: add missing logger import in refresh_unity

Missing logger import causes NameError at runtime when connection
loss handling paths are triggered (lines 82 and 91).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: address code review feedback on thread safety and semantics

Addresses four issues raised in code review:

1. EditorStateCache.GetSnapshot() - Remove shared cached clone
   - Revert to always returning fresh DeepClone() to prevent mutation bugs
   - Main GC optimization remains: state-change detection prevents
     unnecessary _cached rebuilds (the expensive operation)
   - Removed _cachedClone and _cachedCloneSequence fields

2. refresh_unity.py - Fix blocking reason terminology mismatch
   - Changed "asset_refresh" to "asset_import" to match activityPhase
     values from EditorStateCache.cs
   - Ensures asset import is correctly detected as blocking state

3. TransportCommandDispatcher - Fix unsynchronized Count access
   - Moved Pending.Count check inside PendingLock
   - Prevents data races and InvalidOperationException from concurrent
     dictionary access

4. StdioBridgeHost - Fix unsynchronized Count access
   - Moved commandQueue.Count check inside lockObj
   - Ensures all collection access is properly serialized

All changes maintain the GC allocation optimizations while fixing
thread safety violations and semantic contract changes.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: address code review feedback on thread safety and timeout handling

- refresh_unity.py: Track readiness explicitly and return failure on timeout
  instead of silently returning success when wait loop exits without confirming
  ready_for_tools=true

- McpClientConfiguratorBase.cs: Add thread safety guard for Configure() call
  in CheckStatusWithProjectDir(). Changed default attemptAutoRewrite to false
  and added runtime check to prevent calling Configure() from background threads.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
2026-01-20 18:11:25 -08:00
Shutong Wu 53c0ed26ef
Refactor ClaudeCodeConfigurator to use JsonFileMcpConfigurator (#545)
The old CLI file map does not seem to work for the latest CC(V2.1.4), with errors that cannot recognize any symbols after uvx.exe, whether it be --no-cache, --refresh, or --from. I did not find any documentation change of why this cannot use.
I temporarily revert the current Claude Code registration to the old JSON way, which still works. Will look for a fix that could retain the CLI command usage.
2026-01-11 02:07:47 -05:00
Marcus Sanatan f94cb2460a
Simplify MCP client configs (#401)
* First pass at MCP client refactor

* Restore original text instructions

Well most of them, I modified a few

* Move configurators to their own folder

It's less clusterd

* Remvoe override for Windsurf because we no longer need to use it

* Add Antigravity configs

Works like Windsurf, but it sucks ass

* Add some docs for properties

* Add comprehensive MCP client configurators documentation

* Add missing imports (#7)

* Handle Linux paths when unregistering CLI commands

* Construct a JSON error in a much more secure fashion
2025-11-27 18:18:44 -04:00