-
Notifications
You must be signed in to change notification settings - Fork 635
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
Graph Preview States For New Node #12908
Conversation
previewGraphData = engineController.PreviewGraphSyncData(modifiedNodes,verboseLogging); | ||
TargetedWorkspace = workspace; | ||
modifiedNodes = ComputeModifiedNodes(workspace); | ||
previewGraphData = engineController.PreviewGraphSyncData(modifiedNodes, verboseLogging); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aparajit-pratap @mjkkirschner @sm6srw @pinzart90 I have a question, if a graph(in manual mode) has been run once, then the user add a new node, what should this function return? I would expect this function to return me the new node's node id, but it is currently returning an empty set and causing a bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was PreviewGraphSyncData
returning empty list before your change to ComputeModifiedNodes
? To clarify - which function were you referring to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mjkkirschner Previously the function PreviewGraphSyncData
is also returning empty. modifiedNodes
were always all of the nodes in workspace so I would not trust the return value at all. As you can see from the comment from Ben, that TODO from 7 years ago never finished. Right now I am setting the modified nodes as an example, and in my case, I am adding a new node to the existing workspace, but this function does not return the expected results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the description of the PreviewGraphSyncData
function is This is called on the main thread from PreviewGraphSyncData to generate the list of node id's that will be executed on the next run
. I would expect it to return the id of newly added node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks to Turbo Team who fixed this API, this is now working
Please Note:
DynamoRevit
repo will need to be cherry-picked into all the DynamoRevit Release branches that Dynamo supports. Contributors will be responsible for cherry-picking their reviewed commits to the other branches after aLGTM
label is added to the PR.Purpose
Per https://jira.autodesk.com/browse/DYN-4357, enable Graph Preview States for new node
Declarations
Check these if you believe they are true
*.resx
filesRelease Notes
enable Graph Preview States for new node
Reviewers
@DynamoDS/dynamo
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of