<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/111662/">http://git.reviewboard.kde.org/r/111662/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 14th, 2013, 9:42 p.m. UTC, <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;">Hi Tom, thanks a lot for the contribution!
Sadly it seems that Matt is not answering so it'll have to be someone without much knowledge on ksquares doing the review.
I'm going to be a bit annoying i know, but do you think you could try to isolate the new code so that it's only executed when we are on hard mode? This way it'd be easier for someone with no knowledge at all on ksquares to accept the change, based in the fact that if the patch is broken it'll only break the new hard mode, and not the existing ones.
I tried looking at if the code already does that but it seems that it's a bit "convoluted", but if you want to explain to me that the other difficulties don't get changed by the new code i'm also open to that.</pre>
</blockquote>
<p>On August 15th, 2013, 9:29 p.m. UTC, <b>Tom Vincent Peters</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;">Hello Albert, thank you very much for taking the time to review this.
I assume that it will be more annoying for you to look into any form of this code than for me to change it in a way that makes reviewing it easier. So I'm wondering: Should I rather leave the original code untouched, or should I avoid code duplication?
Leaving the original code intact would mean that I either introduce at least one new method in the aiController class or (less preferred) create a whole new class that looks almost like the aiController that is in Diff revision 2 (only without premature return statements for easy/medium levels).
Avoiding code duplication would mean some more work but would surely be possible by saving the results of the calculations of existing AI code in class variables and then reusing these for further calculations. This actually is something I thought about before when I considered introducing new classes that inherit each other. At that time I decided against it to avoid unnecessary structual changes and continued on the path of the code.
I'm not 100% sure that my changes don't affect the other difficulties. Thus it would be unreasonable to argue for keeping the code the way it is. I think it is saner to separate old and new code.
To help you with reviewing: In my old review request there is a small change that puts the internal line index in the tool-tip of each line. These help a lot with interpreting the debug messages in the AI code.</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;">code duplication is bad too. I was hoping you could basically add a few "if (difficulty == hard)" around so that it's clearer only for hard that is executed so the easy/medium code is [almost] untouched (even if some new parameters are added to some methods or something)
Is this possible at all?</pre>
<br />
<p>- Albert</p>
<br />
<p>On July 25th, 2013, 11:28 p.m. UTC, Tom Vincent Peters wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Games and Matt Williams.</div>
<div>By Tom Vincent Peters.</div>
<p style="color: grey;"><i>Updated July 25, 2013, 11:28 p.m.</i></p>
<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;">Hard AI for KSquares,
originally submitted about a year ago: https://svn.reviewboard.kde.org/r/6951/
The new AI level is able to do hard hearted handouts and doublecrosses.
This time there are only minimal UI changes that are required for the new AI level (and some credits).
It seems that the medium ai uses hard hearted handouts more often than before (doesn't seem to influence the strength of the medium AI, see "Testing Done")</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;">100 autoplay games medium against hard
a lot of games against hard ai
some games against medium ai
Test whether medium ai code changed as a side effect of the new code: https://git.reviewboard.kde.org/r/111705/</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/prefs_ai.ui <span style="color: grey">(9774aed)</span></li>
<li>src/ksquareswindow.cpp <span style="color: grey">(092c795)</span></li>
<li>src/main.cpp <span style="color: grey">(58184b6)</span></li>
<li>src/aicontroller.h <span style="color: grey">(beaafa8)</span></li>
<li>src/aicontroller.cpp <span style="color: grey">(0bff5e6)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111662/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>