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
1=== modified file 'src/network/internet_gaming.cc'
2--- src/network/internet_gaming.cc 2019-06-02 09:29:59 +0000
3+++ src/network/internet_gaming.cc 2019-06-05 17:09:01 +0000
4@@ -682,6 +682,14 @@
5 }
6 }
7
8+ else if (subcmd == IGPCMD_CMD) {
9+ // Something went wrong with the command
10+ message += _("Command could not be executed.");
11+ message =
12+ (boost::format("%s %s") % message % InternetGamingMessages::get_message(reason))
13+ .str();
14+ }
15+
16 else if (subcmd == IGPCMD_GAME_OPEN) {
17 // Something went wrong with the newly opened game
18 message = InternetGamingMessages::get_message(reason);
19@@ -901,6 +909,16 @@
20 // beginning
21 // with a "/" - let's see...
22
23+ if (msg == "/help") {
24+ format_and_add_chat("", "", true, _("Supported admin commands:"));
25+ format_and_add_chat("", "", true, _("/motd <msg> sets a permanent greeting message"));
26+ format_and_add_chat("", "", true, _("/announce <msg> send a one time system message"));
27+ format_and_add_chat("", "", true, _("/warn <user> <msg> send a private system message to the given user"));
28+ format_and_add_chat("", "", true, _("/kick <user|game> removes the given user or game from the metaserver"));
29+ format_and_add_chat("", "", true, _("/ban <user> bans a user for 24 hours from the metaserver"));
30+ return;
31+ }
32+
33 // Split up in "cmd" "arg"
34 std::string cmd, arg;
35 std::string temp = msg.substr(1); // cut off '/'
36@@ -937,13 +955,23 @@
37 m.string(arg);
38 net->send(m);
39 return;
40- } else if (!arg.empty() && cmd == "announcement") {
41- // send the request to change the motd
42+ } else if (!arg.empty() && cmd == "announce") {
43+ // send the request to make an announcement
44 SendPacket m;
45 m.string(IGPCMD_ANNOUNCEMENT);
46 m.string(arg);
47 net->send(m);
48 return;
49+ } else if (!arg.empty() && (cmd == "warn" || cmd == "kick" || cmd == "ban")) {
50+ // warn a user by sending a private system message or
51+ // kick a user (for 5 minutes) or a game from the metaserver or
52+ // ban a user for 24 hours
53+ SendPacket m;
54+ m.string(IGPCMD_CMD);
55+ m.string(cmd);
56+ m.string(arg);
57+ net->send(m);
58+ return;
59 } else {
60 // let everything else pass
61 goto normal;
62
63=== modified file 'src/network/internet_gaming_messages.cc'
64--- src/network/internet_gaming_messages.cc 2019-05-25 08:27:20 +0000
65+++ src/network/internet_gaming_messages.cc 2019-06-05 17:09:01 +0000
66@@ -43,6 +43,7 @@
67 igmessages["NO_SUCH_USER"] = _("There is no user with this name logged in.");
68 igmessages["NO_SUCH_GAME"] = _("The game no longer exists, maybe it has just been closed.");
69 igmessages["WRONG_PASSWORD"] = _("Wrong password, please try again.");
70+ igmessages["BANNED"] = _("You have been temporarily banned from online gaming.");
71 igmessages["UNSUPPORTED_PROTOCOL"] = _("The protocol version you are using is not supported!");
72 igmessages["ALREADY_LOGGED_IN"] = _("You are already logged in!");
73 igmessages["DEFICIENT_PERMISSION"] =
74
75=== modified file 'src/network/internet_gaming_protocol.h'
76--- src/network/internet_gaming_protocol.h 2019-06-01 16:31:13 +0000
77+++ src/network/internet_gaming_protocol.h 2019-06-05 17:09:01 +0000
78@@ -299,6 +299,15 @@
79 static const std::string IGPCMD_CHAT = "CHAT";
80
81 /**
82+ * Sent by the client to issue a superuser command.
83+ *
84+ * The client sends this message to the metaserver with the following payload:
85+ * \li string: the command
86+ * \li string: arbitrary parameters.
87+ */
88+static const std::string IGPCMD_CMD = "CMD";
89+
90+/**
91 * Sent by the metaserver to inform the client, that the list of games was changed. No payload is
92 * sent,
93 * 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: