Fixes for KPat Forty & Eight patience game

[Ian Wadham]

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?

Cheers,
Ian W.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-kpat-bug302140
Type: application/octet-stream
Size: 1187 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20200720/a269603f/attachment.obj>