[Bf-committers] Weight Modifiers - Issues

[Campbell Barton]

On Thu, Sep 8, 2011 at 8:49 AM, Thomas Dinges <blender at dingto.org> wrote:
> Hello,
> who "reviewed" the Vertex Weight modifer(s)?

me :)

> IMHO this is totally rushed in and this is not what should happen. We
> agreed only on merge projects which are reviewed well, but on a first
> glance I see some problems with this.
>
> 1) Why 3 Modifiers?
> I'd rather see 1 with different modes, imo.
> This clutters the Modifier menu and really could be avoided. 1 modifier
> with different modes are much better (design wise).

Originally they were one modifier and I requested they be moved into
3, this was because the interface had a lot of buttons and it was hard
to which button effected which functionality - applying a curve on
existing weights is very different to weighting based on the vertex
distance to another object for example, again mixing weights with
another group is different.

> 2) The User Interface Code is not good, lots of columns instead of rows
> in the code. Why don't you look at the other Modifers in the python file
> and take them as an example. And we don't use assignment names such as
> col1 or col2. Maybe in the User Preferences (but this is a special case
> there).
> A column is used for elements underneath each other, not next to each
> other in a row. So don't use split.column if label or prop are in 1 row
> and belonging to each other. :) (weight_vg_mask function)

Not defending bad interfaces but the interface is really easy code to
refactor/rewrite compared to the rest of the patch, for example - if
you were to make a mockup of an improved interface I'm sure it could
be done by Bastian or myself before release.

> 3) UI Design misuse:
> What is that? Boolean check boxes concealed as Radio Buttons?
> <http://www.dict.cc/englisch-deutsch/concealed.html>http://www.pasteall.org/pic/17617
> This is wrapped as an enum, but I can select multiple ones, so either
> this is wrong or they should be wrapped as booleans.
> Anyway this does not fit in the UI design.

No: in fact it is the UI which is at fault (if you consider this wrong
in the first place) - blender supports enums where multiple options
can be selected, this is correct use of the RNA data type - so if we
are to fix anything its the drawing/UI.

IMHO there are bigger UI issues which this is apart where its hard to
tell if buttons are pressed, data vs operator buttons too.

> I am sorry, this may sound a bit harsh and I don't want to offend
> anyone, but this is exactly what some people feared talking about the
> new release cycle, that not well tested/reviewed code gets into trunk .

When reviewing I was mostly checking for bugs, consistency with other
modifiers and usability - (testing the modifiers worked as I expected
& in a useful way).

IMHO it fine to do some UI cleanup once in trunk.

> Please fix the issues.

Some of these issues a are not just things we can `fix` - UI buttons
need some thought if they are to be changed.

>From the 3 points you make, only #2 is a straightforward fix, Bastien
are you able to look into this?


> Best regards,
> Thomas
>
> --
> Thomas Dinges
> Blender Developer, Artist and Musician
>
> www.dingto.org
>
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
>



-- 
- Campbell