[Kde-games-devel] KMahjongg frameworks branch

[Frederik Schwarzer]

Am Sonntag, 22. November 2015, 14:21:21 schrieb Ian Wadham:
> On 22/11/2015, at 6:52 AM, Frederik Schwarzer wrote:
> > Am Samstag, 21. November 2015, 09:01:59 schrieb Ian Wadham:

Hi Ian,

> First off, how can I browse the central KDE Games repositories these
> days?  I used to go to
> https://projects.kde.org/projects/kde/kdegames/kmahjongg/repository/
> but that is no longer kept up-to-date… :-(

Known problem for ... I think since April this year but no fix thus 
far. You can use https://quickgit.kde.org/ ... though a bit messy to 
browse some keywords due to all the private clones. For the games it's 
alright though. Like https://quickgit.kde.org/?p=kmahjongg.git

There is also the gitk application. With "gitk --all ." you cann see 
all branches of your local clone.

> I have found the problem, fixed it and tested the fix on my _local_
> qgraphic branch.  The problem was due to an endless recursion
> (or re-entry) in KMahjongg::loadSettings().  It was treating an
> empty setting for the layout-file path as a new layout, because the
> default setting was not yet saved, so it called startNewGame() for
> the "new layout" and it immediately called back to loadSettings()…
> 
> My patch is attached.  And here is my comment for the commit:
> 
>     Avoid an endless recursion when there is no saved layout
> setting.
> 
> I have not been able to push this commit to the central qgraphic
> branch because there is a clash and I cannot see what is going on
> (see my question at the start of this post).  The clash involves the
> files boardwidget.cpp and (indirectly) boardwidget.h.
> 
> Those files are supposed to have been deleted.  They are superseded
> by the new QGraphisView code, I seem to remember…
> 
> Anyway, here is the patch…

The code in question has changed a bit in frameworks branch so I had 
to adjust the patch.
I did not like the "if this, then nothing, else, something" approach 
before so I refactored-out the empty block. Now with your change, the 
condition has become a bit more complex than feels good for me. So 
please review and comment. It might be better (meaning: more readable) 
to have two nested if()-statements in this case.
https://quickgit.kde.org/?p=kmahjongg.git&a=commitdiff&h=ae317c58b677a7f5edb0dffb1e5082397563a120

Thank you for the patch. It works good here. :)

Regards,
Frederik