[Bf-committers] Weight Modifiers - Issues

[Bastien Montagne]

Hi Thomas,

On Thu Sep 8 01:18:22 CEST 2011, Campbell Barton <ideasman42 at gmail.com> 
wrote:
>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.

Sure! I already made some edits to what you pointed out, let me know if
there are others problems in the UI code! :)

>> 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’ll just add that I highly prefer current UI, compared to the one we had
when only using checkboxes…

>> 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?

Done (I hope :P ).

>> 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

Regards,
Bastien.