[Bf-codereview] Node (issue 7235078)

ideasman42 at gmail.com ideasman42 at gmail.com
Fri Feb 1 14:55:11 CET 2013


Generally fine to merge but suggest changes,

- Unless theres a reason to use lists, use tuples.
- Dont use globals (Looks like theres no need to anyway).
- Keymap removal can be handled nicer (see link inline)


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 = [
if this never changes, suggest to use tuples. Same for other global
lists.

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']
prefer splitting at 120 length max.

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode125
node_efficiency_tools.py:125: global blend_types
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)

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:
'len(node.outputs) > 0'
should be replaced with
'node.outputs'
This is more efficient to test since it only checks the list isnt empty.

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode138
node_efficiency_tools.py:138: for [type, the_list, dst] in [
again, suggest to use typles.

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode313
node_efficiency_tools.py:313: if change in [0.0, 1.0]:
with python3.x checking contains is nice to use sets.
'in {0.0, 1.0}'

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode803
node_efficiency_tools.py:803: bpy.ops.transform.resize(value = (0.0,
0.0, 0.0))
would prefer changing X/Y's explicitly, calling transform op isnt
totally reliable.

https://codereview.appspot.com/7235078/diff/1/node_efficiency_tools.py#newcode1289
node_efficiency_tools.py:1289: def unregister():
Suggest to use this method of removing keymaps. more direct.

http://www.blender.org/documentation/blender_python_api_2_65_9/info_tutorial_addon.html#keymap

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


More information about the Bf-codereview mailing list