[Kde-games-devel] Review Request: Demo mode in Kiriki

[Parker Coates]

> On 2010-04-12 20:00:54, Parker Coates wrote:
> > /trunk/KDE/kdegames/kiriki/src/row.cpp, lines 12-17
> > <http://reviewboard.kde.org/r/3573/diff/2/?file=23236#file23236line12>
> >
> >     You are correct that using an initialiser list is a good idea here, but this change is unrelated to the original goal of this patch.
> >     
> >     Please try to keep each patch down to a single bug fix, feature, or improvement as it makes reviewing much easier and keeps the VCS history cleaner. Six ten line patches are generally much preferred to one thirty line patch.
> 
> Luiz Romário Santana Rios wrote:
>     Okay, but what should I do? Send a patch only for this?

Since this change doesn't make a big difference, normally you would just silently fix it yourself. But as a new contributor without SVN access, that's not so easy. :) I'll just submit this part of the patch for you, if you don't mind.


> On 2010-04-12 20:00:54, Parker Coates wrote:
> > /trunk/KDE/kdegames/kiriki/src/kiriki.cpp, lines 188-190
> > <http://reviewboard.kde.org/r/3573/diff/2/?file=23233#file23233line188>
> >
> >     I'm not sure this is a good idea. After finishing a game, I would not expect a new demo to start after 3 seconds without warning. I could see this being quite confusing to users.
> 
> Luiz Romário Santana Rios wrote:
>     A demo will start only if the current game is also a demo.

Good point.


> On 2010-04-12 20:00:54, Parker Coates wrote:
> > /trunk/KDE/kdegames/kiriki/src/kiriki.cpp, lines 78-80
> > <http://reviewboard.kde.org/r/3573/diff/2/?file=23233#file23233line78>
> >
> >     Does this code actually work as expected? Once the action has been disabled, how does it ever get re-enabled?
> 
> Luiz Romário Santana Rios wrote:
>     Yes, because of the second connect: when a new game is started, gameNewAction emits triggered(false). Thus, the demo action gets enabled.

Oh. You're relying on the fact that the triggered(bool) signal will always be false for a non-checkable action. Very tricky. :)


> On 2010-04-12 20:00:54, Parker Coates wrote:
> > /trunk/KDE/kdegames/kiriki/src/kiriki.kcfg, lines 19-21
> > <http://reviewboard.kde.org/r/3573/diff/2/?file=23234#file23234line19>
> >
> >     I'm not sure if enabling this by default is a good idea or not. I think it could be confusing for user if they don't realise that it's a demo that they're seeing. The might sit there waiting for "their" turn, which will never come. ;)
> 
> Luiz Romário Santana Rios wrote:
>     I was thinking of showing the user that the game is a demo - maybe like in Kubrick. Would it be better?

I think a transparent overlay would be best, but the statusbar indicator you just uploaded is pretty good too as it keeps things nice and simple.


- Parker


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


On 2010-04-13 10:18:52, Luiz Romário Santana Rios wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3573/
> -----------------------------------------------------------
> 
> (Updated 2010-04-13 10:18:52)
> 
> 
> Review request for KDE Games.
> 
> 
> Summary
> -------
> 
> This patch implements demonstration game in Kiriki. Almost complete.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdegames/kiriki/src/configPage.ui 1113356 
>   /trunk/KDE/kdegames/kiriki/src/kiriki.h 1113356 
>   /trunk/KDE/kdegames/kiriki/src/kiriki.cpp 1113356 
>   /trunk/KDE/kdegames/kiriki/src/kiriki.kcfg 1113356 
>   /trunk/KDE/kdegames/kiriki/src/lateralwidget.h 1113356 
>   /trunk/KDE/kdegames/kiriki/src/lateralwidget.cpp 1113356 
>   /trunk/KDE/kdegames/kiriki/src/main.cpp 1113356 
>   /trunk/KDE/kdegames/kiriki/src/scores.cpp 1113356 
> 
> Diff: http://reviewboard.kde.org/r/3573/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Luiz Romário
> 
>