Hi Thomas, On Thu Sep 8 01:18:22 CEST 2011, Campbell Barton <[email protected]> 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. _______________________________________________ Bf-committers mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-committers
