DynamoDS / DynamoRevit

Dynamo Libraries for Revit
https://dynamobim.org
342 stars 188 forks source link

Element.GetParameterValueByName doesnt return element for Level? #1234

Closed ghost closed 7 years ago

ghost commented 8 years ago

Dynamo version

1.1.0

Revit version

2016

Operating system

windows 7

What are you whining about this time, Andrew?

I would have expected that - where an element's parameter storage type is element ID - the "GetParameterValueByName" node would actually return that element. For instance, trying to get the "Level" parameter of a Room element, I want the "Level" object back so I can query its elevation. Instead, I am getting back the name of the level only - the data type being returned is string. For the "Upper Limit" parameter, the element is actually returned. Is there a reason for this inconsistency? Am I missing something obvious? I shouldn't have to do a lookup to match this level name with the document's levels in order to retrieve this element, nor should I have to write a custom script for something as basic as parameter value retrieval. (I am aware that there are existing "Element.Level" scripts available - I am wondering why this behavior is inconsistent in native Dynamo.)

ksobon commented 8 years ago

@AndrewHeumann there is nothing wrong with Dynamo here or its method for Parameter retrieval. What happened is that Revit DB allows for storing of multiple Parameters under the same name:

image

image

What happened is that when you used a Name property it will return one of the two Parameters. It's a bit of a lottery and you lost. One of the Level parameters stores a level name as a string. The other one stores ElementId.

To deal with this behavior Revit API actually implements something called BuiltInParameter that allows you to retrieve a Parameter by its built in and unique name like this:

image

Built In Parameter always points at only single parameter value so its reliable unlike the name of the parameter. You can use a node from Archi-lab package called GetBuiltInParameter like so:

image

kronz commented 8 years ago

Interesting . . . I think I can reproduce this same thing on the Set Parameters level, with a pretty random looking success/failure rate with this. 2016-09-07_1800_001 2016-09-07_1800

ksobon commented 8 years ago

@kronz again, this is expected behavior. If anything I would suggest that you guys implement a BuiltInParameter retrieval so that users can feed either Name or BuiltInParameter Name into the Element.SetParameterByName node. Then I would just educate people about the difference between the two these values.

ThomasMahon commented 8 years ago

I also agree with @ksobon the issue is very easy to see if you add a project parameter with the same name as a System parameter. I recently ran into this when using Dynamo to update custom sheet parameters and the team had created one called "Type" to store data for one of the BS1192 naming convention fields. It obviously clashed with the Revit System Parameter by the same name and was returning the wrong information as a result.

ghost commented 8 years ago

Now I understand why this is happening (thanks @ksobon) but I don't think it's sufficient to just call it "expected behavior" and leave it at that - the node should handle the issue more gracefully (by warning the user that multiple parameters share this name, at the very least) - and ideally even offer the user a means to indicate which one is intended. This could be accomplished via some sort of UI, or an optional supplied type hint, or even offer the option to return the resulting values from ALL matching parameters in a list (although I could see this producing a lot of nasty list management issues to contend with.)

ksobon commented 8 years ago

@AndrewHeumann yeah, I am game if they want to build a UI for selecting one of the many available parameters. I am not sure how that would look like though.

kronz commented 8 years ago

Additional case (closed as a duplicate): https://github.com/DynamoDS/DynamoRevit/issues/1383

kronz commented 8 years ago

Tracking internally with MAGN-10816 Current options being entertained:

andydandy74 commented 8 years ago

Here's a thought: We use prefixes for all our shared/project parameter names so that precisely this kind of thing can never happen... ;-)

aledarba commented 8 years ago

@andydandy74 It is the best approach for sure; however, trying to follow NBS guideliness creates a clash between parameter names, so I am a bit confused and I am not sure if I should follow NBS rules or not...

@kronz are these solutions currently working, or just under development?

Thanks to everyone!! =)

kronz commented 8 years ago

@aledarba thesw are option we are choosing among to develop

aledarba commented 8 years ago

Thank you @kronz , could you please let me know whenever you make a progress?

ksobon commented 8 years ago

This is also duplicate of: https://github.com/DynamoDS/Dynamo/issues/2275

ghost commented 8 years ago

My preference: prefer built in by default, but accept either; toggle (or other option) to force ignore built ins that's set to false by default.

kronz commented 7 years ago

This is fixed in the upcoming 1.3 release https://github.com/DynamoDS/DynamoRevit/pull/1595