Colored Stars work, findings, comments, discussion, etc.

[Jeff Mitchell]

(yes, this is a long email with a lot of info...welcome to amarok-devel, where 
real discussions can happen... :-)  )

Hey guys--

There's been a lot of confusion, at least on my end, about the status of the 
colored stars.  Based on the previous discussion on the (normal) mailing 
list, the consensus among both users and devs generally seemed to be that 
they liked it, if the problems with it were fixed.  Specifically, the 
problems were the following:

1)  The colored stars were not consistent across all places that showed the 
ratings stars.

2)  The predefined colors clashed with some color schemes.

3)  People who didn't like it couldn't turn it off.

I've been told that there was an IRC discussion regarding the feature the 
other day, in which it was generally agreed that it's a bad idea, and in 
which the opinion of at least one other dev seemed to be in contention with 
their earlier statements on the mailing list.

I'd like to first bring up a point:  when a particular person (and I'm not 
just talking me here) is working on a feature, if people wish to discuss its 
future it makes a whole lot of sense for that person to be involved in the 
conversation (i.e. active and present in IRC, or on the mailing lists , 
specifically this one), to provide details on the goal of the feature, its 
development requirements, etc.  This ensures that there is no 
misunderstanding on either side: for the person implementing the feature, the 
wishes of the majority of devs, and for the other devs, misconceptions and 
fallacies about the feature being implemented.  Otherwise it wastes 
everyone's time, because discussions have to be had twice, the person may 
work on code not knowing that the majority of devs don't want it or want it 
done another way, etc.

A few things off the bat...

First:  why is this in stable?  Because it's a continuation of a feature that 
was already there, and when it was taken out, it was promised to users that 
the problems would be fixed and it'd be trialled out for 1.4.whatever.  If 
the overall consensus is that people end up liking it, we'll keep it for 1.4 
and I'll port over to trunk.

Second:  while working on this I've fixed several bugs in the stable branch.  
Specifically, these relate to ratings not being updated everywhere when 
changed (they're now updated instantly in OSD, Context Browser, and Flat Mode 
collection view), as well as inconsistent usage of the values 1-10 and 1-5 
(as opposed to 0.5-5) for the ratings.  As a result of this last change, a 
song can now have a half-star rating, if it's *really* bad.  Some might think 
this doesn't make much sense, but the crucial thing is that we were mapping 9 
values onto a 10-value scale.  So if users manually modified the tag and set 
it to 1, it'd show up as a whole 1 star anyways -- exactly as if they had set 
it to 2.  But every other value from 3 to 10 resulted in its own star-value.  
This made even less sense, so I fixed this.  These bug fixes will be ported 
to trunk in time.

Now to the meat.

AFAIK, the major issue, more than the actual colors used, was the color 
consistency.  Stars in some parts of Amarok remained yellow, while in the 
Playlist they were colored.  This has been fixed, in a way that is fairly 
cheap, in the StarManager class.  In fact, whereas before some components 
would continually re-read the from the source files and convert them to 
pixmaps, this is now done once, allocated on the stack, and pointers to these 
single instances of the stars are passed around.  Even if the colors go it 
may be worth keeping some part of this paradigm to make things cheaper than 
they were before.

The next major issues were both addressed the same way (this part is not 
complete yet in the source code).  Adding two sub-options to the Use Ratings 
checkbox allows users to either turn off the colors and use the defaults, or 
choose their own.  As I mentioned earlier on the mailing list I have what I 
feel is a fairly elegant way of doing this.  Although I'm having issues 
translating Qt Designer to actual running Amarok, this mockup from Designer 
shows what it would look like:

http://img340.imageshack.us/img340/1282/qtdesignerdr3.png

The "Use custom colors" as well as the pushbuttons are only enabled when "Use 
ratings" is checked (and the pushbuttons only enabled if "Use custom colors" 
is checked too).  Each of the groups of buttons will each have a pixmap of 
the star (where the Qt logo is now), colored with the currently chosen color 
for that rating (the second single button is for the half-star).  Clicking on 
any of the buttons in a group will bring up a color chooser for that rating.  
Chosen colors will be reflected immediately in the dialog.

As you can see, a system like this would allow the flexibility that users and 
devs previously expressed wanting in a colored stars feature, so that it can 
be adjusted to work with any color scheme.  The drawback, which some devs 
always have an issue with, is that it requires more configuration options.  I 
think that should we wish to allow these options that this is a fairly 
elegant way of doing it.  The question then becomes whether we should allow 
these.

In 1.4.5 Mark added a feature to make new playlist items colorized.  However, 
because of similar problems -- that is, issues with the colors clashing with 
some color schemes -- an option was added to allow the user to change the 
color.  I didn't hear (on the list) any complaining about this, but I 
wouldn't have expected it, because it makes a lot of sense.

I don't see why this is any different -- or rather, I think that any option 
that involves color will inevitably require a way to change what colors are 
used because of wildly varying color schemes by different users.  I think 
that having the varied stars is a good idea; many of you expressed the same; 
many users did too (I don't actually remember hearing any opinions that the 
idea itself was bad, only that the previous implementation had issues).  So 
it makes sense to me that it would require a way to define the colors, the 
same as for new playlist items.  When you look at it this way, the only 
real "option" being added is the ability to turn the colored stars on and 
off.

I look forward to comments, questions, suggestions, etc.

--Jeff