Skip to content

[DYN-8596] Curve Mapper : Generate warnings#16141

Merged
reddyashish merged 3 commits intoDynamoDS:masterfrom
ivaylo-matov:DYN-8596-Generate-warnings
Apr 23, 2025
Merged

[DYN-8596] Curve Mapper : Generate warnings#16141
reddyashish merged 3 commits intoDynamoDS:masterfrom
ivaylo-matov:DYN-8596-Generate-warnings

Conversation

@ivaylo-matov
Copy link
Copy Markdown
Contributor

Purpose

This PR addresses DYN-8596 , along with several related discussions about the types of warnings generated by the CurveMapper node.

As discussed, the node's info bubble should display a single consolidated warning that summarizes any issues related to the input ports. This improves clarity and avoids overwhelming the user with multiple individual messages.

DYN-8648

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

Curve Mapper now generates a consolidated warning message based on its input values.

Reviewers

@reddyashish
@zeusongit

FYIs

@dnenov
@achintyabhat

Copy link
Copy Markdown

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

[ArbitraryDimensionArrayImport] object minY,
[ArbitraryDimensionArrayImport] object maxY,
List<double> pointsCount,
[ArbitraryDimensionArrayImport] object pointsCount,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is an API break.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

latest commit should resolve this

/// The sine wave is defined by two control points and follows a trigonometric function.
/// </summary>

[IsVisibleInDynamoLibrary(false)]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why add this attribute explicitly for all curves?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

because they show in search

Copy link
Copy Markdown
Collaborator

@reddyashish reddyashish Apr 22, 2025

Choose a reason for hiding this comment

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

Were they showing up before as well? Or some change in this PR that affects it?

Copy link
Copy Markdown
Contributor Author

@ivaylo-matov ivaylo-matov Apr 22, 2025

Choose a reason for hiding this comment

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

They must have been there before
image

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm only showing up in the incanvas search, not in the library component. Good that we are hiding them now.


// Preserve original API signature by wrapping to internal object-based version
// This allows handling of both scalars and lists while avoiding replication
private static List<double> PrivateCalculateValuesX(
Copy link
Copy Markdown
Collaborator

@reddyashish reddyashish Apr 22, 2025

Choose a reason for hiding this comment

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

Works fine but a better way to be to add this as a new public/internal method with 'pointsCount' as '[ArbitraryDimensionArrayImport] object' and add a obsolete tag to the old method(if that is not going to be used anymore). Less code to maintain.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please check latest commit

@reddyashish reddyashish merged commit 51ab6d6 into DynamoDS:master Apr 23, 2025
24 checks passed
</data>
</root>
<data name="CurveMapperEqualMinMaxWarning" xml:space="preserve">
<value>• Min and Max values must not be the same.</value>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Helena suggested to change this message to "Min and Max values must be different.". Can you make that change in the other Curvemapper PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure

@zeusongit zeusongit added this to the 3.5.1 milestone May 5, 2025
@zeusongit
Copy link
Copy Markdown
Contributor

/cherrypick

github-actions Bot pushed a commit that referenced this pull request May 16, 2025
@github-actions
Copy link
Copy Markdown

Successfully created backport PR for RC3.5.1_master:

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.

3 participants