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

[Andreas Scherf]

> On 2010-03-18 21:39:51, Parker Coates wrote:
> > /trunk/KDE/kdegames/kbounce/wall.cpp, lines 29-35
> > <http://reviewboard.kde.org/r/3077/diff/1/?file=20254#file20254line29>
> >
> >     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?

Ok. I remeoved it now.


> On 2010-03-18 21:39:51, Parker Coates wrote:
> > /trunk/KDE/kdegames/kbounce/mainwindow.cpp, lines 280-291
> > <http://reviewboard.kde.org/r/3077/diff/1/?file=20252#file20252line280>
> >
> >     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. :))

I removed thic comment because the lines were exactly the same source as in the switch block. 
Ok you could answer: but there are if statements in the comment and thats different.
Sure but the code does the same. On the other hand i take over maintainership ... 


> On 2010-03-18 21:39:51, Parker Coates wrote:
> > /trunk/KDE/kdegames/kbounce/mainwindow.cpp, lines 258-264
> > <http://reviewboard.kde.org/r/3077/diff/1/?file=20252#file20252line258>
> >
> >     Why add empty switch cases?

To show all different cases. At the moment this a hint only. 
I think a compiler would remove this lines. 
Should i comment them out or remove them completely?


> On 2010-03-18 21:39:51, Parker Coates wrote:
> > /trunk/KDE/kdegames/kbounce/main.cpp, line 31
> > <http://reviewboard.kde.org/r/3077/diff/1/?file=20250#file20250line31>
> >
> >     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.

I added a feature in the commit before and in this one. So i take 0.10.


- Andreas


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


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
> 
>