KasunWijesekara / cuberok

Automatically exported from code.google.com/p/cuberok
GNU General Public License v3.0
0 stars 0 forks source link

Patch for fixing play and pause button behavior #52

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Hi, I have a patch attached to fix a UI problem that has annoyed me for
some time.  In the current GUI, Cuberok has a separate "play" and "pause"
button, and when the "pause" button is set, it never gets updated when
other controls are used to start the player.  This means that the pause
button must be clicked twice in order to actually pause play. 

Example:
  1. Start playing a song
  2. Press Pause
  3. Resume playing in a manner other than pressing pause a second time
(like pressing play, pressing next/previous, or double-clicking a song in
the playlist)
  4. Now the pause button must be pushed twice to re-pause.

I dove into the code and came up with a fix that is a little intrusive, but
seems to work OK.  I merged the play and pause buttons into one single
button that changes icon based on the state of the player.  Now, when the
player is active, you see a "pause" icon, indicating that pressing the
button will cause a pause.  Also, whenever the player is stopped (either by
pressing pause OR by the "stop" button) the "play" button shows, indicating
that the player may resume.  

I have signals sent from other buttons and the list view so that the icon
on the new combined play/pause button will always update and correct itself
whenever the player is stopped or started.  As a bonus, there is now one
less button on the toolbar, freeing up space while also not losing any
functionality.

I have my patch attached, it should apply cleanly to the cuberok 0.10
codebase currently available on the website.

place the patch in cuberok-0.0.10/src and run patch -p0 < playbutton.patch

Tell me if you think you would like to incorporate this into the next release.

Original issue reported on code.google.com by cfo...@gmail.com on 17 Jun 2009 at 11:51

Attachments:

GoogleCodeExporter commented 8 years ago
I like it. But it has some troubles with indicating "pause" state.

Currently when player playing the PlayPause indicated as "pressed" and when 
player 
paused it is not indicated. So I can't see differences between "paused" and 
"stopped" states. It would be better to make like this:
1. when stopped, button is released and "play" icon on it is displayed
2. when playing, button is released and "pause" icon on it is displayed
3. when paused, button is pressed and "pause" icon on it is displayed

Also after cuberok start and if it does not play, this button has icon "pause", 
but 
it should has icon "play".

At last, after cuberok start and if it resumes play track from last session, 
this 
button is not pressed (but it has icon "pause" that is correct)

Also, this patch caused random troubles with restoring playing state on start. 
Sometimes cuberok resumes playing with silence instead of music.

P.S. I had to adapt this patch for r219

Original comment by nomen.in...@gmail.com on 18 Jun 2009 at 7:37

GoogleCodeExporter commented 8 years ago
Also when repeat is off and player finish play last track in playlist, button 
stays 
pressed just like playing is continue.

Original comment by nomen.in...@gmail.com on 18 Jun 2009 at 9:18

GoogleCodeExporter commented 8 years ago
Ok, I've done some fixes & cleanups:
   1. The button behaves as you recommended, no more "pressed" buttons, the button is
always in the "released" state.  This also simplified some unnecessary logic I 
had
dealt with earlier, and definitely looks nicer.
   2. By being careful about when & where I connect my signals & slots, the button is
now accurate when Cuberok starts, whether it is playing a track or not.  Also, 
the
button will return to "Play" when a non-repeating playlist finishes.
   3. Used more logical signals & slots with a boolean to indicate which icon to display.
   4. The tooltip and menu text now change along with the icon.
   5. This patch is now against the R219 version.

Original comment by cfo...@gmail.com on 19 Jun 2009 at 1:02

Attachments:

GoogleCodeExporter commented 8 years ago
I like it.

I can see the only problem on last track in playlist, but it seems to me that 
this 
problem comes not from this patch. The problem is next: when last track in 
playlist 
finished, triggering playbutton have no useful effect, it just changing icon on 
the 
button.

So, I think DrMoriarty may approve this patch. It applys cleanly against r221.

Original comment by nomen.in...@gmail.com on 19 Jun 2009 at 5:39

GoogleCodeExporter commented 8 years ago
Oh, no, things are not so good.

When player just works and it starts playing next track icon ot this button 
changes 
to "play", like track is paused.

Original comment by nomen.in...@gmail.com on 19 Jun 2009 at 7:05

GoogleCodeExporter commented 8 years ago
This issue was closed by r223.

Original comment by drmoriar...@gmail.com on 19 Jun 2009 at 7:10

GoogleCodeExporter commented 8 years ago
new bug was fixed by r224

Original comment by drmoriar...@gmail.com on 19 Jun 2009 at 7:20

GoogleCodeExporter commented 8 years ago
Fixed, I prove it. I guess, issue with last track in playlist will be 
considered 
while implementing new shuffle and repeat modes, won't it?

Original comment by nomen.in...@gmail.com on 19 Jun 2009 at 8:53

GoogleCodeExporter commented 8 years ago
You are right :-)

Original comment by drmoriar...@gmail.com on 19 Jun 2009 at 8:56

GoogleCodeExporter commented 8 years ago
Thanks for cleaning up my bug and accepting the patch!

Original comment by cfo...@gmail.com on 19 Jun 2009 at 10:25

GoogleCodeExporter commented 8 years ago
> Thanks for cleaning up my bug and accepting the patch!

Thank You for patch :-)

Original comment by nomen.in...@gmail.com on 20 Jun 2009 at 6:44