Paths

Table of Contentst

Differential D2129

Node Wrangler - Reset node, Copy Node Attributes
ClosedPublic
Actions

Authored by Christian Brinkmann (poor) on Jul 29 2016, 5:24 PM.

Details

Reviewers

Greg Zaal (gregzaal)

Commits

rBA5929cc941927: Node Wrangler - Reset node by @poor & @zeffii

Summary

This patch basically adds the option to 'Reset' the active node via Back Space, as well as 'Copy Node Attributes' from the active node to all same node types in the selection via Ctrl Shift V.

Both new operators are based on this approach: http://blender.stackexchange.com/a/42338/3710. The code feels a bit "hacky" since it's simply replacing the active node by a new one. When calling 'Copy Node Attributes' Operator, it duplicates the active node and replaces all nodes (of the same type) in the current selection with its respective duplicate. Nevertheless the code works for all node types, regardless whether the nodes are connected and and it's nice to work with (IMO) as long as this is not implemented in C/C++.

In use
http://i.stack.imgur.com/PJlB7.gif

Background
A "reset to default" option makes experimenting with node setups much easier. Not only for large node trees, when it comes to eyeballing, from an artist point of view it's a common functionality in every node based environment regardless of whether it's Houdini, Nuke etc. to "reset" the node by one click (or even a group of properties of that node) in order to starting from scratch.

Best regards,
Christian

Diff Detail

Event Timeline

Christian Brinkmann (poor) updated this revision to Diff 7135.Jul 29 2016, 5:24 PM

Christian Brinkmann (poor) retitled this revision from to Node Wrangler - Reset node, Copy Node Attributes.

Christian Brinkmann (poor) updated this object.

Christian Brinkmann (poor) added a reviewer: Greg Zaal (gregzaal).

Christian Brinkmann (poor) set the repository for this revision to rBA Blender Add-ons.

Christian Brinkmann (poor) added projects: Nodes, Add-ons (Community).

Nice! A few minor notes inline, otherwise:

How is the Copy Node Attributes op different from the existing Copy to Selected (Shift-C) one?

Is it possible to use Backspace as the hotkey for resetting nodes? Without affecting the existing behaviour of resetting whatever property the mouse is hovering over.

To be consistent with the rest of the code style, I'd rather all the code be inside the operator's execute function than in a separate function.

node_wrangler.py
1012	Comments should start with a capital letter (all of them, not just this one)
1014	2 nodes may be selected but neither are active - so better to have a separate check for active node with a more specific error.
1021	If user has a huge selection, it'd be helpful for the error to say which node(s) are not the same type.
1082	Surely if multiple nodes are selected, the user wants to reset all of them? Can just wrap most of this function in a `for node in selection` loop
1111	`resetted` -> `reset` (correct english)
3227	Needs `bl_options = {'REGISTER', 'UNDO'}`
4195	Tooltip should be more descriptive like "Revert node back to default state, but keep connections"

This revision now requires changes to proceed.Aug 8 2016, 12:51 PM

Hi Greg,

Thanks for your review! I implemented all of your notes/remarks and added a lot of checks. Backspace works surprisingly and feels really good. Great idea!

How is the Copy Node Attributes op different from the existing Copy to Selected (Shift-C) one?

Copy to Selected has less checks, returns an error when the node is in a frame (since bpy.ops.node.parent_clear() is deprecated) and calculating the position seems not necessary anymore.

Here is a Copy Node Attributes operator test in a frame: http://i.stack.imgur.com/gEu6V.gif

To be consistent with the rest of the code style, I'd rather all the code be inside the operator's execute function than in a separate function.

No problem. Done.

Christian Brinkmann (poor) updated this revision to Diff 7215.Aug 9 2016, 8:31 PM

Christian Brinkmann (poor) updated this object.

Christian Brinkmann (poor) edited edge metadata.

Christian Brinkmann (poor) removed rBA Blender Add-ons as the repository for this revision.

Apart from how the existing Copy to Selected is broken, they seem to (try to) do the same thing to me. Shall we just replace the current broken one with your new one?

Apart from how the existing Copy to Selected is broken, they seem to (try to) do the same thing to me. Shall we just replace the current broken one with your new one?

Yes, I also noticed that it's basically the same, but the user interaction is slightly different. The new one re-selects the active node (at the end of the process) only and comes with a few reports to indicate what exactly happened. I think it's not necessary to keep the selection (as Copy to Selected does) since the user's intention is to copy to the properties of the active one, which is already set correctly. If it's really requiered to repeat the process any other types as the active node will be ignored, so it's really safe to reselect other nodes as you can see in the first gif.

At the end it's your decision and probably a matter of taste. All I can say is that I heavily tested the new one, it seems to work for all cases and has better interaction (IMO).

Just let me know if I can help!
Thanks again :)

Greg Zaal (gregzaal) mentioned this in rBAc3b04a94d760: Node Wrangler: Rewrite Copy Settings function Fixes a bug with frames….Aug 11 2016, 12:36 PM

Just committed the Copy Attributes part of this patch (replaced existing function) since 2.78 testbuilds are about to be released and then we'll be locked to just bug fixing. After 2.78 I'll add the Reset stuffs.

Is there a reason we need to have two separate operators for Reset Node and Reset Nodes in Selection? If there is only one node in the selection, surely Reset Nodes in Selection would still work?

Of course. I wasn't sure if you like the 'Reset Button' :)

I included a few checks to exclude Groups, Reroutes and Frames from the reset and I finally added the ability to reset all valid nodes in a frame. Also the button changes depending on the context for a more consistent UI now. Heavily tested the whole thing again and it works for all cases. Here is a gif: http://i.stack.imgur.com/NrsdD.gif

What do you think? One last thing: assuming everything is fine now and you decide to take it, can we add 'Dealga McArdle' as author? He has done the base stuff, so I think this would be fair...

In D2129#50125, @Christian Brinkmann (poor) wrote:

Of course. I wasn't sure if you like the 'Reset Button' :)

I included a few checks to exclude Groups, Reroutes and Frames from the reset and I finally added the ability to reset all valid nodes in a frame. Also the button changes depending on the context for a more consistent UI now. Heavily tested the whole thing again and it works for all cases. Here is a gif: http://i.stack.imgur.com/NrsdD.gif

Looks good to me! I'll test it out properly and commit once 2.78 is released.

One last thing: assuming everything is fine now and you decide to take it, can we add 'Dealga McArdle' as author? He has done the base stuff, so I think this would be fair...

Seems like you did most of the work, but if @Dealga McArdle (zeffii) insists and is willing to maintain his code then I don't mind.

This revision is now accepted and ready to land.Aug 12 2016, 9:27 PM

@Greg Zaal (gregzaal), Git is open for BCon1 changes now, so should be fine to commit this now ;)

@Greg Zaal (gregzaal) asked me to chime in here.

i'd be happier if you removed my name from the credits and instead add a comment above where the code is run.

# code by zeffii from http://blender.stackexchange.com/a/42338/3710
# Run through all valid nodes
for node in valid_nodes:        
    # magic

I cannot guarantee that i'll be around or contactable if the code needs to be fixed.

Closed by commit rBA5929cc941927: Node Wrangler - Reset node by @poor & @zeffii (authored by Greg Zaal (gregzaal)). · Explain WhySep 30 2016, 12:06 PM

This revision was automatically updated to reflect the committed changes.

Greg Zaal (gregzaal) mentioned this in rBA5929cc941927: Node Wrangler - Reset node by @poor & @zeffii.

Christian Brinkmann (poor) awarded a token.Sep 30 2016, 2:54 PM

Revision Contents
Changeset List

Path

Size

node_wrangler.py

154 lines

Diff 7135

View Options

Node Wrangler - Reset node, Copy Node AttributesClosedPublicActions