DynamoDS / Dynamo

Open Source Graphical Programming for Design
https://dynamobim.org
Other
1.68k stars 624 forks source link

Nodes with identical behavior but different names #10721

Open vykrum opened 4 years ago

vykrum commented 4 years ago

Dynamo version

2.7.0.8691

Operating system

Windows 10

What did you do?

Looked for nodes to get Isolnes and got

  1. Curve.ByIsoCurveOnSurface
  2. Surface.GetIsoline

What did you expect to see?

The two nodes that perform different tasks

What did you see instead?

The nodes have different names, but are identical Isoline

aparajit-pratap commented 4 years ago

@vykrum yes, they are meant to be identical. These nodes access the same functionality but from different geometry categories - curve and surface. @Amoursol any preference on whether we want to deprecate one of these nodes to make the geom library more concise and maybe less confusing?

Amoursol commented 4 years ago

@aparajit-pratap The Surface.GetIsoline node makes more sense to me as it's first input port is suface (Consistency with the name), whereas the 'Curve.ByIsoCurveOnSurface' requests a baseSurface (Not consistency with name).

We should deprecate the Curve.ByIsoCurveOnSurface node.

vykrum commented 4 years ago

baseSurface (Not consistency with name)

In that case might need to look into this node too ... curveByParameterLineOnSurface

Amoursol commented 4 years ago

Good point @vykrum - We do have a bunch of nodes needing a consistency pass. It's on our radar generally, but always good to have specific cases to look at too! 💪

gregmarr commented 4 years ago

On the other hand, Curve.By or Surface.By or Whatever.By don't generally have the type as their first input as they are creation nodes, so that's not an inconsistency with the rest of the library. The Surface.GetIsoline is a data extraction function, and so it takes the first parameter as the surface. Since the type that results is a Curve and not a Line, I suggest that Curve.ByIsoCurveOnSurface is a better match than Surface.GetIsoline.