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

Refactor preload script to hook into Tidal's actual internal state #404

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ottomated
Copy link

I've reverse-engineered Tidal's internal state and refactored the injected script, which makes it possible to directly subscribe to their Redux store instead of updating on an interval and read the actual data straight from memory instead of from the DOM. This necessitated a lot of changes through the whole app, so feel free to make additional commits if needed.

This also should fix the issue of the API and MPRIS integrations sometimes reporting an outdated value. It should also make it trivial to add seeking and volume support to MPRIS.

@Mastermindzh
Copy link
Owner

Just before the big redux-refactor from TIDAL last year we also had a look at introducing this.
We concluded that it is quite risky given the change history from TIDAL with regards to their internal store but it does seem to have calmed down a bit 😄.

Last year the discussion ended with the fact that we should put this behind a toggle to give people the choice of "scraper".
So for this PR to be considered/merged we would need a few things:

  • Generic scraper class, 1 for the old method 1 for redux-store method.
  • A minimization of breaking changes, the current PR breaks a lot of contracts just because TIDAL chose to use dfifferent names/capitalization than we did.
  • Reverting the string types to enum wherever possible.
  • A full review of the PR (I saw some more stuff that wasn't quite as neat as we'd like)

I can work on this when I have a bit of free time again, I don't mind doing all the work mentioned above but that would a few-pronged approach:

  • Implementing the generic scraper first
  • Implementing a setting to switch between scrapers
  • redoing much of this prs work to fit into the new scraper format
  • refactoring parts of this prs work.
  • eventually we can then add the third scraper that people have been asking for as well (the unofficial API)

Let me know if you want to take this bigger project on or whether I should put it on my own backlog.

@ottomated
Copy link
Author

Feel free to make any changes you see fit, I will probably just maintain my own fork as I have very different opinions on code style.

@Mastermindzh
Copy link
Owner

@ottomated that's ok!
I wish you the best on your fork.

The current setup makes merging the PR quite difficult (you've removed a lot of infrastructure in favor of the internal TIDAL naming conventions for example)

I'll probably end up rewriting chunks of it but I will make sure to drop credit where it's due.

If you ever feel like popping over a PR again because you've added something nice out users can benefit from as well then that would be appreciated.

Good luck and have a great weekend 😊

Copy link

sonarcloud bot commented May 19, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@nyabinary
Copy link

Very nice :3

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.

3 participants