[Kde-games-devel] Re: Review Request: Change levelsets from KConfig to QDom style and create class LevelLoader

[Stefan Majewsky]

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

Ship it!


Looks good. Remarks:


/trunk/KDE/kdegames/kbreakout/src/CMakeLists.txt
<http://svn.reviewboard.kde.org/r/6541/#comment11211>

    renderer.cpp has AFAIR been removed in the 4.6 cycle.



/trunk/KDE/kdegames/kbreakout/src/levelloader.cpp
<http://svn.reviewboard.kde.org/r/6541/#comment11212>

    Argument type -> const QString&



/trunk/KDE/kdegames/kbreakout/src/levelloader.cpp
<http://svn.reviewboard.kde.org/r/6541/#comment11213>

    This test is superfluous. The delete operator does it itself.



/trunk/KDE/kdegames/kbreakout/src/levelloader.cpp
<http://svn.reviewboard.kde.org/r/6541/#comment11214>

    Indentation is inconsistent. You have an indentation width of 4 characters, but a tab width of 8 characters. Please make that consistent with the existing source.


- Stefan


On Feb. 20, 2011, 11:56 p.m., Julian Helfferich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6541/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2011, 11:56 p.m.)
> 
> 
> Review request for KDE Games.
> 
> 
> Summary
> -------
> 
> I have moved the function loadLevel() in GameEngine to an external class LevelLoader and changed the style of the levelsets from a KConfig style to a QDocument object model (QDom) / XML-style. This is the first step in empowering (and encouraging) players to create, share and play their custom levelsets.
> 
> This patch addresses no bug and adds no feature. There are no changes to the user experience.
> 
> The QDom-style levelsets provide two main advantages:
> 1. More robust against errors:
> Checks can be implemented very easily. An invalid levelset now leads to an empty level which can be played (but never won, because no last brick can be destroyed) instead of strange behaviour (mouse visible, but confined to KBreakout-Window). For the old style a list of gift types and a corresponding number of gift types had to be maintained in globals.h . This is not needed with the QDom-style.
> 2. Easily extendable:
> Up to now the only two options are identical with the old style: <line></line> and <gift></gift>. New options are planned including <brick></brick> for the placement of single bricks and the possibility to set the position of a gift. This should empower the user to create his own levelsets with different written styles.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdegames/kbreakout/levelsets/default.levelset 1221881 
>   /trunk/KDE/kdegames/kbreakout/src/CMakeLists.txt 1221881 
>   /trunk/KDE/kdegames/kbreakout/src/brick.h 1221881 
>   /trunk/KDE/kdegames/kbreakout/src/brick.cpp 1221881 
>   /trunk/KDE/kdegames/kbreakout/src/gameengine.h 1221881 
>   /trunk/KDE/kdegames/kbreakout/src/gameengine.cpp 1221881 
>   /trunk/KDE/kdegames/kbreakout/src/gift.cpp 1221881 
>   /trunk/KDE/kdegames/kbreakout/src/globals.h 1221881 
>   /trunk/KDE/kdegames/kbreakout/src/levelloader.h PRE-CREATION 
>   /trunk/KDE/kdegames/kbreakout/src/levelloader.cpp PRE-CREATION 
> 
> Diff: http://svn.reviewboard.kde.org/r/6541/diff
> 
> 
> Testing
> -------
> 
> Testing has been done with a version prior to the port to KGameRenderer. Since the patch does interact with graphics, this should have no effects. Code works as expected, even when dealing with a broken levelset file.
> 
> 
> Thanks,
> 
> Julian
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-games-devel/attachments/20110325/493ae770/attachment.htm