[Kde-games-devel] Review of KF5 ports: KJumpingCube, KGoldrunner and Kubrick

[laurent Montel]

Le Sunday 18 January 2015 17:26:08 Ian Wadham a écrit :
> Hi Laurent and others,

Hi,

> I have walked through all the KF5 changes to KJumpingCube,
> KGoldrunner and Kubrick and (almost) everything looks fine… :-)
> That is to say, I think the functionality of the games will be the same
> as before, except perhaps for a few worries.  I am the author or
> co-author of those three games.
> 
> As you know, I am unable to build and test the games in KF5 yet.
> 
> KJumpingCube port to KF5:
> --------------------------------------
> 
> Just one query: in file kjumpingcube.cpp, around lines 131-135, this change
> appears:
> 
> -   KAction * b = actionCollection()->addAction (QLatin1String
> ("action_button")); -   b->setDefaultWidget (actionButton);	// Show the
> button on the toolbar. +   QAction * b = actionCollection()->addAction
> (QLatin1String ("action_button")); +   //QT5 b->setDefaultWidget
> (actionButton);	// Show the button on the toolbar.

Normal it's not ported yet. As you see there is “//QT5” to inform that we (me) 
need to port to QT5 api.

> 
> The last line is commented out, but equivalent code may still be needed,
> in KF5, for making sure that actionButton, a specially-styled QPushButton,
> is shown on the toolbar.
> 
> Using setDefaultWidget() works in KDE 4 because KAction inherits
> QWidgetAction, which contains that method.  The preceding 15 lines of my
> code show the setup of the actionButton widget.  Maybe use QWidgetAction,
> instead of QAction here?
> 
> KGoldrunner port to KF5:
> ----------------------------------
> 
> Thanks for commenting out the many (now unnecessary) debugging messages as
> you ported them from kDebug() to qDebug()… :-)  I have just five queries...
> 
> 1. Why is DBus support added (main.cpp)?

Because it was created by kapplication/kaboutdata in kde4 => same feature as 
in kde4

> KGoldrunner does not use DBus
> directly. 
> 2. In file src/kgoldrunner.cpp, some code is #iffed out around
> line 240.  That would prevent Key_Escape being used as an alternate
> Pause/Resume key.  Esc has been available in KGr from the very first and in
> its ancestor game since the 1980s.

Not ported "//QT5"

> 3. In file src/kgoldrunner.cpp, some
> code is #iffed out around line 720.

//QT5


> This is needed to translate the names
> of keys that appear in the Pause/Resume message in the game's status bar (a
> fix I included in 2008, at Albert's request IIRC).
> 4. The method
> KGoldrunner::getDirectories() in file kgoldrunner.cpp is archaic code for
> getting both read-only and writable (local) data locations, where all the
> games, levels and other data files are stored.  After it runs, the paths
> are passed everywhere they are needed, via the QString variables called
> systemDataDir (read-only dir) and userDataDir (writable dir).  This code
> should be using QStandardPaths. 

Yes because there is still kdelibs4support.

> Maybe something like:
>         systemDataDir =
> QStandardPaths::locate(QStandardPaths::AppDataLocation, "system",
> QStandardPaths::LocateDirectory); userDataDir =
> QStandardPaths::writableLocation(QStandardPaths::AppDataLocation) +
> "/user/"; 


> 5. When systemDataDir is found, it will need a "/" to be
> appended.  If userDataDir is not found or it has a name but does not exist
> yet, it will need to be created.  So will .../user/levels.  The rest of
> KGoldrunner needs all that to be in place before the game or game editor
> can run.
> 
> Kubrick port to KF5:
> --------------------------
> 
> Thanks for putting in the qCDebug() code to log errors, should they occur. 
> The ported code is looking very nice.  I have just three queries...
> 
> 1. The code following line 64 in file src/gameglview.cpp does not look
> right.  Should it be using QStandardPaths?

Because there is still kdelibs4support.

> If it succeeds, Kubrick should
> have wavy lines in its background.  If it fails, Kubrick will put a 4-way
> gradient in the background and it will appear as if the game is working
> correctly.
> 2. In the file kubrick.cpp, several lines that should put messages in
> statusBar() are #iffed out.

//QT5

> They need to appear.  The texts are drawn from
> const structures and arrays which appear around line 100 and after.
> 3. The said consts have I18N_NOOP around the C-string literals.  Is that
> valid in KF5?
> 
> Thanks again for all your help on these games, Laurent.
> 
> Best regards, Ian W.

I told that I make it compile not that they were ready to merge because there 
is still kdelibs4support and some commented code which needs to be convert to 
QT5 api :)

Regards.


-- 
Laurent Montel | laurent.montel at kdab.com | KDE/Qt Senior Software Engineer
KDAB (France) S.A.S., a KDAB Group company
Tel. France +33 (0)4 90 84 08 53,  http://www.kdab.fr