[Bf-committers] Node Efficiency Tools - can it go to trunk?

[Bartek Skorupa (priv)]

Thank you for this quick review.
Yes you're right about my understanding of bl_label. I did mismatch this and I can change it, np.

About my 'option' properties:
In many cases I can change it to enums, but sometimes StringProperty is used because I need to pass more than one property in one go.
Say I need to set 3 properties:
prop1 = True
prop2 = True
prop3 = False

I wrap it in one StringProperty that gets the value of 'True True False' and then in execute I use string split( ), and eval to get 3 booleans.

Please take a look at line 1325 for example:
layout.operator(NodesLinkActiveToSelected.bl_idname, text="Replace Links").option = 'True True False'

option then is passed to execute of NodesLinkActiveToSelected and split.
see lines: 795 to 798:
        option_split = self.option.split( )
        replace = eval(option_split[0])
        use_node_name = eval(option_split[1])
        use_outputs_names = eval(option_split[2])

This way I get 3 variables out of one property 'option'

Is there a better way of doing it?

Another question:
Could you please explain me why using StringProperty instead of proper EnumProperty is wrong?
Is it a convention, speed issues or anything else?

Thank you in advance

Regards

Bartek Skorupa

www.bartekskorupa.com

On 25 mar 2013, at 15:24, Bastien Montagne <montagne29 at wanadoo.fr> wrote:

> Hi Bartek,
> 
> Just did a (very quick) skim review of your addon, and I agree that 
> there are valuable features in it, worth moving it to trunk. However, I 
> noted at least two points that imho should be addressed before the move:
> 
> First, you seems to mismatch labels and tips of operators! E.g. instead 
> of this line:
> 
>     bl_label = "Copy Settings of Active Node to Selected Nodes"
> 
> I would rather see those:
> 
>     bl_label = "Copy Node Settings"
>     bl_description = "Copy the settings of the active node to selected 
> ones"
> 
> The second point is, I think, more important. You should replace your 
> “multipurpose” "option" StringProperty by relevant properties. E.g.
> 
>     # option: 'from active', 'from node', 'from socket'
>     option = StringProperty()
> 
> Should be replaced by:
> 
>     source = EnumProperty(name="Source", description="A relevant 
> description…", default='FROM_ACTIVE',
>                           items=(('FROM_ACTIVE', "From Active", "A 
> relevant description…", 'ICON_NONE', 1),
>                                  ('FROM_NODE', "From Node", "A relevant 
> description…", 'ICON_NONE', 2),
>                                  ('FROM_SOCKET', "From Socket", "A 
> relevant description…", 'ICON_NONE', 3),
>                                 )
>                          )
> 
> (with relevant changes in other parts of the code).
> 
> Best regards,
> Bastien
> 
> On 25/03/2013 14:00, Bartek Skorupa (priv) wrote:
>> Hey,
>> 
>> After recent commits of node_efficiency_tools.py I as an author call this add-on ready.
>> All of the features that I had in mind have been included, documented on wiki and in video tutorial.
>> Recent API changes have taken into account.
>> It certainly will develop further, but at this stage it's IMHO ready to go trunk.
>> 
>> I'd like to ask for reviewing the code and hopefully permission to move this add-on to trunk.
>> https://svn.blender.org/svnroot/bf-extensions/contrib/py/scripts/addons/node_efficiency_tools.py
>> 
>> Here's the wiki page:
>> http://wiki.blender.org/index.php/Extensions:2.6/Py/Scripts/Nodes/Nodes_Efficiency_Tools
>> 
>> Tracker:
>> http://projects.blender.org/tracker/?func=detail&atid=468&aid=33543&group_id=153
>> 
>> Thread on blenderartists:
>> http://blenderartists.org/forum/showthread.php?274755-ADDON-Nodes-Efficiency-Tools
>> 
>> 
>> Many users appreciate this add-on and wish to have it in official Blender releases.
>> I declare to maintain the code.
>> 
>> Please let me know if you feel that it's worth including or not.
>> 
>> 
>> With Respect,
>> 
>> Bartek Skorupa
>> 
>> www.bartekskorupa.com
>> 
>> _______________________________________________
>> Bf-committers mailing list
>> Bf-committers at blender.org
>> http://lists.blender.org/mailman/listinfo/bf-committers
>> 
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers