[Kde-games-devel] KPat/Freecell broken?

[Parker Coates]

On Mon, Dec 14, 2009 at 12:35, Julian Helfferich wrote:
> 2009/12/14 Parker Coates
>> On Sun, Dec 13, 2009 at 16:56, Andreas Pakulat wrote:
>>> Unfortunately that doesn't help. Freecell still stops doing card-moves
>>> once one stack of cards has been moved.
>>>
>>> A grep for similar disconnects yielded one more result in dealer.cpp,
>>> but also adjusting that one didn't help unfortunately (line 741)
>>>
>>> BTW: There's a disconnect() call in dealer.cpp, unless you need to
>>> disconnect exactly at that point its not overly useful as ~QObject will
>>> disconnect everything from the deleted object too.
>>
>> Crap. Well thanks for trying it out. Back to the drawing board.
>
> i hope i've found the cause for the trouble in freecell. This fix worked for
> me:

Thank you very much, Julian!

> freecell.cpp : Line 137 (in: Freecell::moveCards(CardList &c, FreecellPile
> *from, Pile *to) )
> -- waitfor = c.first();
> ++ waitfor = c.last();
>
> Explanation:
> When a card is clicked and dragged ( cardscene.cpp : mousePressEvent ) the
> program checks from the bottom of the pile to the top and adds the card that
> has been clicked and all above. So the card on the top of the pile is
> c.last(), not c.first()

Ah. That makes perfect sense. For 4.4 KPat is much smarter at
detecting when an animation is in progress, so lots of actions are
prevented until the animation is complete to keep things nicely
sequenced. Freecell was cheating and moving on as soon as the first
card finished animating, but checks recently added elsewhere must have
been preventing it. Waiting for the last card makes sense.

Fortunately there's an even cleaner solution. CardDeck now has a
cardAnimationsDone() signal, so as soon as I get a chance, I'll modify
this logic to make use of that instead of manually connecting to the
animationStopped() signal of individual cards.

> My Freecell worked after this change, i hope it will fix it...

Do you have commit access? If so, please commit. If not, could someone
else please do so?

> I also checked Spider, but there is no CardList.first() that could be
> changed into .last() but i'll have a deeper look to it later.

Thanks for your help with this. Are you a KDE developer, or just a
concerned user? The KPat code base, isn't the friendliest around
(although I am working on slowly improving that. :) ) I'm impressed
you were able to figure it out.

Parker