[Bf-committers] [Bf-blender-cvs] SVN commit: /data/svn/bf-blender [33462] trunk/blender/source/blender/ python/intern/bpy_rna.c: disallow setting RNA attributes while drawing, this is bad practice so enforcing here has the benifit of making sure

Campbell Barton ideasman42 at gmail.com
Wed Dec 8 05:03:22 CET 2010


Hi Doug,
I'm quite sure that this bad practice to do this but weather it should
be disallowed or not is another thing.

Having drawing functions change material/scene/object data is error prone IMHO.
- Can be used as a sloppy way to initialize variables, With the gotcha
that if the panel never shows the variable isn't initialized - Seen
this happen in 2.4x, C code.
- Drawing also wont happen in background mode so this can cause hard
to track problems with a renderfarm.
- Can cause feedback loops where changing the value causes a redraw
which changes the value..... etc. This was happening with the Clay
Addon.

So I rather avoid weirdo bugs where fixing an update function causes
the CPU usage to go to 100% with some UI, or rendering in background
mode fails to initialize some data.
These bugs are often not easy to track which is why I committed this exception.

I'm interested to see the function which is giving the error to see
how it could work differently.

At the moment python properties cant define their own update
functions, I think we need this so changing a lux setting can update
data internally.


Take your point about discussing things on the mailing list first,
though in my experience not many script authors are interested in API
design.
There is bf-python mailing list which is very quiet, but since python
is playing a bigger role now and we have more addon authors I could
try to do this more.
This raises the question: are most extension authors reading
bf-committees mailing list?, should we discuss python api design on
bf-python?

- Campbell


On Tue, Dec 7, 2010 at 9:48 PM, Doug Hammond
<doughammond at hamsterfight.co.uk> wrote:
> Actually, the bigger problem that I just forgot about is that using blender
> lamps in LuxRender is now completely broken too.
>
> Cheers,
> Doug.
>
>
> On 7 December 2010 19:43, Doug Hammond <doughammond at hamsterfight.co.uk>wrote:
>
>> Is there an alternative mechanism for achieving the same result available?
>> From this revision onwards I've lost the ability to set the object's colour
>> from the LuxRender material editor, which is something our users find really
>> useful.
>>
>> TBH, I'm a bit peeved that something like this would be disabled without
>> discussion with addon developers and without a reasonable alternative API in
>> place.
>>
>>
>> Cheers,
>> Doug.
>>
>>
>> On 4 December 2010 06:25, Campbell Barton <ideasman42 at gmail.com> wrote:
>>
>>> Revision: 33462
>>>
>>> http://projects.blender.org/plugins/scmsvn/viewcvs.php?view=rev&root=bf-blender&revision=33462
>>> Author:   campbellbarton
>>> Date:     2010-12-04 07:25:36 +0100 (Sat, 04 Dec 2010)
>>>
>>> Log Message:
>>> -----------
>>> disallow setting RNA attributes while drawing, this is bad practice so
>>> enforcing here has the benifit of making sure people are not manipulating
>>> blender scene data in a drawing panel for eg.
>>>
>>> This is ifdef'd and may be disabled later on, or only enabled in debug
>>> mode.
>>>
>>> This applies to setting any RNA value that has an ID and is not a screen
>>> or window-manager datablock.
>>>
>>> Some addons break this rule and need fixing but from my tests blender UI
>>> scripts are ok.
>>>
>>> Modified Paths:
>>> --------------
>>>    trunk/blender/source/blender/python/intern/bpy_rna.c
>>>
>>> Modified: trunk/blender/source/blender/python/intern/bpy_rna.c
>>> ===================================================================
>>> --- trunk/blender/source/blender/python/intern/bpy_rna.c        2010-12-04
>>> 06:21:08 UTC (rev 33461)
>>> +++ trunk/blender/source/blender/python/intern/bpy_rna.c        2010-12-04
>>> 06:25:36 UTC (rev 33462)
>>> @@ -50,6 +50,7 @@
>>>  #include "DNA_anim_types.h"
>>>  #include "ED_keyframing.h"
>>>
>>> +#define USE_PEDANTIC_WRITE
>>>  #define USE_MATHUTILS
>>>  #define USE_STRING_COERCE
>>>
>>> @@ -67,7 +68,32 @@
>>>
>>>  /* bpyrna vector/euler/quat callbacks */
>>>  static int mathutils_rna_array_cb_index= -1; /* index for our callbacks
>>> */
>>> +#ifdef USE_PEDANTIC_WRITE
>>> +static short rna_disallow_writes= FALSE;
>>>
>>> +static int rna_id_write_error(PointerRNA *ptr, PyObject *key)
>>> +{
>>> +       ID *id= ptr->id.data;
>>> +       if(id) {
>>> +               const short idcode= GS(id->name);
>>> +               if(!ELEM(idcode, ID_WM, ID_SCR)) { /* may need more added
>>> here */
>>> +                       const char *idtype= BKE_idcode_to_name(idcode);
>>> +                       const char *pyname;
>>> +                       if(key && PyUnicode_Check(key)) pyname=
>>> _PyUnicode_AsString(key);
>>> +                       else
>>>      pyname= "<UNKNOWN>";
>>> +
>>> +                       /* make a nice string error */
>>> +                       assert(idtype != NULL);
>>> +                       PyErr_Format(PyExc_RuntimeError, "Writing to ID
>>> classes in this context is not allowed: %.200s, %.200s datablock, error
>>> setting %.200s.%.200s", id->name+2, idtype,
>>> RNA_struct_identifier(ptr->type), pyname);
>>> +
>>> +                       return TRUE;
>>> +               }
>>> +       }
>>> +       return FALSE;
>>> +}
>>> +
>>> +#endif
>>> +
>>>  /* subtype not used much yet */
>>>  #define MATHUTILS_CB_SUBTYPE_EUL 0
>>>  #define MATHUTILS_CB_SUBTYPE_VEC 1
>>> @@ -104,7 +130,13 @@
>>>        float min, max;
>>>        if(self->prop==NULL)
>>>                return 0;
>>> -
>>> +
>>> +#ifdef USE_PEDANTIC_WRITE
>>> +       if(rna_disallow_writes && rna_id_write_error(&self->ptr, NULL)) {
>>> +               return 0;
>>> +       }
>>> +#endif
>>> +
>>>        if (!RNA_property_editable_flag(&self->ptr, self->prop)) {
>>>                PyErr_Format(PyExc_AttributeError, "bpy_prop
>>> \"%.200s.%.200s\" is read-only", RNA_struct_identifier(self->ptr.type),
>>> RNA_property_identifier(self->prop));
>>>                return 0;
>>> @@ -157,6 +189,12 @@
>>>        if(self->prop==NULL)
>>>                return 0;
>>>
>>> +#ifdef USE_PEDANTIC_WRITE
>>> +       if(rna_disallow_writes && rna_id_write_error(&self->ptr, NULL)) {
>>> +               return 0;
>>> +       }
>>> +#endif
>>> +
>>>        if (!RNA_property_editable_flag(&self->ptr, self->prop)) {
>>>                PyErr_Format(PyExc_AttributeError, "bpy_prop
>>> \"%.200s.%.200s\" is read-only", RNA_struct_identifier(self->ptr.type),
>>> RNA_property_identifier(self->prop));
>>>                return 0;
>>> @@ -201,7 +239,13 @@
>>>
>>>        if(self->prop==NULL)
>>>                return 0;
>>> -
>>> +
>>> +#ifdef USE_PEDANTIC_WRITE
>>> +       if(rna_disallow_writes && rna_id_write_error(&self->ptr, NULL)) {
>>> +               return 0;
>>> +       }
>>> +#endif
>>> +
>>>        if (!RNA_property_editable_flag(&self->ptr, self->prop)) {
>>>                PyErr_Format(PyExc_AttributeError, "bpy_prop
>>> \"%.200s.%.200s\" is read-only", RNA_struct_identifier(self->ptr.type),
>>> RNA_property_identifier(self->prop));
>>>                return 0;
>>> @@ -1903,6 +1947,12 @@
>>>  {
>>>        IDProperty *group= RNA_struct_idprops(&self->ptr, 1);
>>>
>>> +#ifdef USE_PEDANTIC_WRITE
>>> +       if(rna_disallow_writes && rna_id_write_error(&self->ptr, key)) {
>>> +               return -1;
>>> +       }
>>> +#endif
>>> +
>>>        if(group==NULL) {
>>>                PyErr_SetString(PyExc_TypeError, "bpy_struct[key] = val: id
>>> properties not supported for this type");
>>>                return -1;
>>> @@ -2727,6 +2777,12 @@
>>>        char *name = _PyUnicode_AsString(pyname);
>>>        PropertyRNA *prop= NULL;
>>>
>>> +#ifdef USE_PEDANTIC_WRITE
>>> +       if(rna_disallow_writes && rna_id_write_error(&self->ptr, pyname))
>>> {
>>> +               return -1;
>>> +       }
>>> +#endif
>>> +
>>>        if(name == NULL) {
>>>                PyErr_SetString(PyExc_AttributeError, "bpy_struct:
>>> __setattr__ must be a string");
>>>                return -1;
>>> @@ -2840,6 +2896,12 @@
>>>        PropertyRNA *prop;
>>>        PointerRNA r_ptr;
>>>
>>> +#ifdef USE_PEDANTIC_WRITE
>>> +       if(rna_disallow_writes && rna_id_write_error(&self->ptr, pyname))
>>> {
>>> +               return -1;
>>> +       }
>>> +#endif
>>> +
>>>        if(name == NULL) {
>>>                PyErr_SetString(PyExc_AttributeError, "bpy_prop:
>>> __setattr__ must be a string");
>>>                return -1;
>>> @@ -5080,7 +5142,11 @@
>>>        PyGILState_STATE gilstate;
>>>
>>>        bContext *C= BPy_GetContext(); // XXX - NEEDS FIXING, QUITE BAD.
>>> -
>>> +#ifdef USE_PEDANTIC_WRITE
>>> +       /* testing, for correctness, not operator and not draw function */
>>> +       const short is_readonly= strstr("draw",
>>> RNA_function_identifier(func)) || !RNA_struct_is_a(ptr->type,
>>> &RNA_Operator);
>>> +#endif
>>> +
>>>        py_class= RNA_struct_py_type_get(ptr->type);
>>>
>>>        /* rare case. can happen when registering subclasses */
>>> @@ -5177,8 +5243,19 @@
>>>                                i++;
>>>                        }
>>>
>>> +#ifdef USE_PEDANTIC_WRITE
>>> +                       rna_disallow_writes= is_readonly ? TRUE:FALSE;
>>> +#endif
>>> +                       /* *** Main Caller *** */
>>> +
>>>                        ret = PyObject_Call(item, args, NULL);
>>>
>>> +                       /* *** Done Calling *** */
>>> +
>>> +#ifdef USE_PEDANTIC_WRITE
>>> +                       rna_disallow_writes= FALSE;
>>> +#endif
>>> +
>>>                        RNA_parameter_list_end(&iter);
>>>                        Py_DECREF(item);
>>>                        Py_DECREF(args);
>>>
>>>
>>> _______________________________________________
>>> Bf-blender-cvs mailing list
>>> Bf-blender-cvs at blender.org
>>> http://lists.blender.org/mailman/listinfo/bf-blender-cvs
>>>
>>
>>
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
>



-- 
- Campbell


More information about the Bf-committers mailing list