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

Better translation and fallback handling #92

Open
wants to merge 3 commits into
base: matrix
Choose a base branch
from

Conversation

beam
Copy link

@beam beam commented Sep 20, 2022

No description provided.

@pkscout
Copy link
Member

pkscout commented Sep 20, 2022

Can you walk me through this? I take it there was an update to the API to get translations (or at least an API call I hadn't seen), and I want to make sure I understand it. A couple specific questions:

  1. At line 169, it looks like you've turned something off and indicated it should have a setting. But why wouldn't we do a fallback on season name just like we do with everything else? I definitely don't want settings for every item for people to decide what should fallback and what shouldn't.
  2. At line 233 it seems like you are proposing eventually to be able to pick a fallback language. Right now I don't like that idea. Or at least it needs some more thought. The scraper should return something whenever possible, so even if we allow a setting for fallback language, it should still fallback to English as a tertiary since I believe everything in TMDb at least has English.

Also, given where we are in the development cycle for Nexus, I would only be able to accept this merge if there is also one for the Nexus version of the scraper as well.

@beam
Copy link
Author

beam commented Sep 24, 2022

  1. I had some issues with the translation of season. Seasons names are generally translated on TMDB, even if there is no specific translation for that season. When someone translates English version, or at least set season name, even if it's same as general translation in English, I don't get general translation of season in my language, but that fallback one in English. But it's more a problem with TMDB than in Add-on. I will fix missing seasons name translations in my language on TMDB.

  2. I thought about the possibility of having more than one fallback language, with English as always the last choice. But it's not necessary.

@beam
Copy link
Author

beam commented Sep 24, 2022

Example of "wrongly" translated seasons is https://www.themoviedb.org/tv/77236-a-discovery-of-witches in Czech language.

"1. sezona" is generally translated name in Czech language, missing explicit translation for Czech and is set translation for English name. Result in Add-on is "Season 1"

TMDB API:

{ 
  "name": "1. sezóna",
  "overview": "",
  "id": 99549,
  "translations": {
      {
        "iso_3166_1": "CZ",
        "iso_639_1": "cs",
        "name": "Český",
        "english_name": "Czech",
        "data": { "name": "", "overview": "" }
      },
      {
        "iso_3166_1": "US",
        "iso_639_1": "en",
        "name": "English",
        "english_name": "English",
        "data": { "name": "Season 1", "overview": "" }
      },
}

"2. sezona" is generally translated name in Czech, missing explicit translated version for Czech and missing explicit translated version for English. Result in Add-on is "2. sezona".

TMDB API:

{
  "name": "2. sezóna",  
  "overview": "",
  "id": 168930,
  "translations": {
      {
        "iso_3166_1": "CZ",
        "iso_639_1": "cs",
        "name": "Český",
        "english_name": "Czech",
        "data": { "name": "", "overview": "" }
      },
      {
        "iso_3166_1": "US",
        "iso_639_1": "en",
        "name": "English",
        "english_name": "English",
        "data": {
          "name": "",
          "overview": "Matthew and Diana travel back to Elizabethan London, where they are hiding in time from the Congregation. Here, they must find a powerful witch teacher to help Diana control her magic and search for the elusive Book of Life. Back in the present day, Diana’s beloved aunts must take shelter with notorious witch hunter Ysabeau De Clermont at her ancestral home."
        }
      },
}

Now i have in Kodi:
Season 1
2. sezona
3. sezona

That's the reason why i should have season name fallback as optional because it's so general name in 99% it doesn't have to be translated, what you think? The same behavior is on Episode detail, but they have in 99% unique name and fallback does the sence.

@pkscout
Copy link
Member

pkscout commented Sep 24, 2022

OK, I get what you're saying about the season fallback being optional. TMDb has always been a pain regarding season translations, and now that I see that this isn't a precursor to settings for every item, I agree. I see there's a new commit to remove the season fallback option, so you can update that again for that option. The settings are a bit of a thing to walk through if you're not familiar with them, so I'll accept the commit with the if False and then update the settings logic myself before I push an update.

Once I've confirmed this commit makes sense on the matrix branch, I'll ask you to submit the same update for the nexus branch.

@beam
Copy link
Author

beam commented Oct 16, 2022

@pkscout I'm not sure about settings title

@pkscout
Copy link
Member

pkscout commented Oct 16, 2022

It's close enough. I can figure out the exact language for the setting. Please go ahead and submit this same set of updates on the Nexus branch. That codebase is slightly different, so you can't just submit the same commit again on that branch. You'll have to do the code update again and submit a different PR for that. Once I have both I'll merge them for testing.

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