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

Config mechanism for plug defaults #1038

Closed
andrewkaufman opened this issue Oct 10, 2014 · 2 comments · Fixed by #1045
Closed

Config mechanism for plug defaults #1038

andrewkaufman opened this issue Oct 10, 2014 · 2 comments · Fixed by #1045
Assignees

Comments

@andrewkaufman
Copy link
Contributor

It should make sense to use the Metadata to specify the values, since that already implements a per-plug-per-node registry. The NodeMenu would apply them, and there'd be a simple method somewhere to allow the same to be done explicitly within a script.

@andrewkaufman andrewkaufman self-assigned this Oct 10, 2014
@andrewkaufman andrewkaufman added this to the Usability in Caribou milestone Oct 10, 2014
andrewkaufman added a commit to andrewkaufman/gaffer that referenced this issue Oct 10, 2014
This occures using NodeMenu.setPlugDefaultValues( node )

Fixes GafferHQ#1038.
@andrewkaufman
Copy link
Contributor Author

I've got a commit with this working (referenced above), but I haven't made a pull request yet, because I don't like that the entire thing is in GafferUI.NodeMenu.

You can register a default like this:
Gaffer.Metadata.registerPlugValue( Gaffer.ExecutableNode.staticTypeId(), "dispatcher.batchSize", "defaultValue", IECore.IntData( 7 ) )

and it will be applied when creating the node via the menu. Scripts could also apply the defaults using GafferUI.NodeMenu.setPlugDefaultValues( node ) but as I said, I don't like it being tied to the GafferUI module. Do you have a better recommendation of where this function should live? On Metadata itself? Is "defaultValue" a reasonable key?

@johnhaddon
Copy link
Member

I think "defaultValue" is a potentially confusing name, since we already have real default values, and what we're actually doing here is overriding them. Methods starting with "set" should always have a corresponding "get" method as well, so that name is a little confusing.

How about we call the metadata entry "userDefault"? And how about we make a new NodeAlgo.py file and add a method called applyUserDefaults() in it? We have various *Algo files now, and it seems like a pretty decent way of collecting various small utilities in a sensible place - it's also what the Imath library does, so it feels fairly coherent to me.

I think it's only a matter of time before we want this functionality in C++, but it's a bit of a pain to implement there, so I'm happy to keep it in Python for now.

Now we have user defaults, I think we can reconsider the Dispatcher registry mechanism again. The objection to registering creators rather than instances was that it prevented startup files from collaborating, each changing only some settings. Now, each file can set some user defaults, but not others, and we get the collaboration we want.

andrewkaufman added a commit to andrewkaufman/gaffer that referenced this issue Oct 14, 2014
Nodes created via the NodeMenu apply default values to their plugs, using the "userDefault" key in the Metadata.

Fixes GafferHQ#1038.
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 a pull request may close this issue.

2 participants