Review Request: KScoreDialog with level selection

View: New views
13 Messages — Rating Filter:   Alert me  

Review Request: KScoreDialog with level selection

by Bugzilla from hobbyblobby@arcor.de :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


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

Review request for KDE Games.


Summary
-------

Hi,

in KSpiral I need a different height-score for each level. For this I've patched the KScoreDialog. I've tested the KScoreDialog in different games (like kdiamond, kbounce, knetwalk) and I think it's fully backwards compatible. Now there are three new functions:
 - addLocalizedLevelName(QPair<QByteArray, QString> level): with this you can add an level to the dialog. If the list of level isn't empty above the tabwidget is a QComboBox shown, where you can choose the level.

 - addLocalizedLevelNames(QMap<QByteArray, QString> level): nearly the same like the function above, but you can add more than one level at once.

 - setLevelName: to read or write the config belonging to your level you need to set the current level name. In the rc-file of your application the groupname is changed from KHightScore_GameDifficulty to KHighScore_GameDifficulty_MAP:levelname.


Diffs
-----

  trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h 1028784
  trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp 1028784

Diff: http://reviewboard.kde.org/r/1743/diff


Testing
-------


Screenshots
-----------

new KScoreDialog with level selection
  http://reviewboard.kde.org/r/1743/s/214/


Thanks,

Felix

_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

Re: Review Request: KScoreDialog with level selection

by Felix Lemke :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


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

(Updated 2009-10-01 13:08:20.394888)


Review request for KDE Games.


Changes
-------

The edit->setFocus()-call has absolutly no effect, because it's called before the dialog is shown. It worked in the past, because the KLineEdit was the only input method in this dialog. But now there is the QComboBox too, so I reimplemented the showEvent().

Another fix is, that the ComboBox selects the element choosen with setLevelName().


Summary
-------

Hi,

in KSpiral I need a different height-score for each level. For this I've patched the KScoreDialog. I've tested the KScoreDialog in different games (like kdiamond, kbounce, knetwalk) and I think it's fully backwards compatible. Now there are three new functions:
 - addLocalizedLevelName(QPair<QByteArray, QString> level): with this you can add an level to the dialog. If the list of level isn't empty above the tabwidget is a QComboBox shown, where you can choose the level.

 - addLocalizedLevelNames(QMap<QByteArray, QString> level): nearly the same like the function above, but you can add more than one level at once.

 - setLevelName: to read or write the config belonging to your level you need to set the current level name. In the rc-file of your application the groupname is changed from KHightScore_GameDifficulty to KHighScore_GameDifficulty_MAP:levelname.


Diffs (updated)
-----

  trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h 1029697
  trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp 1029697

Diff: http://reviewboard.kde.org/r/1743/diff


Testing
-------


Screenshots
-----------

new KScoreDialog with level selection
  http://reviewboard.kde.org/r/1743/s/214/


Thanks,

Felix

_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

Re: Review Request: KScoreDialog with level selection

by Albert Astals Cid-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


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



trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h
<http://reviewboard.kde.org/r/1743/#comment1843>

    levelnames -> level names
    Here and in more places



trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h
<http://reviewboard.kde.org/r/1743/#comment1844>

    add -> adds
    Here and in more places



trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h
<http://reviewboard.kde.org/r/1743/#comment1845>

    The example is wrong, ki18n does not return a QString



trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h
<http://reviewboard.kde.org/r/1743/#comment1847>

    What do you need the translated name for here?



trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp
<http://reviewboard.kde.org/r/1743/#comment1848>

    KComboBox



trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp
<http://reviewboard.kde.org/r/1743/#comment1849>

    This is quite innefficient, i know the other functions are coded the same way, but could you do it the proper way, that is using an iterator?


- Albert


On 2009-10-01 13:08:20, Felix Lemke wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1743/
> -----------------------------------------------------------
>
> (Updated 2009-10-01 13:08:20)
>
>
> Review request for KDE Games.
>
>
> Summary
> -------
>
> Hi,
>
> in KSpiral I need a different height-score for each level. For this I've patched the KScoreDialog. I've tested the KScoreDialog in different games (like kdiamond, kbounce, knetwalk) and I think it's fully backwards compatible. Now there are three new functions:
>  - addLocalizedLevelName(QPair<QByteArray, QString> level): with this you can add an level to the dialog. If the list of level isn't empty above the tabwidget is a QComboBox shown, where you can choose the level.
>
>  - addLocalizedLevelNames(QMap<QByteArray, QString> level): nearly the same like the function above, but you can add more than one level at once.
>
>  - setLevelName: to read or write the config belonging to your level you need to set the current level name. In the rc-file of your application the groupname is changed from KHightScore_GameDifficulty to KHighScore_GameDifficulty_MAP:levelname.
>
>
> Diffs
> -----
>
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h 1029697
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp 1029697
>
> Diff: http://reviewboard.kde.org/r/1743/diff
>
>
> Testing
> -------
>
>
> Screenshots
> -----------
>
> new KScoreDialog with level selection
>   http://reviewboard.kde.org/r/1743/s/214/
>
>
> Thanks,
>
> Felix
>
>

_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

Re: Review Request: KScoreDialog with level selection

by Dmitry Suzdalev-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hi guys!

Actually I'm not sure if this really should be added to KScoreDialog.

It seems to me that if every game which has a slightly different layout of its
highscore system would patch libkdegames, we would end up with a mess in API
and in implementation.

What do others think?

And also is this  change absolutely needed to be in KScoreDialog and why?

Cheers,
Dmitry.

On Friday 02 October 2009 02:33:26 Albert Astals Cid wrote:

> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1743/#review2516
> -----------------------------------------------------------
>
>
>
> trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h
> <http://reviewboard.kde.org/r/1743/#comment1843>
>
>     levelnames -> level names
>     Here and in more places
>
>
>
> trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h
> <http://reviewboard.kde.org/r/1743/#comment1844>
>
>     add -> adds
>     Here and in more places
>
>
>
> trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h
> <http://reviewboard.kde.org/r/1743/#comment1845>
>
>     The example is wrong, ki18n does not return a QString
>
>
>
> trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h
> <http://reviewboard.kde.org/r/1743/#comment1847>
>
>     What do you need the translated name for here?
>
>
>
> trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp
> <http://reviewboard.kde.org/r/1743/#comment1848>
>
>     KComboBox
>
>
>
> trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp
> <http://reviewboard.kde.org/r/1743/#comment1849>
>
>     This is quite innefficient, i know the other functions are coded the
>  same way, but could you do it the proper way, that is using an iterator?
>
>
> - Albert
>
> On 2009-10-01 13:08:20, Felix Lemke wrote:
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > http://reviewboard.kde.org/r/1743/
> > -----------------------------------------------------------
> >
> > (Updated 2009-10-01 13:08:20)
> >
> >
> > Review request for KDE Games.
> >
> >
> > Summary
> > -------
> >
> > Hi,
> >
> > in KSpiral I need a different height-score for each level. For this I've
> > patched the KScoreDialog. I've tested the KScoreDialog in different games
> > (like kdiamond, kbounce, knetwalk) and I think it's fully backwards
> > compatible. Now there are three new functions: -
> > addLocalizedLevelName(QPair<QByteArray, QString> level): with this you
> > can add an level to the dialog. If the list of level isn't empty above
> > the tabwidget is a QComboBox shown, where you can choose the level.
> >
> >  - addLocalizedLevelNames(QMap<QByteArray, QString> level): nearly the
> > same like the function above, but you can add more than one level at
> > once.
> >
> >  - setLevelName: to read or write the config belonging to your level you
> > need to set the current level name. In the rc-file of your application
> > the groupname is changed from KHightScore_GameDifficulty to
> > KHighScore_GameDifficulty_MAP:levelname.
> >
> >
> > Diffs
> > -----
> >
> >   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h 1029697
> >   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp 1029697
> >
> > Diff: http://reviewboard.kde.org/r/1743/diff
> >
> >
> > Testing
> > -------
> >
> >
> > Screenshots
> > -----------
> >
> > new KScoreDialog with level selection
> >   http://reviewboard.kde.org/r/1743/s/214/
> >
> >
> > Thanks,
> >
> > Felix
>
> _______________________________________________
> kde-games-devel mailing list
> kde-games-devel@...
> https://mail.kde.org/mailman/listinfo/kde-games-devel
>
_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

Re: Review Request: KScoreDialog with level selection

by Felix Lemke :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message



> On 2009-10-01 22:33:33, Albert Astals Cid wrote:
> > trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h, line 200
> > <http://reviewboard.kde.org/r/1743/diff/2/?file=12252#file12252line200>
> >
> >     What do you need the translated name for here?

The level will automatically added to the list of level names. And in this list the untranslated name is used as identifier and the translated name as string to show.


> On 2009-10-01 22:33:33, Albert Astals Cid wrote:
> > trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h, line 185
> > <http://reviewboard.kde.org/r/1743/diff/2/?file=12252#file12252line185>
> >
> >     The example is wrong, ki18n does not return a QString

Sorry, it should be i18n(), not ki18n().


> On 2009-10-01 22:33:33, Albert Astals Cid wrote:
> > trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp, line 63
> > <http://reviewboard.kde.org/r/1743/diff/2/?file=12253#file12253line63>
> >
> >     KComboBox

I'll change it.


> On 2009-10-01 22:33:33, Albert Astals Cid wrote:
> > trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h, line 182
> > <http://reviewboard.kde.org/r/1743/diff/2/?file=12252#file12252line182>
> >
> >     levelnames -> level names
> >     Here and in more places

I hope, I found all.


> On 2009-10-01 22:33:33, Albert Astals Cid wrote:
> > trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp, line 213
> > <http://reviewboard.kde.org/r/1743/diff/2/?file=12253#file12253line213>
> >
> >     This is quite innefficient, i know the other functions are coded the same way, but could you do it the proper way, that is using an iterator?

I changed it also for addLocalizedConfigGroupNames(...)


> On 2009-10-01 22:33:33, Albert Astals Cid wrote:
> > trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h, line 183
> > <http://reviewboard.kde.org/r/1743/diff/2/?file=12252#file12252line183>
> >
> >     add -> adds
> >     Here and in more places

Done


- Felix


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


On 2009-10-01 13:08:20, Felix Lemke wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1743/
> -----------------------------------------------------------
>
> (Updated 2009-10-01 13:08:20)
>
>
> Review request for KDE Games.
>
>
> Summary
> -------
>
> Hi,
>
> in KSpiral I need a different height-score for each level. For this I've patched the KScoreDialog. I've tested the KScoreDialog in different games (like kdiamond, kbounce, knetwalk) and I think it's fully backwards compatible. Now there are three new functions:
>  - addLocalizedLevelName(QPair<QByteArray, QString> level): with this you can add an level to the dialog. If the list of level isn't empty above the tabwidget is a QComboBox shown, where you can choose the level.
>
>  - addLocalizedLevelNames(QMap<QByteArray, QString> level): nearly the same like the function above, but you can add more than one level at once.
>
>  - setLevelName: to read or write the config belonging to your level you need to set the current level name. In the rc-file of your application the groupname is changed from KHightScore_GameDifficulty to KHighScore_GameDifficulty_MAP:levelname.
>
>
> Diffs
> -----
>
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h 1029697
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp 1029697
>
> Diff: http://reviewboard.kde.org/r/1743/diff
>
>
> Testing
> -------
>
>
> Screenshots
> -----------
>
> new KScoreDialog with level selection
>   http://reviewboard.kde.org/r/1743/s/214/
>
>
> Thanks,
>
> Felix
>
>

_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

Re: Review Request: KScoreDialog with level selection

by Felix Lemke :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


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

(Updated 2009-10-02 09:25:32.729541)


Review request for KDE Games.


Changes
-------

Fix the issues pointed out by Albert.


Summary
-------

Hi,

in KSpiral I need a different height-score for each level. For this I've patched the KScoreDialog. I've tested the KScoreDialog in different games (like kdiamond, kbounce, knetwalk) and I think it's fully backwards compatible. Now there are three new functions:
 - addLocalizedLevelName(QPair<QByteArray, QString> level): with this you can add an level to the dialog. If the list of level isn't empty above the tabwidget is a QComboBox shown, where you can choose the level.

 - addLocalizedLevelNames(QMap<QByteArray, QString> level): nearly the same like the function above, but you can add more than one level at once.

 - setLevelName: to read or write the config belonging to your level you need to set the current level name. In the rc-file of your application the groupname is changed from KHightScore_GameDifficulty to KHighScore_GameDifficulty_MAP:levelname.


Diffs (updated)
-----

  trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h 1029697
  trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp 1029697

Diff: http://reviewboard.kde.org/r/1743/diff


Testing
-------


Screenshots
-----------

new KScoreDialog with level selection
  http://reviewboard.kde.org/r/1743/s/214/


Thanks,

Felix

_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

Re: Review Request: KScoreDialog with level selection

by Bugzilla from majewsky@gmx.net :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Am Freitag 02 Oktober 2009 09:22:34 schrieb Dmitry Suzdalev:
> Actually I'm not sure if this really should be added to KScoreDialog.
>
> It seems to me that if every game which has a slightly different layout of
>  its highscore system would patch libkdegames, we would end up with a mess
>  in API and in implementation.
>
> What do others think?

From what Felix said (he's sitting next to me currently), people have already
commented on his blog article that they would also like to use this
functionality. Also for me, separating highscores by level name opens the
possibility to use KScoreDialog in Palapeli.

Of course, we need to keep a balance between commonly needed functionality and
feature bloat. Highscores per level do IMO fall in the former category.

> And also is this  change absolutely needed to be in KScoreDialog and why?

If it was a separate KScoreWidget, which is only embedded inside a
KScoreDialog, there could have been a KScoreDialogWithLevels, which holds
different KScoreWidgets for each level. I cannot judge from here whether it is
possible to rewrite KScoreDialog in such a way without BC problems. Also, this
feels like it would bloat the code even more than the proposed patch.

Greetings
Stefan


_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

signature.asc (204 bytes) Download Attachment

Re: Review Request: KScoreDialog with level selection

by Bugzilla from schwarzerf@gmail.com :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


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


just another typo


trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp
<http://reviewboard.kde.org/r/1743/#comment1856>

    chossing -> choosing :)


- Frederik


On 2009-10-02 09:25:32, Felix Lemke wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1743/
> -----------------------------------------------------------
>
> (Updated 2009-10-02 09:25:32)
>
>
> Review request for KDE Games.
>
>
> Summary
> -------
>
> Hi,
>
> in KSpiral I need a different height-score for each level. For this I've patched the KScoreDialog. I've tested the KScoreDialog in different games (like kdiamond, kbounce, knetwalk) and I think it's fully backwards compatible. Now there are three new functions:
>  - addLocalizedLevelName(QPair<QByteArray, QString> level): with this you can add an level to the dialog. If the list of level isn't empty above the tabwidget is a QComboBox shown, where you can choose the level.
>
>  - addLocalizedLevelNames(QMap<QByteArray, QString> level): nearly the same like the function above, but you can add more than one level at once.
>
>  - setLevelName: to read or write the config belonging to your level you need to set the current level name. In the rc-file of your application the groupname is changed from KHightScore_GameDifficulty to KHighScore_GameDifficulty_MAP:levelname.
>
>
> Diffs
> -----
>
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h 1029697
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp 1029697
>
> Diff: http://reviewboard.kde.org/r/1743/diff
>
>
> Testing
> -------
>
>
> Screenshots
> -----------
>
> new KScoreDialog with level selection
>   http://reviewboard.kde.org/r/1743/s/214/
>
>
> Thanks,
>
> Felix
>
>

_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

Re: Review Request: KScoreDialog with level selection

by Dmitry Suzdalev-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Friday 02 October 2009 13:26:21 Stefan Majewsky wrote:

> Am Freitag 02 Oktober 2009 09:22:34 schrieb Dmitry Suzdalev:
> > Actually I'm not sure if this really should be added to KScoreDialog.
> >
> > It seems to me that if every game which has a slightly different layout
> > of its highscore system would patch libkdegames, we would end up with a
> > mess in API and in implementation.
> >
> > What do others think?
>
> From what Felix said (he's sitting next to me currently), people have
>  already commented on his blog article that they would also like to use
>  this functionality. Also for me, separating highscores by level name opens
>  the possibility to use KScoreDialog in Palapeli.

I've read Felix's blog and also thought a bit more about this.

Guys in the blog comments say that this can be useful for Level packs support.
As I'm currently adding a level packs support to KAtomic, I could potentially
use this too (though KAtomic doesn't use a KScoreDialog atm).

But in this case the API added by Felix needs to be generalized to some more
common concept.

And this would require a very different implementation.
Because LevelPacks consist of separate levels. So highscores are for each
level in pack. So layout of widgets in score dialog will differ and unerlying
implementation must differ.
KGoldRunner and KAtomic are examples of games with such highscore structure.
Both of them don't use KScoreDialog because currently it doesn't fit their
model.

We discussed this once on this list and all decided that our highscore system
would need some reimplementation. Back then we gathered all suggestions on a
wiki page. *searches*. here:
http://techbase.kde.org/Projects/Games/Ideas#Reworked_highscore_system

But this never got implemented because no one had found a time for this :)

Looking at Felix's patch it looks more like some "hack to make this work with
current KScoreDialog implementation". It doesn't look like a clean solution or
clean extension from my POV :)

That's why I suggested inheriting dialog inside kspiral's sources and
introduce this "hacky" part there - to keep KScoreDialog's implementation
(relatively) simple...

Another option is what you've suggested: to implement an additional
KScoreDialogWithLevels perhaps by factoring out some common code in some
baseclass/mixin.

I also must admit that this is my opinion and KScoreDialog has a maintainer,
and also we have some other devs here, so I think i'm not the one to judje,
just expressing my thoughts.

I don't want to stop the Felix's initiative, I just want to find a right place
for it :)

Cheers,
Dmitry.
_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

Re: Review Request: KScoreDialog with level selection

by Felix Lemke :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


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

(Updated 2009-10-02 10:46:10.989895)


Review request for KDE Games.


Changes
-------

chossing->choosing


Summary
-------

Hi,

in KSpiral I need a different height-score for each level. For this I've patched the KScoreDialog. I've tested the KScoreDialog in different games (like kdiamond, kbounce, knetwalk) and I think it's fully backwards compatible. Now there are three new functions:
 - addLocalizedLevelName(QPair<QByteArray, QString> level): with this you can add an level to the dialog. If the list of level isn't empty above the tabwidget is a QComboBox shown, where you can choose the level.

 - addLocalizedLevelNames(QMap<QByteArray, QString> level): nearly the same like the function above, but you can add more than one level at once.

 - setLevelName: to read or write the config belonging to your level you need to set the current level name. In the rc-file of your application the groupname is changed from KHightScore_GameDifficulty to KHighScore_GameDifficulty_MAP:levelname.


Diffs (updated)
-----

  trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h 1029697
  trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp 1029697

Diff: http://reviewboard.kde.org/r/1743/diff


Testing
-------


Screenshots
-----------

new KScoreDialog with level selection
  http://reviewboard.kde.org/r/1743/s/214/


Thanks,

Felix

_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

Re: Review Request: KScoreDialog with level selection

by Parker Coates :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


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


On Fri, Oct 2, 2009 at 03:22, Dmitry Suzdalev wrote:
> Actually I'm not sure if this really should be added to KScoreDialog.
>
> It seems to me that if every game which has a slightly different layout of its
> highscore system would patch libkdegames, we would end up with a mess in API
> and in implementation.
>
> What do others think?
>
> And also is this  change absolutely needed to be in KScoreDialog and why?

Tsk, tsk, Dmitry! Review board comments are supposed to stay on the ReviewBoard! :D

As you might remember, a while back Matt Williams and I started work on a replacement for KScoreDialog. The idea was to split the implementation into a score loading, saving, sorting, etc. backend (KScoreManager) which would be flexible enough to handle several different types of scores. The score GUIs would be provided separately and largely interchangable. Also if a game had very particular needs, it wouldn't be too much effort to write an entirely new GUI for it.

That was the idea. Unfortunately, it quickly died as we both moved on to other things. Matt did most of the work on the original API, and I made a first run at an implementation. Neither are near final, but if I remember correctly, I got it to the point where the functionality roughly matched that of KScoreDialog.

In this highscore utopia we were planning, KScoreDialog would die an slow death in unused obscurity. Unfortunately that utopia never came, so we're force to find a practical solution. Personally, I don't this patch is a great solution, but I can't really think of a better way to implement this into KScoreDialog.

What do you think KSpiral should do instead?

- Parker


On 2009-10-02 10:46:10, Felix Lemke wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1743/
> -----------------------------------------------------------
>
> (Updated 2009-10-02 10:46:10)
>
>
> Review request for KDE Games.
>
>
> Summary
> -------
>
> Hi,
>
> in KSpiral I need a different height-score for each level. For this I've patched the KScoreDialog. I've tested the KScoreDialog in different games (like kdiamond, kbounce, knetwalk) and I think it's fully backwards compatible. Now there are three new functions:
>  - addLocalizedLevelName(QPair<QByteArray, QString> level): with this you can add an level to the dialog. If the list of level isn't empty above the tabwidget is a QComboBox shown, where you can choose the level.
>
>  - addLocalizedLevelNames(QMap<QByteArray, QString> level): nearly the same like the function above, but you can add more than one level at once.
>
>  - setLevelName: to read or write the config belonging to your level you need to set the current level name. In the rc-file of your application the groupname is changed from KHightScore_GameDifficulty to KHighScore_GameDifficulty_MAP:levelname.
>
>
> Diffs
> -----
>
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h 1029697
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp 1029697
>
> Diff: http://reviewboard.kde.org/r/1743/diff
>
>
> Testing
> -------
>
>
> Screenshots
> -----------
>
> new KScoreDialog with level selection
>   http://reviewboard.kde.org/r/1743/s/214/
>
>
> Thanks,
>
> Felix
>
>

_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

Re: Review Request: KScoreDialog with level selection

by Albert Astals Cid-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


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



trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h
<http://reviewboard.kde.org/r/1743/#comment1858>

    Do you really think it's good that setLevelName calls addLocalizedLevelName? For me "Set" only "sets" not "setsAndAdds", so if you do it, you can either remove it (if i convince you) or add it to the documentation so people like me knows what is happening


- Albert


On 2009-10-02 10:46:10, Felix Lemke wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1743/
> -----------------------------------------------------------
>
> (Updated 2009-10-02 10:46:10)
>
>
> Review request for KDE Games.
>
>
> Summary
> -------
>
> Hi,
>
> in KSpiral I need a different height-score for each level. For this I've patched the KScoreDialog. I've tested the KScoreDialog in different games (like kdiamond, kbounce, knetwalk) and I think it's fully backwards compatible. Now there are three new functions:
>  - addLocalizedLevelName(QPair<QByteArray, QString> level): with this you can add an level to the dialog. If the list of level isn't empty above the tabwidget is a QComboBox shown, where you can choose the level.
>
>  - addLocalizedLevelNames(QMap<QByteArray, QString> level): nearly the same like the function above, but you can add more than one level at once.
>
>  - setLevelName: to read or write the config belonging to your level you need to set the current level name. In the rc-file of your application the groupname is changed from KHightScore_GameDifficulty to KHighScore_GameDifficulty_MAP:levelname.
>
>
> Diffs
> -----
>
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h 1029697
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp 1029697
>
> Diff: http://reviewboard.kde.org/r/1743/diff
>
>
> Testing
> -------
>
>
> Screenshots
> -----------
>
> new KScoreDialog with level selection
>   http://reviewboard.kde.org/r/1743/s/214/
>
>
> Thanks,
>
> Felix
>
>

_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel

Re: Review Request: KScoreDialog with level selection

by Felix Lemke :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message



> On 2009-10-02 17:32:48, Albert Astals Cid wrote:
> > trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h, line 200
> > <http://reviewboard.kde.org/r/1743/diff/4/?file=12266#file12266line200>
> >
> >     Do you really think it's good that setLevelName calls addLocalizedLevelName? For me "Set" only "sets" not "setsAndAdds", so if you do it, you can either remove it (if i convince you) or add it to the documentation so people like me knows what is happening

You're right. But it is similar to the call of "setConfigGroup". But it is important to explain it in the documentation. I'll change it (Perhaps you have a proposal for the phrase). You see my English is not the best.


- Felix


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


On 2009-10-02 10:46:10, Felix Lemke wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1743/
> -----------------------------------------------------------
>
> (Updated 2009-10-02 10:46:10)
>
>
> Review request for KDE Games.
>
>
> Summary
> -------
>
> Hi,
>
> in KSpiral I need a different height-score for each level. For this I've patched the KScoreDialog. I've tested the KScoreDialog in different games (like kdiamond, kbounce, knetwalk) and I think it's fully backwards compatible. Now there are three new functions:
>  - addLocalizedLevelName(QPair<QByteArray, QString> level): with this you can add an level to the dialog. If the list of level isn't empty above the tabwidget is a QComboBox shown, where you can choose the level.
>
>  - addLocalizedLevelNames(QMap<QByteArray, QString> level): nearly the same like the function above, but you can add more than one level at once.
>
>  - setLevelName: to read or write the config belonging to your level you need to set the current level name. In the rc-file of your application the groupname is changed from KHightScore_GameDifficulty to KHighScore_GameDifficulty_MAP:levelname.
>
>
> Diffs
> -----
>
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.h 1029697
>   trunk/KDE/kdegames/libkdegames/highscore/kscoredialog.cpp 1029697
>
> Diff: http://reviewboard.kde.org/r/1743/diff
>
>
> Testing
> -------
>
>
> Screenshots
> -----------
>
> new KScoreDialog with level selection
>   http://reviewboard.kde.org/r/1743/s/214/
>
>
> Thanks,
>
> Felix
>
>

_______________________________________________
kde-games-devel mailing list
kde-games-devel@...
https://mail.kde.org/mailman/listinfo/kde-games-devel