Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flatpak'ed Discord Rich Presence support #273

Merged
merged 8 commits into from
Mar 9, 2019

Conversation

gasinvein
Copy link
Member

RP clients talks to Discord through socket $XDG_RUNTIME_DIR/discord-ipc-0, however, we can't export it from flatpak's sandbox. Instead, a "proxied" socket $XDG_RUNTIME_DIR/discord/ipc-0 is exported.
On the app's side, we have to add a symlink to make Discord RP available to games under the path they expect.
See flathub/com.discordapp.Discord#29 for details.
Testing is desirable.

@flathubbot
Copy link
Contributor

NOTE: This comment was posted by the test instance of buildbot

Started test build 154

@flathubbot
Copy link
Contributor

NOTE: This comment was posted by the test instance of buildbot

Build 154 successful
To test this build, install it from the testing repository:

flatpak install --user http://repo-test.flathub.org:8080/build-repo/145/com.valvesoftware.Steam.flatpakref

@nanonyme
Copy link
Collaborator

nanonyme commented Dec 7, 2018

I'm a bit confused architecturally how this would work

This removes the need to start Discord before Steam
@flathubbot
Copy link
Contributor

NOTE: This comment was posted by the test instance of buildbot

Started test build 159

@flathubbot
Copy link
Contributor

NOTE: This comment was posted by the test instance of buildbot

Build 159 successful
To test this build, install it from the testing repository:

flatpak install --user http://repo-test.flathub.org:8080/build-repo/150/com.valvesoftware.Steam.flatpakref

@gasinvein
Copy link
Member Author

@nanonyme we start socat inside Discord's sandbox to forward data from $XDG_RUNTIME_DIR/discord-ipc-0 to $XDG_RUNTIME_DIR/discord/ipc-0.
The directory $XDG_RUNTIME_DIR/discord/ can be shared with host and other flatpak'ed apps. Because this socket path is not standard and clients expect it to be under $XDG_RUNTIME_DIR/discord-ipc-0, we should add an corresponding symlink on client's side, so the client can talk to Discord.

@nanonyme
Copy link
Collaborator

nanonyme commented Dec 7, 2018

Are you essentially trying to make two sandboxed apps talk to each other?

@gasinvein
Copy link
Member Author

gasinvein commented Dec 7, 2018 via email

@nanonyme
Copy link
Collaborator

nanonyme commented Dec 7, 2018

I think this is rather questionable approach. Should instead be some portal proxying communication between the two apps.

@gasinvein
Copy link
Member Author

gasinvein commented Dec 7, 2018 via email

@gasinvein gasinvein changed the title [wip] Add Discord Rich Presence support Add flatpak'ed Discord Rich Presence support Dec 25, 2018
@flathubbot
Copy link
Contributor

NOTE: This comment was posted by the test instance of buildbot

Started test build 958

@flathubbot
Copy link
Contributor

NOTE: This comment was posted by the test instance of buildbot

Build 958 successful
To test this build, install it from the testing repository:

flatpak install --user http://repo-test.flathub.org:8080/build-repo/891/com.valvesoftware.Steam.flatpakref

@nanonyme
Copy link
Collaborator

nanonyme commented Jan 4, 2019

Have you now done testing on this?

@gasinvein
Copy link
Member Author

I still don't know which (if any) of my games on Steam supports Discord RP, so I didn't actually test this with games. Yet the send-presence sample client seems to work fine.

@nanonyme
Copy link
Collaborator

nanonyme commented Jan 4, 2019

Is there any point to add the support if no games support it? Do some games support it?

@gasinvein
Copy link
Member Author

Some games with Discord-RP support are mentioned by Discord, but I own none of them.

@sat3ll
Copy link
Contributor

sat3ll commented Jan 4, 2019

https://www.libretro.com/index.php/upcoming-retroarch-1-7-4-details-on-discord-integration/

RetroArch supports Discord RP.

@nanonyme
Copy link
Collaborator

nanonyme commented Jan 11, 2019

What if we instead just patch this line https://github.com/discordapp/discord-rpc/blob/master/src/connection_unix.cpp#L68?
We compile Discord from sources anyway. Would allow running no extra code in the wrapper.

@gasinvein
Copy link
Member Author

gasinvein commented Jan 12, 2019

@nanonyme RP clients aren't obliged to use this library, they can implement communication with Discord themselves.

@flathubbot
Copy link
Contributor

Started test build 568

@flathubbot
Copy link
Contributor

Build 568 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/529/com.valvesoftware.Steam.flatpakref

@nanonyme
Copy link
Collaborator

nanonyme commented Mar 9, 2019

@gasinvein if you've tested Steam still runs with this, I'm fine with merging. My only test machine is currently borken.

@gasinvein
Copy link
Member Author

Yes, Steam is expected to work as usual.

@carlocastoldi
Copy link

Sorry for the slight OT, but do you think a similar work should be done for Mumble in order to fix #997?

Definitely a portal would be better, but I doubt that either flatpak
team would do that just for Discord, or Discord's developers would care
about flatpak.

On the plus side: mumble it's a open source project, so if we wanted to experiment with things we could try to

@gasinvein
Copy link
Member Author

Sorry for the slight OT, but do you think a similar work should be done for Mumble in order to fix #997?

It looks like Mumble is doing the same bad thing Discord does - puts its socket directly under $XDG_RUNTIME_DIR/ without a subdir. I hope there is no way we're going to add the same terrible warkaround for oss Mumble as for proprietary Discord. This should be solved in Mumble itself.

@carlocastoldi
Copy link

is there any convention for the path to use inside $XDG_RUNTIME_DIR/? I see with proprietary apps you're setting them to $XDG_RUNTIME_DIR/app/<app-id>, but that seems flatpak specific

@nanonyme
Copy link
Collaborator

nanonyme commented Nov 13, 2022

Just about any sub-directory works. Using app id is more or less guaranteed unique path.

@carlocastoldi
Copy link

I'm preparing a MR for Mumble. I notice that if i delete the subdir in $XDG_RUNTIME_DIR/, flatpak Steam no longer has access to it and its files:

  • host:
❯ ls /run/user/1000/ -lah
drwxr-xr-x  2 carlo carlo  80 14 nov 16.33 mumble
❯ ls /run/user/1000/mumble -lah
totale 0
drwxr-xr-x  2 carlo carlo  80 14 nov 16.33 .
drwx------ 21 carlo carlo 580 14 nov 16.33 ..
srwxrwxrwx  1 carlo carlo   0 14 nov 16.33 MumbleOverlayPipe
srwxr-xr-x  1 carlo carlo   0 14 nov 16.33 MumbleSocket
  • flatpak (zero hard links):
[📦 com.valvesoftware.Steam com.valvesoftware.Steam]$ ls $XDG_RUNTIME_DIR -lah
drwxr-xr-x 0 carlo carlo  40 14 nov 16.32 mumble

What am I missing here? Should the subdir never be deleted during flatpak's runtime? I read somewhere that XDG_RUNTIME_DIR can be garbage collected if not touched in 6 hours. Does it happen to directories as well?

@gasinvein
Copy link
Member Author

I'm preparing a MR for Mumble. I notice that if i delete the subdir in $XDG_RUNTIME_DIR/, flatpak Steam no longer has access to it and its files:

You mean delete the subdir before running Steam flatpak, or while it's already running? For the former, the solution is to add :create suffix to the filesystem permission, i.e. --filesystem=xdg-run/mumble:create

@carlocastoldi
Copy link

Yeah, I meant while Steam flatpak is running

@gasinvein
Copy link
Member Author

Yeah, I meant while Steam flatpak is running

Then it's kinda expected behaviour. --filesystems in flatpak sandbox are bind-mounted, so when the source is deleted and recreated, it won't appear again at the mount target path.

@carlocastoldi
Copy link

oh ok! So we have to assume either that:

  • subdirs in /run/user/$UID/ are note cleaned up after a while they are not being touched
  • flatpaks apps don't stay open more than ~6 hours

@carlocastoldi
Copy link

btw, instead of bumping this old PR, I opened mine for Mumble: #1008

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants