[Bf-committers] Python API change breaks Rigify

[Nathan Vegdahl]

> However this isnt about py devs writing inefficient code, but my point
> is that allowing bad interactions between drawing UI and managing
> settings is error prone and a UI shouldn't be editing blend file data
> anyway.
> We dont accept this in our C code (as a convention), why allow bad,
> error-prone practice in Python?

Sorry if I didn't make it clear: I fully agree on this point, and it's
the real reason I've been swayed.

I just also wanted to make the point that additional API support will
likely be necessary because of the restriction.  Supporting
non-empty-by-default collections is a good example.  Probably there
aren't too many things like this, but they will need to be addressed
as they come up.

> This is possible to do now, if you define an RNA property you can give
> it an update callback to run.

That probably takes care of 90% of the use-cases right there. :-)

--Nathan


On Sun, Feb 17, 2013 at 8:00 AM, Campbell Barton <ideasman42 at gmail.com> wrote:
> On Mon, Feb 18, 2013 at 2:18 AM, Nathan Vegdahl <cessen at cessen.com> wrote:
>> I believe there are some other use-cases that this restriction
>> hinders/prevents without additional features in the API.
>>
>> IIRC, last time this came up, Matt Ebb spoke up regarding his
>> RenderMan exporter, where selecting a shader file adds its parameters
>> as properties to the material, or something like that.  I'm not sure
>> what the best solution for such use-cases would be.  Of course a
>> "process shader" button could be added, similar to the "add rigify
>> layers" button.  But that's not ideal for a clean, transparent user
>> experience.  Maybe some kind of UI hooks...?
>
> This is possible to do now, if you define an RNA property you can give
> it an update callback to run.
>
>>> Also, manipulating data can cause a redraw, which enters a feedback
>>> loop using 100% CPU while idle.
>>
>> Well, you can't prevent Python code from doing things that are
>> arbitrarily expensive.  IMO that's better to enforce via code review.
>
> Fair point, infact using the update callback you could create a
> feedback loop too so probably wasn't the best example to give. -
> Though new devs did run into it and complain.
>
> However this isnt about py devs writing inefficient code, but my point
> is that allowing bad interactions between drawing UI and managing
> settings is error prone and a UI shouldn't be editing blend file data
> anyway.
> We dont accept this in our C code (as a convention), why allow bad,
> error-prone practice in Python?
>
>> But still, I agree, this is a good restriction to have in retrospect.
>> Although from my perspective it's more about forcing the API to grow
>> to accommodate doing these things properly.  Now I get to poke you
>> when I can't accomplish something due to this restriction!  Bwa ha ha
>> ha ha. ;-)
>
> This restriction can be worked around if its really important. but
> prefer not to unless its really needed.
>
>>> At the moment Rigify just has a button to add layers if they are not
>>> created, ideally blender could support collections that default to a
>>> non-empty list, but this isn't such a common thing to be doing.
>>
>> Can we put this on a todo list somewhere, so that it doesn't get
>> forgotten?  It may not be common, but it is useful.  I would like it
>> for Rigify.
>
> Yep, I think TODO would be non-empty defaults for collections, belongs here
> http://wiki.blender.org/index.php/Dev:2.5/Source/Development/Todo/Scripting
>
>> --Nathan
>>
>>
>> On Sat, Feb 16, 2013 at 8:32 PM, Campbell Barton <ideasman42 at gmail.com> wrote:
>>> On Fri, Feb 15, 2013 at 12:59 AM, Nathan Vegdahl <cessen at cessen.com> wrote:
>>>> I'm having difficulty tracking down the revision that caused this, but
>>>> this error is now coming up in Rigify:
>>>>
>>>> ----
>>>> Traceback (most recent call last):
>>>>   File "/home/cessen/Projects/blender/build/bin/2.65/scripts/addons/rigify/ui.py",
>>>> line 173, in draw
>>>>     bone.rigify_parameters.add()
>>>> AttributeError: Writing to ID classes in this context is not allowed:
>>>> Armature, Object datablock, error setting PoseBone.
>>>> ----
>>>>
>>>> It appears to be caused by a change in the Python API (this code
>>>> worked before).  It seems that ID classes can no longer be written to
>>>> within the draw method of Panels.  What is the benefit of this
>>>> restriction, and why do we need it?
>>>>
>>>> And in the mean time, can we please revert that restriction so that
>>>> Rigify (and possibly other existing code) continues to function as
>>>> before?
>>>>
>>>> For those who are curious: to reproduce the error in Rigify, you have
>>>> to manually construct a metarig, adding a rig type to a new bone.  The
>>>> default human metarig does not give this error because of how it is
>>>> auto-constructed.  The error is emitted to stdout, and does not show
>>>> up as a python in Blender's UI.  However, it does break Rigify
>>>> functionality.
>>>>
>>>> --Nathan
>>>
>>> To follow up on this - Nathan and I discussed in IRC and found a workaround.
>>>
>>> Restricting ID writing on draw has been in since 2010, rev33462.
>>>
>>> I'm still glad this restriction exists, its too easy to initialize
>>> data in a panel draw function that then makes the application behave
>>> incorrectly if the panel happens to start collapsed for example - or
>>> the tool is accessed from a different part of the interface the data
>>> may not be initialized.
>>> Also, manipulating data can cause a redraw, which enters a feedback
>>> loop using 100% CPU while idle.
>>>
>>> At the moment Rigify just has a button to add layers if they are not
>>> created, ideally blender could support collections that default to a
>>> non-empty list, but this isn't such a common thing to be doing.
>>>
>>> --
>>> - Campbell
>>> _______________________________________________
>>> 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
>
>
>
> --
> - Campbell
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers