fix(tekla): openings as curves #440
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #440 +/- ##
=====================================
Coverage 9.43% 9.43%
=====================================
Files 223 223
Lines 4209 4209
Branches 487 487
=====================================
Hits 397 397
Misses 3796 3796
Partials 16 16 ☔ View full report in Codecov by Sentry. |
| @@ -28,6 +31,8 @@ public static IServiceCollection AddTeklaConverters(this IServiceCollection serv | |||
| ConverterSettingsStore<TeklaConversionSettings> | |||
| >(); | |||
|
|
|||
| serviceCollection.AddTransient<ITypedConverter<TSM.BooleanPart, IEnumerable<Base>>, OpeningToSpeckleConverter>(); | |||
There was a problem hiding this comment.
AFAIK you do not need to register ITypedConverters explicitly. Some magic (serviceCollection.AddMatchingInterfacesAsTransient(converterAssembly)) can handle them. Did you have a problem before with that?
| yield return line; | ||
| } | ||
| break; | ||
|
|
||
| default: | ||
| yield break; |
There was a problem hiding this comment.
regardles we can log here instead swallowing. pre-existing problem
| else if (parent is TSM.BooleanPart boolPart) | ||
| { | ||
| solid = boolPart.OperativePart?.GetSolid(); | ||
| } |
There was a problem hiding this comment.
do we have else case somehow. smells a bit
| } | ||
|
|
||
| var vertexEnum = loop.GetVertexEnumerator(); | ||
| while (vertexEnum.MoveNext()) |
There was a problem hiding this comment.
OMG. 3 nested while loop, how crazy we are ( ͡°( ͡° ͜ʖ( ͡° ͜ʖ ͡°)ʖ ͡°) ͡°)
at least this converter deserves inline comments/notes. hard to understand from first look. also wanna know why we need that while loops, seems like API thing but some explanation around structure would be good
| return new TG.Point(point.X, point.Y, Math.Max(minZ, Math.Min(maxZ, point.Z))); | ||
| } | ||
|
|
||
| public IEnumerable<Base> Convert(TSM.BooleanPart target) |
There was a problem hiding this comment.
i would put public methods above always, but it is QoL...
|
|
||
| var vertices = new List<TG.Point>(); | ||
| var vertexEnum = loop.GetVertexEnumerator(); | ||
| while (vertexEnum.MoveNext()) |
There was a problem hiding this comment.
i am hesitant with these dudes, but happy to do live PR review together with you on this
|
Lets not merge this to target proper ear cut implementation. Closing it |
Description
We were failing to send proper openings to viewer. After some discussion, we decided to represent the openings with curves. I added the OpeningsToSpeckleConverter to detect openings which are represented as BooleanPart in Tekla.
There are two parts of cuts mainly: polygon cut and part cut. Polygon cut was working fine, but with the part cut, my logic was adding lines to whole cutting part.
Therefore I added logic matching with the plane coordinates.
Checklist: