From bf97d8c029494685eb649e59155f561c880edacd Mon Sep 17 00:00:00 2001
From: Dimitrie Stefanescu <didimitrie@gmail.com>
Date: Thu, 5 Jun 2025 14:36:15 +0100
Subject: [PATCH] fix: cleans up rhino layer names properly for baking

---
 .../HostApp/RhinoLayerBaker.cs                | 45 ++++++++++++++-----
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoLayerBaker.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoLayerBaker.cs
index 4bb1fb24b..edf1cae3c 100644
--- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoLayerBaker.cs
+++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoLayerBaker.cs
@@ -48,18 +48,25 @@ public class RhinoLayerBaker : TraversalContextUnpacker
   /// <remarks>Make sure this is executing on the main thread, using e.g RhinoApp.InvokeAndWait.</remarks>
   public void CreateAllLayersForReceive(IEnumerable<Collection[]> paths, string baseLayerName)
   {
-    CreateBaseLayer(baseLayerName);
-    var uniquePaths = new Dictionary<string, Collection[]>();
-    foreach (var path in paths)
+    try
     {
-      var names = path.Select(o => string.IsNullOrWhiteSpace(o.name) ? "unnamed" : o.name);
-      var key = string.Join(",", names!);
-      uniquePaths[key] = path;
-    }
+      CreateBaseLayer(baseLayerName);
+      var uniquePaths = new Dictionary<string, Collection[]>();
+      foreach (var path in paths)
+      {
+        var names = path.Select(o => string.IsNullOrWhiteSpace(o.name) ? "unnamed" : o.name);
+        var key = string.Join(",", names!);
+        uniquePaths[key] = path;
+      }
 
-    foreach (var uniquePath in uniquePaths)
+      foreach (var uniquePath in uniquePaths)
+      {
+        var layerIndex = CreateLayerFromPath(uniquePath.Value, baseLayerName);
+      }
+    }
+    catch (Exception ex) when (!ex.IsFatal())
     {
-      var layerIndex = CreateLayerFromPath(uniquePath.Value, baseLayerName);
+      throw new SpeckleException("Could not create all layers for receive.", ex);
     }
   }
 
@@ -86,6 +93,22 @@ public class RhinoLayerBaker : TraversalContextUnpacker
     throw new SpeckleException($"Did not find a layer in the cache with the name {layerFullName}");
   }
 
+  /// <summary>
+  /// Cleans up layer names to be "rhino" proof. Note this can be improved, as "()[] and {}" are illegal only at the start.
+  /// https://docs.mcneel.com/rhino/6/help/en-us/index.htm#information/namingconventions.htm?Highlight=naming
+  /// </summary>
+  /// <param name="layerName"></param>
+  /// <returns></returns>
+  private string CleanLayerName(string layerName) =>
+    layerName
+      .Replace("{", "")
+      .Replace("}", "")
+      .Replace("(", "")
+      .Replace(")", "")
+      .Replace("[", "")
+      .Replace("]", "")
+      .Replace(":", "");
+
   /// <summary>
   /// Creates a layer based on the given collection path and adds it to the Rhino document.
   /// </summary>
@@ -101,14 +124,14 @@ public class RhinoLayerBaker : TraversalContextUnpacker
     {
       currentLayerName += s_pathSeparator + (string.IsNullOrWhiteSpace(collection.name) ? "unnamed" : collection.name);
 
-      currentLayerName = currentLayerName.Replace("{", "").Replace("}", ""); // Rhino specific cleanup for gh (see RemoveInvalidRhinoChars)
+      currentLayerName = CleanLayerName(currentLayerName); //.Replace("{", "").Replace("}", ""); // Rhino specific cleanup for gh (see RemoveInvalidRhinoChars)
       if (_hostLayerCache.TryGetValue(currentLayerName, out int value))
       {
         previousLayer = currentDocument.Layers.FindIndex(value);
         continue;
       }
 
-      var cleanNewLayerName = collection.name.Replace("{", "").Replace("}", "");
+      var cleanNewLayerName = CleanLayerName(collection.name); //.Replace("{", "").Replace("}", "").Replace("(", "").Replace(")", "");
       Layer newLayer = new() { Name = cleanNewLayerName, ParentLayerId = previousLayer?.Id ?? Guid.Empty };
 
       // set material