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

Proposed by Notabilis
Status: Merged
Merged at revision: 9148
Proposed branch: lp:~widelands-dev/widelands/bug-1826744-lobby-commands
Merge into: lp:widelands
Diff against target: 93 lines (+40/-2)
3 files modified
src/network/internet_gaming.cc (+30/-2)
src/network/internet_gaming_messages.cc (+1/-0)
src/network/internet_gaming_protocol.h (+9/-0)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1826744-lobby-commands
Reviewer Review Type Date Requested Status
GunChleoc Approve
Toni Förster Approve
Review via email: mp+368285@code.launchpad.net

Commit message

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

Description of the change

/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.

To post a comment you must log in.
9141. By Notabilis

Fixed mistake in previous commit.

Revision history for this message
Toni Förster (stonerl) wrote :

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)?

Revision history for this message
bunnybot (widelandsofficial) wrote :

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.

Revision history for this message
Notabilis (notabilis27) wrote :

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?

Revision history for this message
Toni Förster (stonerl) wrote :

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 :)

9142. By Notabilis

Changed /announced to /announce.

Revision history for this message
Notabilis (notabilis27) wrote :

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

Revision history for this message
bunnybot (widelandsofficial) wrote :

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.

Revision history for this message
GunChleoc (gunchleoc) wrote :

1 small string nit.

9143. By Notabilis

Reduced doubled space to single space.

9144. By Notabilis

"Implemented" /ban command.

Revision history for this message
Notabilis (notabilis27) wrote :

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).

Revision history for this message
bunnybot (widelandsofficial) wrote :

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.

Revision history for this message
Toni Förster (stonerl) wrote :

LGTM :)

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

review: Approve
Revision history for this message
Notabilis (notabilis27) wrote :

The related metaserver branch is now deployed.

Revision history for this message
Toni Förster (stonerl) wrote :

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

review: Needs Fixing
Revision history for this message
Toni Förster (stonerl) wrote :

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

Revision history for this message
GunChleoc (gunchleoc) wrote :

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.

Revision history for this message
Toni Förster (stonerl) wrote :

I'm happy :)

@bunnybot merge

review: Approve
Revision history for this message
Notabilis (notabilis27) wrote :

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.

Revision history for this message
GunChleoc (gunchleoc) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
682 }682 }
683 }683 }
684684
685 else if (subcmd == IGPCMD_CMD) {
686 // Something went wrong with the command
687 message += _("Command could not be executed.");
688 message =
689 (boost::format("%s %s") % message % InternetGamingMessages::get_message(reason))
690 .str();
691 }
692
685 else if (subcmd == IGPCMD_GAME_OPEN) {693 else if (subcmd == IGPCMD_GAME_OPEN) {
686 // Something went wrong with the newly opened game694 // Something went wrong with the newly opened game
687 message = InternetGamingMessages::get_message(reason);695 message = InternetGamingMessages::get_message(reason);
@@ -901,6 +909,16 @@
901 // beginning909 // beginning
902 // with a "/" - let's see...910 // with a "/" - let's see...
903911
912 if (msg == "/help") {
913 format_and_add_chat("", "", true, _("Supported admin commands:"));
914 format_and_add_chat("", "", true, _("/motd <msg> sets a permanent greeting message"));
915 format_and_add_chat("", "", true, _("/announce <msg> send a one time system message"));
916 format_and_add_chat("", "", true, _("/warn <user> <msg> send a private system message to the given user"));
917 format_and_add_chat("", "", true, _("/kick <user|game> removes the given user or game from the metaserver"));
918 format_and_add_chat("", "", true, _("/ban <user> bans a user for 24 hours from the metaserver"));
919 return;
920 }
921
904 // Split up in "cmd" "arg"922 // Split up in "cmd" "arg"
905 std::string cmd, arg;923 std::string cmd, arg;
906 std::string temp = msg.substr(1); // cut off '/'924 std::string temp = msg.substr(1); // cut off '/'
@@ -937,13 +955,23 @@
937 m.string(arg);955 m.string(arg);
938 net->send(m);956 net->send(m);
939 return;957 return;
940 } else if (!arg.empty() && cmd == "announcement") {958 } else if (!arg.empty() && cmd == "announce") {
941 // send the request to change the motd959 // send the request to make an announcement
942 SendPacket m;960 SendPacket m;
943 m.string(IGPCMD_ANNOUNCEMENT);961 m.string(IGPCMD_ANNOUNCEMENT);
944 m.string(arg);962 m.string(arg);
945 net->send(m);963 net->send(m);
946 return;964 return;
965 } else if (!arg.empty() && (cmd == "warn" || cmd == "kick" || cmd == "ban")) {
966 // warn a user by sending a private system message or
967 // kick a user (for 5 minutes) or a game from the metaserver or
968 // ban a user for 24 hours
969 SendPacket m;
970 m.string(IGPCMD_CMD);
971 m.string(cmd);
972 m.string(arg);
973 net->send(m);
974 return;
947 } else {975 } else {
948 // let everything else pass976 // let everything else pass
949 goto normal;977 goto normal;
950978
=== 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 @@
43 igmessages["NO_SUCH_USER"] = _("There is no user with this name logged in.");43 igmessages["NO_SUCH_USER"] = _("There is no user with this name logged in.");
44 igmessages["NO_SUCH_GAME"] = _("The game no longer exists, maybe it has just been closed.");44 igmessages["NO_SUCH_GAME"] = _("The game no longer exists, maybe it has just been closed.");
45 igmessages["WRONG_PASSWORD"] = _("Wrong password, please try again.");45 igmessages["WRONG_PASSWORD"] = _("Wrong password, please try again.");
46 igmessages["BANNED"] = _("You have been temporarily banned from online gaming.");
46 igmessages["UNSUPPORTED_PROTOCOL"] = _("The protocol version you are using is not supported!");47 igmessages["UNSUPPORTED_PROTOCOL"] = _("The protocol version you are using is not supported!");
47 igmessages["ALREADY_LOGGED_IN"] = _("You are already logged in!");48 igmessages["ALREADY_LOGGED_IN"] = _("You are already logged in!");
48 igmessages["DEFICIENT_PERMISSION"] =49 igmessages["DEFICIENT_PERMISSION"] =
4950
=== 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 @@
299static const std::string IGPCMD_CHAT = "CHAT";299static const std::string IGPCMD_CHAT = "CHAT";
300300
301/**301/**
302 * Sent by the client to issue a superuser command.
303 *
304 * The client sends this message to the metaserver with the following payload:
305 * \li string: the command
306 * \li string: arbitrary parameters.
307 */
308static const std::string IGPCMD_CMD = "CMD";
309
310/**
302 * Sent by the metaserver to inform the client, that the list of games was changed. No payload is311 * Sent by the metaserver to inform the client, that the list of games was changed. No payload is
303 * sent,312 * sent,
304 * as e.g. clients in a game are not really interested about other games and we want to keep traffic313 * as e.g. clients in a game are not really interested about other games and we want to keep traffic

Subscribers

People subscribed via source and target branches

to status/vote changes: