Skip to content
This repository has been archived by the owner on Jul 18, 2021. It is now read-only.

Attempt to fix for MacOS users (copy environment table) #21

Merged
merged 4 commits into from
Oct 27, 2019
Merged

Attempt to fix for MacOS users (copy environment table) #21

merged 4 commits into from
Oct 27, 2019

Conversation

ddavness
Copy link
Contributor

The IPC layer for Unix uses process environments that aren't copied to the slave processes - this should help, I guess?

This probably relates to discordjs/RPC#54

Other changes:

  • Added a setting which defines a custom timeout if you're forcing launching Discord (it was hardcoded which is not ideal) - defaults to 30 seconds.

  • Added comments to the config file - it's a slightly questionable way to do this but I don't see a choice for a better way (JSON doesn't natively allow comments).

  • Developer-wise, standard is now a dev dependency of roPresence. Code has been standardized.

  • Version bump to 1.0.5

Will be kept as a draft until a macOS user confirms to me this works!

The IPC layer for unix uses process environments that aren't copied to the slave processes - this should help, I guess?

Added a setting which defines a custom timeout if you're forcing launching Discord - defaults to 30 seconds.

Added comments to the config file - slightly questionable but meh
@ddavness ddavness mentioned this pull request Sep 23, 2019
@sasial-dev
Copy link
Contributor

Works on windows.
Am not a fan of the terminal JSON.

@ddavness
Copy link
Contributor Author

I know, it's out there for debugging reasons - I actually intend to remove it in a future commit.

Diff reference:
https://github.com/JiveOff/roPresence/pull/21/files#diff-168726dbe96b3ce427e7fedce31bb0bcR24-R27

@sasial-dev
Copy link
Contributor

Got it.

@ddavness
Copy link
Contributor Author

@theLMGN, @Monkey8729, @GameAddict7 - please give me feedback on whether this works on your machines. Thank you!

@JiveOff
Copy link
Owner

JiveOff commented Oct 1, 2019

Any news?

@ddavness
Copy link
Contributor Author

ddavness commented Oct 1, 2019

As of now, nothing.

I'm going to check whether the MacOS behaviour can be applied to Linux aswell.

@JiveOff
Copy link
Owner

JiveOff commented Oct 1, 2019 via email

@ddavness
Copy link
Contributor Author

ddavness commented Oct 1, 2019

Update: Linux presents the same behavior of macOS

Since they're both Unix derived, I can safely assume this patch applies to both environments:
I was able to reproduce the bug described in #12 and #18, and succeed with the patch presented.

image
(At left, the current version, at the right, with the patch).

If we disable the patch we get this:

a

See how the environment variable XDG_RUNTIME_DIR (which discord-rpc relies on) isn't passed from the master process to the slave process.

Since that this fix works, I'm marking this as ready for review.

The preliminary tests work now, we can proceed.
@ddavness ddavness marked this pull request as ready for review October 1, 2019 23:00
Why is this even needed there?

== PULL REQUEST CONTEXT ==
Fixes: #12
Fixes: #18
@ddavness
Copy link
Contributor Author

ddavness commented Oct 1, 2019

This should do the trick! 🙂

@JiveOff JiveOff merged commit d8e557e into JiveOff:master Oct 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants