<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://svn.reviewboard.kde.org/r/6903/">http://svn.reviewboard.kde.org/r/6903/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 15th, 2012, 11:32 p.m., <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6903/diff/1/?file=47664#file47664line46" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdegames/kdiamond/src/main.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int main(int argc, char ** argv)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">46</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">KCmdLineOptions</span> <span class="n">options</span><span class="p">;</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">We do not support setting the difficulty level through a command line option anymore?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm not a friend of dropping features, either, but seriously: Who uses this? If it's worth the effort, one can consider adding functionality to the library to autogenerate the options from the KgDifficulty object, but it's not my top priority.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 15th, 2012, 11:32 p.m., <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6903/diff/1/?file=47671#file47671line33" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdegames/libkdegames/kgdifficulty.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">33</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">class</span> <span class="n">KDEGAMES_EXPORT</span> <span class="n">KgDifficultyLevel</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm not a fan of two classes in the same header, could i convince you to split it?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I don't see the added value. I usually split headers when one of both classes can be used without the other one, but that's clearly not the case. Also, splitting increases compilation time.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 15th, 2012, 11:32 p.m., <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6903/diff/1/?file=47671#file47671line36" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdegames/libkdegames/kgdifficulty.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">36</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">enum</span> <span class="n">StandardLevel</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">QDISABLE_COPY?
QProperties?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Will be added.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 15th, 2012, 11:32 p.m., <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6903/diff/1/?file=47671#file47671line90" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdegames/libkdegames/kgdifficulty.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">90</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">Q_PROPERTY</span><span class="p">(</span><span class="k">const</span> <span class="n">KgDifficultyLevel</span><span class="o">*</span> <span class="n">currentLevel</span> <span class="n">READ</span> <span class="n">currentLevel</span> <span class="n">WRITE</span> <span class="n">select</span> <span class="n">NOTIFY</span> <span class="n">changed</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think it's a good practive if you get the notify signals to be the property name + changed, i.e. currentLevel -> currentLevelChanged
this way from QML you can do the typical
onCurrentLevelChanged: console.log("foo", currentLevel)</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Good point. Without the properties, selected() and changed() sounded concise and to the point, but the QML argument is convincing.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 15th, 2012, 11:32 p.m., <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6903/diff/1/?file=47671#file47671line95" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdegames/libkdegames/kgdifficulty.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">95</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">KgDifficulty</span><span class="p">(</span><span class="n">QObject</span><span class="o">*</span> <span class="n">parent</span> <span class="o">=</span> <span class="mi">0</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Mark as explicit?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 15th, 2012, 11:32 p.m., <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6903/diff/1/?file=47671#file47671line111" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdegames/libkdegames/kgdifficulty.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">111</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="kt">void</span> <span class="n">addStandardLevelRange</span><span class="p">(</span><span class="n">KgDifficultyLevel</span><span class="o">::</span><span class="n">StandardLevel</span> <span class="n">from</span><span class="p">,</span> <span class="n">KgDifficultyLevel</span><span class="o">::</span><span class="n">StandardLevel</span> <span class="n">to</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Can we get a addStandardLevel again? in case i just want to add Easy and Hard? I guess I could call addStandardLevelRange(Easy, Easy) but
addStandardLevelRange(Easy) is much more natural</pre>
</blockquote>
<p>On February 15th, 2012, 11:37 p.m., <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;">"but addStandardLevelRange(Easy) is much more natural" should read "but addStandardLevel(Easy) is much more natural"</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I did not consider addLevel(new KgDifficultyLevel(level)) too chatty, but I'll add addStandardLevel().</pre>
<br />
<p>- Stefan</p>
<br />
<p>On February 15th, 2012, 9:33 p.m., Stefan Majewsky wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Games.</div>
<div>By Stefan Majewsky.</div>
<p style="color: grey;"><i>Updated Feb. 15, 2012, 9:33 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;">Another day, another updated libkdegames component. KgDifficulty is a rewrite of KGameDifficulty which splits logic from GUI and provides a cleaner API for the logic part.
New features:
* KgDifficulty automatically saves and restores the level selection across application runs.
* The new KgDifficultyLevel base class allows applications to add more data and logic to their difficulty levels. (Usage of standard difficulty levels is simple as ever, and even simpler, thanks to the new addStandardLevelRange method.)
* Everything in KgDifficulty is exposed through Q_PROPERTY for QML-readiness.
Removed features:
* The standard level KGameDifficulty::Configurable has been removed since it's the same as ::Custom from the KgDifficulty point of view.
* setRestartOnChange() has been removed. If you want that behavior, call setGameRunning() as appropriate. If you don't want that behavior, just don't.
* The localization API (e.g. localizedLevelStrings) has been removed. The information can be retrieved by iterating over the levels(). This is the only point where application code is going to be more complex right now, but I plan to make KgDifficulty a first class citizen of KScoreDialog when I merge my old KScore2 branch (then as KgScore).
Compatibility: Highscore data which was created using KGameDifficulty metadata continues to work since the same data values, translations etc. are used internally. Since the difficulty level selection logic moves from the application to the library, compatibility cannot be retained, but that's a minor one-time issue caused by the update. I do not consider that problematic.
The fine print: The QtWidgets-dependent part has been separated into the KgDifficultyGUI namespace containing the single method KgDifficultyGUI::init. I intend to move this to a separate library once all QtWidgets dependencies are split from the core libkdegames.</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;">My usual guinea pig (KDiamond) behaves fine. The tab selection in KScoreDialog (through setConfigGroup()) is broken for some reason, but I confirmed that it was already broken before the port. Might have to do with the deprecation warning for KScoreDialog::setConfigGroup().</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>trunk/KDE/kdegames/kdiamond/src/board.h <span style="color: grey">(1280243)</span></li>
<li>trunk/KDE/kdegames/kdiamond/src/board.cpp <span style="color: grey">(1280243)</span></li>
<li>trunk/KDE/kdegames/kdiamond/src/game.h <span style="color: grey">(1280243)</span></li>
<li>trunk/KDE/kdegames/kdiamond/src/game.cpp <span style="color: grey">(1280243)</span></li>
<li>trunk/KDE/kdegames/kdiamond/src/main.cpp <span style="color: grey">(1280243)</span></li>
<li>trunk/KDE/kdegames/kdiamond/src/mainwindow.h <span style="color: grey">(1280243)</span></li>
<li>trunk/KDE/kdegames/kdiamond/src/mainwindow.cpp <span style="color: grey">(1280243)</span></li>
<li>trunk/KDE/kdegames/libkdegames/CMakeLists.txt <span style="color: grey">(1280243)</span></li>
<li>trunk/KDE/kdegames/libkdegames/includes/CMakeLists.txt <span style="color: grey">(1280243)</span></li>
<li>trunk/KDE/kdegames/libkdegames/includes/KgDifficulty <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdegames/libkdegames/kgamedifficulty.h <span style="color: grey">(1280243)</span></li>
<li>trunk/KDE/kdegames/libkdegames/kgdifficulty.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdegames/libkdegames/kgdifficulty.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/6903/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>