Skip to content

Added AssetDatabase.Refresh() to all command tool calls #475

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.

Sign up for GitHub

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

Open
strich wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added AssetDatabase.Refresh() to all command tool calls #475

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions MCPForUnity/Editor/Tools/CommandRegistry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using MCPForUnity.Editor.Resources;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using UnityEditor;

namespace MCPForUnity.Editor.Tools
{
Expand Down Expand Up @@ -232,6 +233,7 @@ public static Func<JObject, object> GetHandler(string commandName)
/// <returns>The result for synchronous commands, or null for async commands (TCS will be completed later)</returns>
public static object ExecuteCommand(string commandName, JObject @params, TaskCompletionSource<string> tcs)
{
AssetDatabase.Refresh();
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find MCPForUnity -name "CommandRegistry.cs" -type f

Repository: CoplayDev/unity-mcp

Length of output: 106

🏁 Script executed:

git ls-files "MCPForUnity/Editor/Tools/CommandRegistry.cs"

Repository: CoplayDev/unity-mcp

Length of output: 106

🏁 Script executed:

fd -t f "CommandRegistry.cs" --max-results 5

Repository: CoplayDev/unity-mcp

Length of output: 155

🏁 Script executed:

cat -n "MCPForUnity/Editor/Tools/CommandRegistry.cs"

Repository: CoplayDev/unity-mcp

Length of output: 19060

🏁 Script executed:

# Search for where ExecuteCommand and InvokeCommandAsync are called
rg -n "ExecuteCommand|InvokeCommandAsync" --type cs -g '!CommandRegistry.cs' | head -30

Repository: CoplayDev/unity-mcp

Length of output: 1061

🏁 Script executed:

# Find tool and resource implementations to see what they do
fd -t f -e "cs" --path "*Editor*" | xargs grep -l "\[McpForUnityTool\]\|\[McpForUnityResource\]" 2>/dev/null | head -10

Repository: CoplayDev/unity-mcp

Length of output: 314

🏁 Script executed:

# Find tool and resource implementations
find MCPForUnity/Editor -type f -name "*.cs" -exec grep -l "\[McpForUnityTool\]\|\[McpForUnityResource\]" {} \;

Repository: CoplayDev/unity-mcp

Length of output: 274

🏁 Script executed:

# Look for actual tool implementations
find MCPForUnity/Editor/Tools -type f -name "*.cs" ! -name "CommandRegistry.cs" | head -20

Repository: CoplayDev/unity-mcp

Length of output: 655

🏁 Script executed:

# Check a few tool implementations to see what they do
echo "=== ExecuteMenuItem.cs ===" && head -60 MCPForUnity/Editor/Tools/ExecuteMenuItem.cs
echo -e "\n=== ReadConsole.cs ===" && head -60 MCPForUnity/Editor/Tools/ReadConsole.cs
echo -e "\n=== ManageAsset.cs ===" && head -60 MCPForUnity/Editor/Tools/ManageAsset.cs

Repository: CoplayDev/unity-mcp

Length of output: 6648

🏁 Script executed:

# Check BatchExecute to see command execution patterns
cat -n MCPForUnity/Editor/Tools/BatchExecute.cs | head -110

Repository: CoplayDev/unity-mcp

Length of output: 4882

🏁 Script executed:

# Check if there are any guards around the Refresh calls - search for conditions before the refresh
rg -B5 "AssetDatabase.Refresh" MCPForUnity/Editor/Tools/CommandRegistry.cs

Repository: CoplayDev/unity-mcp

Length of output: 842

Reconsider unconditional AssetDatabase.Refresh() for performance reasons.

AssetDatabase.Refresh() is a blocking, expensive operation that scans the entire project filesystem and triggers asset garbage collection. Calling it before every command execution creates significant performance issues, especially for:

  • High-frequency command usage or rapid sequential command invocations
  • Batch operations (up to 25 commands per batch would trigger 25 consecutive refreshes)
  • Commands that don't modify assets (e.g., ExecuteMenuItem, ReadConsole)

Consider these alternatives:

  1. Opt-in approach: Add a property to tool/resource attributes (e.g., RequiresAssetRefresh = true) so only commands that modify assets trigger a refresh
  2. Targeted refresh: Pass specific asset paths to AssetDatabase.Refresh(path) when known
  3. Command-level control: Let individual commands call refresh internally when needed
  4. Post-modification refresh: Only refresh after commands that return modification indicators
🤖 Prompt for AI Agents 
In MCPForUnity/Editor/Tools/CommandRegistry.cs around line 236, remove the
unconditional call to AssetDatabase.Refresh() and implement a selective refresh
strategy: add a boolean flag/property (e.g., RequiresAssetRefresh) on
command/tool attributes or command descriptors so only commands that modify
assets request a refresh; support passing specific asset paths for targeted
refresh when available; allow commands to perform their own refresh internally
when they know they changed assets; and ensure batch execution aggregates
refresh requests to run a single Refresh (or targeted Refresh with collected
paths) after the batch completes to avoid repeated global refreshes.

var handlerInfo = GetHandlerInfo(commandName);

if (handlerInfo.IsAsync)
Expand All @@ -256,6 +258,7 @@ public static object ExecuteCommand(string commandName, JObject @params, TaskCom
/// <param name="params">Parameters to pass to the command (optional).</param>
public static Task<object> InvokeCommandAsync(string commandName, JObject @params)
{
AssetDatabase.Refresh();
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Same performance concern applies, with additional batch execution impact.

This method is explicitly documented (line 255) as being "used internally for features like batch execution," making the unconditional AssetDatabase.Refresh() call particularly problematic:

  • Batch operations: Multiple commands in sequence would trigger multiple expensive refreshes, even though Unity coalesces file system changes and one refresh would suffice
  • Async blocking: The synchronous AssetDatabase.Refresh() blocks the async pathway, negating the benefits of async command execution
  • Redundant refreshes: Consecutive refresh calls provide no additional benefit since the first call captures all pending changes

The same alternatives suggested for ExecuteCommand apply here, with additional consideration for batching scenarios where refresh should ideally happen once before or after the batch, not per command.

🤖 Prompt for AI Agents 
In MCPForUnity/Editor/Tools/CommandRegistry.cs at line 261, remove the
unconditional synchronous AssetDatabase.Refresh() call because this method is
used for batch execution and async workflows; instead make refresh behavior
controllable and non-blocking by (a) adding an optional parameter or return flag
(e.g., bool requestRefresh = false) so callers can defer refresh for batches,
(b) when immediate refresh is needed schedule it via EditorApplication.delayCall
(or similar main-thread async scheduling) rather than blocking the async path,
and/or (c) for bulk operations prefer
AssetDatabase.StartAssetEditing()/StopAssetEditing() or a single consolidated
Refresh() around the batch — implement one of these approaches so individual
command executions do not trigger redundant, blocking refreshes.

var handlerInfo = GetHandlerInfo(commandName);
var payload = @params ?? new JObject();

Expand Down