Bug #908

Incorrect method CMusicPlayer::previous()

Added by lubos over 8 years ago. Updated about 8 years ago.

Status:Closed Start date:05/17/2010
Priority:Normal Due date:
Assignee:kervala % Done:

100%
Category:Client: General
Target version:Version 0.8.0

Description

Better implementation is:

if (_Songs.empty())
    return;

if (_CurrentSong == 0)
    _CurrentSong%=_Songs.size()-1;
else
    _CurrentSong--;

play ();

History

#1 Updated by kaetemi over 8 years ago

How is that better?

#2 Updated by lubos over 8 years ago

When is _CurrentSong 0 and is decremented, then plays random song.
(Little fix - _CurrentSong=_Songs.size()-1; )

#3 Updated by kaetemi over 8 years ago

Just wondering
why not

if (!_Songs.empty())
{
    if (_CurrentSong)
        --_CurrentSong;
    else
        _CurrentSong = _Songs.size()-1;

    play ();
}

=)

#4 Updated by kervala over 8 years ago

I tried (to be sure) and _CurrentSong-- when _CurrentSong == 0 gives _CurrentSong = 0xffffffff and _CurrentSong %= _Songs.size() gives _CurrentSong = _Songs.size() - 1

So excepted for an "better understanding" aspect, it's not a bug :)

EDIT: Btw I'm not sure this behavior is identical on all systems...

#5 Updated by kaetemi over 8 years ago

I quite like this one too:

--_CurrentSong;
_CurrentSong = CurrentSong & ((~CurrentSong) >> 16);

(ignore any typos)
Math is generally faster than branching ^^

#6 Updated by kervala over 8 years ago

  • Status changed from New to Assigned
  • Assignee set to kervala

Thanks for noticing this bug and for providing patch :)

#7 Updated by kervala over 8 years ago

  • Status changed from Assigned to Resolved
  • % Done changed from 0 to 100

Applied in changeset r173.

#8 Updated by vl about 8 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF