discord: add ability to use custom applications for Rich Presence #140
Conversation
…ENCE_ASSET and SET_RICH_PRESENCE_ID natives this is useful for people who want to change fivem's rich presence image, it can be set from scripts and also from the CitizenFX.ini file (DiscordId and DiscordImage)
| @@ -1,4 +1,7 @@ | |||
| #include <StdInc.h> | |||
| #include <windows.h> | |||
There was a problem hiding this comment.
... why? These header files are included by StdInc anyway.
| static time_t g_startTime = time(nullptr); | ||
|
|
||
| static std::string DiscordId; |
There was a problem hiding this comment.
Naming conventions, perhaps g_discordAppId?
| discordPresence.largeImageKey = "fivem_large"; | ||
| if (!g_richPresenceOverrideAsset.empty()) | ||
| { | ||
| discordPresence.largeImageKey = g_richPresenceOverrideAsset.c_str(); |
There was a problem hiding this comment.
where's the indentation?
| Discord_Initialize("382624125287399424", &handlers, 1, nullptr); | ||
| std::wstring fpath = MakeRelativeCitPath(L"CitizenFX.ini"); | ||
|
|
||
| if (GetFileAttributes(fpath.c_str()) == INVALID_FILE_ATTRIBUTES) |
There was a problem hiding this comment.
INI overrides are likely undesirable. This code is also extremely ugly. 😕
There was a problem hiding this comment.
i wanna make sure i understand this correctly, you are unhappy about the fact that the ini settings can be overwritten by natives?
| } | ||
| else { | ||
| char DiscordIdS[512]; | ||
| wcstombs(DiscordIdS, path, sizeof(DiscordIdS)); |
There was a problem hiding this comment.
... no, no, no, no, use ToWide/ToNarrow.
| } | ||
|
|
||
| } | ||
| Discord_Initialize(DiscordId.c_str(), &handlers, 1, nullptr); |
There was a problem hiding this comment.
Variable naming, and a padding newline?
| @@ -108,4 +158,40 @@ static InitFunction initFunction([]() | |||
|
|
|||
| g_richPresenceChanged = true; | |||
| }); | |||
|
|
|||
| fx::ScriptEngine::RegisterNativeHandler("SET_RICH_PRESENCE_ASSET", [](fx::ScriptContext& context) | |||
There was a problem hiding this comment.
Service-specific, so make the native like SET_DISCORD_RICH_PRESENCE_ASSET. Also, you forgot to update codegen_cfx_natives.lua?
| { | ||
| DiscordId = "382624125287399424"; | ||
| } | ||
| Discord_Shutdown(); |
There was a problem hiding this comment.
Is this call actually safe? Also, probably should be called SET_DISCORD_APPLICATION_ID or so.
There was a problem hiding this comment.
according to discord's docs, it is, it terminates the thread allowing the application to shut down gracefully or discord rich presence to be reinitialized, without this, i am pretty sure the threads would just stack one onto another every time you changed the app id.
|
|
||
| if (GetFileAttributes(fpath.c_str()) == INVALID_FILE_ATTRIBUTES) | ||
| { | ||
| DiscordId = "382624125287399424"; |
There was a problem hiding this comment.
Default ID is mentioned so often it probably should be a #define.
|
that should be about it, last thing that's left is understand the INI override problem you were talking about, i wanna make sure i know what you're asking for before i make any changes 😄 |
|
@vecchiotom You should use convars instead of reading from an INI file. |
|
we are talking about the client tho, not the server, and the INI file is CitizenFX.ini, which to my knowledge is what is used for client side configuration 😄 |
|
Is this meant to be user editable though? I was assuming this is so that servers can show off that you are playing on their server with a custom icon and name. If that's the case then a serverinfo convar would work best since that's sent when you connect(See: NetLibrary::ProcessOOB). |
|
let's see what i can do 😄 |
|
@bladecoding still trying to find out how to get a convar value on the client, mind giving me a little clue? please 🙏 😄 |
…INI configuration
|
ok, pretty confident everything you asked for has now been reviewed and fixed 😄 @blattersturm |
| apiset 'client' | ||
| returns "void" | ||
| doc [[! | ||
| <summary> |
There was a problem hiding this comment.
No definition of <param />?
| apiset 'client' | ||
| returns "void" | ||
| doc [[! | ||
| <summary> |
There was a problem hiding this comment.
Again no param definitions. Also, the descriptions are a little vague and non-specific.
| { | ||
| discordPresence.largeImageKey = g_richPresenceOverrideAsset.c_str(); | ||
| } | ||
| else { |
There was a problem hiding this comment.
{ not on a new line? :(
| @@ -41,12 +51,21 @@ static void UpdatePresence() | |||
| line1 = g_richPresenceOverride; | |||
| } | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Random added whitespace!
| @@ -89,6 +111,8 @@ static InitFunction initFunction([]() | |||
| OnKillNetworkDone.Connect([]() | |||
| { | |||
| g_richPresenceOverride = ""; | |||
| NativeInvoke::Invoke<HashString("SET_DISCORD_APP_ID"),char*,char*>(DEFAULT_APP_ID); | |||
There was a problem hiding this comment.
Unsure whether it's a good idea to call natives inline like that from here.
There was a problem hiding this comment.
yeah the idea was to avoid repetitions in the code, but sure, i can fix this :)
| g_discordAppId = DEFAULT_APP_ID; | ||
| } | ||
|
|
||
| Discord_Shutdown(); |
There was a problem hiding this comment.
Can this not be done in the tick routine using a appid != lastappid check (or similar)?
|
pushed some changes to address the issues as requested :) |
this is useful for people who want to change fivem's rich presence image, it can be set from scripts using natives and also from the CitizenFX.ini file (DiscordId and DiscordImage)
P.S. this is my real first time doing anything with cpp, i have probably overcomplicated some things, if you'd like for me to change anything, just review the commit and i will try my best to fix it :)