<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/5238/">http://svn.reviewboard.kde.org/r/5238/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 5th, 2010, 8:05 p.m., <b>Stefan Majewsky</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;">I test-compiled, but all I got is a completely blank (as in #FFFFFF) view. Probably something is wrong with the diff; I got a merge conflict in mainwindow.cpp (at the instantiation of m_view) while all other patches I got from you up to now did always apply cleanly.</pre>
</blockquote>
<p>On September 5th, 2010, 8:34 p.m., <b>Parker Coates</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;">It patches cleanly here. Do you have local changes, maybe?</pre>
</blockquote>
<p>On September 5th, 2010, 11:21 p.m., <b>Stefan Majewsky</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;">No, I checked git-status before applying the patch. If it patches cleanly on your site, does it work when compiled?</pre>
</blockquote>
<p>On September 9th, 2010, 6:51 a.m., <b>Brian Croom</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;">Ok, I think I've got this issue figured out.
Following KMines, the patch simply passes in Settings::theme() when constructing the KGameRenderer, however kblocks.kcfg has no default value for the theme, so no theme is loaded. I believe that the proper fix for this is to add a default value to the config file, correct? Alternatively, should KGameRendererPrivate::setTheme() call KGameTheme::loadDefault() when it receives 0-length theme string instead of just bailing?
Note that this bug affects KMines as well: deleting kminesrc from the config directory and then running the game results in no theme being loaded and thus a blank view.</pre>
</blockquote>
<p>On September 9th, 2010, 12:47 p.m., <b>Parker Coates</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;">Definitely add a default to the .kcfg file. Nowadays, that's the preferred way of changing the default theme in new releases, too. Is KMines the only other game that you've ported that is affected by this issue?
I'll let Stefan comment on whether or not it makes sense to call loadDefault() when KGR has no valid theme.
</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;">Yes, I just checked everything and KMines is the one that ends up with the theme failing to load.</pre>
<br />
<p>- Brian</p>
<br />
<p>On September 2nd, 2010, 10:36 p.m., Brian Croom wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/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 and Stefan Majewsky.</div>
<div>By Brian Croom.</div>
<p style="color: grey;"><i>Updated 2010-09-02 22:36:16</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;">As KNetWalk's use of QWidget as its graphics stack was marked in red on the porting status wiki page, and because I wanted to see if I had acquired a good enough working understanding of the QGraphicsView framework to be able to accomplish it, I decided to attempt a full port of this game. I referred to the KMines source while restructuring the code, as it has various conceptually similar elements (a grid with square cells that are activated by the user during gameplay, etc.)
Doing this required some rather invasive changes to the GUI code. In addition to switching to QGraphicsView, this patch also:
- Removes the distinction between mouse and keyboard input modes. They now work together nicely, as they should.
- Adds the ability to pause the game, and hide the puzzle while it is paused.
- Cleans up code in various places and removes a number of obsolete class members that I presume to have been vestiges from before the KDE port.
My only disappointment is that switching to QGraphicsView did not get rid of the rendering artifacts that often appear when the cables are being rotated. Particularly on the cells with T-shaped cable junctions, white pixels often appear at the edge of the pixmap during the rotation animation. I would greatly appreciate any suggestions for how to avoid that happening.</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;">I have come to enjoy playing this game quite a lot, and have given it considerable testing. All the usual things seem to work well including resizing, theme changes, changing difficulty level, and pausing/resuming.</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/knetwalk/src/fielditem.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/fielditem.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/cell.cpp <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/CMakeLists.txt <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/abstractgrid.h <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/abstractgrid.cpp <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/cell.h <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/gamewidget.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/gamewidget.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/globals.h <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/knetwalk.kcfg <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/knetwalkui.rc <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/main.cpp <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/mainwindow.h <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/mainwindow.cpp <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/renderer.h <span style="color: grey">(1168585)</span></li>
<li>/trunk/KDE/kdegames/knetwalk/src/renderer.cpp <span style="color: grey">(1168585)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5238/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>