[Kde-games-devel] KMahjongg frameworks branch

[Ian Wadham]

Hi Frederik,

On 22/11/2015, at 11:40 PM, Frederik Schwarzer wrote:
> 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:
> 
>> 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.

I have had a good look at the frameworks branch's version of
KMahjongg::loadSettings() and it looks good to me.

I think, to keep matters simple, I will replace loadSettings() on the
qgraphic branch with the entire method from the frameworks branch
and commit it as a consolidated change.  Then I am sure our two
fragments of code for KMahjongg::loadSettings() can stay in step
as we continue testing.

I could cherry-pick changes from frameworks into qgraphic, but my git
skills are not good and I would be worried about making a mistake.

> 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

The code really is getting to be a rat's nest, what with all the negatives.
It might be cleaner to pre-define a boolean or two, with nice readable
names, then have simpler if() statements.  OTOH, "if it ain't broke don't fix it".

BTW, I tried choosing "random layout mode" in the Settings dialog.  It works
well, except for two things I did not like (as a user):

a) There is no way to know what layout you have been randomly given.

b) If you like one of the random layouts and then go back to non-random
     mode, what you get is a change of layout back to whatever was last
     saved in non-random mode, maybe hours or weeks before and maybe
     no longer of interest.

So there is no easy way to remember a random layout and use it again later.

Problem a) could be solved by putting the layout's name in KMahjongg's
title bar, which would be a user-friendly thing to do in any case (see KPat).

Cheers, Ian W.