feat: add Discord RPC Bridge to XIVLauncher.Common.Unix - #369
Bronya-Rand wants to merge 5 commits into
Conversation
|
I didn't see the discussion for moving the RPC bridge, but I am curious what the problem was with the existing approach seeing as only that plugin ended up needing bridging? I've always preferred keeping functionality/fixes specific to certain platforms like Discord internal to the plugin when possible Otherwise the implementation looks fine here, I'd need to review the referenced package too though |
The main issue with the old WineRPCBridge was that it was getting quite old and I didn't really have the original source code available to maintain, much less understand what it was doing to actually maintain it without actually having AI tell me where. It also only supported Native Discord clients and not Sandboxed ones like Flatpak/Snap and some third-party Discord clients. I originally tried to fix this by using WinSock but I went with Option 2 and originally planned to ship a dedicated bridge binary for Linux and macOS in a plugin update that users would have to run alongside XIV in order to get RPC working. However someone in the forum for Discord RPC got upset over having to trust something I built, accused me of a few things and then after asking in #plugin-dev why my PR has been still in queue for 2 days when other plugin updates got merged was when Franz and Era sort of implied that PAC had been debating whether to let the update pass as it could set a precedent that plugins can come with binary files and make users execute said code. Thus, Franz proposed that
So I converted the original bridge binary as a NuGet and merged it into XIVLauncher.Core with the added checkbox and custom port number for added customization and this is exactly what the PR is attempting to do. Reference package is here: https://github.com/Bronya-Rand/DiscordRPCBridge-Wine. Just 2 small files really. |
|
Thanks for the explainer, I'll double check everything soon I'm surprised to see the I'm not entirely sure how I feel about having to make changes in the mainline launcher because a technically-unsupported fork offers wine builds without the feature, but I also don't want to become the gatekeeper of what passes for approval if others really want it (though, the people complaining in that thread are going to technically have to trust you anyway, it's either me reviewing the package or PAC). |
Honestly when I heard people say that the new socket code broke for them I was as surprised as you were thinking in the same way. Even XOM has that issue yet its on Wine 11. It only took a hour of investigation with different versions of Proton/Wine to know that, for some reason or whatever, some Wine/Protons don't come bundled with
This I can understand as realistically the review moves away from PAC to you, Franz and Goat which is basically the same thing all over again. And yes I do get that XIVLauncher-RB is not officially supported and it was basically it + XOM that caused me to invest in the whole bridge project to get those on 10-34, that Sarelek 10-29 build, XOM on macOS and otherwise to work. That is why I originally built the binary so that it would not be part of XIVLauncher but its own thing but given limitations on Wine on starting Unix apps and all and Franz/Era's thoughts on that most users might end up breaking something running something via the Terminal (aside from macOS users screaming over Terminal use), it was kind of the only other way to bring RPC back that was more something any plugin can use than just a binary every plugin must build and have people use. |
|
Heya. Just was checking in if Blooym and/or Goat had a peek into it and if they said anything if something needed to be addressed or something was confusing. |
rankynbass
left a comment
There was a problem hiding this comment.
You should use NumericSettingsEntry instead of creating a code path for SettingsEntry. It causes doubling of input boxes for anything using NumericSettingsEntry
…at/discord-rpc-bridge
# Conflicts:
# src/XIVLauncher.Core/Components/SettingsPage/SettingsEntry{T}.cs
Adjusted now. |
rankynbass
left a comment
There was a problem hiding this comment.
Looks good. Compiles and runs on my machine, and the doubling problem is gone.
After discussion with PAC on the ongoing Discord Rich Presence 2.1.0.0 update, a consensus (I think?) was made in which XIVLauncher.Core would be the one handling RPC similarly to XIV On Mac as a replacement fallback from the old WineRPCBridge that the plugin relied on long ago.
I assumed where the runner should be by the class name and structured it similarly to the other Unix entries, I then slid it between the compatibility tools and game startup though given how light the NuGet is, it will probably rarely show the RPC startup text. All settings for Discord RPC is in the Game tab in between the Game DPI dropdown and XIVLauncher Auth.
Hopefully all is good here for merging and to unblock the plugin update. For source of the NuGet, see here: https://github.com/Bronya-Rand/DiscordRPCBridge-Wine