Skip to content
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
92 changes: 80 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,91 @@ 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);

// 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");
}
}

return new { status = "success", message = $"Created material at {materialPath} with shader {shaderName}" };
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);
}
else
{
Debug.LogWarning(
$"Could not find suitable color property to apply color during material creation at {materialPath}"
);
}
Comment on lines +544 to +561
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider explicit error when specified colorProperty doesn't exist.

When a user explicitly specifies colorProperty but that property doesn't exist on the material, the code silently falls back to _BaseColor or _Color. This differs from SetMaterialColor (line 192), which returns an error in similar cases.

Consider adding an explicit check and error return when the user-specified property doesn't exist:

🔎 Suggested improvement for explicit property validation
 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))
+    if (!string.IsNullOrEmpty(colorProperty))
     {
-        material.SetColor(colorProperty, color);
+        if (material.HasProperty(colorProperty))
+        {
+            material.SetColor(colorProperty, color);
+        }
+        else
+        {
+            return new { status = "error", message = $"Specified color property '{colorProperty}' does not exist on shader {shaderName}" };
+        }
     }
     else if (material.HasProperty("_BaseColor"))
     {
         material.SetColor("_BaseColor", color);
     }
     else if (material.HasProperty("_Color"))
     {
         material.SetColor("_Color", color);
     }
     else
     {
-        Debug.LogWarning(
-            $"Could not find suitable color property to apply color during material creation at {materialPath}"
-        );
+        return new { status = "error", message = "Could not find suitable color property (_BaseColor or _Color) on shader" };
     }
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
MCPForUnity/Editor/Tools/ManageMaterial.cs around lines 544-561: the current
logic falls back to _BaseColor/_Color when a user-specified colorProperty is
provided but missing on the material; change it to explicitly treat a missing
user-specified property as an error (match SetMaterialColor at ~line 192).
Concretely, if colorProperty is not null/empty and
material.HasProperty(colorProperty) is false, log an error (or return a failure)
indicating the specified property doesn't exist on the material and abort
applying the color; only allow fallback to _BaseColor/_Color when no explicit
colorProperty was provided. Ensure the error path returns the same error/false
contract used by SetMaterialColor.

}

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);
}
}
}
}
}
82 changes: 82 additions & 0 deletions Server/src/transport/unity_instance_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,93 @@ 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."""
try:
from transport.unity_transport import _current_transport

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:
ids.append(f"{project}@{hash_value}")
if len(ids) == 1:
chosen = ids[0]
self.set_active_instance(ctx, chosen)
logger.info(
"Auto-selected sole Unity instance via PluginHub: %s",
chosen,
)
return chosen
except (ConnectionError, ValueError, KeyError, TimeoutError, AttributeError, RuntimeError) 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:
from transport.legacy.unity_connection import get_unity_connection_pool

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)
logger.info(
"Auto-selected sole Unity instance via stdio discovery: %s",
chosen,
)
return chosen
except (ConnectionError, ValueError, KeyError, TimeoutError, AttributeError, RuntimeError) 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,
)
except Exception as exc:
if isinstance(exc, (SystemExit, KeyboardInterrupt)):
raise
logger.debug(
"Auto-select path encountered an unexpected error (%s)",
type(exc).__name__,
exc_info=True,
)

return None

async def _inject_unity_instance(self, context: MiddlewareContext) -> None:
"""Inject active Unity instance into context if available."""
ctx = context.fastmcp_context

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
39 changes: 39 additions & 0 deletions Server/tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
SERVER_ROOT = Path(__file__).resolve().parents[2]
if str(SERVER_ROOT) not in sys.path:
sys.path.insert(0, str(SERVER_ROOT))
SERVER_SRC = SERVER_ROOT / "src"
if str(SERVER_SRC) not in sys.path:
sys.path.insert(0, str(SERVER_SRC))

# Ensure telemetry is disabled during test collection and execution to avoid
# any background network or thread startup that could slow or block pytest.
Expand Down Expand Up @@ -86,3 +89,39 @@ class _DummyMiddlewareContext:
fastmcp_server.middleware = fastmcp_server_middleware
sys.modules.setdefault("fastmcp.server", fastmcp_server)
sys.modules.setdefault("fastmcp.server.middleware", fastmcp_server_middleware)

# Stub minimal starlette modules to avoid optional dependency imports.
starlette = types.ModuleType("starlette")
starlette_endpoints = types.ModuleType("starlette.endpoints")
starlette_websockets = types.ModuleType("starlette.websockets")
starlette_requests = types.ModuleType("starlette.requests")
starlette_responses = types.ModuleType("starlette.responses")


class _DummyWebSocketEndpoint:
pass


class _DummyWebSocket:
pass


class _DummyRequest:
pass


class _DummyJSONResponse:
def __init__(self, *args, **kwargs):
pass


starlette_endpoints.WebSocketEndpoint = _DummyWebSocketEndpoint
starlette_websockets.WebSocket = _DummyWebSocket
starlette_requests.Request = _DummyRequest
starlette_responses.JSONResponse = _DummyJSONResponse

sys.modules.setdefault("starlette", starlette)
sys.modules.setdefault("starlette.endpoints", starlette_endpoints)
sys.modules.setdefault("starlette.websockets", starlette_websockets)
sys.modules.setdefault("starlette.requests", starlette_requests)
sys.modules.setdefault("starlette.responses", starlette_responses)
Loading