Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions MCPForUnity/Editor/Tools/ManageGameObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1450,7 +1450,11 @@ string searchMethod
{
var compToken = componentsToAddArray.First;
if (compToken.Type == JTokenType.String)
{
typeName = compToken.ToString();
// Check for properties in top-level componentProperties parameter
properties = @params["componentProperties"]?[typeName] as JObject;
}
else if (compToken is JObject compObj)
{
typeName = compObj["typeName"]?.ToString();
Expand Down
86 changes: 74 additions & 12 deletions MCPForUnity/Editor/Tools/ManageMaterial.cs
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,8 @@ private static object CreateMaterial(JObject @params)
{
string materialPath = NormalizePath(@params["materialPath"]?.ToString());
string shaderName = @params["shader"]?.ToString() ?? "Standard";
JToken colorToken = @params["color"];
string colorProperty = @params["property"]?.ToString();

JObject properties = null;
JToken propsToken = @params["properties"];
Expand Down Expand Up @@ -494,25 +496,85 @@ private static object CreateMaterial(JObject @params)
return new { status = "error", message = $"Could not find shader: {shaderName}" };
}

Material material = new Material(shader);

// Check for existing asset to avoid silent overwrite
if (AssetDatabase.LoadAssetAtPath<Material>(materialPath) != null)
{
return new { status = "error", message = $"Material already exists at {materialPath}" };
}
AssetDatabase.CreateAsset(material, materialPath);

if (properties != null)

Material material = null;
var shouldDestroyMaterial = true;
try
{
MaterialOps.ApplyProperties(material, properties, ManageGameObject.InputSerializer);
}

EditorUtility.SetDirty(material);
AssetDatabase.SaveAssets();
material = new Material(shader);

return new { status = "success", message = $"Created material at {materialPath} with shader {shaderName}" };
// Apply color param during creation (keeps Python tool signature and C# implementation consistent).
// If "properties" already contains a color property, let properties win.
bool shouldApplyColor = false;
if (colorToken != null)
{
if (properties == null)
{
shouldApplyColor = true;
}
else if (!string.IsNullOrEmpty(colorProperty))
{
// If colorProperty is specified, only check that specific property.
shouldApplyColor = !properties.ContainsKey(colorProperty);
}
else
{
// If colorProperty is not specified, check fallback properties.
shouldApplyColor = !properties.ContainsKey("_BaseColor") && !properties.ContainsKey("_Color");
}
}

if (shouldApplyColor)
{
Color color;
try
{
color = MaterialOps.ParseColor(colorToken, ManageGameObject.InputSerializer);
}
catch (Exception e)
{
return new { status = "error", message = $"Invalid color format: {e.Message}" };
}

if (!string.IsNullOrEmpty(colorProperty) && material.HasProperty(colorProperty))
{
material.SetColor(colorProperty, color);
}
else if (material.HasProperty("_BaseColor"))
{
material.SetColor("_BaseColor", color);
}
else if (material.HasProperty("_Color"))
{
material.SetColor("_Color", color);
}
}

AssetDatabase.CreateAsset(material, materialPath);
shouldDestroyMaterial = false; // material is now owned by the AssetDatabase

if (properties != null)
{
MaterialOps.ApplyProperties(material, properties, ManageGameObject.InputSerializer);
}

EditorUtility.SetDirty(material);
AssetDatabase.SaveAssets();

return new { status = "success", message = $"Created material at {materialPath} with shader {shaderName}" };
}
finally
{
if (shouldDestroyMaterial && material != null)
{
UnityEngine.Object.DestroyImmediate(material);
}
}
}
}
}
93 changes: 93 additions & 0 deletions Server/src/transport/unity_instance_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
into the request-scoped state, allowing tools to access it via ctx.get_state("unity_instance").
"""
from threading import RLock
import time
import logging

from fastmcp.server.middleware import Middleware, MiddlewareContext
Expand Down Expand Up @@ -49,6 +50,8 @@ def __init__(self):
super().__init__()
self._active_by_key: dict[str, str] = {}
self._lock = RLock()
self._auto_select_none_until: float = 0.0
self._auto_select_backoff_seconds = 5.0

def get_session_key(self, ctx) -> str:
"""
Expand Down Expand Up @@ -83,11 +86,101 @@ def clear_active_instance(self, ctx) -> None:
with self._lock:
self._active_by_key.pop(key, None)

async def _maybe_autoselect_instance(self, ctx) -> str | None:
"""Auto-select sole Unity instance when no active instance is set."""
from transport.unity_transport import _current_transport
from transport.legacy.unity_connection import get_unity_connection_pool

if self._auto_select_none_until and time.time() < self._auto_select_none_until:
return None

transport = _current_transport()
if PluginHub.is_configured():
try:
sessions_data = await PluginHub.get_sessions()
sessions = sessions_data.sessions or {}
ids: list[str] = []
for session_info in sessions.values():
project = getattr(session_info, "project", None) or "Unknown"
hash_value = getattr(session_info, "hash", None)
if hash_value and isinstance(hash_value, str) and hash_value.strip():
ids.append(f"{project}@{hash_value}")
if len(ids) == 1:
chosen = ids[0]
self.set_active_instance(ctx, chosen)
self._auto_select_none_until = 0.0
logger.info(
"Auto-selected sole Unity instance via PluginHub: %s",
chosen,
)
return chosen
except (ConnectionError, ValueError, KeyError, TimeoutError) as exc:
logger.debug(
"PluginHub auto-select probe failed (%s); falling back to stdio",
type(exc).__name__,
exc_info=True,
)
except AttributeError as exc:
logger.debug(
"PluginHub auto-select probe failed (%s); falling back to stdio",
type(exc).__name__,
exc_info=True,
)
except Exception as exc:
if isinstance(exc, (SystemExit, KeyboardInterrupt)):
raise
logger.debug(
"PluginHub auto-select probe failed with unexpected error (%s); falling back to stdio",
type(exc).__name__,
exc_info=True,
)

if transport != "http":
try:
pool = get_unity_connection_pool()
instances = pool.discover_all_instances(force_refresh=True)
ids = [getattr(inst, "id", None) for inst in instances]
ids = [inst_id for inst_id in ids if inst_id]
if len(ids) == 1:
chosen = ids[0]
self.set_active_instance(ctx, chosen)
self._auto_select_none_until = 0.0
logger.info(
"Auto-selected sole Unity instance via stdio discovery: %s",
chosen,
)
return chosen
except (ConnectionError, ValueError, KeyError, TimeoutError) as exc:
logger.debug(
"Stdio auto-select probe failed (%s)",
type(exc).__name__,
exc_info=True,
)
except AttributeError as exc:
logger.debug(
"Stdio auto-select probe failed (%s)",
type(exc).__name__,
exc_info=True,
)
except Exception as exc:
if isinstance(exc, (SystemExit, KeyboardInterrupt)):
raise
logger.debug(
"Stdio auto-select probe failed with unexpected error (%s)",
type(exc).__name__,
exc_info=True,
)

self._auto_select_none_until = time.time() + self._auto_select_backoff_seconds
return None

async def _inject_unity_instance(self, context: MiddlewareContext) -> None:
"""Inject active Unity instance into context if available."""
ctx = context.fastmcp_context
Comment on lines 177 to 179
Copy link

Choose a reason for hiding this comment

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

style: Auto-selection runs on every tool call when no active instance is set, potentially causing redundant discovery scans. Consider caching the "no instances available" result temporarily to avoid repeated expensive operations.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: Server/src/transport/unity_instance_middleware.py
Line: 142:144

Comment:
**style:** Auto-selection runs on every tool call when no active instance is set, potentially causing redundant discovery scans. Consider caching the "no instances available" result temporarily to avoid repeated expensive operations.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.


active_instance = self.get_active_instance(ctx)
if not active_instance:
active_instance = await self._maybe_autoselect_instance(ctx)
if active_instance:
# If using HTTP transport (PluginHub configured), validate session
# But for stdio transport (no PluginHub needed or maybe partially configured),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -630,5 +630,137 @@ public void GetComponentData_WorksWithMultipleMaterials()
UnityEngine.Object.DestroyImmediate(material2);
UnityEngine.Object.DestroyImmediate(testObject);
}

[Test]
public void AddComponent_StringArrayFormat_AppliesComponentProperties()
{
// Arrange - Create a GameObject to add component to
var testObject = new GameObject("AddComponentTestObject");

// Create params using string array format with top-level componentProperties
var addComponentParams = new JObject
{
["action"] = "add_component",
["target"] = testObject.name,
["search_method"] = "by_name",
["componentsToAdd"] = new JArray { "Rigidbody" },
["componentProperties"] = new JObject
{
["Rigidbody"] = new JObject
{
["mass"] = 7.5f,
["useGravity"] = false,
["drag"] = 2.0f
}
}
};

// Act
var result = ManageGameObject.HandleCommand(addComponentParams);

// Assert - Verify component was added
var rigidbody = testObject.GetComponent<Rigidbody>();
Assert.IsNotNull(rigidbody, "Rigidbody component should be added to GameObject");

// Verify properties were set correctly during component creation
Assert.AreEqual(7.5f, rigidbody.mass, 0.001f,
"Mass should be set to 7.5 via componentProperties during add_component");
Assert.AreEqual(false, rigidbody.useGravity,
"UseGravity should be set to false via componentProperties during add_component");
Assert.AreEqual(2.0f, rigidbody.drag, 0.001f,
"Drag should be set to 2.0 via componentProperties during add_component");

// Verify result indicates success
Assert.IsNotNull(result, "Should return a result object");
var resultObj = result as JObject ?? JObject.FromObject(result);
Assert.IsTrue(resultObj.Value<bool>("success"),
"Result should indicate success when adding component with properties");

// Clean up
UnityEngine.Object.DestroyImmediate(testObject);
}

[Test]
public void AddComponent_ObjectFormat_StillAppliesComponentProperties()
{
// Arrange - Create a GameObject to add component to
var testObject = new GameObject("AddComponentObjectFormatTestObject");

// Create params using object array format (existing behavior)
var addComponentParams = new JObject
{
["action"] = "add_component",
["target"] = testObject.name,
["search_method"] = "by_name",
["componentsToAdd"] = new JArray
{
new JObject
{
["typeName"] = "Rigidbody",
["properties"] = new JObject
{
["mass"] = 3.5f,
["useGravity"] = true
}
}
}
};

// Act
var result = ManageGameObject.HandleCommand(addComponentParams);

// Assert - Verify component was added
var rigidbody = testObject.GetComponent<Rigidbody>();
Assert.IsNotNull(rigidbody, "Rigidbody component should be added to GameObject");

// Verify properties were set correctly
Assert.AreEqual(3.5f, rigidbody.mass, 0.001f,
"Mass should be set to 3.5 via inline properties");
Assert.AreEqual(true, rigidbody.useGravity,
"UseGravity should be set to true via inline properties");

// Clean up
UnityEngine.Object.DestroyImmediate(testObject);
}

[Test]
public void AddComponent_ComponentNameFormat_AppliesComponentProperties()
{
// Arrange - Create a GameObject to add component to
var testObject = new GameObject("AddComponentNameFormatTestObject");

// Create params using componentName format (existing behavior)
var addComponentParams = new JObject
{
["action"] = "add_component",
["target"] = testObject.name,
["search_method"] = "by_name",
["componentName"] = "Rigidbody",
["componentProperties"] = new JObject
{
["Rigidbody"] = new JObject
{
["mass"] = 5.0f,
["drag"] = 1.5f
}
}
};

// Act
var result = ManageGameObject.HandleCommand(addComponentParams);

// Assert - Verify component was added
var rigidbody = testObject.GetComponent<Rigidbody>();
Assert.IsNotNull(rigidbody, "Rigidbody component should be added to GameObject");

// Verify properties were set correctly
Assert.AreEqual(5.0f, rigidbody.mass, 0.001f,
"Mass should be set to 5.0 via componentName format");
Assert.AreEqual(1.5f, rigidbody.drag, 0.001f,
"Drag should be set to 1.5 via componentName format");

// Clean up
UnityEngine.Object.DestroyImmediate(testObject);
}
}
}