[Kde-games-devel] Review Request: Porting Bomber from KGameCanvas to QGraphicsView

[Stefan Majewsky]

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


Looks great overall.


trunk/KDE/kdegames/bomber/src/board.cpp
<http://svn.reviewboard.kde.org/r/6908/#comment13254>

    Compiler complains:
    
    bomber/src/board.h: In constructor ‘BomberBoard::BomberBoard(KGameRenderer*, QGraphicsView*, QObject*)’:
    bomber/src/board.h:191:8: warning: ‘BomberBoard::m_bomb’ will be initialized after [-Wreorder]
    bomber/src/board.h:181:17: warning:   ‘KGameRenderer* BomberBoard::m_renderer’ [-Wreorder]
    bomber/src/board.cpp:57:1: warning:   when initialized here [-Wreorder]
    



trunk/KDE/kdegames/bomber/src/bomberwidget.h
<http://svn.reviewboard.kde.org/r/6908/#comment13255>

    Unused member. (Also makes no sense, BomberGameWidget is the QGraphicsView itself.)



trunk/KDE/kdegames/bomber/src/bomberwidget.cpp
<http://svn.reviewboard.kde.org/r/6908/#comment13256>

    It's usually easier to group all items below one master item which does not paint anything by itself, so you only need to show/hide that item. Just if you like.



trunk/KDE/kdegames/bomber/src/bomberwidget.cpp
<http://svn.reviewboard.kde.org/r/6908/#comment13257>

    You're looking for QGraphicsItem::setZValue.



trunk/KDE/kdegames/bomber/src/building.cpp
<http://svn.reviewboard.kde.org/r/6908/#comment13258>

    Houston, we have a memory leak. Granted, it was there before your work, but while you're at it, could you take a look at it?



trunk/KDE/kdegames/bomber/src/building.cpp
<http://svn.reviewboard.kde.org/r/6908/#comment13259>

    Please throw away all these raise() functions. Really, you want to use QGraphicsItem::setZValue.


And don't forget to add yourself to the author list in main.cpp (look for "about.addAuthor"). It has a field to specify your exact contribution.

- Stefan Majewsky


On March 12, 2012, 4:47 p.m., Mehmet Emre wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6908/
> -----------------------------------------------------------
> 
> (Updated March 12, 2012, 4:47 p.m.)
> 
> 
> Review request for KDE Games.
> 
> 
> Description
> -------
> 
> I've ported bomber to QGraphicsView as requested here: http://techbase.kde.org/Projects/Games/Porting. I've made some modifications on the classes below:
> - Explodable
> - BomberBoard
> - BomberGameWidget
> - Building
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdegames/bomber/src/board.h 1284898 
>   trunk/KDE/kdegames/bomber/src/board.cpp 1284898 
>   trunk/KDE/kdegames/bomber/src/bomberwidget.h 1284898 
>   trunk/KDE/kdegames/bomber/src/bomberwidget.cpp 1284898 
>   trunk/KDE/kdegames/bomber/src/building.h 1284898 
>   trunk/KDE/kdegames/bomber/src/building.cpp 1284898 
>   trunk/KDE/kdegames/bomber/src/explodable.h 1284898 
>   trunk/KDE/kdegames/bomber/src/explodable.cpp 1284898 
> 
> Diff: http://svn.reviewboard.kde.org/r/6908/diff/
> 
> 
> Testing
> -------
> 
> I've done these tests and solved the problems that I've bumped into:
> - Played bomber's first two levels.
> - Paused the game
> - Changed settings during the game and while game is paused.
> 
> 
> Thanks,
> 
> Mehmet Emre
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20120312/a6d5ce86/attachment-0001.html>