[Kde-games-devel] Review Request: New features for knetwalk

[Albert Astals Cid]

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


Please do not change the whitespace of files if you don't have to, makes patches much more difficult to read.
isWrapped should be const
I also wonder if the lock and intellilock functions don't belong better to the grid than to the main window.

- Albert


On 2009-09-13 20:18:31, Markus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1597/
> -----------------------------------------------------------
> 
> (Updated 2009-09-13 20:18:31)
> 
> 
> Review request for KDE Games.
> 
> 
> Summary
> -------
> 
> This adds a couple new features and wishlist items to knetwalk.
> 
> Overview:
> 1. auto-locking of pieces after they are rotated
> 2. auto-locking of adjacent pieces if you lock a piece
> 3. changing of cable animation time (wish/bug #186483)
> 4. advanced input mode, using Mouse and Keyboard together
> 5. board shifting in all modes where the connections wrap around the board
> 
> The changes in detail:
> - added a page to the settings dialog where you can check if you want all these
> things (they are all configurable independently of each other), and removed the
> "Use Keyboard" entry in the menu
> - changed the animation algorithm of the cells so as to be compatible with the
> implementation of feature #2 (no visual difference, and a reduction in code
> size)
> - added a lock() method which checks for feature #2 and helps in implementation
> of feature #1
> - feature #1: after you have rotated a cell (i.e. when you change the next
> cell) it will be locked automatically. This counts as a normal lock (i.e. as if
> you had clicked with the mouse or used the keyboard), and so is 100% compatible
> with feature #2
> - feature #2: if a cell is locked, the adjacent cells will be checked if they
> can be surely locked, i.e. there is no other position where they can fit with
> the already locked cells (p.ex. a vertical bar next to another which is already
> locked). This helps with the problem of Parker Coates (wish/bug #167588)
> where cells are not locked because they are never rotated.
> - feature #3: added an option to the settings dialog to change the animation
> time (from 0 to 1000 ms). For this I had to change some details of the cell
> implementation.
> - feature #4: The settings page has an option to play with keyboard
> alone (as it has been until now), or to use a combined mode in which a cell is
> selected as the mouse hovers over it, and you can use the mouse and the
> keyboard to rotate or lock it. This is the original mouse mode, but now with
> some keyboard controls enabled.
> - feature #5: When connections can be made around the edges of the board
> (Expert mode), the board can be shifted by keyboard to make it easier to see the
> missing links.
> 
> 
> This addresses bugs 167588 and 186483.
>     https://bugs.kde.org/show_bug.cgi?id=167588
>     https://bugs.kde.org/show_bug.cgi?id=186483
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdegames/knetwalk/src/CMakeLists.txt 1014066 
>   /trunk/KDE/kdegames/knetwalk/src/abstractgrid.h 1014066 
>   /trunk/KDE/kdegames/knetwalk/src/abstractgrid.cpp 1014066 
>   /trunk/KDE/kdegames/knetwalk/src/cell.h 1014066 
>   /trunk/KDE/kdegames/knetwalk/src/cell.cpp 1014066 
>   /trunk/KDE/kdegames/knetwalk/src/globals.h 1014066 
>   /trunk/KDE/kdegames/knetwalk/src/knetwalk.kcfg 1014066 
>   /trunk/KDE/kdegames/knetwalk/src/knetwalkui.rc 1014066 
>   /trunk/KDE/kdegames/knetwalk/src/mainwindow.h 1014066 
>   /trunk/KDE/kdegames/knetwalk/src/mainwindow.cpp 1014066 
>   /trunk/KDE/kdegames/knetwalk/src/settings.kcfgc 1014066 
>   /trunk/KDE/kdegames/knetwalk/src/settings.ui PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/1597/diff
> 
> 
> Testing
> -------
> 
> I fixed three bugs compared to the original version of the patch as published some time ago on bugs.kde.org, and now everything seems to work fine.
> Note that I haven't split the patch as suggested on bugs.kde.org, because some features are inter-depending. I will remove all unwanted features from the patch once this has been reviewed.
> 
> 
> Thanks,
> 
> Markus
> 
>