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

Implement Nuke's lib.read #476

Merged
merged 9 commits into from
Dec 13, 2019
Merged

Conversation

davidlatwe
Copy link
Collaborator

@davidlatwe davidlatwe commented Nov 28, 2019

Motivation

The Avalon data read-write methods implemented in Nuke was driven by prefixing knob name so to differ which knob is node's default and which is user-defined.

But the prefix may change, and the logic was not the same as other hosts, so if there's a way to differ user-defined and default knobs without prefixing knob name, I think we should opt that in.

Changes

  • For unifying host implementation and pattern, adding lib.read to retrieve user-defined knobs from any given node, just like in other hosts. For layout type knob, like nuke.Tab_Knob, divider knob (nuke.Text_Knob with empty string value), and unnamed knob will be ignored.

  • Backward compatible for knobs which prefixed with "avalon:" or "ak:". (Resolved this comment)

  • Added flags optional arg on lib.Knobby for setting knob flag. Current read-only string data imprinting was implemented by using nuke.Text_Knob. But nuke.Text_Knob could be a layout divider if the initial value is an empty string, which could be confused with other read-only string data while reading. This change reduced the confusion by enabling knob flag setting while imprinting (e.g. nuke.String_Knob + nuke.READ_ONLY) .

@davidlatwe
Copy link
Collaborator Author

Just patched my motivation in above comment ☝️

And, in the following GIF you may see the difference between divider and text knob, the string knob and read-only string knob.

knobs

Note that the string knob with nuke.READ_ONLY applied, user still able to select and copy string, but not with text knob.

@tokejepsen
Copy link
Collaborator

Looks good! Nice work @davidlatwe

@davidlatwe
Copy link
Collaborator Author

This will be merged tomorrow if no other objections 🚀

@davidlatwe davidlatwe merged commit 6175006 into getavalon:master Dec 13, 2019
@davidlatwe davidlatwe mentioned this pull request Dec 13, 2019
@davidlatwe davidlatwe deleted the nuke-read branch December 13, 2019 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants