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

[Parker Coates]

> 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. :))
> 
> Andreas Scherf wrote:
>     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 ...

I hadn't realised that they were identical to the switch statement. In that case, I guess we can safely assume that there was no good reason for keeping this code around.


> 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?
> 
> Andreas Scherf wrote:
>     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?

I guess this is more a matter of style than anything else. If I had unhandled enum values, I probably would have just included an empty default case. But I could see the argument for including all the values explicitly even if they don't do anything.

Again an issue of style, but I probably would have dropped the first two break statements.


- Parker


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


On 2010-03-18 22:04:20, Andreas Scherf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3077/
> -----------------------------------------------------------
> 
> (Updated 2010-03-18 22:04:20)
> 
> 
> 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
> 
>