Bug 129076

Created attachment 16580 [details]
add 2 popup menu to select Max rate

i have already posted this patch in ktorrent forum

Patch seems OK, but I cannot set upload limit lower than 5KB/s. Maybe you should change that.

And could you create patch with

cd your_local_ktorrent_copy/
svn diff > patchname.patch

since it's much easier to apply it. This way I had to manually enter filenames...

BTW, thanks for the patch ;)

Anything below 5 KB/s probably won't make much of a dent in most BitTorrent user's connections these days, so I wouldn't go too nuts with the resolution to keep the menu managable.

In Azureus, the selection seems to algorithmically offer a certain number of choices situated around the limits set in the main preferences dialog.

Created attachment 16581 [details]
version 2

now should work with value <5 too

The patch has a few problems, IMHO:

1. It doesn't communicate the active speed limits. They should be highlighted with a small bullet next to them or a bold type face.

2. The steppings are a bit weird. When I set upload to 40, it offers "40, 45, 48, 49, 50, ..." for example. 48 and 49 are overdoing it - "40, 45, 50" would be quite enough.

3. The "max rate in kb/s" sub-popup heading isn't particularly elegant English. I'd opt for "Upload in kb/s" and "Download in kb/s" or, if it's supposed to be generic, "Speed limit in kb/s".

Otherwise, definitely a great addition to the systray's usefulness.

>1. It doesn't communicate the active speed limits. They should be highlighted with a small bullet next to them or a bold type face.
 
I agree with this.

>2. The steppings are a bit weird. When I set upload to 40, it offers "40, 45, 48, 49, 50, ..." for example. 48 and 49 are overdoing it - "40, 45, 50" would be quite enough.

I'm not quite sure what algorithm he used, but eventually you could always select desired value by choosing the nearest one and then redoing the whole process. I never had to do it more than twice myself.

>3. The "max rate in kb/s" sub-popup heading isn't particularly elegant English. I'd opt for "Upload in kb/s" and "Download in kb/s" or, if it's supposed to be generic, "Speed limit in kb/s". 

I was about to change this with my commit (which I haven't done yet) along with couple of other minor fixes.

So khrothgar, will you make the modifications with your code or should I commit this patch first?

> I'm not quite sure what algorithm he used, but eventually you could always select desired value by choosing the nearest one and then redoing the whole process. I never had to do it more than twice myself. 

You misunderstood my point there. IMHO, the purpose of accessing those settings from the systray icon's popup menu is speed, and speedy navigation of an UI is accomplished by simplicity. Hence, 5 KB/s increments are enough especially around a set value like 40 KB/s - and odd numbers like "48" and "49" are particularly awkward. It looks like a bug in the algorithm.

As for your comment, someone who's willing to use the systray icon facilities twice to set a very specific value can just as easily click "Configure" in the same menu.

Well, you're right I admit.

Created attachment 16596 [details]
add 2 popup menu to select Max rate

fixes to problems signaled by Eike Hein

Sorry for my poor english.

The steppings and wording look good now, my only remaining nitpick would be the "->" used to highlight the currently set speed limits. That's pretty non-standard as far as KDE UI designs go. Maybe the ToggleAcction design use by Konqueror, Kate, Konversation & Co to select the document/channel encoding in a popup menu could be used. I'll attach a shot. 

Created attachment 16597 [details]
Active encoding highlight in Kate's encoding selection

Created attachment 16598 [details]
add 2 popup menu to select Max rate

now have checkable item
thanks for the suggestion

Looks great now. Thanks for your effort!

Oops, yet another nitpick I'm afraid: The algorithm doesn't cope very well when a limit is set to "Unlimited".

When I have upload on 40, it offers reasonable choices below and above 40. When it's set to "Unlimited", however, it offers 350-650, which is way outside the range that is useful to me. "Unlimited" should probably be treated as a special case that offers some smaller choices as well.

if it will offer 10-200 is ok?
same matter for download or it's ok with 350-650 range?

Hmm, tough choice. 

Over here in Germany, most broadband connections are asynchronous DSL, with 2 or 6 Mbit/s download capacity, but significantly less upload capacity. For example, I can do up to 70 KB/s up and 700 KB/s down. 

My upload limit us usually set to 40 KB/s. When I'm not relying on speedy web access, I sometimes set it to 50, or even unlimited over night, when I'm seeding anything. Occassionally I will also lower it to 20 or 25, when I have another P2P app running in parallel. My download speed is usually set to unlimited.

10-200 in the upload selection and 350-650 in the download selection would be ideal for me, but I have no idea what's common in KTorrent's other "markets".

Azureus appears to offer 5-200 (up) and 75-475 (down) in the "Unlimited" case, which would be similar. Assuming they've gone through a couple of iterations on their menu as well, 10-200 up would probably work out fine, but the download selection may have to offer some lower choices as well.


BTW, another suggestion: When one of the limits is set to an odd number like, say "37", the menu will offer rates like "42, 47 ...". Maybe it would be nicer to round any odd number to the next nicer one, i.e. 37 to 35, internally. So, 37 would still be shown as currently set, but it would offer 40, 45 etc. above it. 

I have a problem. when i set max speed from systray and close ktorrent change doesn't persist.
seems that Settings::setMaxUploadRate(v) set only a variable in Settings class without apply it to the system.

Created attachment 16602 [details]
add 2 popup menu to select Max rate

Hm, small problem with the rounding logic, IMHO: If it's set to "39", it should probably internally round to 40 and replace that in the list, not 35.

(Basically, 36-37 -> 35; 38-39 -> 40.)

Created attachment 16620 [details]
add 2 popup menu to select Max rate

i have added Settings::writeConfig() in SetMaxRate::rateSelected so if you
close ktorrent before open Configure the change persist

Created attachment 16621 [details]
add 2 popup menu to select Max rate

sorry previuos is wrong

The limits are now correctly saved across sessions. It still rounds wrong, though.

now if i set max speed manually through configure to "39" why round to 40 in systray popup. i haven't understood the problem

The goal of my suggestion is visual polish :). 

I initally remarked that when you happen to have set your upload rate to 37 (perhaps deliberately, or by clicking in the Configure dialog a few times too often unintentionally), the popup menu would display suggestions like "42" and "47" around it. For various reasons, the human mind works better with even steppings of ten or its half, 5 - i.e., 40 and 45 are nicer to work with in a popup menu.

So, when a value like '37' is selected, the code should internally round to the nearest 'nice' number and replace it in the list. For 37, that's 35. So you'd get a list like:

25
30
[37]
40
45

That's what you've got working right now, which is great. The problem, then, is that you currently always round down, never up, even when the number suggests otherwise. I.e. currently, you run into this:

30
[39]
40

... when it should be:

35
[39]
45

39 should replade 40, not 35, because 39 is nearer to 40 than 35. 

I know, I'm a pain in the ass - but that's actually the last nitpick I have, I promise ;).

Created attachment 16624 [details]
add 2 popup menu to select Max rate

Great work :). I think from a user experience point of view the patch is really solid now. Thank you for putting up with my suggestions.

Ivan, what do you think? Ready to commit?

thank you for the suggestion

I think we can commit, allthough should we not make the menu a bit smaller ? How does this look in a 800x600 resolution ? I think it will probably be to big to fit fully on the screen.

Created attachment 16627 [details]
KTorrent upload speed limit popup menu on 800x600

Created attachment 16628 [details]
KTorrent download speed limit popup menu on 800x600

Should fit even with fairly large font sizes, I think.

(Notably, KTorrent itself doesn't fit on a 800x600 screen. It's impossible to "Maximize" it as it doesn't fit maximized to 800x600.)

Yeah, I know we plan on fixing this, but we have to be sure that any further additions will fit in 800x600, otherwise we would have to fix that to.

we can reduce the number of choice however if it will be too big create it self a new column

Believe me, I know the problem ;). Over in the Konversation camp, the main culprit is the prefs dialog, which is quite telling.

Anyhow, I think the menu as it exists now should just about fit even with 12pt fonts - and it's not like it breaks when it doesn't fit; the menu widget goes into multi-column mode in that event. The large encoding selections mentioned above exhibit that trait on many screens, for example.

Created attachment 16629 [details]
menu splitted

SVN commit 551869 by ivasic:

ADD: Added popup menu to system tray to quickly select transfer rate limits. Big thanks to Khrothgar for providing the patch and Eike Hein for suggestions.

BUG: 129076

 M  +9 -0      ktorrent.cpp  
 M  +3 -0      ktorrent.h  
 M  +10 -0     ktorrentcore.cpp  
 M  +3 -1      ktorrentcore.h  
 M  +110 -18   trayicon.cpp  
 M  +38 -17    trayicon.h  

Great stuff; thanks to all involved.