[Kde-games-devel] Review Request: Added difficuly level into kbounce.

[Parker Coates]

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



/trunk/KDE/kdegames/kbounce/board.cpp
<http://reviewboard.kde.org/r/3077/#comment4020>

    Spaces on either side of '=' would be good here. I realise that setBallVelocity code was already this way, but the style of the surrounding code seems to dictate that spaces should be used.



/trunk/KDE/kdegames/kbounce/gamewidget.cpp
<http://reviewboard.kde.org/r/3077/#comment4021>

    Something weird is happening in the diff here.



/trunk/KDE/kdegames/kbounce/main.cpp
<http://reviewboard.kde.org/r/3077/#comment4022>

    This doesn't follow the KDE version numbering system. "0.9.1" is the next logical version number if only bugs have been fixed. "0.10" if features have been added.



/trunk/KDE/kdegames/kbounce/mainwindow.cpp
<http://reviewboard.kde.org/r/3077/#comment4023>

    Why add empty switch cases?



/trunk/KDE/kdegames/kbounce/mainwindow.cpp
<http://reviewboard.kde.org/r/3077/#comment4019>

    I agree that it's a good idea to delete old code rather than comment it out. What is version control for, after all? However, I would discourage you from removing it as part of your patch. The KBounce code isn't "yours" so it's possible (though unlikely) that there is an important reason for that comment being there. Removing it also has nothing to do the changes you're hoping to introduce with this patch.
    
    (I realise this is nitpicky, but I would be bothered if somebody did this to my code. KBounce is a slightly different scenario, because it is currently unmaintained. Of course, if you'd like to take over maintainership, feel free to delete as many comments as you'd like. :))



/trunk/KDE/kdegames/kbounce/wall.cpp
<http://reviewboard.kde.org/r/3077/#comment4018>

    First off, you should be using the initialisation list for initialising variables.
    
    Secondly, since wall velocity is no longer actually a constant, I seems a bit weird to keep a named constant around just for default initialisation. I would say get rid of KBounceWall::WALL_VELOCITY.
    
    Thirdly, since the KBounceBoard::newLevel() calls setWallVelocity on all the walls anyway, do you really need to both initialising the velocity with a default value?


- Parker


On 2010-03-18 21:38:16, Andreas Scherf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3077/
> -----------------------------------------------------------
> 
> (Updated 2010-03-18 21:38:16)
> 
> 
> Review request for KDE Games.
> 
> 
> Summary
> -------
> 
> Fullfilled the wishlist item 219507. I added 3 difficulty levels into kbounce (easy,medium,hard)
> Medium hat the default settings and (Easy,Hard) have different velocity settings for walls and balls.
> The problem is now is Difficulty:Hard playable anymore?
> 
> 
> This addresses bug 219507.
>     https://bugs.kde.org/show_bug.cgi?id=219507
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdegames/kbounce/backgroundselector.cpp 1104903 
>   /trunk/KDE/kdegames/kbounce/board.h 1104903 
>   /trunk/KDE/kdegames/kbounce/board.cpp 1104903 
>   /trunk/KDE/kdegames/kbounce/gamewidget.h 1104903 
>   /trunk/KDE/kdegames/kbounce/gamewidget.cpp 1104903 
>   /trunk/KDE/kdegames/kbounce/main.cpp 1104903 
>   /trunk/KDE/kdegames/kbounce/mainwindow.h 1104903 
>   /trunk/KDE/kdegames/kbounce/mainwindow.cpp 1104903 
>   /trunk/KDE/kdegames/kbounce/wall.h 1104903 
>   /trunk/KDE/kdegames/kbounce/wall.cpp 1104903 
> 
> Diff: http://reviewboard.kde.org/r/3077/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andreas
> 
>