Skip to content
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

DYN-7570 Purple package nodes overlay for non-custom nodes #15532

Merged
merged 3 commits into from
Oct 9, 2024

Conversation

dnenov
Copy link
Collaborator

@dnenov dnenov commented Oct 9, 2024

Purpose

Continuation of and older task that might not have been fully addressed the first time around - a link to the original PR can be found here: #14507

Context

Currently, only CustomFunctions were flagged up as 'Package' nodes in the UI. This PR attempts to add functions coming from Packages, regardless of their node type.

Changes (the gif is a bit long, sorry):

DynamoSandbox_SJEgDBX38v

Limitations

In the example below, a node coming from the Forma package AssemblyName is not found in the list of packages insdie the NodePackageDictionary:

  • assemblyName = {DynamoForma, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null}

image

image

Conclusion

For this function to work correctly, package authors must make sure that the nodes are contained within the same assembly that the package will be listed under by the PackageExtension (please, correct me if I am wrong on this one).

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

  • added PackageName check to mark a node as a CustomNode
  • removed limitation on how the PackageName is generated, which would only limit that to nodes with IsCustomFuction set to true

Reviewers

@QilongTang
@zeusongit
@reddyashish

FYIs

@Amoursol

dnenov added 3 commits October 7, 2024 18:19
- now adds package name check to determine if a node belongs to a package
- for whatever reason, only custom functions were allowed to be given PackageName
- removed this limitation
Copy link

github-actions bot commented Oct 9, 2024

UI Smoke Tests

Test: success. 11 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests

@QilongTang QilongTang added this to the 3.4 milestone Oct 9, 2024
@QilongTang
Copy link
Contributor

For this function to work correctly, package authors must make sure that the nodes are contained within the same assembly that the package will be listed under by the PackageExtension (please, correct me if I am wrong on this one).

I think you mean For this function to work correctly, package authors must make sure that the nodes are contained within the assembly that marked as node library by package author.

When reviewing a different task, @aparajit-pratap has indicated that DynamoForma does not follow such structure and use a dedicated dll/helper to load all the nodes. Maybe that's the reason this is still going to break for Forma.

@QilongTang QilongTang changed the title Package info fix DYN-7570 Purple package nodes overlay for non-custom nodes Oct 9, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-7570

@QilongTang QilongTang merged commit 99dc05a into DynamoDS:master Oct 9, 2024
35 of 45 checks passed
@dnenov
Copy link
Collaborator Author

dnenov commented Oct 9, 2024

For this function to work correctly, package authors must make sure that the nodes are contained within the same assembly that the package will be listed under by the PackageExtension (please, correct me if I am wrong on this one).

I think you mean For this function to work correctly, package authors must make sure that the nodes are contained within the assembly that marked as node library by package author.

When reviewing a different task, @aparajit-pratap has indicated that DynamoForma does not follow such structure and use a dedicated dll/helper to load all the nodes. Maybe that's the reason this is still going to break for Forma.

Yes, exactly, apologies for the poor wording there. I hope this means that for the majority of the cases, the above should be sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants