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

[Andreas Pakulat]

On 14.12.09 12:50:24, Parker Coates wrote:
> 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!

Yeah, thanks a lot.
 
> > 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.

That might actually solve the spider problem. I've tried some debugging
and for some (unfortunately unknown reason) the bookkeeping of
carddeck.cpp m_cardsWaitedFor gets out of sync with reality. Thats why
mousePressEvent then ignores all subsequent mouse events, the
m_cardsWaitedFor is not 0 once the run is moved to the bottom-left
stack.

I also saw that this only happens when you manually complete a run, if
you complete a run by fetching another round of 10 cards from the
right-hand stack and one of the cards completes a run this doesn't
interrupt the flow!

The following is my current debug output, the "card started/stopped
animation" is printed from CardDeck::cardStartedAnimation and
CardDeck::cardStoppedAnimation. The numbers are the "current" and the
"new" value of the counter variable m_cardsWaitedFor:

SpiderPile::moveCards                                                   
card started animation 0 1                                              
card started animation 1 2                                              
card started animation 2 3                                              
card started animation 3 4                                              
card started animation 4 5                                              
card started animation 5 6                                              
card started animation 6 7                                              
card started animation 7 8                                              
card started animation 8 9                                              
card started animation 9 10                                             
card started animation 10 11
card started animation 11 12
card started animation 12 13
card started animation 13 14
card started animation 14 15
card started animation 15 16
Spider::checkPileDeck()
Spider::getRun()
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card stopped animation 16 15
card started animation 15 16
card started animation 16 17
card started animation 17 18
card stopped animation 18 17
card stopped animation 17 16
card stopped animation 16 15
card stopped animation 15 14
card stopped animation 14 13
card stopped animation 13 12
card stopped animation 12 11
card stopped animation 11 10
card stopped animation 10 9
card stopped animation 9 8
card stopped animation 8 7
card stopped animation 7 6
card stopped animation 6 5
card stopped animation 5 4
card stopped animation 4 3
card stopped animation 3 2
Spider::cardStopped()
card started animation 2 3
card started animation 3 4
card stopped animation 4 3
card stopped animation 3 2

I'll see if I can dig deeper...

> > 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'll commit tomorrow unless Julian is faster.
 
Andreas

-- 
A tall, dark stranger will have more fun than you.