Bug 5913 - Favorites start at #0
: Favorites start at #0
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Player UI
: 7.0
: PC Windows XP
: P2 trivial (vote)
: ---
Assigned To: Unassigned bug - please assign me!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-26 01:18 UTC by Philip Meyer
Modified: 2009-09-08 09:26 UTC (History)
3 users (show)

See Also:
Category: ---


Attachments
show zeroth fave (1.27 KB, patch)
2007-11-07 16:27 UTC, KDF
Details | Diff
display one-indexed, remap buttons (2.28 KB, patch)
2007-11-07 20:18 UTC, KDF
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Meyer 2007-10-26 01:18:27 UTC
It seems that favorites start at #0?  I had two favorites.

I added a playlist as a favorite, so this became favorite #2.
From the playerUI, this looks a little confusing in the favorites menu, as it shows "favorites 3 of 3", but when you go the favorite 3 in the list it says #2.

Would it not be a little less confusing to start numbering at 1?
Comment 1 Blackketter Dean 2007-10-26 13:58:56 UTC
Yes, it would be less confusing, it's just plain wrong.  On my 7.0 system, the favorite number isn't even displayed on the zeroth favorite.

Comment 2 KDF 2007-10-26 16:16:35 UTC
This will make it messy unless you simply remove '0' as an option (meaning one less button you can use for direct access)

If you still want it, then the order ends up as
1 - 2 - 3 - 4 - 5 - 6 - 7 - 8 - 9 - 0 - 10 - 11

or, you convince people that 0 loads favourite #10 (oddly just as confusing given the variety of users that show up)

having fave #0 show up is easy.  There is just an if(fave) where there should be an if(defined fave)
Comment 3 KDF 2007-11-07 16:27:48 UTC
Created attachment 2379 [details]
show zeroth fave

simple to show zeroth fave, corresponding to press-hold 0 key.
Comment 4 KDF 2007-11-07 16:29:40 UTC
other option would be to start at 1 for favorites index (it's in a few different places) and remap the buttons such that press-hold 0 loads fave #10. needs a ruling.
Comment 5 Blackketter Dean 2007-11-07 16:44:05 UTC
Yes.  Button zero should map to favorite 10.
Comment 6 KDF 2007-11-07 16:58:31 UTC
argh.  it seems 0 is already mapped to 10 button-wise, but the first favorite is stored as zero.  player UI for lists uses arrays all over the place. these are zero-indexed in perl.  it's going to get ugly shifting the index wherever favorites are concerned, yet allowing all other cases to remain unaffected.  

I wonder if if might be simpler to map button 1 to favorite 0, 2 to 1, etc and simply munge the display info so that favourite 0 is shown as #1 anywhere it may be displayed.

Comment 7 KDF 2007-11-07 20:18:48 UTC
Created attachment 2385 [details]
display one-indexed, remap buttons

This remaps, also splits the button actions so that we don't get numberscroll_0 AND playFavorite_0.
Comment 8 Adrian Smith 2007-11-10 09:04:57 UTC
I wrestled with this at the time.  The problem is that favorites reuses all the xmlbrowser code for buttons, web, cli and jive access so really does need to use zero based lists (which is what they do)

I think we should just number the player UI buttons from 0 and make 0 map to favorite 0.
Alternatively we add/delete one for player UI, but it must be done in the player UI code not anywhere esle in the favorites code...

[Kevin - I would expect side effects for all the web and jive code by chaning the index mappings...]
Comment 9 KDF 2007-11-10 10:59:53 UTC
absolutely.  My choice would be to 'show zeroth fave' and move on.  
Comment 10 Adrian Smith 2007-11-17 04:49:02 UTC
Do we have agreement on using the zero favorite and moving on?
Comment 11 KDF 2007-11-17 14:25:12 UTC
I'm in agreement, but clearly it needs more.
for now, I've committed a fix to show the zeroth favourite properly, change 14804
Comment 12 Blackketter Dean 2007-11-17 17:46:23 UTC
Is there a simple way to have the UI show the zeroth item as "#1" and map the 1 button to that item?

Zero-based numbering is good for computers and programmers, but confusing for everybody else.
Comment 13 Adrian Smith 2007-11-18 02:56:37 UTC
OK let me have another look at doing this just for the player UI.
Comment 14 Adrian Smith 2007-11-18 05:59:22 UTC
change 14809 should now display favorites starting from 1

Note I've needed to add a new string for the case when a url is included within a menu level of the favorites system.  This used to display x.y.z which does not look good, so I have hidden this, but it uses a new string which needs localising.  String is: FAVORITES_FAVORITE
Comment 15 Andy Grundman 2007-11-21 10:14:45 UTC
Marking fixed, please test and reopen if you still see this problem.
Comment 16 Philip Meyer 2007-11-21 14:48:47 UTC
When I add a favorite via the SoftSqueeze player UI (eg. add a playlist to the favorites), I see unexpanded strings "{FAVORITES_FAVORITE_NUM}4{FAVORITES_RIGHT_TO_DELETE}".
Comment 17 KDF 2007-11-21 15:13:38 UTC
Have you done a full update from svn?  there was a point where some strings files were broken after a translations updates (was in the last day or two).  Otherwise, it is probably the strings.bin which caches the current language.  

current svn does not show this problem here.
Comment 18 Philip Meyer 2007-11-22 01:20:02 UTC
Stopped SC, SVN'ed up, cleared my SC cache again, and restarted SC.  I still see the missing strings (this is using SoftSqueeze 3.5).
Comment 19 Michael Herger 2007-11-22 01:31:17 UTC
Do you see error messages concerning the strings in your log file?
Comment 20 KDF 2007-11-22 01:33:56 UTC
Start looking at the logs, especially for missing string dumps. I can confirm that there is no problem with current svn, linux or xp, sb2, sb3, transporter or softsqueeze 3.5.  

try svn status, see if you have any conflicts or unmatched files with svn.  you may need to svn revert, especially if you have applied any patches, etc.
Comment 21 Philip Meyer 2007-11-22 03:00:06 UTC
All of my SVN files are latest version (nothing patched that requires reverting), I have checked.

I have checked the logs - can't see any warnings/errors about missing strings.

The only thing I see when I add a playlist as a favorite in the log is:
[10:51:01.8615] Slim::Utils::Misc::msg (1214) Warning: [10:51:01.8611] tempfile(): temporary filename requested but not opened.
Possibly unsafe, consider using tempfile() with OPEN set to true

When I right-arrow to add a playlist as a favorite I see the "Saving favorites... <name>" and then "{FAVORITES_FAVORITE_NUM}4{FAVORITES_RIGHT_TO_DELETE}".  If I go back, there is still an option to add to favorites.  If I right-arrow to add the playlist as a favorite again, I see the "Saving favorites... <name>" but not the following message (I assume this is perhaps intended, as it finds that the favorite already exists).

If I then go to the Favorites, I can see my favorite has been added, and I get the correct string expansions for deleting the favorite.
Comment 22 KDF 2007-11-22 08:48:42 UTC
ok, now I see it.  seems to be playlists ONLY.
in some cases, nothing is added but when it is, the strings aren't right.
always helps to be very specific about the steps to make it happen :)
Comment 23 Philip Meyer 2007-11-22 08:59:46 UTC
I was specific in comment #16, but admittedly, not when I tried it again in comment #18 :-)
Comment 24 KDF 2007-11-22 09:07:43 UTC
sorry...a one liner containing the word "playlist" is not what I mean.
be specific about exact steps.
1) Browse playlist
2) right to track list
3) up to favourites
4) right to add
5) bad string.

not to mention, it as nothing to do with favorites starting at 0.
however, should be fixed shortly. thanks.
Comment 25 KDF 2007-11-22 09:20:12 UTC
ok, first part: fixing strings done in change 14942.
remaining problem is that some playlists are not being found as existing favorites.
Comment 26 KDF 2007-11-22 09:42:34 UTC
and change 14943 should now find the existing faves. marking as fixed again, but of course reopen if there is anything else.
Comment 27 Chris Owens 2008-03-07 09:05:14 UTC
This bug is being closed since it was resolved for a version which is now released!  Please download the new version of SqueezeCenter (formerly SlimServer) at http://www.slimdevices.com/su_downloads.html

If you are still seeing this bug, please re-open it and we will consider it for a future release.