<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/128556/">https://git.reviewboard.kde.org/r/128556/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 30th, 2016, 11:02 a.m. CDT, <b>Albert Astals Cid</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;">You say
**
when an invisible or indestructable brick explodes, the remaining brick counter is decreased even though these bricks did not contribute to the number of remaining bricks.
**
But the only place i see remainingBricks being decremented is handleDeletion and it seems it already accounts for this issue, no? What am i mising?</pre>
</blockquote>
<p>On July 30th, 2016, 1:44 p.m. CDT, <b>Julian Helfferich</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 for looking into it. Yes this is true in most cases. However, the brick type of exploding bricks is set to "BurningBrick" before handleDeletion is called. Thus, handleDeletion does not cover this issue.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Typically, when an invisible brick becomes visible or an indestructable brick becomes destructable, the remainingBricks counter is increased. This, however, did not happen in burn() when the brick type was set to "BurningBrick" (which are both visible and destructable) and is exactly what I have added.</p></pre>
</blockquote>
<p>On August 1st, 2016, 3:59 p.m. CDT, <b>Albert Astals Cid</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;">Then wouldn't it make more sense to handle BurningBrick in handleDeletion so that it is centralized where we do this kind of handling of deleting but not really a brick?</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;">I agree with you. However, as the brick type is set to "BurningBrick" before handleDeletion is called (in case of an explosion), the function has no way of knowing whether the brick to be deleted was invisible or indestructible before. It could have been just an ordinary brick.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There are several places in the code where the remainingBrick counter is increased. For example, when an invisible brick becomes visible after being hit by a ball, or when indestructible bricks become destructible with the magic wand gift. Thus, it doesn't feel wrong to me to increase the counter in burn().</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The simplest way I can think of to use your approach, would be to add a new parameter bool reduceBrickCounter to handleDeletion so that the calling function can decide whether or not to reduce the brick counter.</p></pre>
<br />
<p>- Julian</p>
<br />
<p>On July 29th, 2016, 9:21 p.m. CDT, 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 July 29, 2016, 9:21 p.m.</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=328811">328811</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kbreakout
</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;">As described in bug 328811, some levels clear before all bricks have been destroyed. This happes for example in Level 6, when invisible bricks are destroyed by the "Burning Ball". KBreakout keeps count of the remaining bricks. Indestructable and invisible bricks do not contribute to the remaining bricks. When bricks are destroyed, the remaining brick counter is decreased accordingly. However, when an invisible or indestructable brick explodes, the remaining brick counter is decreased even though these bricks did not contribute to the number of remaining bricks.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In the patch, the remaining brick counter is increased if an invisible or indestructible brick is set to explode.</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;">Print out number of remaining bricks.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">a) Before the patch: Confirm that number of remaining bricks is wrong.
b) After the patch: Confirm that number of remaining bricks is correct. Confirm that levels no longer clear prematurely.</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/qml/logic.js <span style="color: grey">(dc6a2d3)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128556/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>