Node Tools r4422 review

[ideasman42, 2013-03-27 00:16:46]

Code review & some suggestions.

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

https://codereview.appspot.com/7651047/diff/1/node_efficiency_tools.py#newcod...
node_efficiency_tools.py:103: def set_convenience_variables(context):
suggest to not use globals here and just:
return nodes, links

... so using would look like this (renamed set --> get)

nodes, links = get_convenience_variables()

https://codereview.appspot.com/7651047/diff/1/node_efficiency_tools.py#newcod...
node_efficiency_tools.py:138: def poll(cls, context):
All both functions are the same, use use a mix-in class: eg

class NodeToolBase:
    @classmethod
    def poll(cls, context):
        space = context.space_data
        return space.type == 'NODE_EDITOR' and space.node_tree is not None

... then use the class like this and dont include a poll function in the
operator subclass.
class MergeNodes(bpy.types.Operator, NodeToolBase):

https://codereview.appspot.com/7651047/diff/1/node_efficiency_tools.py#newcod...
node_efficiency_tools.py:191: for the_list in [selected_mix, selected_shader,
selected_math]:
'the_list' is used for different purposes, would prefer to use a name that makes
sense for each use.

https://codereview.appspot.com/7651047/diff/1/node_efficiency_tools.py#newcod...
node_efficiency_tools.py:197: loc_x = the_list[0][1] + 350.0
*picky* - personal preference to use 2d vectors for all locx,locy in this
script.

# if you dont want to be accessing the nodes original location, this copies the
vector.
xy = node.location.copy()

https://codereview.appspot.com/7651047/diff/1/node_efficiency_tools.py#newcod...
node_efficiency_tools.py:603: node.location = [x,y]
*picky*, you can just do this.
node.location = x, y

https://codereview.appspot.com/7651047/diff/1/node_efficiency_tools.py#newcod...
node_efficiency_tools.py:799: use_node_name = eval(option_split[1])
Using eval() just to pass 3 bools is not really good idea.
For this we have bpy.props.BoolVectorProperty(), an array of bools.

Goes for all other uses of eval() here.

https://codereview.appspot.com/7651047/diff/1/node_efficiency_tools.py#newcod...
node_efficiency_tools.py:858: class AlignNodes(bpy.types.Operator):
*picky* - again, this would benefit from using 2d Vector()'s (readability)

https://codereview.appspot.com/7651047/diff/1/node_efficiency_tools.py#newcod...
node_efficiency_tools.py:976: class SelectParentChildren(bpy.types.Operator):
This could be added into blenders C code, seems generally useful and we have for
object-mode, pose-mode.