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

SubDL subtitles provider #1082

Merged
merged 3 commits into from May 18, 2024
Merged

Conversation

KingLucius
Copy link
Contributor

@KingLucius KingLucius commented May 9, 2024

  • Adds SubDL subtitles provider.
  • Imdb Id passing now correctly.

Closes #1080
Closes #1076

@KingLucius
Copy link
Contributor Author

there is another option that we can getMetaData() inside ShowMirrosDialog() and pass the imdb to openOnlineSubPicker()

Copy link
Collaborator

@fire-light42 fire-light42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition, however I will be a bit harsh with the code quality.

What you have done is hardcoded imdb into too many places using a string instead of a long, why?. This may work for this one thing, but does not allow for easy future expansion as it ties cs3 to imdb.

@KingLucius
Copy link
Contributor Author

KingLucius commented May 9, 2024

Nice addition, however I will be a bit harsh with the code quality.

What you have done is hardcoded imdb into too many places using a string instead of a long, why?. This may work for this one thing, but does not allow for easy future expansion as it ties cs3 to imdb.

In most cases you need and will use the imdb ID as a full one with ttxxxxx and that's the same format that be added from trakt & TMDB apis
Only Opensubtitles uses only the numbers without tt
So using it as string and only replace in Opensubtitles sub provider is the logic thing to do
To avoid adding the tt in most cases
Even in MainApi it's added as string because originally imdb ID is string not int like tmdb

@fire-light42
Copy link
Collaborator

Alright, however normally with stuff like this (like mal and anilist) when it can be stored as an int is is stored as an int

@KingLucius
Copy link
Contributor Author

Alright, however normally with stuff like this (like mal and anilist) when it can be stored as an int is is stored as an int

Sure, I think only imdb needs to be stored as string, all others would be stored as Int

@KingLucius
Copy link
Contributor Author

KingLucius commented May 18, 2024

We can later update MainApi to store applicable IDs as int.
Then we can remove the toInt() later

If you agree, I can me Todo before merging

@fire-light42 fire-light42 merged commit 469a712 into recloudstream:master May 18, 2024
1 of 2 checks passed
@KingLucius KingLucius deleted the subdlProvider branch May 18, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants