Mostly simple changes like suppressing includes but also a change about socket classes
Details
- Reviewers
nhirsl mlaurent - Group Reviewers
KDE Games - Commits
- R413:ed7861ed8600: Finish removing dependance to KDElibs4Support
Diff Detail
- Repository
- R413 KsirK
- Branch
- nokdelibs4support (branched from frameworks)
- Lint
No Linters Available - Unit
No Unit Test Coverage
At this point, my first attempt to remove kdelibs4support was almost the same. However, I haven't submitted the change. There are two main reasons for this:
- I haven't tested xmpp part of the game, even everything else worked good
- Irislib and jabber part are outdated and there are newer versions available
To cover first item I was going to start new email thread (still saved in the draft), but since we have the review, I'll discuss it here.
Testing xmpp part
Is there anything special needed for setting up xmpp game?
For this purpose I asked our sysadmins and promptly got two accounts on kdetalk (thanks to Ben!):
- ksirk-dev1@kdetalk.net - password 'f59ArDehbEjgvhit'
- ksirk-dev2@kdetalk.net - password 'C9cbf3iMxSR8jqKn'
However I couldn't setup a game. Unfortunately, didn't have time to dive deeper and find root cause.
New iris and jabber library
In the meantime, I pulled latest iris lib and with a few minor changes mostly in cmake files I compiled it and integrated into ksirk. Changes in Jabber part (from Kopete) and the rest of ksirk were also aligned and minimal. Cutestuff also changed location within the iris lib which makes diff almost unusable.
I do have all these changes in the local branch, but not tested. In this case I can verify that connecting to kdetalk.net succeeds, but the first next thing - getting roster list fails. Again (unfortunately), not sure when I can continue investigation in this direction.
Now the question is how to proceed?
Gael, have you tested xmpp part of the game?
If changes in this review work, I would vote for pushing them and than later upgrading iris and jabber (if at all). But I'm not sure what's the best option if we can't verify that iris and jabber changes are OK.
I must admit that I did not test the xmpp part… And in fact it does not work. There is an error in connecting a socket error signal. That should at least be corrected.
But I just tried too a Jabber based game on the ksirk currently available on KDE Neon and it does not work either. But this time the console output gives useful information.
I propose to correct the evident bug to have a situation in par with the current released version and then to push these changes before trying to make the Jabber work again with the new versions you worked on. In this case, should we remove the Jabber option from the GUI while it is not working ?
P.S.: to answer your question, there is nothing special necessary to play over Jabber except a working account
P.P.S.: the Jabber stuff was an interesting experiment but I don't know if anybody ever used it for real games… So, should we do large efforts to maintain it ?
I by error added a new commit to this review request instead of creating a new one :-(
I agree. It would be ideal to have the same behavior after removal of kdelibs4support. That would make the upgrade to newer iris and jabber libs easier.
We can remove Jabber option from GUI until we fix Jabber gameplay.
P.S.: to answer your question, there is nothing special necessary to play over Jabber except a working account
P.P.S.: the Jabber stuff was an interesting experiment but I don't know if anybody ever used it for real games… So, should we do large efforts to maintain it ?
Well, that's good question. I can't say for sure for how long, but Jabber part seems to be broken for a while. There was no single bug report for Jabber game, either.
Now, what effort should we invest here?
I wanted to upgrade to newer libs which at the end:
- Removes need for kdelibs4support
- Fixes a bunch of compile time warnings and speeds up compile time
- Aligns with upstream
Unfortunately, solely this upgrade didn't fix KsirK integration - it is still not possible to setup a game.
So, maybe we shouldn't invest that much time now, what do you think?
The plan could be:
- Remove kdelibs4support as in this review
- Fix and align behavior with current master
- Remove Jabber from GUI
- Upgrade iris and kopete parts
- Try to fix gameplay
If we can't do no.5 in a reasonable time, Jabber will be disabled (non visible from the GUI) until again someone (e.g. new group of students) decide to experiment with XMPP gameplay.
Does this sound reasonable?
- Revert to QTcpSocket
- Hides the button starting new Jabber game
Implements first steps of plan by @nhirsl. Next step is to update iris and kopete libs.