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

Bastien Montagne montagne29 at wanadoo.fr
Mon Mar 25 16:02:17 CET 2013


Hi,

On 25/03/2013 15:51, Bartek Skorupa (priv) wrote:
> 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?
Yes there is! :)

In such case, you should do that:

     props = layout.operator(NodesLinkActiveToSelected.bl_idname, text="Replace Links").prop1 = True
     props.prop2 = True
     props.prop3 = False

> 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?
First of all, it is a convention. True/false options should be booleans, 
options with a well defined set of choices should be enums, etc.

This also helps on documentation level (as each enum items has its own 
label/description, you do not need comments in code, and doc can be 
retrieved easily by multiple ways).

And there is also an UI interest, as using enums you can get a nice 
drop-down list as a control, or you can even auto-generate a menu where 
each entry will execute the operator with one of the options of the enum, …

By the way, note that if you have a set of related booleans options that 
are not mutually exclusive, you can use an enum with 'ENUM_FLAG' option 
(in that case you are just limited to at most 32 different flags - the 
length of a classical integer ;) ).

Kind regards,
Bastien

> 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
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
>


More information about the Bf-committers mailing list