On Thu, Sep 8, 2011 at 8:49 AM, Thomas Dinges <[email protected]> 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 > [email protected] > http://lists.blender.org/mailman/listinfo/bf-committers > -- - Campbell _______________________________________________ Bf-committers mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-committers
