[Bf-codereview] Node (issue 7235078)

[bartekskorupa.com at gmail.com]

Most of issues have been addressed. Only 2 left because I have doubts
I'll need to discuss.


https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py
File node_efficiency_tools.py (right):

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode42
node_efficiency_tools.py:42: rl_outputs = [
On 2013/02/01 13:55:11, ideasman42 wrote:
> if this never changes, suggest to use tuples. Same for other global
lists.

Done.

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode73
node_efficiency_tools.py:73: blend_types = ['MIX', 'ADD', 'MULTIPLY',
'SUBTRACT', 'SCREEN', 'DIVIDE', 'DIFFERENCE', 'DARKEN', 'LIGHTEN',
'OVERLAY', 'DODGE', 'BURN', 'HUE', 'SATURATION', 'VALUE', 'COLOR',
'SOFT_LIGHT', 'LINEAR_LIGHT']
On 2013/02/01 13:55:11, ideasman42 wrote:
> prefer splitting at 120 length max.

Done.

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode125
node_efficiency_tools.py:125: global blend_types
On 2013/02/01 13:55:11, ideasman42 wrote:
> None of these need to be global, unless you want to re-assign (which
looks not
> to be the case - this goes for all other `global` use in this file)

Done.

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode136
node_efficiency_tools.py:136: if node.select and len(node.outputs) > 0:
On 2013/02/01 13:55:11, ideasman42 wrote:
> 'len(node.outputs) > 0'
> should be replaced with
> 'node.outputs'
> This is more efficient to test since it only checks the list isnt
empty.

Done.

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode138
node_efficiency_tools.py:138: for [type, the_list, dst] in [
On 2013/02/01 13:55:11, ideasman42 wrote:
> again, suggest to use typles.

Done.

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode313
node_efficiency_tools.py:313: if change in [0.0, 1.0]:
On 2013/02/01 13:55:11, ideasman42 wrote:
> with python3.x checking contains is nice to use sets.
> 'in {0.0, 1.0}'

Done.

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode1289
node_efficiency_tools.py:1289: def unregister():
Here I have doubts. Doesn't it remove the whole keymap? What if I have
several add ons using the same keymap? I'm afraid items for those other
addons will be removed once I de-activate this addon. Am I totally wrong
here?

https://codereview.appspot.com/7235078/