Allow integer slider to support 64 bit ints #10972
Conversation
|
@aparajit-pratap an interesting set of failures, including the ones your predicted. |
| case "UInt32": | ||
| case "UInt64": | ||
| return BuildIntNode(Convert.ToInt32(value)); | ||
| case "UInt64": |
There was a problem hiding this comment.
can we use nameof?
I guess with c# 7 we should be doing:
https://stackoverflow.com/a/4478490
|
@QiltongTang do you know how to change the description of the output port tooltip for a UI node? The only thing remaining in this PR is to change the output port type of the integer slider to report |
|
@aparajit-pratap I would try look at when outport model constructor got called when placing integer slider: https://github.com/DynamoDS/Dynamo/blob/master/src/DynamoCore/Graph/Nodes/PortModel.cs#L335 |
|
|
|
Thanks @QilongTang! |
|
@aparajit-pratap Seems there are regressions with this PR? |
@QilongTang there are 2 regressions remaining. I addressed many of them yesterday. Will address the remaining ones today. In the meantime do you have any comments on the rest of the review? |
QilongTang
left a comment
There was a problem hiding this comment.
Overall looks solid, just a few minor comments!
Purpose
https://jira.autodesk.com/browse/DYN-1085
IntegerSlider64Bitthat derives fromSliderBase<long>to support 64 bit integersCoreNodeModels.Input.IntegerSlidertype from JSON as aCoreNodeModels.Input.IntegerSlider64Bitso that the new 64 bit slider node is instantiatedCoreNodeModels.Input.IntegerSlidertype so that new DYN's can be opened by older versions of DynamoDeserialize 32 bit integer slider from old XML format as new 64 bit integer slider- This does not seem to be straightforward since in order to not break the API I needed to keep the legacy class, which is why I had to retain the legacy behaviour of xml-based 32 bit integer sliders (to be deserialized as 32 bit). Now only when you save the XML file (when saved as JSON) and reopened, will the slider behave as a 64 bit integer slider node, not before.Declarations
Check these if you believe they are true
*.resxfilesReviewers
@DynamoDS/dynamo