[Kde-games-devel] KTron's review

[Albert Astals Cid]

A Diumenge, 15 de març de 2009, Stas Verberkt va escriure:
> On dinsdag 10 maart 2009 00:02:57 Albert Astals Cid wrote:
> > A Dimarts, 24 de febrer de 2009, Legolas va escriure:
> > > Hello games-list,
> > >
> > > Has been in review for two weeks now. That said, how do you feel about
> > > moving KTron/KSnake to the KDEGames module wednesday/thursday?
> >
> > My comments:
> >  * ChangeLog file seems like it could be killed
>
> Probably all text files (README, TODO, ChangeLog) could be killed. As they
> are outdated and I prefer using bugzilla for wishes etc and svn log as a
> changelog.

Since you are the maintainer now feel free to kill them or improve them.

>
> >  * Given that there are already two games using the fontutils method i
> > invented for blinken i think we should think of moving it to libkdegames.
>
> That'd be a good idea.

What really should be a good idea is moving that to kdelibs since kdeedu has 
also two users of this class, i'll try to move that fordward.

>
> >  * Do we really have to mention KTron/KSnake in the Game Type? I think
> > that they just add confusion to the user
>
> How should it be? They are different game types.

Just drop KTron: and KSnake: and you'll have
 * Player vs Computer
 * Player vs Player
 * Single Player

I think that is easy to see that Single Player is going to be different from 
the PvC and PvP. I personally think that's enough for the user. What are 
others opinion on this?

>
> >  * In single player mode i set my name as "foo" but when highscore shows
> > it says "kdesvn" (my user name)
>
> Just committed a fix.
>
> >  * It would be nice if Right Player / KSnake label is dynamic, that is,
> > says Right Player when on "KTron" modes and "Player Name" when on
> > "KSnake" mode, also Left Player line should disappear on "KSnake" mode
>
> Do you mean the text in the shortcuts dialog, the configuration dialog or
> both? The configuration dialog has the texts in a .ui file, so I'm not sure
> how easy I can make it dynamic.

Actually i meant only configuration dialog but both should be done, otherwise 
the user will get confused.

Comming from a .ui file is not a problem, you still have a pointer to the 
label in which you can call setText(i18n("Foo"))

Changing the texts of the actions could be a bit more complicated but also 
doable, if others think this is a good thing to do i can help you with it.

>
> >  * It would be good if Right Player / Player Name was filled with
> > KUser::FullName if exists or KUser::loginName if it does not
>
> As above. I could also use the given player name, altough the distinction
> between left and right makes it easier to see which player has which
> controls.

I think you did not understand me, i mean the lineedit first run contents, not 
the label text

>
> >  * Also Left Player should have a default name (think something catchy!)
>
> Hmm :) You could use things like the player name (in the configuration) or
> Opponent. But I do like the left / right distinction.

I think you did not understand me, i mean the lineedit first run contents, not 
the label text

Albert

>
> >  * As a coding practice, in KDE we usually prefix private class variables
> > with m_ so they are easier to recognize when having a quick look at a
> > random method.
>
> I'll do a big update as soon as I find the time.
>
> >  * namespace ObstacleType {
> >         enum Type {
> >                 Bush
> >         };
> > }
> >  makes more sense inside as enum inside class Obstacle
>
> Comitted a fix.
>
> > Not sure how much of the code is inherited and how much is new to see how
> > that is your "fault" or original's coder :D
>
> I changed a lot of the code (moved things to seperate files, made it a bit
> OOP). So I guess most is my fault ;)
>
> > Albert
>
> Thanks for the comments. Could you give me a bit more help / feedback on
> the naming issues?
> I'm not completely sure about how to handle them.
>
> Regards,
>
> Stas
>
> _______________________________________________
> kde-games-devel mailing list
> kde-games-devel at kde.org
> https://mail.kde.org/mailman/listinfo/kde-games-devel