Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
f713810
Fix issue #308: Find py files in MCPForUnityTools and version.txt
JohanHoltby Oct 9, 2025
48d912e
Follow-up: address CodeRabbit feedback for #308 (<GetToolsFolderIdent…
JohanHoltby Oct 9, 2025
68f09e1
Follow-up: address CodeRabbit feedback for #308 – centralize GetTools…
JohanHoltby Oct 9, 2025
069f1be
Fixing so the MCP don't removes _skipDirs e.g. __pycache__
JohanHoltby Oct 10, 2025
b1f696d
skip empty folders with no py files
JohanHoltby Oct 10, 2025
88a6491
Rabbit: "Fix identifier collision between different package roots."
JohanHoltby Oct 10, 2025
6a4d55b
Update MCPForUnity/Editor/Helpers/ServerInstaller.cs
JohanHoltby Oct 10, 2025
fac3b95
Rabbbit: Cleanup may delete server’s built-in tool subfolders — restr…
JohanHoltby Oct 10, 2025
3fef23e
Merge branch 'fix-issue-308' of https://github.com/Valectric/unity-mc…
JohanHoltby Oct 10, 2025
571ca8c
Fixed minor + missed onadding rabit change
JohanHoltby Oct 12, 2025
da98ba9
Revert "Fixed minor + missed onadding rabit change"
msanatan Oct 17, 2025
d502dae
Merge branch 'main' into pr-309
msanatan Oct 17, 2025
81f5006
refactor: remove Unity project tools copying and version tracking fun…
msanatan Oct 17, 2025
9a152e3
refactor: consolidate module discovery logic into shared utility func…
msanatan Oct 17, 2025
6e5ccb5
Remove unused imports
msanatan Oct 17, 2025
650806f
feat: add Python tool registry and sync system for MCP server integra…
msanatan Oct 17, 2025
38fe922
feat: add auto-sync processor for Python tools with Unity editor inte…
msanatan Oct 17, 2025
04798f9
feat: add menu item to reimport all Python files in project
msanatan Oct 17, 2025
4c8c611
Fix infinite loop error
msanatan Oct 17, 2025
8753c30
refactor: move Python tool sync menu items under Window/MCP For Unity…
msanatan Oct 17, 2025
9627e47
Update docs
msanatan Oct 18, 2025
7ba6967
Remove duplicate header
msanatan Oct 18, 2025
366936a
feat: add OnValidate handler to sync Python tools when asset is modified
msanatan Oct 18, 2025
1d6c55b
test: add unit tests for Python tools asset and sync services
msanatan Oct 18, 2025
1c676c4
Update MCPForUnity/Editor/Helpers/PythonToolSyncProcessor.cs
msanatan Oct 18, 2025
141064b
style: remove trailing whitespace from Python tool sync files
msanatan Oct 18, 2025
e58285e
test: remove incomplete unit tests from ToolSyncServiceTests
msanatan Oct 18, 2025
b0ec0eb
perf: optimize Python file reimport by using AssetDatabase.FindAssets…
msanatan Oct 18, 2025
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
1 change: 0 additions & 1 deletion MCPForUnity/Editor/Data/McpClients.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Runtime.InteropServices;
using MCPForUnity.Editor.Models;

namespace MCPForUnity.Editor.Data
Expand Down
107 changes: 107 additions & 0 deletions MCPForUnity/Editor/Data/PythonToolsAsset.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
using System;
using System.Collections.Generic;
using System.Linq;
using UnityEngine;

namespace MCPForUnity.Editor.Data
{
/// <summary>
/// Registry of Python tool files to sync to the MCP server.
/// Add your Python files here - they can be stored anywhere in your project.
/// </summary>
[CreateAssetMenu(fileName = "PythonTools", menuName = "MCP For Unity/Python Tools")]
public class PythonToolsAsset : ScriptableObject
{
[Tooltip("Add Python files (.py) to sync to the MCP server. Files can be located anywhere in your project.")]
public List<TextAsset> pythonFiles = new List<TextAsset>();

[Header("Sync Options")]
[Tooltip("Use content hashing to detect changes (recommended). If false, always copies on startup.")]
public bool useContentHashing = true;

[Header("Sync State (Read-only)")]
[Tooltip("Internal tracking - do not modify")]
public List<PythonFileState> fileStates = new List<PythonFileState>();

/// <summary>
/// Gets all valid Python files (filters out null/missing references)
/// </summary>
public IEnumerable<TextAsset> GetValidFiles()
{
return pythonFiles.Where(f => f != null);
}

/// <summary>
/// Checks if a file needs syncing
/// </summary>
public bool NeedsSync(TextAsset file, string currentHash)
{
if (!useContentHashing) return true; // Always sync if hashing disabled

var state = fileStates.FirstOrDefault(s => s.assetGuid == GetAssetGuid(file));
return state == null || state.contentHash != currentHash;
}
Comment on lines +37 to +43
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 18, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Non‑asset TextAssets get empty GUIDs → state collisions

AssetDatabase.GetAssetPath on non‑asset TextAssets returns empty, so AssetPathToGUID is empty. Multiple unsaved TextAssets then share the same key, corrupting fileStates. Guard and skip non‑asset entries.

-        public bool NeedsSync(TextAsset file, string currentHash)
+        public bool NeedsSync(TextAsset file, string currentHash)
         {
-            if (!useContentHashing) return true; // Always sync if hashing disabled
-
-            var state = fileStates.FirstOrDefault(s => s.assetGuid == GetAssetGuid(file));
+            if (!useContentHashing) return true; // Always sync if hashing disabled
+            if (!TryGetAssetGuid(file, out var guid)) return true; // treat as needing sync but don't record
+            var state = fileStates.FirstOrDefault(s => s.assetGuid == guid);
             return state == null || state.contentHash != currentHash;
         }
@@
-        public void RecordSync(TextAsset file, string hash)
+        public void RecordSync(TextAsset file, string hash)
         {
-            string guid = GetAssetGuid(file);
-            var state = fileStates.FirstOrDefault(s => s.assetGuid == guid);
+            if (!TryGetAssetGuid(file, out var guid))
+            {
+                Debug.LogWarning($"[{nameof(PythonToolsAsset)}] Skipping sync state for non-asset TextAsset '{file?.name}'. Save it under Assets/ to track.");
+                return;
+            }
+            var state = fileStates.FirstOrDefault(s => s.assetGuid == guid);
@@
-        private string GetAssetGuid(TextAsset asset)
-        {
-            return UnityEditor.AssetDatabase.AssetPathToGUID(UnityEditor.AssetDatabase.GetAssetPath(asset));
-        }
+        private string GetAssetGuid(TextAsset asset)
+        {
+            return UnityEditor.AssetDatabase.AssetPathToGUID(UnityEditor.AssetDatabase.GetAssetPath(asset));
+        }
+
+        private bool TryGetAssetGuid(TextAsset asset, out string guid)
+        {
+            guid = UnityEditor.AssetDatabase.AssetPathToGUID(UnityEditor.AssetDatabase.GetAssetPath(asset));
+            return !string.IsNullOrEmpty(guid);
+        }

Also applies to: 48-62, 73-76

🤖 Prompt for AI Agents
In MCPForUnity/Editor/Data/PythonToolsAsset.cs around lines 37-43 (also apply
same check to ranges 48-62 and 73-76): AssetDatabase.GetAssetPath returns an
empty path for non-asset/unsaved TextAsset which makes AssetPathToGUID return an
empty GUID, causing collisions in fileStates; modify methods that call
GetAssetGuid/GetAssetPath to detect empty asset paths (or empty GUIDs) and skip
processing those TextAssets (return false from NeedsSync, or do not add/update
entries in fileStates) so non-asset TextAssets are ignored. Ensure the guard is
applied consistently in all three affected ranges so you never use an empty GUID
as a key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is super unlikely, devs are just going to drag Python files the scriptable object, or add them to the list directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

@msanatan, you're right—in the typical workflow where developers drag Python asset files from the Project window into the inspector, those will always have valid GUIDs. I understand the defensive guard isn't needed for the expected use case. Thanks for clarifying! 🐰


🧠 Learnings used
Learnt from: JohanHoltby
PR: CoplayDev/unity-mcp#309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.


/// <summary>
/// Records that a file was synced
/// </summary>
public void RecordSync(TextAsset file, string hash)
{
string guid = GetAssetGuid(file);
var state = fileStates.FirstOrDefault(s => s.assetGuid == guid);

if (state == null)
{
state = new PythonFileState { assetGuid = guid };
fileStates.Add(state);
}

state.contentHash = hash;
state.lastSyncTime = DateTime.UtcNow;
state.fileName = file.name;
}

/// <summary>
/// Removes state entries for files no longer in the list
/// </summary>
public void CleanupStaleStates()
{
var validGuids = new HashSet<string>(GetValidFiles().Select(GetAssetGuid));
fileStates.RemoveAll(s => !validGuids.Contains(s.assetGuid));
}

private string GetAssetGuid(TextAsset asset)
{
return UnityEditor.AssetDatabase.AssetPathToGUID(UnityEditor.AssetDatabase.GetAssetPath(asset));
}

/// <summary>
/// Called when the asset is modified in the Inspector
/// Triggers sync to handle file additions/removals
/// </summary>
private void OnValidate()
{
// Cleanup stale states immediately
CleanupStaleStates();

// Trigger sync after a delay to handle file removals
// Delay ensures the asset is saved before sync runs
UnityEditor.EditorApplication.delayCall += () =>
{
if (this != null) // Check if asset still exists
{
MCPForUnity.Editor.Helpers.PythonToolSyncProcessor.SyncAllTools();
}
};
}
}

[Serializable]
public class PythonFileState
{
public string assetGuid;
public string fileName;
public string contentHash;
public DateTime lastSyncTime;
}
}
11 changes: 11 additions & 0 deletions MCPForUnity/Editor/Data/PythonToolsAsset.cs.meta

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

192 changes: 192 additions & 0 deletions MCPForUnity/Editor/Helpers/PythonToolSyncProcessor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
using System.IO;
using System.Linq;
using MCPForUnity.Editor.Data;
using MCPForUnity.Editor.Services;
using UnityEditor;
using UnityEngine;

namespace MCPForUnity.Editor.Helpers
{
/// <summary>
/// Automatically syncs Python tools to the MCP server when:
/// - PythonToolsAsset is modified
/// - Python files are imported/reimported
/// - Unity starts up
/// </summary>
[InitializeOnLoad]
public class PythonToolSyncProcessor : AssetPostprocessor
{
private const string SyncEnabledKey = "MCPForUnity.AutoSyncEnabled";
private static bool _isSyncing = false;

static PythonToolSyncProcessor()
{
// Sync on Unity startup
EditorApplication.delayCall += () =>
{
if (IsAutoSyncEnabled())
{
SyncAllTools();
}
};
}

/// <summary>
/// Called after any assets are imported, deleted, or moved
/// </summary>
private static void OnPostprocessAllAssets(
string[] importedAssets,
string[] deletedAssets,
string[] movedAssets,
string[] movedFromAssetPaths)
{
// Prevent infinite loop - don't process if we're currently syncing
if (_isSyncing || !IsAutoSyncEnabled())
return;

bool needsSync = false;

// Only check for .py file changes, not PythonToolsAsset changes
// (PythonToolsAsset changes are internal state updates from syncing)
foreach (string path in importedAssets.Concat(movedAssets))
{
// Check if any .py files were modified
if (path.EndsWith(".py"))
{
needsSync = true;
break;
}
}

// Check if any .py files were deleted
if (!needsSync && deletedAssets.Any(path => path.EndsWith(".py")))
{
needsSync = true;
}

if (needsSync)
{
SyncAllTools();
}
}

/// <summary>
/// Syncs all Python tools from all PythonToolsAsset instances to the MCP server
/// </summary>
public static void SyncAllTools()
{
// Prevent re-entrant calls
if (_isSyncing)
{
McpLog.Warn("Sync already in progress, skipping...");
return;
}

_isSyncing = true;
try
{
if (!ServerPathResolver.TryFindEmbeddedServerSource(out string srcPath))
{
McpLog.Warn("Cannot sync Python tools: MCP server source not found");
return;
}

string toolsDir = Path.Combine(srcPath, "tools", "custom");

var result = MCPServiceLocator.ToolSync.SyncProjectTools(toolsDir);

if (result.Success)
{
if (result.CopiedCount > 0 || result.SkippedCount > 0)
{
McpLog.Info($"Python tools synced: {result.CopiedCount} copied, {result.SkippedCount} skipped");
}
}
else
{
McpLog.Error($"Python tool sync failed with {result.ErrorCount} errors");
foreach (var msg in result.Messages)
{
McpLog.Error($" - {msg}");
}
}
}
catch (System.Exception ex)
{
McpLog.Error($"Python tool sync exception: {ex.Message}");
}
finally
{
_isSyncing = false;
}
}

/// <summary>
/// Checks if auto-sync is enabled (default: true)
/// </summary>
public static bool IsAutoSyncEnabled()
{
return EditorPrefs.GetBool(SyncEnabledKey, true);
}

/// <summary>
/// Enables or disables auto-sync
/// </summary>
public static void SetAutoSyncEnabled(bool enabled)
{
EditorPrefs.SetBool(SyncEnabledKey, enabled);
McpLog.Info($"Python tool auto-sync {(enabled ? "enabled" : "disabled")}");
}

/// <summary>
/// Menu item to reimport all Python files in the project
/// </summary>
[MenuItem("Window/MCP For Unity/Tool Sync/Reimport Python Files", priority = 99)]
public static void ReimportPythonFiles()
{
// Find all Python files (imported as TextAssets by PythonFileImporter)
var pythonGuids = AssetDatabase.FindAssets("t:TextAsset", new[] { "Assets" })
.Select(AssetDatabase.GUIDToAssetPath)
.Where(path => path.EndsWith(".py", System.StringComparison.OrdinalIgnoreCase))
.ToArray();

foreach (string path in pythonGuids)
{
AssetDatabase.ImportAsset(path, ImportAssetOptions.ForceUpdate);
}

int count = pythonGuids.Length;
McpLog.Info($"Reimported {count} Python files");
AssetDatabase.Refresh();
}

/// <summary>
/// Menu item to manually trigger sync
/// </summary>
[MenuItem("Window/MCP For Unity/Tool Sync/Sync Python Tools", priority = 100)]
public static void ManualSync()
{
McpLog.Info("Manually syncing Python tools...");
SyncAllTools();
}

/// <summary>
/// Menu item to toggle auto-sync
/// </summary>
[MenuItem("Window/MCP For Unity/Tool Sync/Auto-Sync Python Tools", priority = 101)]
public static void ToggleAutoSync()
{
SetAutoSyncEnabled(!IsAutoSyncEnabled());
}

/// <summary>
/// Validate menu item (shows checkmark when enabled)
/// </summary>
[MenuItem("Window/MCP For Unity/Tool Sync/Auto-Sync Python Tools", true, priority = 101)]
public static bool ToggleAutoSyncValidate()
{
Menu.SetChecked("Window/MCP For Unity/Tool Sync/Auto-Sync Python Tools", IsAutoSyncEnabled());
return true;
}
}
}
11 changes: 11 additions & 0 deletions MCPForUnity/Editor/Helpers/PythonToolSyncProcessor.cs.meta

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions MCPForUnity/Editor/Helpers/ServerInstaller.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public static void EnsureServerInstalled()
// Copy the entire UnityMcpServer folder (parent of src)
string embeddedRoot = Path.GetDirectoryName(embeddedSrc) ?? embeddedSrc; // go up from src to UnityMcpServer
CopyDirectoryRecursive(embeddedRoot, destRoot);

// Write/refresh version file
try { File.WriteAllText(Path.Combine(destSrc, VersionFileName), embeddedVer ?? "unknown"); } catch { }
McpLog.Info($"Installed/updated server to {destRoot} (version {embeddedVer}).");
Expand Down Expand Up @@ -410,6 +411,7 @@ private static bool TryGetEmbeddedServerSource(out string srcPath)
}

private static readonly string[] _skipDirs = { ".venv", "__pycache__", ".pytest_cache", ".mypy_cache", ".git" };

private static void CopyDirectoryRecursive(string sourceDir, string destinationDir)
{
Directory.CreateDirectory(destinationDir);
Expand Down
8 changes: 8 additions & 0 deletions MCPForUnity/Editor/Importers.meta

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions MCPForUnity/Editor/Importers/PythonFileImporter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using UnityEngine;
using UnityEditor.AssetImporters;
using System.IO;

namespace MCPForUnity.Editor.Importers
{
/// <summary>
/// Custom importer that allows Unity to recognize .py files as TextAssets.
/// This enables Python files to be selected in the Inspector and used like any other text asset.
/// </summary>
[ScriptedImporter(1, "py")]
public class PythonFileImporter : ScriptedImporter
{
public override void OnImportAsset(AssetImportContext ctx)
{
var textAsset = new TextAsset(File.ReadAllText(ctx.assetPath));
ctx.AddObjectToAsset("main obj", textAsset);
ctx.SetMainObject(textAsset);
}
}
}
11 changes: 11 additions & 0 deletions MCPForUnity/Editor/Importers/PythonFileImporter.cs.meta

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading