[Bf-committers] Freestyle branch status report May 2012

[Tamito KAJIYAMA]

Dan and Campbell,

Thanks for your review of the Python part.  I am going to address all the
suggested changes and improvements.

The current naming convention of the Freestyle Python API is largely historical
and based on the underlying C++ class hierarchy.  Previously the Python API was
auto-generated by SWIG.  Then we rewrote the Python API by manual coding in
order to get rid of the C++ syntax from the Python API.

For what concerns an automatic generation of the Python API from the C++ code,
there is an application requirement.  The Freestyle Python API is a wrapper of the
underlying C++ class system, and the Python and C++ layers have been connected
by a specific mechanism that allows you to define a Python subclass of a C++ class.
The present API implementation heavily relies on this mechanism, and indeed
it works very well.  Implementing a similar (automated) functionality in makerna
would be a major task.

-- 
KAJIYAMA, Tamito <rd6t-kjym at asahi-net.or.jp>


-----Original Message----- 
From: Campbell Barton 
Sent: Tuesday, May 22, 2012 6:57 AM 
To: bf-blender developers 
Subject: Re: [Bf-committers] Freestyle branch status report May 2012 

On Tue, May 22, 2012 at 4:05 AM, Dan Eicher <dan at trollwerks.org> wrote:
> Quickly looking over the python part of the patch, there's lots of
> places where you should be using PyGetSetDef for attributes instead of
> function getters/setters -- for eg StrokeShader.name instead of
> StrokeShader.getName().
>
> Don't know if makesrna can do C++ but that would make it a lot easier
> than maintaining a bunch of hand written python bindings. Probably
> really depends on if the generated code is compiled by C++ or not
> because the access functions would be a breeze.
>
> Dan

Yep, if you can use PyGetSetDef this would make a lot more sense for
BPy_FrsMaterial_methods for eg,
Now mathutils defines mathutils.Color too which could be used instead
of r/g/b accessors.

if the data isnt in DNA then wrapping like this is needed but think it
could be improved.

Would be better to do full codereview but noticed some other things.

- suffers from api clogging we had in 2.4x - eg:
-- StrokeVertex_setX
-- StrokeVertex_setY
-- StrokeVertex_setPoint(x, y)
-- StrokeVertex_x
-- StrokeVertex_y
-- StrokeVertex_getPoint
... all have docstrings too - 12 definitions which could be replaced
with one attribute ".point" ? and one docstring

- PyTuple_SET_ITEM can be used when you create the tuple.
- keywords are ignored in some initializers.
- api doesnt use pep8/blender api naming conventions (lowercase +
underscores for methods and attributes, CamelCase for classes)
- scripts use import * - bad practice.
- defines "__getitem__" method rather then implementing Mapping or
Sequence protocols - PyMappingMethods / PySequenceMethods.
- bool_from_PyBool accepts any object type and doesn't typecheck -
could lead to confusing errors if "foo.bar = None" is equivalent to
"foo.bar = False" and "foo.bar = 2.0" is equivalent to "foo.bar =
True" rather then raising an error.