DynamoDS / DynamoRevit

Dynamo Libraries for Revit
https://dynamobim.org
336 stars 187 forks source link

improvement request: Add wrapper for Rebar #1689

Open eibre opened 7 years ago

eibre commented 7 years ago

Many of the nodes in the Dynamo For Revit package can not be used with existing Rebars selected by OOTB Dynamo nodes because they are wrapped as Revit.Elements.UnknownElement

The wrapper already exists in the Dynamo For Revit package, if approved by @moethu I would prefer it moved to DynamoRevit.

image

https://github.com/tt-acm/DynamoForRebar/issues/72

moethu commented 7 years ago

@eibre agree, this would be nice. Back then when I developed this package it needed to be independent of DynamoRevit, which brought some limitations. Feel free to move the wrappers to DynamoRevit.

smangarole commented 7 years ago

@kronz and @mjkkirschner : Ping.

I think we need your help in making this decision.

ksobon commented 7 years ago

@eibre that's a good idea, but...Dynamo for Rebar is a 3rd party package. If we decide to roll that Rebar wrapper into DynamoRevit, great, but then @moethu would still have to update his package to now work with the new wrapper. There are a lot of moving parts here.

@moethu couldn't you just update your package to allow for UnknownElement input, and then feed it through your internal constructors? I am sure that's a lot of work but maybe it's worth it.

moethu commented 7 years ago

@ksobon yes this would be the easiest solution. will do that, should be done quite fast

eibre commented 7 years ago

@ksobon @moethu Are you talking about changing the input types from Revit.Elements.Rebar to Revit.Elements.Element? Is there no other reason to have a wrapper than be able to specify the input type?

If it's preferred to keep most Revit elements as 3rd party packages, shouldn't there be a way for packages to "hook" into DynamoRevit and add wrappers on startup?

Personally, I would still like to have the Rebar wrappers and the Create nodes as a part of DynamoRevit, but I see that it will require some cleanup.

ksobon commented 7 years ago

I don't mind having it all be part of DynamoRevit either but that involves a little more work that @moethu would not be getting compensated for. Paging @kronz

Yes, it would be nice to have ability to extend DynamoRevit classes, but currently that's not exactly possible. This, however, echos what I was saying here already: https://github.com/DynamoDS/Dynamo/issues/7105

moethu commented 7 years ago

Agree, extending wrappers of DynamoRevit would be great for anyone writing new packages - but it will be hard to maintain it coordinated because people might create duplicates or similar wrappers for same classes.

Adding Rebar to DynamoRevit wouldn't be the best solution because I'd have to wait until the next DynamoRevit release to update the rebar package and if someone continues using the old rebar package there would be conflicting wrappers.

I think the best solution would be to accept input of type Revit.Elements.Element and drop the rebar internal wrapper entirely. Later down the line there can still be a rebar wrapper in DynamoRevit.

kronz commented 7 years ago

Let me get a technical consult from @mjkkirschner and @ikeough