[Bf-committers] Freestyle branch status report May 2012

[Campbell Barton]

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.