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

Bartek Skorupa (priv) bartekskorupa at bartekskorupa.com
Tue Mar 26 17:55:45 CET 2013


Thank you very much.

Bartek Skorupa

www.bartekskorupa.com

On 26 mar 2013, at 17:53, CoDEmanX <codemanx at gmx.de> wrote:

> if the number of properties you pass to an operator differ, e.g. one 
> prop per Custom Property of the active object, then you shouldn't do 
> something like
> 
> prop1 = ...
> prop2 = ...
> ...
> prop20 = ...
> 
> to allow for up to 20 properties be passed to the operator. Why? Cause 
> you don't need 20 properties if you mostly need to pass just one or two, 
> and there may be situations in which you need more than 20. So a fixed 
> amount is really bad.
> 
> Instead, you can use a CollectionProperty and add an item for every 
> element you need to pass.
> 
> Here's an example:
> 
> http://www.pasteall.org/40829/python
> 
> 
> If ideasman is fine with your code, then it's ok to move to trunk!
> 
> 
> Am 25.03.2013 17:33, schrieb Bartek Skorupa (priv):
>> Hey,
>> @CoDEmanX:
>> I don't quite get what you mean by:
>>> But please don't do this if the amount of props varies (prop1-propN)!
>> Could you elaborate, please?
>> 
>> 
>> Bartek Skorupa
>> 
>> www.bartekskorupa.com
>> 
>> On 25 mar 2013, at 17:19, CoDEmanX <codemanx at gmx.de> wrote:
>> 
>>> Proper formatting:
>>> 
>>> props = layout.operator(NodesLinkActiveToSelected.bl_idname,
>>> text="Replace Links")
>>> 
>>> props.prop1 = True
>>> props.prop2 = True
>>> props.prop3 = False
>>> 
>>> But please don't do this if the amount of props varies (prop1-propN)!
>>> Use a CollectionProperty instead. I think i did this in the Game
>>> Property Visulizer addon to replace the evil eval().
>>> 
>>> 
>>> Am 25.03.2013 16:08, schrieb Bartek Skorupa (priv):
>>>> Thank you for the explanation.
>>>>> props = layout.operator(NodesLinkActiveToSelected.bl_idname, text="Replace Links").prop1 = True
>>>>>     props.prop2 = True
>>>>>     props.prop3 = False
>>>> Oops… I didn't know you can do it :-) Really…
>>>> 
>>>> Every day I learn something.
>>>> I will change all those properties accordingly.
>>>> 
>>>> Thank you once again
>>>> 
>>>> Regards
>>>> 
>>>> Bartek Skorupa
>>>> 
>>>> www.bartekskorupa.com
>>>> 
>>>> On 25 mar 2013, at 16:02, Bastien Montagne <montagne29 at wanadoo.fr> wrote:
>>>> 
>>>>> 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
>>>>>> 
>>>>> _______________________________________________
>>>>> 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
>> 
>> _______________________________________________
>> 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