<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/125855/">https://git.reviewboard.kde.org/r/125855/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Oktober 29th, 2015, 1:59 vorm. UTC, <b>Frederik Schwarzer</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Please take my comments as courious questions or suggestions and not as demands to fulfill for a "ship it". :)</p></pre>
</blockquote>
<p>On Oktober 30th, 2015, 12:07 vorm. UTC, <b>Dustin Steinack</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On Oktober 30th, 2015, 3:12 vorm. UTC, <b>Frederik Schwarzer</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If anyone who knows more about this is reading this, please comment.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Just please fix the one casing issue.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'll leave the approval to Albert. :)</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sorry for the casing :)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It may be an ideas to declare the gameGenerationNum variable using the qt typedefs to avoid the casting and allow an easy serialization. But I'm not quite sure whether this is justifiable for this little patch.</p></pre>
<br />
<p>- Dustin</p>
<br />
<p>On Oktober 30th, 2015, 12:07 vorm. UTC, Dustin Steinack wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Games.</div>
<div>By Dustin Steinack.</div>
<p style="color: grey;"><i>Updated Okt. 30, 2015, 12:07 vorm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=353845">353845</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kmahjongg
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This fix enables kmahjongg to write the current game number to the savegame file. </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>boardwidget.h <span style="color: grey">(6e1d3f5)</span></li>
<li>boardwidget.cpp <span style="color: grey">(6a4f033)</span></li>
<li>kmahjongg.cpp <span style="color: grey">(eb1dfa1)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125855/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>