Merge pull request #1203 from specklesystems/dev

Back merge dev -> main

.github/workflows/pr.yml

@@ -10,7 +10,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
-        uses: actions/checkout@v5
+        uses: actions/checkout@v6
         with:
           fetch-depth: 0
 

.github/workflows/release.yml

@@ -16,7 +16,7 @@ jobs:
       file_version: ${{ steps.set-version.outputs.file_version }}
     steps:
       - name: Checkout
-        uses: actions/checkout@v5
+        uses: actions/checkout@v6
         with:
           fetch-depth: 0
 

Connectors/Rhino/Speckle.Connectors.GrasshopperShared/Components/Dev/TokenUrlComponent.cs

@@ -15,7 +15,7 @@ public class TokenUrlComponent : GH_Component
 {
   public TokenUrlComponent()
     : base(
-      "Speckle Model URL",
+      "Speckle Model URL with Token",
       "URL",
       "Create a Speckle model link using URL and developer token",
       ComponentCategories.PRIMARY_RIBBON,

Connectors/Rhino/Speckle.Connectors.GrasshopperShared/Components/Objects/ExpandSpeckleProperties.cs

@@ -27,8 +27,7 @@ public class ExpandSpeckleProperties : GH_Component, IGH_VariableParameterCompon
   protected override Bitmap Icon => Resources.speckle_properties_expand;
   public override GH_Exposure Exposure => GH_Exposure.secondary;
 
-  protected override void RegisterInputParams(GH_InputParamManager pManager)
-  {
+  protected override void RegisterInputParams(GH_InputParamManager pManager) =>
     pManager.AddParameter(
       new SpecklePropertyGroupParam(),
       "Properties",
@@ -36,7 +35,6 @@ public class ExpandSpeckleProperties : GH_Component, IGH_VariableParameterCompon
       "Speckle Properties to expand",
       GH_ParamAccess.item
     );
-  }
 
   protected override void RegisterOutputParams(GH_OutputParamManager pManager) { }
 

Connectors/Rhino/Speckle.Connectors.GrasshopperShared/Components/Operations/Receive/ReceiveAsyncComponent.cs

@@ -130,7 +130,7 @@ public class ReceiveAsyncComponent : GH_AsyncComponent<ReceiveAsyncComponent>
     {
       var autoReceiveMi = Menu_AppendItem(
         menu,
-        "Load automatically",
+        "Load new versions automatically",
         (s, e) =>
         {
           AutoReceive = !AutoReceive;

Connectors/Rhino/Speckle.Connectors.GrasshopperShared/Parameters/SpecklePropertyGoo.cs

@@ -79,6 +79,9 @@ public class SpecklePropertyGoo : GH_Goo<object>, ISpecklePropertyGoo
       case int i:
         Value = i;
         return true;
+      case long l:
+        Value = l;
+        return true;
       case string s:
         Value = s;
         return true;

Connectors/Rhino/Speckle.Connectors.GrasshopperShared/Parameters/SpecklePropertyGroupGoo.cs

@@ -161,7 +161,12 @@ public partial class SpecklePropertyGroupGoo : GH_Goo<Dictionary<string, ISpeckl
       else
       {
         SpecklePropertyGoo entry = new();
-        entry.CastFrom(kvp.Value);
+        if (!entry.CastFrom(kvp.Value))
+        {
+          throw new ArgumentException(
+            $"Property '{kvp.Key}' has unsupported type '{kvp.Value?.GetType().Name ?? "null"}'"
+          );
+        }
         val = entry;
       }
 

Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoLayerBaker.cs

@@ -123,6 +123,12 @@ public class RhinoLayerBaker : TraversalContextUnpacker
       }
 
       var cleanNewLayerName = RhinoUtils.CleanLayerName(collection.name);
+
+      if (!ModelComponent.IsValidComponentName(cleanNewLayerName))
+      {
+        throw new SpeckleException($"Layer name '{currentLayerName}' is not valid");
+      }
+
       Layer newLayer = new() { Name = cleanNewLayerName, ParentLayerId = previousLayer?.Id ?? Guid.Empty };
 
       // set material
@@ -150,7 +156,7 @@ public class RhinoLayerBaker : TraversalContextUnpacker
       int index = currentDocument.Layers.Add(newLayer);
       if (index == -1)
       {
-        throw new SpeckleException($"Could not create layer '{currentLayerName}'.");
+        throw new SpeckleException($"Could not create layer '{currentLayerName}'");
       }
 
       _hostLayerCache.Add(currentLayerName, index);

Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoUtils.cs

@@ -1,27 +1,41 @@
+using System.Text;
+
 namespace Speckle.Connectors.Rhino.HostApp;
 
 public static class RhinoUtils
 {
-  public static string CleanBlockDefinitionName(string str)
-  {
-    return ReplaceChars(str, @"\/", "_");
-  }
+  private static readonly HashSet<char> s_skipChars = ['[', ']', '(', ')', '{', '}'];
+  private static readonly HashSet<char> s_replaceWithHyphen = [':', ';'];
+
+  public static string CleanBlockDefinitionName(string str) => str.Replace('/', '_').Replace('\\', '_');
 
   // 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
   public static string CleanLayerName(string str)
   {
-    str = ReplaceChars(str, @"[](){}", "");
-    return ReplaceChars(str, @":;", "-");
-  }
+    var sb = new StringBuilder(str.Length);
 
-  private static string ReplaceChars(string str, string invalidChars, string replaceString)
-  {
-    foreach (char c in invalidChars)
+    foreach (char c in str)
     {
-      str = str.Replace(c.ToString(), replaceString);
+      if (char.IsControl(c))
+      {
+        continue; // skip control characters (shoutout cnx-2809)
+      }
+
+      if (s_skipChars.Contains(c))
+      {
+        continue; // skip brackets
+      }
+
+      if (s_replaceWithHyphen.Contains(c))
+      {
+        sb.Append('-');
+        continue;
+      }
+
+      sb.Append(c);
     }
 
-    return str;
+    return sb.ToString();
   }
 }

Converters/Revit/Speckle.Converters.RevitShared/Helpers/DisplayValueExtractor.cs

@@ -74,15 +74,18 @@ public sealed class DisplayValueExtractor
         }
         return areaDisplay;
 
-      // NOTE: this is only for Rebar and not AreaReinforcement, RebarInSystem
-      // AreaReinforcement and RebarInSystem pass through GetGeometryDisplayValue which get DisplayValues as per hostApp
-      // Rebar elements need special handling as get_Geometry() doesn't work properly
-      // We either represent them as centerlines or as solids based on settings
+      // Rebar: get_Geometry() returns null, use GetTransformedCenterlineCurves/GetFullGeometryForView + apply reference point transform
       case DB.Structure.Rebar rebar:
         return _converterSettings.Current.SendRebarsAsVolumetric
           ? GetRebarVolumetricDisplayValue(rebar)
           : GetRebarCenterlineDisplayValue(rebar);
 
+      // AreaReinforcement/PathReinforcement get_Geometry() returns curves in document coordinates
+      // unlike Rebar which needs reference point transform applied, these are already correct
+      case DB.Structure.AreaReinforcement:
+      case DB.Structure.PathReinforcement:
+        return GetAreaReinforcementDisplayValue(element);
+
       // handle specific types of objects with multiple parts or children
       // curtain and stacked walls should have their display values in their children
       case DB.Wall wall:
@@ -107,7 +110,7 @@ public sealed class DisplayValueExtractor
     using DB.Transform? compoundTransform =
       localToDocument is not null && documentToWorld is not null
         ? documentToWorld.Multiply(localToDocument)
-        : localToDocument; // don't want to accidentally dispose of the ReferencePointTransform
+        : localToDocument;
 
     DB.Transform? localToWorld = compoundTransform ?? documentToWorld;
 
@@ -423,7 +426,7 @@ public sealed class DisplayValueExtractor
       return false; // exit fast on a potential hot path
     }
 
-    DB.GraphicsStyle? bjk = null; // ask ogu why this variable is named like this
+    DB.GraphicsStyle? bjk; // ask ogu why this variable is named like this
 
     if (!_graphicStyleCache.ContainsKey(geomObj.GraphicsStyleId.ToString().NotNull()))
     {
@@ -547,8 +550,9 @@ public sealed class DisplayValueExtractor
 
     if (geometryElements != null)
     {
+      DB.Transform? documentToWorld = _converterSettings.Current.ReferencePointTransform?.Inverse;
       SortGeometry(rebar, collections, geometryElements, null);
-      return ProcessGeometryCollections(rebar, collections, null);
+      return ProcessGeometryCollections(rebar, collections, documentToWorld);
     }
 
     // Return empty list if no geometry is found - imo not critical
@@ -589,16 +593,40 @@ public sealed class DisplayValueExtractor
         )
       );
     }
+    DB.Transform? documentToWorld = _converterSettings.Current.ReferencePointTransform?.Inverse;
 
     List<DisplayValueResult> displayValue = new();
     foreach (var curve in curves)
     {
-      displayValue.Add(DisplayValueResult.WithoutTransform(GetCurveDisplayValue(curve)));
+      if (documentToWorld is not null)
+      {
+        using var transformedCurve = curve.CreateTransformed(documentToWorld);
+        displayValue.Add(DisplayValueResult.WithoutTransform(GetCurveDisplayValue(transformedCurve)));
+      }
+      else
+      {
+        displayValue.Add(DisplayValueResult.WithoutTransform(GetCurveDisplayValue(curve)));
+      }
     }
 
     return displayValue;
   }
 
+  /// <summary>
+  /// Gets display value for AreaReinforcement and PathReinforcement.
+  /// </summary>
+  /// <remarks>
+  /// These elements' get_Geometry() returns curves already in document coordinates.
+  /// Unlike Rebar.GetTransformedCenterlineCurves() which requires reference point transform,
+  /// these curves should not be transformed - they're already in the correct space.
+  /// </remarks>
+  private List<DisplayValueResult> GetAreaReinforcementDisplayValue(DB.Element element)
+  {
+    var collections = GetSortedGeometryFromElement(element, null, null);
+    // pass null for transform - curves are already in correct document coordinates
+    return ProcessGeometryCollections(element, collections, null);
+  }
+
   /// <summary>
   /// Represents sorted collections of different geometry types extracted from an element.
   /// Used to pass multiple geometry collections as a single parameter to improve code readability