Add Discord support by vadi2 · Pull Request #1716 · Mudlet/Mudlet

vadi2 · 2018-05-28T12:08:22Z

Brief overview of PR changes/additions

Add Discord integration to Mudlet.

Motivation for adding to Mudlet

Integrate with Discord like AAA games do, give MUDs another excellent, free channel for advertising, and, ultimately, make MUDs more modern and appealing.

Other info (issues closed, discussion etc)

Windows: https://ci.appveyor.com/api/buildjobs/3cxqfec12bka8il8/artifacts/src%2Fmudlet.zip
macOS: https://transfer.sh/jOII2/Mudlet-3.13.0-testing-PR1716-d02f94c.dmg
Linux: https://transfer.sh/7jGHI/Mudlet-3.13.0-testing-PR1716-d02f94c0-linux-x64.AppImage.tar

Current draft of the GMCP spec is: https://pastebin.com/Zt7Wbkwk. Documentation: https://wiki.mudlet.org/w/Manual:Scripting#Discord_Rich_Presence

Games supporting Discord out of the box: MidMUD, StickMUD, Achaea, Lusternia, Imperian, Aetolia

Currently available Lua API is documented here.

Sample previews:

vadi2 · 2018-05-28T12:18:24Z

@keneanung could use your help in sorting out how it should build, as always. Right now I have to use LD_LIBRARY_PATH pointing to the Discord binary.

We also have the option of compiling it, it's open-source under MIT (so all OK @SlySven) but I'd rather compile less things, not more.

SlySven

You also need stuff for the CMake project files...

SlySven · 2018-05-28T15:57:42Z

3rdparty/discord/discord-rpc-linux/discord-rpc/linux-static/include/discord_rpc.h

+
+#define DISCORD_REPLY_NO 0
+#define DISCORD_REPLY_YES 1
+#define DISCORD_REPLY_IGNORE 2


Can we have these at the top so they are obvious please...

This is a third-party library's code

SlySven · 2018-05-28T15:58:51Z

3rdparty/discord/discord-rpc-win/discord-rpc/win32-dynamic/include/discord_rpc.h

+
+#define DISCORD_REPLY_NO 0
+#define DISCORD_REPLY_YES 1
+#define DISCORD_REPLY_IGNORE 2


^ that was a little terse, sorry was in a rush :)

SlySven · 2018-05-28T16:08:44Z

src/mudlet.cpp

 #include "edbee/models/textgrammar.h"
 #include "edbee/texteditorwidget.h"
 #include "edbee/views/texttheme.h"
+#include "discord.h"


This has to be conditional, at least by platform - it won't work on FreeBSD because the Desktop application is not available for that platform. It may also not be wanted by some packagers - for instance since Discord is not packaged by Debian but provided by the Discord people themselves it is probable that @csmall will not want it included in Debian packages...

Based on my previous hackery, may I suggest an environmental variable WITH_DISCORD being set to NO to disable a #define INCLUDE_DISCORD on the supported platforms which is used to include/exclude the functionality.

Also you mentioned the code was MIT licenced - so it will need to be conditionally included in the "About dialog" - but it should be fairly straight-forward to conditionally add another licence to the listing there.

I'll fix the include.

I don't want yet another flag for this, we already have a lot of flags for trivial things and I don't see players wanting to use a what would be a less than functional Mudlet

Is it possible to load the functionality at runtime, depending on if the libraries are available?

http://doc.qt.io/qt-5/qlibrary.html#details looks like so! I will investigate this approach.

Conditional loading of the library dependent on availability will be a PITA for developers because they will have to arrange for the libraries concerned to both be available / not available to test both conditions won't they? I guess adding/removing some symlinks is one crude way to do it.

The Discord libraries - are they C or C++ code? I ask because that QLibrary seems to be aimed at dlopenìng (which I think is the name for run-time, explicit, loading of libraries on demand as opposed to the implicit, link stage of compilation, incorporation of library functions into an executable) C libraries only...

All it takes is one LD_PRELOAD_PATH and only developers working with Discord code would need to do that - I prefer that option over more fobs in the build system.

It's C code, I think it'll be OK!

SlySven · 2018-05-28T16:11:23Z

src/mudlet.pro

 }

+linux {
+    HEADERS += ../3rdparty/discord/discord-rpc-linux/discord-rpc/linux-dynamic/include


Are the other two paths correct - they look different to this one in that they are missing a /discord element straight after the ../3rdparty directory:
../3rdparty/discord/discord-rpc-linux/discord-rpc/linux-dynamic/include
verses:
../3rdparty/discord-rpc-osx/discord-rpc/osx-dynamic/include
../3rdparty/discord-rpc-win/discord-rpc/win32-dynamic/include

Thanks for the catch :)

keneanung · 2018-05-28T18:14:22Z

Do you plan to make it controllable via lua? We could also define custom GMCP/MSDP extensions instead of using triggers.

vadi2 · 2018-05-29T09:57:10Z

Yes and yes.

Did you want to look at getting the GMCP/MSDP extensions communicated with other clients / community?

SlySven

I do like what you are trying to achieve. 😀
I'm just sad that I won't be able to use it. 😭

Does the Discord API support opening up Discord in the channel of a specific MUD Game - I am sure some MUDs already have official ones but that may not always be the case and users on raiding parties or other activities may want per team or other sub-groupings and so may want to be able to suggest a channel to be opened in the Discord application depending on the situation - and they may want to automate that into a Mudlet script/package/module...

SlySven · 2018-05-29T15:53:14Z

src/discord.cpp

 #include "pre_guard.h"
+#include <functional>
 #include "../3rdparty/discord/discord-rpc-linux/discord-rpc/linux-dynamic/include/discord_register.h"
 #include "../3rdparty/discord/discord-rpc-linux/discord-rpc/linux-dynamic/include/discord_rpc.h"


This and the preceding header are redundant as you've already #included them in the discord.h file! {Ironic considering you just have or are remove such duplicates from the existing Mudlet source files. 🤣 }

Fixed, thanks for spotting this.

SlySven · 2018-05-29T15:54:38Z

src/discord.cpp

 #include "../3rdparty/discord/discord-rpc-linux/discord-rpc/linux-dynamic/include/discord_rpc.h"
 #include "post_guard.h"

 static const char* APPLICATION_ID = "450571881909583884";


Is this a GUID for Mudlet on Discord? We might want to note that in a comment so we know what this magic number is...

Documented.

SlySven · 2018-05-29T15:59:58Z

src/discord.cpp

+    }
+
+    char buffer[256];
+    sprintf(buffer, "Playing %s", name.toUtf8().constData());


🌏 I18n: Discord is aiming to cover more than English IIRC so we may need to tr(...) the string; so, will this work:

sprintf(buffer, tr("Playing %1").arg(name).toUtf8().constData());

Oh, right! Thanks.

I also went with:

sprintf(buffer, "%s", tr("Playing %1").arg(name).toUtf8().constData());

That is more secure.

Ehh... I am not sure this is a good idea. If it shows for an italian user "giocando a clessindra", nobody elsewhere will know what are they doing.

Consider the Spanish players, previously using MudletRL, are they going to want to see:

Jugando: Reinos de leyenda

or:

Playing: Reinos de leyenda

The Spanish Discord users want to see the former while the English want to see the latter and the German Discord users want Spielt: Reinos de leyenda. Optimally, Discord should translate this depending on the side viewing the status.

I was looking to see how well developed Discord was for non-Americanish languages to see whether it is likely they'd look at this but was not able to find anything out. Also, it is not something that this bot is going to solve - and since they dropped their free service on 2018/05/17 it is not one that we can afford to invite to our channel... 🙁

Discord does this just fine. Using Discord with German client, I would see "Spielt Mudlet" and not "Playing Mudlet". Here is a screenshot for future reference:

SlySven · 2018-05-29T16:00:45Z

src/discord.cpp

+    char buffer[256];
+    sprintf(buffer, "Playing %s", name.toUtf8().constData());
+    mDiscordPresence.details = buffer;
+    mDiscordPresence.largeImageKey = name.toLower().toUtf8().constData();


Is toLower() appropriate here?

Yes, Discord lowercases the asset names

SlySven · 2018-05-29T16:23:10Z

src/discord.cpp

-void Discord::timerEvent(QTimerEvent *event)
+void Discord::timerEvent(QTimerEvent* event)
 {
+    if (!mLoaded) {


Is, aborting on fail always the way to go with short methods like this, would not:

if (mLoaded) { Discord_RunCallbacks(); }

be just as functional but even briefer?

SlySven · 2018-05-29T16:25:29Z

src/discord.cpp

+    mDiscordPresence.partySize = 5;
+    mDiscordPresence.partyMax = 6;
+    mDiscordPresence.matchSecret = "zdgfghrfsyheqrwqgshbfxdq35 4 5";
+    mDiscordPresence.joinSecret = "ASFDFSHR512345 RASGSADWr";


Can we have an explanation somewhere about what these magic numbers mean/are for - or even the whole mDiscordPresence?

Fake numbers testing for the API for now: https://discordapp.com/developers/docs/rich-presence/how-to#updating-presence

SlySven · 2018-05-29T16:27:45Z

src/discord.h

 #include <QTimerEvent>
+#include <QLibrary>
 #include "../3rdparty/discord/discord-rpc-linux/discord-rpc/linux-dynamic/include/discord_register.h"
 #include "../3rdparty/discord/discord-rpc-linux/discord-rpc/linux-dynamic/include/discord_rpc.h"


Are these platform specific - so need #if defined(Q_OS_LINUX) etc. wrappings?

vadi2 · 2018-05-30T09:29:18Z

Does the Discord API support opening up Discord in the channel of a specific MUD Game

I'm not sure, but I like what you're thinking! We'll get to this in time...

vadi2 · 2018-05-30T09:31:42Z

Problem: 1 profile open, easy - discord should say you're playing X game.

2 profiles open, different games - which should be reported? :(

Perhaps the currently focused tab, that would work in multiview too. This is also where @SlySven's idea of not reporting to Discord comes in - a person may want to have some characters they keep secret, while some characters are public... so yes should have a per-profile option for this. Opt in or opt out is the question - opt-in and people won't notice to enable it, opt-out and their secret characters might get leaked.

I think opt-in with a way to notify the user that they can enable it would be the way to go.

vadi2 · 2018-05-30T18:38:34Z

src/discord.cpp

+        char buffer[128];
+        auto status = mStatus.toUtf8();
+        strncpy(buffer, status.constData(), status.length());
+        discordPresence.state = status.constData();


@SlySven could use your help in making this nicer... it has a few warnings.

vadi2 · 2018-09-26T02:51:25Z

Updated.

SlySven · 2018-09-30T01:18:00Z

src/dlgProfilePreferences.cpp

    auto detail = mudlet->mDiscord.getDetailText(mpHost);
    if (!detail.isEmpty()) {
-        detail = QStringLiteral("(\"%1\")").arg(detail);
+        detail = QStringLiteral("<br/>(\"%1\")").arg(detail);


Please note <br/> IS NOT VALID HTML! That is actually an XHTML tag - the one I think you should be using is <br> - as shown in https://doc.qt.io/qt-5/richtext-html-subset.html - Qt actually states in that link: "Qt's text widgets are able to display rich text, specified using a subset of HTML 4 markup."

This tag works fine as intended in Qt

SlySven · 2018-09-30T01:19:40Z

src/dlgProfilePreferences.cpp

+        <img src=":/icons/discord-rich-presence-large-icon.png"/>
+        <p style="color: #989A9F;" id="large-icon">%2</p>
+      </td>
+    <td class="tg-jn9l"><p class="detail" id="detail">%3 %4</p></td>


Needs one more space to be indented properly I think.

Thanks, fixed

Kebap · 2018-09-30T13:58:50Z

Setting a custom discord application ID worked as expected and usingMudletDiscordID changed from true to false as well

…ekit # Conflicts: # CI/travis.osx.install.sh # src/dlgProfilePreferences.cpp # src/ui/profile_preferences.ui

Kebap · 2018-10-02T06:19:13Z

For some reason now, in my test profile, I am allowed to check the "Discord" box upon connection, even though this is no "known" game at all. Is that intended behavior?

Kebap · 2018-10-02T06:36:35Z

For some reason, now my tests won't work anymore. Not sure if it was setting that checkbox, or some foul play with setting and removing the custom application ID somehow, now all my set-commands will yield "true" but not display in Discord anymore. Will have to investigate further.

It seems the profile remembers, if I set a custom ID and then will activate the checkbox in connection dialogue, right?

vadi2 · 2018-10-02T07:08:21Z

I'm not too certain, I'll look at that code. Discord box should only be enabled for "known" games and afaik there's no functionality to "remember" a game.

I'd like to build one in so games using GMCP don't have to wait for a Mudlet update in order to enable the box for them, but that's in the future.

vadi2 · 2018-10-02T08:14:39Z

Yes, there's a feature that if a custom Discord ID was known, that game is remembered to have Discord support (thanks @SlySven!).

But that shouldn't break the set-commands, I'll have a look.

vadi2 · 2018-10-02T10:22:11Z

Turning things on and off seems to work as expected here.

keneanung

I haven't tested it, but the lua part looks good.

The infrastructure part is something I'd like to see improved at some point (ew checking in binaries), but it's good enough for now.

vadi2 added 6 commits May 28, 2018 11:26

Added Discord release v3.3.0

b64c8c2

Added Discord class in

2c4cfd4

Added in handlers

83234a7

Added QTimer header

eef74f2

Got the Discord game loop going

f36ae14

Added a fake notification

02c989d

vadi2 added the 0 - In progress label May 28, 2018

vadi2 self-assigned this May 28, 2018

vadi2 requested a review from a team as a code owner May 28, 2018 12:08

vadi2 requested a review from a team May 28, 2018 12:08

vadi2 added 2 commits May 28, 2018 14:29

Fix path to compiled libraries.

70985a3

Remove chrono

5184857

SlySven reviewed May 28, 2018

View reviewed changes

WIP setting game name and status

0797547

vadi2 added 4 commits May 29, 2018 13:21

Removed static, unused folders

e0133fb

Added libdiscord-rpc.so

0801149

Fixed missing /discord

777791a

WIP loading library

d38542e

SlySven reviewed May 29, 2018

View reviewed changes

WIP setting game status

79b7bdf

vadi2 added 3 commits May 30, 2018 13:53

Added Lua API for manipulating game name and status

0b00fcf

Adding a few more functions

b4f382e

Fixed state corruption

2754dd8

vadi2 commented May 30, 2018

View reviewed changes

Fixed UpdatePresence() to be sane

5b2f841

SlySven reviewed Sep 30, 2018

View reviewed changes

vadi2 added 6 commits October 1, 2018 06:26

Review feedback

6cad47b

Reset index of profile preferences

c936aad

Merge remote-tracking branch 'SlySven/development' into add-basic-gam…

5d73cdb

…ekit # Conflicts: # CI/travis.osx.install.sh # src/dlgProfilePreferences.cpp # src/ui/profile_preferences.ui

Merged Discord options back in

faedfe2

Update documentation comments

bb38ad1

Harmonised privacy options for Discord read/write

659fa7d

Minor formatting tweaks

3b2f595

vadi2 added 8 commits October 2, 2018 14:21

Remove unneeded comment

b754fa9

Use Mudlets own image if none is currently set

87c751a

Merge branch 'development' into add-basic-gamekit

00d0655

Fix merge error

6530ccb

Merge branch 'development' into add-basic-gamekit

64fe515

Minor code tweaks and formatting

7d40966

Remove accidental empty xml

53bc6cd

Build from master installers branch

91c3628

keneanung approved these changes Oct 4, 2018

View reviewed changes

Minor code cleanup

1f663c3

vadi2 merged commit 4de6f97 into Mudlet:development Oct 5, 2018

vadi2 deleted the add-basic-gamekit branch October 5, 2018 04:26

Kebap mentioned this pull request Dec 1, 2020

Add documentation for getDiscordTimeStamps() function #4420

Closed

Kebap mentioned this pull request Jul 2, 2021

Add resetDiscordData() #5304

Closed

Uh oh!

Conversation

vadi2 commented May 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Brief overview of PR changes/additions

Motivation for adding to Mudlet

Other info (issues closed, discussion etc)

Uh oh!

vadi2 commented May 28, 2018

Uh oh!

SlySven left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SlySven May 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keneanung commented May 28, 2018

Uh oh!

vadi2 commented May 29, 2018

Uh oh!

SlySven left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vadi2 commented May 28, 2018 •

edited

Loading

SlySven May 28, 2018 •

edited

Loading

Kebap commented Sep 30, 2018 •

edited

Loading