fix: address CodeRabbit feedback on resource leaks and shader safety

- Fix material creation to handle missing shaders (URP/HDRP fallbacks)
- Add try/finally blocks for RenderTexture resource management
- Fix Python closure variable binding (ruff B023)

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
main
David Sarno 2025-09-03 17:11:30 -07:00
parent 290c913a0f
commit 264b585ceb
2 changed files with 34 additions and 19 deletions

View File

@ -179,8 +179,18 @@ namespace MCPForUnity.Editor.Tools
} }
else if (lowerAssetType == "material") else if (lowerAssetType == "material")
{ {
Material mat = new Material(Shader.Find("Standard")); // Default shader // Prefer provided shader; fall back to common pipelines
// TODO: Apply properties from JObject (e.g., shader name, color, texture assignments) var requested = properties?["shader"]?.ToString();
Shader shader =
(!string.IsNullOrEmpty(requested) ? Shader.Find(requested) : null)
?? Shader.Find("Universal Render Pipeline/Lit")
?? Shader.Find("HDRP/Lit")
?? Shader.Find("Standard")
?? Shader.Find("Unlit/Color");
if (shader == null)
return Response.Error($"Could not find a suitable shader (requested: '{requested ?? "none"}').");
var mat = new Material(shader);
if (properties != null) if (properties != null)
ApplyMaterialProperties(mat, properties); ApplyMaterialProperties(mat, properties);
AssetDatabase.CreateAsset(mat, fullPath); AssetDatabase.CreateAsset(mat, fullPath);
@ -1261,24 +1271,29 @@ namespace MCPForUnity.Editor.Tools
{ {
// Ensure texture is readable for EncodeToPNG // Ensure texture is readable for EncodeToPNG
// Creating a temporary readable copy is safer // Creating a temporary readable copy is safer
RenderTexture rt = RenderTexture.GetTemporary( RenderTexture rt = null;
preview.width, Texture2D readablePreview = null;
preview.height
);
Graphics.Blit(preview, rt);
RenderTexture previous = RenderTexture.active; RenderTexture previous = RenderTexture.active;
try
{
rt = RenderTexture.GetTemporary(preview.width, preview.height);
Graphics.Blit(preview, rt);
RenderTexture.active = rt; RenderTexture.active = rt;
Texture2D readablePreview = new Texture2D(preview.width, preview.height); readablePreview = new Texture2D(preview.width, preview.height);
readablePreview.ReadPixels(new Rect(0, 0, rt.width, rt.height), 0, 0); readablePreview.ReadPixels(new Rect(0, 0, rt.width, rt.height), 0, 0);
readablePreview.Apply(); readablePreview.Apply();
}
finally
{
RenderTexture.active = previous; RenderTexture.active = previous;
RenderTexture.ReleaseTemporary(rt); if (rt != null) RenderTexture.ReleaseTemporary(rt);
}
byte[] pngData = readablePreview.EncodeToPNG(); var pngData = readablePreview.EncodeToPNG();
previewBase64 = Convert.ToBase64String(pngData); previewBase64 = Convert.ToBase64String(pngData);
previewWidth = readablePreview.width; previewWidth = readablePreview.width;
previewHeight = readablePreview.height; previewHeight = readablePreview.height;
UnityEngine.Object.DestroyImmediate(readablePreview); // Clean up temp texture UnityEngine.Object.DestroyImmediate(readablePreview);
} }
catch (Exception ex) catch (Exception ex)
{ {

View File

@ -630,8 +630,8 @@ def register_manage_script_edits_tools(mcp: FastMCP):
if not m: if not m:
continue continue
# Expand $1, $2... in replacement using this match # Expand $1, $2... in replacement using this match
def _expand_dollars(rep: str) -> str: def _expand_dollars(rep: str, _m=m) -> str:
return _re.sub(r"\$(\d+)", lambda g: m.group(int(g.group(1))) or "", rep) return _re.sub(r"\$(\d+)", lambda g: _m.group(int(g.group(1))) or "", rep)
repl = _expand_dollars(text_field) repl = _expand_dollars(text_field)
sl, sc = line_col_from_index(m.start()) sl, sc = line_col_from_index(m.start())
el, ec = line_col_from_index(m.end()) el, ec = line_col_from_index(m.end())
@ -767,8 +767,8 @@ def register_manage_script_edits_tools(mcp: FastMCP):
if not m: if not m:
continue continue
# Expand $1, $2... backrefs in replacement using the first match (consistent with mixed-path behavior) # Expand $1, $2... backrefs in replacement using the first match (consistent with mixed-path behavior)
def _expand_dollars(rep: str) -> str: def _expand_dollars(rep: str, _m=m) -> str:
return _re.sub(r"\$(\d+)", lambda g: m.group(int(g.group(1))) or "", rep) return _re.sub(r"\$(\d+)", lambda g: _m.group(int(g.group(1))) or "", rep)
repl_expanded = _expand_dollars(repl) repl_expanded = _expand_dollars(repl)
# Let C# side handle validation using Unity's built-in compiler services # Let C# side handle validation using Unity's built-in compiler services
sl, sc = line_col_from_index(m.start()) sl, sc = line_col_from_index(m.start())