Merge lp:~widelands-dev/widelands/bug-1826744-lobby-commands into lp:widelands

[Notabilis]

Adding support for /warn and /kick commands of superusers in the internet gaming lobby.

/warn is sending a system message to a single user.
/kick is banning the IP of the user for 24 hours to avoid immediate reconnects.

[Toni Förster (stonerl)]

Could you change "/announcement" to "/announce"? That's the command for the multiplayer lobby.

Also, could you implement /help for the lobby (for users and superusers)?

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5145. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/540930580.
Appveyor build 4927. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1826744_lobby_commands-4927.

[Notabilis (notabilis27)]

I can rename /announce, no problem. But what is /help supposed to do for normal users? They only have /me as a "command". And the /help command for superusers is already part of this branch, or do you want something different there?

[Toni Förster (stonerl)]

Sorry, I was under the impression that users could use the help command as well.

The it is just /announcement command that needs to be changed :)

[Notabilis (notabilis27)]

Ah, okay. Now it makes sense. :)
The /help command visible in the diff is within an if-block that only triggers for superusers.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5151. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/541357325.
Appveyor build 4933. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1826744_lobby_commands-4933.

[GunChleoc (gunchleoc)]

1 small string nit.

[Notabilis (notabilis27)]

Thanks for the reviews. The strings are fixed now. I also "added" the requested /ban command (actually, it will simply be forwarded to the metaserver).

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5159. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/541867359.
Appveyor build 4941. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1826744_lobby_commands-4941.

[Toni Förster (stonerl)]

LGTM :)

But we need the server to be in place to test this better.

review: Approve

[Notabilis (notabilis27)]

The related metaserver branch is now deployed.

[Toni Förster (stonerl)]

Admins shouldn't be able to kick or ban themselves or other admins.

review: Needs Fixing

[Toni Förster (stonerl)]

But I guess this should be solved on the server side...

[GunChleoc (gunchleoc)]

Yes it should. Are you happy with your testing otherwise?

If so, we should merge this so it will be easier for Notabilis to continue with the needed changes on the server.

[Toni Förster (stonerl)]

I'm happy :)

@bunnybot merge

review: Approve

[Notabilis (notabilis27)]

Thanks for the reviews. A metaserver branch with the requested change is open at https://github.com/widelands/widelands_metaserver/pull/60 and already deployed for testing.

[GunChleoc (gunchleoc)]

I have found some things that still need fixing/improving - most of them on the server side. So, I have created a new Metaserver issue:

https://github.com/widelands/widelands_metaserver/issues/63

This branch can go in though :)

review: Approve

=== modified file 'src/network/internet_gaming.cc'
--- src/network/internet_gaming.cc	2019-06-02 09:29:59 +0000
+++ src/network/internet_gaming.cc	2019-06-05 17:09:01 +0000
@@ -682,6 +682,14 @@
 				}
 			}
 
+			else if (subcmd == IGPCMD_CMD) {
+				// Something went wrong with the command
+				message += _("Command could not be executed.");
+				message =
+				   (boost::format("%s %s") % message % InternetGamingMessages::get_message(reason))
+					  .str();
+			}
+
 			else if (subcmd == IGPCMD_GAME_OPEN) {
 				// Something went wrong with the newly opened game
 				message = InternetGamingMessages::get_message(reason);
@@ -901,6 +909,16 @@
 		// beginning
 		// with a "/" - let's see...
 
+		if (msg == "/help") {
+			format_and_add_chat("", "", true, _("Supported admin commands:"));
+			format_and_add_chat("", "", true, _("/motd <msg> sets a permanent greeting message"));
+			format_and_add_chat("", "", true, _("/announce <msg> send a one time system message"));
+			format_and_add_chat("", "", true, _("/warn <user> <msg> send a private system message to the given user"));
+			format_and_add_chat("", "", true, _("/kick <user|game> removes the given user or game from the metaserver"));
+			format_and_add_chat("", "", true, _("/ban <user> bans a user for 24 hours from the metaserver"));
+			return;
+		}
+
 		// Split up in "cmd" "arg"
 		std::string cmd, arg;
 		std::string temp = msg.substr(1);  // cut off '/'
@@ -937,13 +955,23 @@
 			m.string(arg);
 			net->send(m);
 			return;
-		} else if (!arg.empty() && cmd == "announcement") {
-			// send the request to change the motd
+		} else if (!arg.empty() && cmd == "announce") {
+			// send the request to make an announcement
 			SendPacket m;
 			m.string(IGPCMD_ANNOUNCEMENT);
 			m.string(arg);
 			net->send(m);
 			return;
+		} else if (!arg.empty() && (cmd == "warn" || cmd == "kick" || cmd == "ban")) {
+			// warn a user by sending a private system message or
+			// kick a user (for 5 minutes) or a game from the metaserver or
+			// ban a user for 24 hours
+			SendPacket m;
+			m.string(IGPCMD_CMD);
+			m.string(cmd);
+			m.string(arg);
+			net->send(m);
+			return;
 		} else {
 			// let everything else pass
 			goto normal;

=== modified file 'src/network/internet_gaming_messages.cc'
--- src/network/internet_gaming_messages.cc	2019-05-25 08:27:20 +0000
+++ src/network/internet_gaming_messages.cc	2019-06-05 17:09:01 +0000
@@ -43,6 +43,7 @@
 	igmessages["NO_SUCH_USER"] = _("There is no user with this name logged in.");
 	igmessages["NO_SUCH_GAME"] = _("The game no longer exists, maybe it has just been closed.");
 	igmessages["WRONG_PASSWORD"] = _("Wrong password, please try again.");
+	igmessages["BANNED"] = _("You have been temporarily banned from online gaming.");
 	igmessages["UNSUPPORTED_PROTOCOL"] = _("The protocol version you are using is not supported!");
 	igmessages["ALREADY_LOGGED_IN"] = _("You are already logged in!");
 	igmessages["DEFICIENT_PERMISSION"] =

=== modified file 'src/network/internet_gaming_protocol.h'
--- src/network/internet_gaming_protocol.h	2019-06-01 16:31:13 +0000
+++ src/network/internet_gaming_protocol.h	2019-06-05 17:09:01 +0000
@@ -299,6 +299,15 @@
 static const std::string IGPCMD_CHAT = "CHAT";
 
 /**
+ * Sent by the client to issue a superuser command.
+ *
+ * The client sends this message to the metaserver with the following payload:
+ * \li string:    the command
+ * \li string:    arbitrary parameters.
+ */
+static const std::string IGPCMD_CMD = "CMD";
+
+/**
  * Sent by the metaserver to inform the client, that the list of games was changed. No payload is
  * sent,
  * as e.g. clients in a game are not really interested about other games and we want to keep traffic