Fixes for KPat Forty & Eight patience game

[Albert Astals Cid]

El dimarts, 21 de juliol de 2020, a les 0:47:31 CEST, Ian Wadham va escriure:
> 
> > On 21 Jul 2020, at 7:48 am, Albert Astals Cid <aacid at kde.org> wrote:
> > 
> > El dilluns, 20 de juliol de 2020, a les 9:05:26 CEST, Ian Wadham va escriure:
> >> Hi Albert,
> >> 
> >>> On 18 Jul 2020, at 5:38 am, Albert Astals Cid <aacid at kde.org> wrote:
> >>> 
> >>> El divendres, 17 de juliol de 2020, a les 8:35:23 CEST, Ian Wadham va escriure:
> >>>> 
> >>>>> On 17 Jul 2020, at 6:38 am, Albert Astals Cid <aacid at kde.org> wrote:
> >>>>> 
> >>>>> El divendres, 10 de juliol de 2020, a les 7:38:40 CEST, Ian Wadham va escriure:
> >>>>>> Hi guys,
> >>>>>> 
> >>>>>> Good news. In brief, I have found two or three causes of the 10 bugs listed in
> >>>>>> https://bugs.kde.org/buglist.cgi?quicksearch=kpat%20forty%20eight.
> >>>>>> 
> >>>>>> However, I am going to need help from an expert with a knowledge of the KPat code to work out the most appropriate patches.
> >>>>>> 
> >> <snip>
> >>>>>> I will also need help to get patches committed, when they are ready, because I do not have access to or knowledge of the newer KDE Community repositories and procedures.
> >>>>>> 
> >>>>> 
> >>>>> Unfortunately I don't think we have many KPat experts around anymore, Shlomi and Fabian are the two that did changes to somewhat core parts lately.
> >>>> 
> >>>> I was hoping Parker Coates might be able to advise us. I did see a post on kde-games-devel from him not long ago.
> >>>> 
> >>>>> My suggestion would be for you to put the patch as a Merge Request in invent.kde.org
> >>>> 
> >>>> I have looked at that online, but I am really not up to it, Albert. The last review tool I had anything to do with was ReviewBoard and I have not committed or checked out anything for about five years. I have been keeping an eye on things using “anongit" and sometimes offering suggestions or a “diff -u” style patch (not a “git diff” style one). But as of now I cannot even figure out how to get a read-only git clone of KPat from invent.kde.org.
> >>> 
> >>> git clone https://invent.kde.org/games/kpat.git
> >> 
> >> Thanks, Albert, I was able to get a local git clone, as above.
> >> 
> >> As you may have seen, I have sorted out the Forty & Eight bugs on bugs.kde.org and eliminated some duplicates. I have also finalised my changes to KPat and am doing final testing on my Apple OSX KDE 4 version of KPat. One patch is tested and ready to present, but I am not able to test it on the KF5 version of KPat.
> >> 
> >> I have the change on a branch in my local clone, but have not committed it, nor written a commit message there.
> >> 
> >> Attached is a patch in “git diff” format to show you what I mean. It fixes the illegal move bug in kpat/patsolve/fortyeightsolver.cpp.
> >> 
> >> I have taken the liberty of adding some comments to explain what is going on in the code and make it less cryptic. Also I have re-written the conditional code for multi-card moves, to use multi-character identifiers and make it more readable. I could have fixed the whole thing with one line and no comments, but that is not how I like to work… :-)
> >> 
> >> So how would you like to proceed from here?
> >> 
> >> <snip>
> >>>>> and that way we all can have a look at it and maybe try to understand it together.
> >>>> 
> >>>> Indeed. I look forward to that.
> >> 
> >> <snip>
> >>>> I think the best I can do, over the next week or so, is first to tidy up the bug reports in:
> >>>> 
> >>>> https://bugs.kde.org/buglist.cgi?quicksearch=kpat%20forty%20eight,
> >>>> 
> >>>> flagging duplicate reports, collating test cases, etc. Then I can add my patches as attachments to whichever are the “main” reports (with appropriate comments). Then maybe links to that work in Bugzilla could help kick off any discussion on a merge request.
> >> 
> >> Close to finishing that.
> >> 
> >>>> What do you think, Albert?
> >>> 
> >>> I guess that could work, not optimal, but better than nothing :)
> >> 
> >> So how would you like to proceed from here, using the attached fix for https://bugs.kde.org/show_bug.cgi?id=302140 as a trial?
> > 
> > The patch seems reasonable (from the i've no idea what you're really talking about) scenario :D
> > 
> > Do you have a numbered hand in which to reproduce the issue?
> 
> Certainly. The best examples were submitted by Duncan Kinnear in comments https://bugs.kde.org/show_bug.cgi?id=290647#c7 through https://bugs.kde.org/show_bug.cgi?id=290647#c10. Comments 7 and 9 provide attached KPat save-files from which you can load a game and observe the bug. Comments 8 and 10 have attached screenshots of the same game-states with the invalid move highlighted by “Hint” action… but you cannot actually play a move where two cards are of the same suit and not in ascending sequence. Demo, Hint and solver play it happily and claim that the game is winnable. That’s the issue.
> 
> The saved game at https://bugs.kde.org/show_bug.cgi?id=290647#c9 is the better of the two examples. The invalid move is used by the solver to claim that the game is winnable. The 5 of Clubs is under the King of Clubs but the Demo plays both cards onto the 6 of Clubs. For that to be valid, the two cards to move would have to be 4 and 5 of Clubs. This example is particularly good because when the bug is fixed, the game becomes unwinnable, as seems to be the case however I try to win it.
> 
> There are other attached save-files in https://bugs.kde.org/show_bug.cgi?id=302140, which is the “main” report for this bug, but they are rather old and I have had no success at using them so far.

Hmm, question to try to understand the code a bit better.

When you say "starting with the top two cards"  you mean 8/K or J/10 in https://i.imgur.com/5paxqtH.png ? i guess you mean J/10 as in "cards in the top of the pile" and not as in "cards closer to the top part of the screen", right?

Cheers,
  Albert

> 
> Cheers,
> Ian W.
> 
>