Skip to content

Conversation

@ivaylo-matov
Copy link
Contributor

@ivaylo-matov ivaylo-matov commented Apr 3, 2025

Purpose

This PR aims to address DYN-8596.

Curve Mapper now blocks list inputs on X and Y domain ports: minX, maxX, minY, maxY. If the user passes a list to any those input ports the node displays an "Expects argument types: double" warning instead of the generic runtime error. The output for both X and Y values in that case is null.
It's also prevents input replication, which previously caused CalculateValues to execute multiple times unintentionally.

Dynamo normally replicates function calls when a list is passed into a node. This breaks Curve Mapper’s logic.
To prevent this:

  • We now check in CurveMapperGenerator.CalculateValues if any of the X or Y limits are lists IEnumerable
  • If so, we return [null, null] and throw an ArgumentException with the message "Expects argument types: double"
  • The thrown exception tells Dynamo to show the familiar type mismatch warning used by built-in nodes

🙌 All credit to @aparajit-pratap

P.S.: I had to included the changes that enable support for List to allow uneven distribution of points, to ensure that the node remains fully functional. All feedback from 16030 at the time of publishing has been incorporated into this version.

DYN-8596

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 blocks list inputs on X and Y domain ports: minX, maxX, minY, maxY.

Reviewers

@reddyashish
@zeusongit

FYIs

@achintyabhat
@dnenov

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

@reddyashish
Copy link
Collaborator

So this #16030 is not needed? Those changes are in here?

@ivaylo-matov
Copy link
Contributor Author

So this #16030 is not needed? Those changes are in here?

Correct.
.. just saw the conflicts. On it now

Copy link
Collaborator

@reddyashish reddyashish left a comment

Choose a reason for hiding this comment

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

Looks good with couple of comments.

@reddyashish reddyashish merged commit c2f4070 into DynamoDS:master Apr 3, 2025
24 checks passed
@github-actions
Copy link

github-actions bot commented Apr 3, 2025

Backport failed for RC3.5.0_master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin RC3.5.0_master
git worktree add -d .worktree/cherrypick-16078 origin/RC3.5.0_master
cd .worktree/cherrypick-16078
git switch --create cherrypick-16078
git cherry-pick -x c2f4070e070589d135d523f2acdaa9f2a45ab7a2

@reddyashish
Copy link
Collaborator

/cherrypick

@github-actions
Copy link

github-actions bot commented Apr 3, 2025

Backport failed for RC3.5.0_master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin RC3.5.0_master
git worktree add -d .worktree/cherrypick-16078 origin/RC3.5.0_master
cd .worktree/cherrypick-16078
git switch --create cherrypick-16078
git cherry-pick -x c2f4070e070589d135d523f2acdaa9f2a45ab7a2

@reddyashish
Copy link
Collaborator

Will cherrypick this manually.

@reddyashish
Copy link
Collaborator

Also @ivaylo-matov I think you missed the resources change here #16030 to get into this PR.

reddyashish pushed a commit to reddyashish/Dynamo that referenced this pull request Apr 3, 2025
zeusongit pushed a commit that referenced this pull request Apr 3, 2025
@ivaylo-matov
Copy link
Contributor Author

ivaylo-matov commented Apr 4, 2025

apologies for the oversight, @reddyashish
I've added it here: #16094 along with one GenerateRenderValues in the node model.

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