Add Comments about TransformTo (#436)

specklesystems · Dec 5, 2024 · 7c28a9f · 7c28a9f
1 parent c2bafc6
commit 7c28a9f
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 5 deletions.
diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs
@@ -192,16 +192,18 @@ localToGlobalMap.AtomicObject is ITransformable transformable // and ICurve
           && localToGlobalMap.AtomicObject["units"] is string units
         )
         {
+          //TODO TransformTo will be deprecated as it's dangerous and requires ID transposing which is wrong!
+          //ID needs to be copied to the new instance
           var id = localToGlobalMap.AtomicObject.id;
           ITransformable? newTransformable = null;
           foreach (var mat in localToGlobalMap.Matrix)
           {
             transformable.TransformTo(new Transform() { matrix = mat, units = units }, out newTransformable);
-            transformable = newTransformable; // we need to keep the reference to the new object, as we're going to use it in the cache'
+            transformable = newTransformable; // we need to keep the reference to the new object, as we're going to use it in the cache
           }
 
           localToGlobalMap.AtomicObject = (newTransformable as Base)!;
-          localToGlobalMap.AtomicObject.id = id; // restore the id, as it's used in the cache'
+          localToGlobalMap.AtomicObject.id = id; // restore the id, as it's used in the cache
           localToGlobalMap.Matrix = new HashSet<Matrix4x4>(); // flush out the list, as we've applied the transforms already
         }
 

diff --git a/Sdk/Speckle.Converters.Common/LocalToGlobalConverterUtils.cs b/Sdk/Speckle.Converters.Common/LocalToGlobalConverterUtils.cs
@@ -33,10 +33,13 @@ public Base TransformObjects(Base atomicObject, IReadOnlyCollection<Matrix4x4> m
 
     if (atomicObject is ITransformable c)
     {
+      //TODO TransformTo will be deprecated as it's dangerous and requires ID transposing which is wrong!
+      //ID needs to be copied to the new instance
+      string id = atomicObject.id.NotNull();
       foreach (var transform in transforms)
       {
         c.TransformTo(transform, out ITransformable newObj);
-        c = newObj;
+        c = newObj; // we need to keep the reference to the new object, as we're going to use it in the cache
       }
 
       if (c is not Base)
@@ -46,8 +49,7 @@ public Base TransformObjects(Base atomicObject, IReadOnlyCollection<Matrix4x4> m
         );
       }
 
-      string id = atomicObject.id.NotNull();
-      atomicObject = (Base)c;
+      atomicObject = (Base)c; // restore the id, as it's used in the cache
       atomicObject.id = id;
 
       // .TransformTo only transfers typed properties, we need to add back the dynamic ones: