Implement Discord Social SDK #676
Conversation
…nabled by default, includes option to disable.
|
I guess it would also close this |
| private: | ||
| std::shared_ptr<discordpp::Client> m_client = nullptr; | ||
| std::atomic<bool> m_running; | ||
| const uint64_t m_app_id = 1406867596585865326; |
There was a problem hiding this comment.
Is this intentionally hard-coded? Where does it come from?
There was a problem hiding this comment.
m_app_id is intentionally hardcoded. It's the private Discord application the SDK gets art assets from.
SerialPrograms/Source/Integrations/DiscordSocial/DiscordSocial.cpp
Outdated
Show resolved
Hide resolved
SerialPrograms/Source/Integrations/DiscordSocial/DiscordSocial.cpp
Outdated
Show resolved
Hide resolved
- Use global strings for program name, make the latest GitHub release URL a private member. - Add a new, permanent Discord invite URL for SocialSDK. - Fix erroneous spaces before angle brackets in a couple of places.
| log(message, "Internal", severity); | ||
| }, m_log_level); | ||
|
|
||
| std::thread(&DiscordSocial::thread_loop, this).detach(); |
There was a problem hiding this comment.
Never use detach since it's impossible to clean up due to having an unbounded lifetime. If you destruct this class, and the thread is still running, you dangle all the fields that it references.
Store the thread into the class. Then in the destructor after you signaled the thread to exit, join the thread.
m_thread = std::thread(&DiscordSocial::thread_loop, this);
In destructor:
m_running.store(false, std::memory_order_release);
m_thread.join();
// Now it's safe to destroy the client.
if (m_client) m_client.reset();
| m_client = std::make_shared<Client>(); | ||
| if (!m_client){ | ||
| log("Failed to initialize DiscordSocialSDK.", "run()", LoggingSeverity::Error); | ||
| m_client.reset(); |
There was a problem hiding this comment.
This isn't exception safe. If an exception is thrown further down, m_client remains initialized which is probably not what you intended.
Make the client first and store into a local variable:
std::shared_ptrClient> client = std::make_shared<Client>();
Then at the the end of the function (before you start the thread), move it into m_client.
It's possible for the thread creation itself to throw. So to be truly correct, you'll need something like this:
m_client = std::move(client);
try{
std::thread(&DiscordSocial::thread_loop, this).detach();
}catch (...){
m_client.reset();
throw;
}
But see my other comment about not using detach().
|
|
||
| RunCallbacks(); | ||
| update_rich_presence(); | ||
| std::this_thread::sleep_for(std::chrono::seconds(1)); |
There was a problem hiding this comment.
Do we need to do this every second? Is it possible to diff to see if anything changed. And only update if something changed?
Ideally CC itself should push these changes out, but now is not the time to re-architect that part so this is fine.
There was a problem hiding this comment.
DiscordSDK requires some sort of a main loop that calls RunCallbacks() regularly for internal logging and other set callbacks. 1 second with logging only set to errors won't spam, but a loop running on status change (which is possible) would be more invasive (likely require OAuth2 or additional scopes: currently there are none).
If we want CC to dictate rich presence update, that would require a direct program state reference (or something thereof). Not sure how feasible when initialized from Main.cpp.
2 participants
Allows setting rich presence for the user running ComputerControl. Displays program name, status, elapsed time.
Enabled by default, can be toggled off. Requires a Discord client to be running on user's computer.
Includes debug and release .lib and binary files for Windows, Linux, MacOS. Linux and MacOS building currently not implemented.