[Kde-games-devel] Review Request: Port KNetWalk to use QGraphicsView and KGameRenderer

Brian Croom brian.s.croom at gmail.com
Thu Sep 9 09:34:37 CEST 2010



> On 2010-09-06 03:02:02, Parker Coates wrote:
> > /trunk/KDE/kdegames/knetwalk/src/cell.cpp, line 82
> > <http://svn.reviewboard.kde.org/r/5238/diff/1/?file=35067#file35067line82>
> >
> >     You still get that tiny graphic artefact if add a "m_cablesItem->setTransformationMode(Qt::SmoothTransformation);" here although it does seem to be a bit less noticeable.
> >     
> >     Speaking of which, enabling smooth transformations makes the rotation animations look *much* nicer, but I'm not sure how much more computationally expensive it is. It would take some profiling (that I don't have time to do to ;)) to find out.

Wow that really does make a huge difference. I think I like this game even more now, if that's possible :)


> On 2010-09-06 03:02:02, Parker Coates wrote:
> > /trunk/KDE/kdegames/knetwalk/src/fielditem.cpp, lines 85-90
> > <http://svn.reviewboard.kde.org/r/5238/diff/1/?file=35069#file35069line85>
> >
> >     Instead of implementing an empty paint() method, you should set the ItemHasNoContents flag as it's more efficient.

I wasn't aware of that flag, however isn't it still necessary to implement paint() as it is pure virtual in QGraphicsItem?


> On 2010-09-06 03:02:02, Parker Coates wrote:
> > /trunk/KDE/kdegames/knetwalk/src/mainwindow.cpp, line 217
> > <http://svn.reviewboard.kde.org/r/5238/diff/1/?file=35077#file35077line217>
> >
> >     Why get rid of the Renderer singleton? You now have a few classes holding and passing around pointers to a KGameRenderer object, which I don't really love. Getting rid of the singleton is fine if you can cleanly and conveniently keep everyone informed without it, but I'm really not sure that this is an improvement.

Honestly, it's just because that's what I've been doing for the other ports, and it seemed cool to be able to get rid of a source file. I agree that keeping it as a singleton makes good sense though.


- Brian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5238/#review7417
-----------------------------------------------------------


On 2010-09-02 22:36:16, Brian Croom wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5238/
> -----------------------------------------------------------
> 
> (Updated 2010-09-02 22:36:16)
> 
> 
> Review request for KDE Games and Stefan Majewsky.
> 
> 
> Summary
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdegames/knetwalk/src/fielditem.cpp PRE-CREATION 
>   /trunk/KDE/kdegames/knetwalk/src/fielditem.h PRE-CREATION 
>   /trunk/KDE/kdegames/knetwalk/src/cell.cpp 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/CMakeLists.txt 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/abstractgrid.h 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/abstractgrid.cpp 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/cell.h 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/gamewidget.h PRE-CREATION 
>   /trunk/KDE/kdegames/knetwalk/src/gamewidget.cpp PRE-CREATION 
>   /trunk/KDE/kdegames/knetwalk/src/globals.h 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/knetwalk.kcfg 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/knetwalkui.rc 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/main.cpp 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/mainwindow.h 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/mainwindow.cpp 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/renderer.h 1168585 
>   /trunk/KDE/kdegames/knetwalk/src/renderer.cpp 1168585 
> 
> Diff: http://svn.reviewboard.kde.org/r/5238/diff
> 
> 
> Testing
> -------
> 
> 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.
> 
> 
> Thanks,
> 
> Brian
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-games-devel/attachments/20100909/58e794d9/attachment.htm 


More information about the kde-games-devel mailing list