[Kde-games-devel] Review Request 125855: Fix bug #353845 save and load game number properly

[Frederik Schwarzer]

> On Oct. 29, 2015, 1:59 a.m., Frederik Schwarzer wrote:
> > Please take my comments as courious questions or suggestions and not as demands to fulfill for a "ship it". :)
> 
> Dustin Steinack wrote:
>     Thank you very much for your review. :) I adapt the patch according to your suggestions. Except the type of the gameNum variable which I'm not sure about. It shouldn't run into any problems.

The gameNum variable is of type long and you convert it to qint64 in order to make is serializable for saving. The conversion should be OK, I guess. qint64 is 64 bit and long can be 32 bit or 64 bit depending to the system. I am not an experienced developer ... but this is making me uneasy. :D
It is not necessary here because you are casting to a longer type but I would think about using qint64 as type for the variable.

If anyone who knows more about this is reading this, please comment.

For now, I would leave it like this. There is already another variable that is cast from int to qint32 ... which is making me more uneasy than your new conversion and it seems to work.

Just please fix the one casing issue.

I'll leave the approval to Albert. :)


> On Oct. 29, 2015, 1:59 a.m., Frederik Schwarzer wrote:
> > boardwidget.h, line 135
> > <https://git.reviewboard.kde.org/r/125855/diff/1/?file=413227#file413227line135>
> >
> >     Casing: setGameNum() (as in getGameNum())
> >     And why not naming the variable gameNum?

You forgot the casing. :)
setGameNum() to make it same as getGameNum().


- Frederik


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125855/#review87637
-----------------------------------------------------------


On Oct. 30, 2015, 12:07 a.m., Dustin Steinack wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125855/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2015, 12:07 a.m.)
> 
> 
> Review request for KDE Games.
> 
> 
> Bugs: 353845
>     http://bugs.kde.org/show_bug.cgi?id=353845
> 
> 
> Repository: kmahjongg
> 
> 
> Description
> -------
> 
> This fix enables kmahjongg to write the current game number to the savegame file. 
> 
> The game number will be displayed in the status line if a saved games is loaded. Loading an old savegame will not change the status line.
> 
> 
> Diffs
> -----
> 
>   boardwidget.h 6e1d3f5 
>   boardwidget.cpp 6a4f033 
>   kmahjongg.cpp eb1dfa1 
> 
> Diff: https://git.reviewboard.kde.org/r/125855/diff/
> 
> 
> Testing
> -------
> 
> Loading savegame with game number included in a patched version of kmahjongg: works as expected.
> Loading savegame with game number included in an unpatched version of kmahjongg: game correctly restored, status line unaffected.
> Loading savegame with game number excluded in a patched version of kmahjongg: game correctly restored, status line unaffected.
> 
> 
> Thanks,
> 
> Dustin Steinack
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20151030/523208d8/attachment-0001.html>