<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/126919/">https://git.reviewboard.kde.org/r/126919/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 29th, 2016, 11:16 a.m. 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;">In line 354 of the same file the author already tries to handle the 4th quarter.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> nQuarterSize <span style="color: #666666">=</span> (nQuarter <span style="color: #666666"><</span> <span style="color: #666666">3</span> <span style="color: #666666">?</span> nFullSize <span style="color: #666666">/</span> <span style="color: #666666">4</span> <span style="color: #666666">:</span> nFullSize <span style="color: #666666">-</span> <span style="color: #666666">3</span> <span style="color: #666666">*</span> nFullSize <span style="color: #666666">/</span> <span style="color: #666666">4</span>);
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Can you fix the issue in that line instead?
I mean, if I divide by 4, I am always at the lower bound of possibilities. E.g. with a size of 75, I get 18.75 mathematically, which is 18 with the '/'.
If I then take three fourth of the size (size*3/4), it's 56,25, resulting in 56 being subtracted from the 75, which is 19. Boom?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So I would suggest something like:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"><span style="color: #666666">-</span> nQuarterSize <span style="color: #666666">=</span> (nQuarter <span style="color: #666666"><</span> <span style="color: #666666">3</span> <span style="color: #666666">?</span> nFullSize <span style="color: #666666">/</span> <span style="color: #666666">4</span> <span style="color: #666666">:</span> nFullSize <span style="color: #666666">-</span> <span style="color: #666666">3</span> <span style="color: #666666">*</span> nFullSize <span style="color: #666666">/</span> <span style="color: #666666">4</span>);
<span style="color: #666666">+</span> <span style="color: #008000; font-weight: bold">if</span> (nQuarter <span style="color: #666666"><</span> <span style="color: #666666">3</span>) {
<span style="color: #666666">+</span> nQuarterSize <span style="color: #666666">=</span> nFullSize <span style="color: #666666">/</span> <span style="color: #666666">4</span>;
<span style="color: #666666">+</span> } <span style="color: #008000; font-weight: bold">else</span> {
<span style="color: #666666">+</span> <span style="color: #008000; font-weight: bold">if</span> ((nFullSize <span style="color: #666666">*</span> <span style="color: #666666">3</span> <span style="color: #666666">%</span> <span style="color: #666666">4</span>) <span style="color: #666666">==</span> <span style="color: #666666">0</span>) {
<span style="color: #666666">+</span> nQuarterSize <span style="color: #666666">=</span> nFullSize <span style="color: #666666">-</span> nFullSize <span style="color: #666666">*</span> <span style="color: #666666">3</span> <span style="color: #666666">/</span> <span style="color: #666666">4</span>;
<span style="color: #666666">+</span> } <span style="color: #008000; font-weight: bold">else</span> {
<span style="color: #666666">+</span> nQuarterSize <span style="color: #666666">=</span> nFullSize <span style="color: #666666">-</span> nFullSize <span style="color: #666666">*</span> <span style="color: #666666">3</span> <span style="color: #666666">/</span> <span style="color: #666666">4</span> <span style="color: #666666">-</span> <span style="color: #666666">1</span>;
<span style="color: #666666">+</span> }
<span style="color: #666666">+</span> }
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">around line 354 so nQuarterSize is correct throughout the whole rest of that method.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Or am I barking up the wrong tree here?</p></pre>
</blockquote>
<p>On January 29th, 2016, 7:28 p.m. UTC, <b>Mathias Kraus</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;">This would also fix the crash, but the solution from Julian is how it was supposed to work. Because of the division by 4, the sum of the four quarters would be less than nFullSize. That was the reason for the special handling of the fourth quarter, which will be equal or bigger than the privious three. But the index of the fourth quarter should start at the end of the third quarter, not three times the possibly bigger fourth quarter, so the solution from Julian is correct.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The solution from above will set the bonus to a block already set in quarter 3. Let's assume nFullSize is a multiple of 4, e.g. 16. In this case the last quarter would have a size of 3. The start index would be calculated to 9, which would be part of the third quarter and could overwrite an already set bonus from m_blocks.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The calculation of the fourth quarter is also not correct, it should be</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">nQuarterSize <span style="color: #666666">=</span> (nQuarter <span style="color: #666666"><</span> <span style="color: #666666">3</span> <span style="color: #666666">?</span> nFullSize <span style="color: #666666">/</span> <span style="color: #666666">4</span> <span style="color: #666666">:</span> nFullSize <span style="color: #666666">-</span> <span style="color: #666666">3</span> <span style="color: #666666">*</span> (nFullSize <span style="color: #666666">/</span> <span style="color: #666666">4</span>));
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">else the last quarter will be to small. Julian, could you also fix tha and then commit?</p></pre>
</blockquote>
<p>On January 29th, 2016, 7:30 p.m. UTC, <b>Mathias Kraus</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;">BTW, thank's Julian for looking into the problem :)</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;">Thank you for the explanation. Maybe a few lines of this might make a good comment in the court. :)</p></pre>
<br />
<p>- Frederik</p>
<br />
<p>On January 28th, 2016, 1:30 p.m. UTC, Julian Helfferich 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 Julian Helfferich.</div>
<p style="color: grey;"><i>Updated Jan. 28, 2016, 1:30 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
granatier
</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;">The crash takes place in game.cpp - Game::createBonus(). The total amount of blocks is nFullSize. To place the bonuses, the game iterates over 4 quarters, the first three containing nFullSize/4 blocks and the last containing the remaining blocks. Thus, nQuarterSize can be larger for the last quarter than for the previous three. Now, when a bonus is assigned to a block, the index of the block is calculated as</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">nIndex = nQuarter * nQuarterSize + i</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">where i iterates from zero to nQuarterSize. The idea is that nQuarter * nQuarterSize is the number of blocks of the previous quarters. However, since nQuarterSize can be larger for the last quarter, this can lead to an index out of bounds when a bonus is to be placed in one of the last blocks. The fixed version is</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">nIndex = nQuarter * (nFullSize/4) + i</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">where nFullSize/4 is the size of the first three quarters.</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;">Started the game a lot of times. Crash did not happen.</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>src/game.cpp <span style="color: grey">(371fac9)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126919/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>