New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Element.GetParameterValueByName doesnt return element for Level? #1234

Closed
AndrewHeumann opened this Issue Aug 29, 2016 · 15 comments

Comments

Projects
None yet
6 participants
@AndrewHeumann

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

This comment has been minimized.

Show comment
Hide comment
@ksobon

ksobon Sep 1, 2016

Contributor

@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

Contributor

ksobon commented Sep 1, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@kronz

kronz Sep 7, 2016

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

kronz commented Sep 7, 2016

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

This comment has been minimized.

Show comment
Hide comment
@ksobon

ksobon Sep 7, 2016

Contributor

@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.

Contributor

ksobon commented Sep 7, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@ThomasMahon

ThomasMahon Sep 15, 2016

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.

ThomasMahon commented Sep 15, 2016

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.

@AndrewHeumann

This comment has been minimized.

Show comment
Hide comment
@AndrewHeumann

AndrewHeumann Sep 15, 2016

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.)

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

This comment has been minimized.

Show comment
Hide comment
@ksobon

ksobon Sep 16, 2016

Contributor

@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.

Contributor

ksobon commented Sep 16, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@kronz

kronz Nov 3, 2016

Additional case (closed as a duplicate): #1383

kronz commented Nov 3, 2016

Additional case (closed as a duplicate): #1383

@kronz

This comment has been minimized.

Show comment
Hide comment
@kronz

kronz Nov 3, 2016

Tracking internally with MAGN-10816
Current options being entertained:

  • Special Get/Set BuiltIn Paramater nodes, and Existing Get/Set does not use BuiltIn
  • Boolean input on Get set for "Use Named/BuiltIn Parameter" default set to Named?
  • If there is a BuiltIn, always use that. This will disallow the use of Named parameters in the situation where there is a BuiltIn and a Named.
  • If there is a BuiltIn, always use that. Add a toggle that is set by default to false that says "Ignore BuiltIn Parameter"

kronz commented Nov 3, 2016

Tracking internally with MAGN-10816
Current options being entertained:

  • Special Get/Set BuiltIn Paramater nodes, and Existing Get/Set does not use BuiltIn
  • Boolean input on Get set for "Use Named/BuiltIn Parameter" default set to Named?
  • If there is a BuiltIn, always use that. This will disallow the use of Named parameters in the situation where there is a BuiltIn and a Named.
  • If there is a BuiltIn, always use that. Add a toggle that is set by default to false that says "Ignore BuiltIn Parameter"
@andydandy74

This comment has been minimized.

Show comment
Hide comment
@andydandy74

andydandy74 Nov 3, 2016

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

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

This comment has been minimized.

Show comment
Hide comment
@aledarba

aledarba Nov 3, 2016

@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!! =)

aledarba commented Nov 3, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@kronz

kronz Nov 3, 2016

@aledarba thesw are option we are choosing among to develop

kronz commented Nov 3, 2016

@aledarba thesw are option we are choosing among to develop

@aledarba

This comment has been minimized.

Show comment
Hide comment
@aledarba

aledarba Nov 3, 2016

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

aledarba commented Nov 3, 2016

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

@ksobon

This comment has been minimized.

Show comment
Hide comment
@ksobon

ksobon Nov 3, 2016

Contributor

This is also duplicate of: DynamoDS/Dynamo#2275

Contributor

ksobon commented Nov 3, 2016

This is also duplicate of: DynamoDS/Dynamo#2275

@AndrewHeumann

This comment has been minimized.

Show comment
Hide comment
@AndrewHeumann

AndrewHeumann Nov 4, 2016

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.

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

This comment has been minimized.

Show comment
Hide comment
@kronz

kronz Mar 14, 2017

This is fixed in the upcoming 1.3 release
#1595

kronz commented Mar 14, 2017

This is fixed in the upcoming 1.3 release
#1595

@kronz kronz closed this Mar 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment