MegaGlest Forum

MegaGlest => Feature requests => Closed feature requests => Topic started by: ctz on 16 September 2013, 06:52:35

Title: Include version string in IRC real name
Post by: ctz on 16 September 2013, 06:52:35

It is not possible to easily determine the MegaGlest version of a player with the name MG_name_### in the #megaglest-lobby channel.  As a result, one may possibly ask to play a game, only to find that the versions are incompatible.

Currently, this is the result of a /whois on a MegaGlest client:

Code: [Select]

/whois MG_lyra_68
MG_lyra_68 [~MG_lyra_6@(host)]
realname : megaglest
channels : #megaglest-lobby
[...]
The current "real name" of all MegaGlest clients is "megaglest".  Adding the version string to the IRC "real name" (e.g. so that it is "megaglest 3.8.0-beta1" in the case of the current beta version) may make it easier to determine a player's MG version--just do a /whois.

It's not possible to do a /whois from within MegaGlest, but this might still be useful to people who sit around in the channel with their IRC clients.

Implementation of this should require adding the version string to the last argument of irc_connect in source/shared_lib/sources/platform/posix/ircclient.cpp lines 790 and 1001.

Title: Re: Include version string in IRC real name
Post by: tomreyn on 16 September 2013, 17:06:14

I'd like to see version numbers in the realname field, too.

FreeNode runs ircd-seven, their fork or the charybdis IRC server. According to ircd-seven's ircd_defs.h (https://dev.freenode.net/redmine/projects/ircd-seven/repository/revisions/master/entry/include/ircd_defs.h#L96) (compare to charydbis ircd_defs.h (https://github.com/atheme/charybdis/blob/master/include/ircd_defs.h#L70)) the maximum realname length is 50 characters.

This is definitely enough to squeeze in both the game version, and most likely the compiler, too.

Code: [Select]

$ echo -n '|v12.12.12-beta12|GNUC: 40403 [64bit]|' | wc -c
38

It may even suffice to also add the build time (and date) stamp, but it may be necessary to format this as a Unix epoch to save space.

Code: [Select]

$ echo -n '|v12.12.12-beta12|GNUC: 40403 [64bit]|Jun 25 2013 22:02:29|' | wc -c
59

$ echo -n '|v12.12.12-beta12|GNUC: 40403 [64bit]|1372190549|' | wc -c
49

We need to be careful about the compiler string, though, since it can vary much, so maybe it'd be better to put this last, i.e. to have |VERSION|BUILD TIMESTAMP|COMPILER| as in

Code: [Select]

|v12.12.12-beta12|1372190549|GNUC: 40403 [64bit]|.

I have a feeling I brought this suggestion up with softcoder before, though, and that he said this would require patching the 3rd party library we depend on for the IRC code (and maintaining those patches) which he'd prefer not to. It would be good to have this repeated or corrected here because my memory is flaky.

Another option would be to replace the lobby group chat by one run by either the clients themselves (peer-to-peer, but this involves NAT issues, so rather not), or XMPP (https://en.wikipedia.org/wiki/XMPP) (AKA "Jabber", a more modern protocol than IRC) with the MUC (http://xmpp.org/extensions/xep-0045.html) (multi-user chat) extension. I would think that libpurple provides both.

It could also be something running on the masterserver, maybe something HTTP like/compatible. Ideally the transport layer should be push (https://en.wikipedia.org/wiki/Push_technology), not pull though, such as websocket (https://en.wikipedia.org/wiki/WebSocket) (and maybe 'activity streams' for the application/message protocol). This, however, would add plenty of new client (game) dependencies (https://en.wikipedia.org/wiki/Comparison_of_WebSocket_implementations) which aren't nice either.

The upside of both XMPP/MUC (over BOSH or websocket) or some other application protocol based on websocket would be that it's usually easy to connect to it from a web application (yes there are also web clients for IRC but those are technologically awful).

Yet another option for push-style group chats is SIMPLE (https://en.wikipedia.org/wiki/SIMPLE) (based on SIP), but I think this is actually a misnomer.

If we should ever shift to another protocol for the lobby chat (I don't really see this happening any time soon due to the extra dependencies it would introduce and the limited improvements it would provide), we should think about whether, and if so, how this should also be used during the game setup phase and / or in-game.

Back on topic: For now I guess we need to keep it simple, which may mean not having additional client information in the realname field.

Title: Re: Include version string in IRC real name
Post by: ctz on 19 September 2013, 03:46:35

I implemented this.  No need to patch a 3rd party library.

Here is the result of a whois:

Code: [Select]

/whois MG_lyra_297
MG_lyra_297 [~MG_lyra_2@(host)]
realname : MegaGlestv3.8.0-dev|r4548M|Linux-X64|GNUC: 40703 [
channels : #megaglest-lobby
[...]
The realname includes the version, SVN revision, platform, and compiler.  As you can see, the compiler string is incomplete because didn't fit in 50 characters.  Some space could be saved by removing "MegaGlest" I suppose.

Here is a diff of the changes I made:

(click to show/hide)

Code: [Select]

Index: source/glest_game/menu/menu_state_masterserver.cpp
===================================================================
--- source/glest_game/menu/menu_state_masterserver.cpp (revision 4548)
+++ source/glest_game/menu/menu_state_masterserver.cpp (working copy)
@@ -305,6 +305,13 @@
     else {
      ircArgs.push_back("");
     }
+#define REALNAMEMAXLEN 50
+    string IRCrealname =
+        "MegaGlest" +
+        glestVersionString + "|r"
+        + getSVNRevisionString().substr(5) /* skip "Rev: " */
+        + "|" + getPlatformNameString() + "|" + getCompilerNameString();
+    ircArgs.push_back(IRCrealname.substr(0,REALNAMEMAXLEN));
 
     MutexSafeWrapper safeMutexIRCPtr(&mutexIRCClient,string(extractFileFromDirectoryPath(__FILE__).c_str()) + "_" + intToStr(__LINE__));
 
Index: source/shared_lib/include/platform/posix/ircclient.h
===================================================================
--- source/shared_lib/include/platform/posix/ircclient.h (revision 4548)
+++ source/shared_lib/include/platform/posix/ircclient.h (working copy)
@@ -67,8 +67,8 @@
     string username;
  string channel;
  string nick;
-
  string playerName;
+ string realname;
 
     bool hasJoinedChannel;
 
@@ -118,6 +118,7 @@
 
  string getChannel() const { return channel;}
  string getNick() const { return nick;}
+ string getRealname() const { return realname;}
 
  string getExecute_cmd_onconnect() const { return execute_cmd_onconnect; }
  void setExecute_cmd_onconnect(string value) { execute_cmd_onconnect = value; }
Index: source/shared_lib/sources/platform/posix/ircclient.cpp
===================================================================
--- source/shared_lib/sources/platform/posix/ircclient.cpp (revision 4548)
+++ source/shared_lib/sources/platform/posix/ircclient.cpp (working copy)
@@ -526,6 +526,7 @@
     lastNickListUpdate = time(NULL);
     wantToLeaveChannel = false;
     playerName = "";
+    realname = "megaglest";
 }
 
 void IRCThread::disconnect() {
@@ -720,7 +721,7 @@
          ircSession=NULL;
          safeMutex.ReleaseLock(true);
 
-            if(argv.size() != 5) {
+            if(argv.size() != 6) {
                 if(SystemFlags::VERBOSE_MODE_ENABLED || IRCThread::debugEnabled) printf ("===> IRC Usage: <server> <nick> <channel> : got params [%ld]\n",(long int)argv.size());
                 return;
             }
@@ -772,6 +773,9 @@
 //            this->password = argv[4];
 //            }
             this->username = argv[1];
+            if (argv.size() >= 6) {
+                this->realname = argv[5];
+            }
             if(argv.size() >= 4) {
              this->username = argv[3];
             }
@@ -787,7 +791,7 @@
             }
 
             safeMutex.Lock();
-            if(irc_connect(ircSession, argv[0].c_str(), IRC_SERVER_PORT, 0, this->nick.c_str(), this->username.c_str(), "megaglest")) {
+            if(irc_connect(ircSession, argv[0].c_str(), IRC_SERVER_PORT, 0, this->nick.c_str(), this->username.c_str(), this->realname.c_str())) {
              safeMutex.ReleaseLock();
                 if(SystemFlags::VERBOSE_MODE_ENABLED || IRCThread::debugEnabled) printf ("===> IRC Could not connect: %s\n", irc_strerror (irc_errno(ircSession)));
                 return;
@@ -998,7 +1002,7 @@
  if(connectRequired == false) {
 #if !defined(DISABLE_IRCCLIENT)
  MutexSafeWrapper safeMutex1(&mutexIRCSession,string(__FILE__) + "_" + intToStr(__LINE__));
-        if(irc_connect(ircSession, argv[0].c_str(), IRC_SERVER_PORT, 0, this->nick.c_str(), this->username.c_str(), "megaglest")) {
+        if(irc_connect(ircSession, argv[0].c_str(), IRC_SERVER_PORT, 0, this->nick.c_str(), this->username.c_str(), this->realname.c_str())) {
             if(SystemFlags::VERBOSE_MODE_ENABLED || IRCThread::debugEnabled) printf ("===> IRC Could not connect: %s\n", irc_strerror (irc_errno(ircSession)));
             return;
         }


Title: Re: Include version string in IRC real name
Post by: tomreyn on 2 October 2013, 17:45:16

Personally I think it would be better to have the version string on a separate field, or combined with the svn revision, i.e. one of these:

Code: [Select]

MegaGlest|v3.8.0-dev|r4548M|Linux-X64|GNUC: 40703
MegaGlest|v3.8.0-dev+r4548M|Linux-X64|GNUC: 40703

Other than that I like this change (but cannot comment on the patch). If merged, it would also provide the foundation for removing the MG_ prefix (https://forum.megaglest.org/index.php?topic=9121.0) off the nickname field.

There is one more field we can play with, which is the IRC username (~ + 9 characters). Currently this is just a (usually cut off) copy of the nickname. Instead, this could contain the random numbers which are currently part of the nickname. Or, maybe better, it could contain the MegaGlest (exactly 9 characters) identifier, allowing for more usable space in the real name field (which could then start with the 1 to 3 character sized random number).

That's a lot of back and forth, so here's what I think would be best:

username: ~MegaGlest (fixed)
realname: random number {1-3} + | + version string (variable length) + | + platform string (variable length) + | + compiler string (variable length)
nickname: player name (variable length)

realname and nickname may be cut off, depending on the total length of its elements.

Example:
username: ~MegaGlest
realname: 123|v3.8.0-dev|r4548M|Linux-X64|GNUC: 40703
nickname: Tom_Reynolds

Edit:
Hmm, nope, this doesn' work either, since we need unique nicknames not just amongst our userbase but also amongst all of the Freenode network. If someone will use a simple player name like Tom then it's pretty certain this is already taken by someone else on Freenode. So we can't get rid og the MG_ nickname prefix, but this doesn't impact the patch discussed here at all.

Title: Re: Include version string in IRC real name
Post by: titi on 29 October 2013, 09:48:03

MG_ is used to seperate "normal" irc users from users inside the MG lobby.
So all in all I like Lyras way the best for the moment.

Title: Re: Include version string in IRC real name
Post by: andy_5995 on 15 March 2017, 20:41:03

A good suggestion. This was recently implemented by titi, post v3.13.0 release

Closing.,,