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

AGD-3133: Add Dynamo Preference Non-UI Flag for legacy behavior #14575

Merged
merged 5 commits into from
Nov 13, 2023

Conversation

saintentropy
Copy link
Contributor

@saintentropy saintentropy commented Nov 6, 2023

Purpose

Add Non preference UI flag to Dynamo to allow users to enable legacy behavior of Dynamo Player graph parsing related to Outputs. When the player was first introduced there were no isOutput property on the nodes. The various players used the renamed watch as a single to the Player to use them as outputs. Now that Dynamo has the output property the old behavior is causing bug reports. This change will allow the player to remove the old option but support it for a period of time. This flag and support will be removed in Dynamo 4.0.

https://jira.autodesk.com/browse/AGD-3133

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

For Dynamo Player, the legacy behavior for setting an output was a renamed Watch node. This method was still supported when the IsOutput property was introduced. Dynamo Player is now deprecating the renamed Watch node method and adding support via Dynamo config file. This config option will be removed in a future release.

Reviewers

@mjkkirschner @twastvedt

FYIs

@QilongTang

@saintentropy saintentropy changed the title Add flag Add Dynamo Preference Non-UI Flag for legacy behavior Nov 6, 2023
@mjkkirschner
Copy link
Member

Whats the use case here?
Can it be tested, or it's entirely just a passthrough?

Concern is that it will be very easy for a dev to remove this thinking it's not used etc.

@QilongTang QilongTang added this to the 3.0 milestone Nov 7, 2023
@QilongTang
Copy link
Contributor

Should we use inheritance so DynamoRevit actually has a settings object slightly different from sandbox? Does this apply to C3D? My concern is that this setting seems to only surface in Player and is not settable in Dynamo.

@saintentropy
Copy link
Contributor Author

I will update the comments on this implementation but @mjkkirschner and @QilongTang the goal here is to fix bugs that are being reported due to a legacy issue with the Dynamo Player server. (I added more detail to the description) We have planned to depreciate this behavior but follow the Revit model of providing an options to customers that really need the old way for one release year but require them to make a change in a config file.

@saintentropy
Copy link
Contributor Author

Not sure if that answers you question @QilongTang. The goal is to give each host the option to set this config for the application

@mjkkirschner
Copy link
Member

@saintentropy can you add a test?

@mjkkirschner
Copy link
Member

hey @saintentropy theres also a failure in here from some other preferences tests

@saintentropy
Copy link
Contributor Author

@mjkkirschner Good to go?

@mjkkirschner mjkkirschner merged commit 40e4357 into DynamoDS:master Nov 13, 2023
13 checks passed
@twastvedt twastvedt changed the title Add Dynamo Preference Non-UI Flag for legacy behavior AGD-3131: Add Dynamo Preference Non-UI Flag for legacy behavior Jan 30, 2024
@twastvedt twastvedt changed the title AGD-3131: Add Dynamo Preference Non-UI Flag for legacy behavior AGD-3133: Add Dynamo Preference Non-UI Flag for legacy behavior Jan 30, 2024
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