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 functionality to turn on/off Running lights for faders based on user in-call status #6

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

majorsimon
Copy link

@majorsimon majorsimon commented Feb 2, 2022

NB: upstream project discord-rpc has ...issues... with subscriptions. A PR to fix these in that project has been merged into a github fork majorsimon/RPC, replacing the dependency on npm "discord-rpc" with the GH repo, at least until upstream issue resolved (see discordjs/RPC#159). This aims to resolve the issues with the subscriptions, but in reality it seems a little flaky?

NB: upstream project discord-rpc has ...issues... with subscriptions. A
PR to fix these in that project has been merged into a github fork
majorsimon/RPC, replacing the dependency on npm "discord-rpc" with the
GH repo, at least until upstream issue resolved (see discordjs/RPC#159).
@jpwilliams
Copy link
Member

jpwilliams commented Feb 2, 2022

Thanks for the PR, @majorsimon! Mm discord-rpc is indeed a bit of a wonky library currently.

Frustrating that the issue seems to be even worse on their latest 4.0.1. Do we know why they closed discordjs/RPC#133? I haven't seen any acknowledgement of the issue yet on the issues in their repo yet; I'll have to dive in to reproduce the fun with the logs you've added.

@majorsimon
Copy link
Author

majorsimon commented Feb 2, 2022

I've no idea why they closed the PR! It seems from the commit log that there was some attempt to rectify the subs problem (there's a commit w/ message "new sub model") on same day as they closed the PR... /shrug. Spent the whole day yesterday trying to figure out why simple changes to add the syncRunning stuff here didn't work, only to find it was this strange lib.

I was still seeing intermittently weird behaviour with the new changes, so the issue may go deeper. Could always add a basic timer job to udpate Running but subs are so much cleaner! Happy if this isn't merged until upstream gets cleaned; I can work on an alt on a timer if needed. I just wanted my R keys to light up when I'm on a call 😭

@majorsimon
Copy link
Author

majorsimon commented Feb 2, 2022

It appears as though PR was closed correctly! The project has moved to client.on(EVENT, f); client.subscribe(EVENT, args);. Fabulously, the TotallyTyped definitions have not kept up, so I've had to add a couple of (temporary?) @ts-ignores :(

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.

2 participants