multilang: strange behaviour when selecting dts templates

[philcerf]

Hey.

Was playing with multilang in develop and noticed the following strange behaviour.

For monster I have e.g. these (amongst others):

  {
    "id": 1,
    "language": "en",
    "type": "monster",
    "default": false,
    "platform": "telegram",
    "template": {
      "content": "{{round iv}}% {{name}}{{#if form}}{{#isnt formName 'Normal'}} {{formName}}{{/isnt}}{{/if}} cp:{{cp}} L:{{level}} {{atk}}/{{def}}/{{sta}} {{boostWeatherEmoji}}\nEnd: {{time}}, Time left: {{tthm}}m {{tths}}s \n {{#if weatherChange}}{{weatherChange}}\n{{/if}} {{{addr}}} \n quick: {{quickMoveName}}, charge {{chargeMoveName}} \n {{#if pvp_rankings_great_league}}{{#compare bestGreatLeagueRank '<=' pvpDisplayMaxRank}}{{#compare bestGreatLeagueRankCP '>=' pvpDisplayGreatMinCP}}**Great league:**\n{{/compare}}{{/compare}}{{#each pvp_rankings_great_league}}{{#if this.rank}}{{#compare this.rank '<=' ../pvpDisplayMaxRank}}{{#compare this.cp '>=' ../pvpDisplayGreatMinCP}} - {{pokemonName this.pokemon}} #{{this.rank}} @{{this.cp}}CP (Lvl. {{this.level}})\n{{/compare}}{{/compare}}{{/if}}{{/each}}{{/if}}{{#if pvp_rankings_ultra_league}}{{#compare bestUltraLeagueRank '<=' pvpDisplayMaxRank}}{{#compare bestUltraLeagueRankCP '>=' pvpDisplayUltraMinCP}}**Ultra League:**\n{{/compare}}{{/compare}}{{#each pvp_rankings_ultra_league}}{{#if this.rank}}{{#compare this.rank '<=' ../pvpDisplayMaxRank}}{{#compare this.cp '>=' ../pvpDisplayUltraMinCP}} - {{pokemonName this.pokemon}} #{{this.rank}} @{{this.cp}}CP (Lvl. {{this.level}})\n{{/compare}}{{/compare}}{{/if}}{{/each}}{{/if}} Maps: [Google]({{{googleMapUrl}}}) | [Apple]({{{appleMapUrl}}})",
      "sticker": "{{{stickerUrl}}}",
      "location": true,
      "webpage_preview": true
    }
  },
  {
    "id": "-0-pvp-mapimg-maplnk",
    "language": "en",
    "type": "monster",
    "default": true,
    "platform": "telegram",
    "template": {
      "content": "*{{name}}{{#is gender 1}}{{#isnt id 32}}\u00A0{{genderData.emoji}}{{/isnt}}{{else}}{{#is gender 2}}{{#isnt id 29}}\u00A0{{genderData.emoji}}{{/isnt}}{{/is}}{{/is}}{{#if form}}{{#isnt formName 'Normal'}} {{formName}}{{/isnt}}{{/if}} {{cp}}\u00A0CP (lvl\u00A0{{level}}){{#if boost}}\u2003{{boostWeatherEmoji}}{{/if}}*\n📊\u00A0*{{round iv}}% ({{atk}}/{{def}}/{{sta}}) IV*\n{{#if pvp_rankings_great_league}}{{#compare bestGreatLeagueRank '<=' pvpDisplayMaxRank}}{{#compare bestGreatLeagueRankCP '>=' pvpDisplayGreatMinCP}}🔵\u00A0*Great League:*\n{{/compare}}{{/compare}}{{#each pvp_rankings_great_league}}{{#if this.rank}}{{#compare this.rank '<=' ../pvpDisplayMaxRank}}{{#compare this.cp '>=' ../pvpDisplayGreatMinCP}}\u2003•\u00A0{{pokemonName this.pokemon}}: #{{this.rank}} ({{this.cp}}\u00A0CP, lvl\u00A0{{this.level}})\n{{/compare}}{{/compare}}{{/if}}{{/each}}{{/if}}{{#if pvp_rankings_ultra_league}}{{#compare bestUltraLeagueRank '<=' pvpDisplayMaxRank}}{{#compare bestUltraLeagueRankCP '>=' pvpDisplayUltraMinCP}}⚫\u00A0*Ultra League:*\n{{/compare}}{{/compare}}{{#each pvp_rankings_ultra_league}}{{#if this.rank}}{{#compare this.rank '<=' ../pvpDisplayMaxRank}}{{#compare this.cp '>=' ../pvpDisplayUltraMinCP}}\u2003•\u00A0{{pokemonName this.pokemon}}: #{{this.rank}} ({{this.cp}}\u00A0CP, lvl\u00A0{{this.level}})\n{{/compare}}{{/compare}}{{/if}}{{/each}}{{/if}}🕓\u00A0*{{time}} ({{tthm}}min\u00A0{{tths}}s)*\n[\u200B]({{{staticMap}}})🗺️\u00A0[Google Maps]({{{googleMapUrl}}})\u2003[Apple Maps]({{{appleMapUrl}}})",
      "sticker": "",
      "location": true,
      "webpage_preview": true
    }
  },
  {
    "id": "-0-pvp-mapimg-nomaplnk",
    "language": "en",
    "type": "monster",
    "default": false,
    "platform": "telegram",
    "template": {
      "content": "*{{name}}{{#is gender 1}}{{#isnt id 32}}\u00A0{{genderData.emoji}}{{/isnt}}{{else}}{{#is gender 2}}{{#isnt id 29}}\u00A0{{genderData.emoji}}{{/isnt}}{{/is}}{{/is}}{{#if form}}{{#isnt formName 'Normal'}} {{formName}}{{/isnt}}{{/if}} {{cp}}\u00A0CP (lvl\u00A0{{level}}){{#if boost}}\u2003{{boostWeatherEmoji}}{{/if}}*\n📊\u00A0*{{round iv}}% ({{atk}}/{{def}}/{{sta}}) IV*\n{{#if pvp_rankings_great_league}}{{#compare bestGreatLeagueRank '<=' pvpDisplayMaxRank}}{{#compare bestGreatLeagueRankCP '>=' pvpDisplayGreatMinCP}}🔵\u00A0*Great League:*\n{{/compare}}{{/compare}}{{#each pvp_rankings_great_league}}{{#if this.rank}}{{#compare this.rank '<=' ../pvpDisplayMaxRank}}{{#compare this.cp '>=' ../pvpDisplayGreatMinCP}}\u2003•\u00A0{{pokemonName this.pokemon}}: #{{this.rank}} ({{this.cp}}\u00A0CP, lvl\u00A0{{this.level}})\n{{/compare}}{{/compare}}{{/if}}{{/each}}{{/if}}{{#if pvp_rankings_ultra_league}}{{#compare bestUltraLeagueRank '<=' pvpDisplayMaxRank}}{{#compare bestUltraLeagueRankCP '>=' pvpDisplayUltraMinCP}}⚫\u00A0*Ultra League:*\n{{/compare}}{{/compare}}{{#each pvp_rankings_ultra_league}}{{#if this.rank}}{{#compare this.rank '<=' ../pvpDisplayMaxRank}}{{#compare this.cp '>=' ../pvpDisplayUltraMinCP}}\u2003•\u00A0{{pokemonName this.pokemon}}: #{{this.rank}} ({{this.cp}}\u00A0CP, lvl\u00A0{{this.level}})\n{{/compare}}{{/compare}}{{/if}}{{/each}}{{/if}}🕓\u00A0*{{time}} ({{tthm}}min\u00A0{{tths}}s)*\n[\u200B]({{{staticMap}}})🗺️\u00A0[Google Maps]({{{googleMapUrl}}})\u2003[Apple Maps]({{{appleMapUrl}}})",
      "sticker": "",
      "location": false,
      "webpage_preview": true
    }
  },

...

  {
    "id": "-0-pvp-mapimg-maplnk",
    "language": "de",
    "type": "monster",
    "default": true,
    "platform": "telegram",
    "template": {
      "content": "*{{name}}{{#is gender 1}}{{#isnt id 32}}\u00A0{{genderData.emoji}}{{/isnt}}{{else}}{{#is gender 2}}{{#isnt id 29}}\u00A0{{genderData.emoji}}{{/isnt}}{{/is}}{{/is}}{{#if form}}{{#isnt formName 'Normal'}} {{formName}}{{/isnt}}{{/if}} {{cp}}\u00A0WP (lvl\u00A0{{level}}){{#if boost}}\u2003{{boostWeatherEmoji}}{{/if}}*\n📊\u00A0*{{round iv}}% ({{atk}}/{{def}}/{{sta}}) IV*\n{{#if pvp_rankings_great_league}}{{#compare bestGreatLeagueRank '<=' pvpDisplayMaxRank}}{{#compare bestGreatLeagueRankCP '>=' pvpDisplayGreatMinCP}}🔵\u00A0*Superliga:*\n{{/compare}}{{/compare}}{{#each pvp_rankings_great_league}}{{#if this.rank}}{{#compare this.rank '<=' ../pvpDisplayMaxRank}}{{#compare this.cp '>=' ../pvpDisplayGreatMinCP}}\u2003•\u00A0{{pokemonName this.pokemon}}: #{{this.rank}} ({{this.cp}}\u00A0WP, lvl\u00A0{{this.level}})\n{{/compare}}{{/compare}}{{/if}}{{/each}}{{/if}}{{#if pvp_rankings_ultra_league}}{{#compare bestUltraLeagueRank '<=' pvpDisplayMaxRank}}{{#compare bestUltraLeagueRankCP '>=' pvpDisplayUltraMinCP}}⚫\u00A0*Hyperliga:*\n{{/compare}}{{/compare}}{{#each pvp_rankings_ultra_league}}{{#if this.rank}}{{#compare this.rank '<=' ../pvpDisplayMaxRank}}{{#compare this.cp '>=' ../pvpDisplayUltraMinCP}}\u2003•\u00A0{{pokemonName this.pokemon}}: #{{this.rank}} ({{this.cp}}\u00A0WP, lvl\u00A0{{this.level}})\n{{/compare}}{{/compare}}{{/if}}{{/each}}{{/if}}🕓\u00A0*{{time}} ({{tthm}}min\u00A0{{tths}}s)*\n[\u200B]({{{staticMap}}})🗺️\u00A0[Google Maps]({{{googleMapUrl}}})\u2003[Apple Maps]({{{appleMapUrl}}})",
      "sticker": "",
      "location": true,
      "webpage_preview": true
    }
  },
  {
    "id": "-0-pvp-mapimg-nomaplnk",
    "language": "de",
    "type": "monster",
    "default": false,
    "platform": "telegram",
    "template": {
      "content": "*{{name}}{{#is gender 1}}{{#isnt id 32}}\u00A0{{genderData.emoji}}{{/isnt}}{{else}}{{#is gender 2}}{{#isnt id 29}}\u00A0{{genderData.emoji}}{{/isnt}}{{/is}}{{/is}}{{#if form}}{{#isnt formName 'Normal'}} {{formName}}{{/isnt}}{{/if}} {{cp}}\u00A0WP (lvl\u00A0{{level}}){{#if boost}}\u2003{{boostWeatherEmoji}}{{/if}}*\n📊\u00A0*{{round iv}}% ({{atk}}/{{def}}/{{sta}}) IV*\n{{#if pvp_rankings_great_league}}{{#compare bestGreatLeagueRank '<=' pvpDisplayMaxRank}}{{#compare bestGreatLeagueRankCP '>=' pvpDisplayGreatMinCP}}🔵\u00A0*Superliga:*\n{{/compare}}{{/compare}}{{#each pvp_rankings_great_league}}{{#if this.rank}}{{#compare this.rank '<=' ../pvpDisplayMaxRank}}{{#compare this.cp '>=' ../pvpDisplayGreatMinCP}}\u2003•\u00A0{{pokemonName this.pokemon}}: #{{this.rank}} ({{this.cp}}\u00A0WP, lvl\u00A0{{this.level}})\n{{/compare}}{{/compare}}{{/if}}{{/each}}{{/if}}{{#if pvp_rankings_ultra_league}}{{#compare bestUltraLeagueRank '<=' pvpDisplayMaxRank}}{{#compare bestUltraLeagueRankCP '>=' pvpDisplayUltraMinCP}}⚫\u00A0*Hyperliga:*\n{{/compare}}{{/compare}}{{#each pvp_rankings_ultra_league}}{{#if this.rank}}{{#compare this.rank '<=' ../pvpDisplayMaxRank}}{{#compare this.cp '>=' ../pvpDisplayUltraMinCP}}\u2003•\u00A0{{pokemonName this.pokemon}}: #{{this.rank}} ({{this.cp}}\u00A0WP, lvl\u00A0{{this.level}})\n{{/compare}}{{/compare}}{{/if}}{{/each}}{{/if}}🕓\u00A0*{{time}} ({{tthm}}min\u00A0{{tths}}s)*\n[\u200B]({{{staticMap}}})🗺️\u00A0[Google Maps]({{{googleMapUrl}}})\u2003[Apple Maps]({{{appleMapUrl}}})",
      "sticker": "",
      "location": false,
      "webpage_preview": true
    }
  },

So there's basically 1 (with en, however default=false) from the upstream example dts.json, than my own ones, with first one en with default=true some more in en with default=false and then the same for de.

Now when I do /language de and set a tracking rule without any templates it correctly uses the anticipated one, but when I use /language en it always goes to template1, even if I e.g. change the order and put it behind "my" en-default.

Cheers.

[jfberry]

This is the order of matching from the code:

        // Exact match
        let findDts = this.dts.find((template) => template.type === templateType && template.id.toString() === templateName && template.platform === platform && template.language == language)

        // First right template and platform and no language (likely backward compatible choice)
        if (!findDts) {
            findDts = this.dts.find((template) => template.type === templateType && template.id.toString() === templateName && template.platform === platform && !template.language)
        }

        // Default of right template type, platform and language
        if (!findDts) {
            findDts = this.dts.find((template) => template.type === templateType && template.default && template.platform === platform && template.language == language)
        }

        // First default of right template type and platform with empty language
        if (!findDts) {
            findDts = this.dts.find((template) => template.type === templateType && template.default && template.platform === platform && !template.language)
        }

        // First default of right template type and platform
        if (!findDts) {
            findDts = this.dts.find((template) => template.type === templateType && template.default && template.platform === platform)
        }

Perhaps the order of my logic is faulty

[philcerf]

Uhm.... I assume it's allowed that different templates have the same name if e.g. at least language differs?

If so, then I think it would need to be something like this:

• type & platform always need to match
• next, match of:…
  1. language, ID, default=true (i.e. default gives even more precedence, than just an exact match, but one could also omit this)
  2. language, ID, default=false (i.e. exact match)

(then, if duplicate IDs with different languages are allowed, which would make sense, IMO)

3. language, default=true (i.e. if no matching ID, take the default of the selected lang)
4. language, default=false (i.e. if no matching ID and no default, take any of the selected lang)

(starting from here, one could make it better if one had an order of precedence of languages,... e.g. something like: if no lang matches, then take de first (if that's the local language), next en, next whatever. If one doesn't have this, I'd continue as follows:)

5. ID, default=true (i.e. if no matching language, take the default of the selected ID)
6. ID, default=false (i.e. if no matching language and no default, take any of the selected ID)

last but not least:

7. default=true
8. any

And I'd consider the special cases like "empty language" to be identical to no matching language.

[jfberry]

Yes

I tried to take into account explicitly language is not present in the sequence, I am not clear why the sequence you propose is better probably need some worked examples which are realistic compared to previous iterations of configuration bearing in mind really you should be matching platform, language, template type, template name and anything else is really just a guess

[philcerf]

Well, AFAIU your code it's the following:

1. type, platform, language, ID, default=* (I guess it would take whatever comes first?)
2. type, platform, language="", ID, default=* Having the fallback 2nd is IMO subotimal cause would probably override any possibly better choices, if there is only one template with language node set, e.g. imagine one has the following templates: monster, telegram, de, foo, true monster, telegram, de, bar, false monster, telegram, "", foo, false monster, telegram, en, bar, true

Imagine a user has set lang=en and templatefoo, then he'd get no exact match, and would probably end up with the 3rd choice, right? Even though this is probably just some leftover (as it has not language) and there is a higher matching default=true version for en, which just happens to be named bar. However, this is anyway just relevant for cases, where people forgot to set the language upon upgrading to the new version

3. type, platform, language, ID=*, default=yes
4. type, platform, language="", ID=*, default=yes
5. type, platform, language=, ID=, default=yes

I think that misses at least those cases were one has a better default=false match (e.g. no matching language, but matching ID) and again, the language="" case should probably come last.

However, I still don't get why in my case above, it doesn't work,... cause I do have languages all set, and there even is an exact match. Or is there? What does the code, if there is no template set in the DB? Does it perhaps implicitly use "1"?

[jfberry]

The fallback in (2) was moved higher to support people running without languages at all, but perhaps too high The code before multi-language would just try the template selected if not then the first with default=true. Of course with no language it's much easier to see that the two stages is fine. type and platform are obviously a given. No point selecting the wrong type or wrong platform. Picking the wrong template shouldn't happen unless default is true, so really should only be considered as part of those choices. Should it ever pick the wrong language? Perhaps it should only fall back to the system locale from current language rather than language = ''. Language='' was a deliberate attempt to not pick the wrong language but rather a fallback for historic configuration

[philcerf]

Should it ever pick the wrong language?

Depends,... I would say yes, at least when otherwise there wouldn't be a template at all.

Other than that, the "best" solution would be perhaps what I've wrote very above, that one could set a (server-side) preference of the order of languages, maybe implicitly, perhaps via the order in availableLanguages (which would then also allow to select the default language if the user hasn't set anything.

But it would be better if one had a separate setting for that, which could be only a subset of availableLanguages, this would allow to e.g. select "de, en" (in Germany most (or many ^^) people speak English, too,... but a fallback to French would perhaps make less sense and in that case it could be better to pick a wrong language.

[philcerf]

btw: I've just checked and when one specifies no template in the command, it indeed seems to write 1 into the DB, which is the perfect reason why I see the original problem.

[jfberry]

Ok so now we kind of understand the parameters - it'd be nice to work out a definitive list of orders then we can try it with some examples. I'll happily change the code but I don't want to keep changing it. And I can fix any case sensitivity problems at the same time (referred to in #313) - I assume the database always gets the lower case version and I should be dragging the template name to lower case before comparing

[philcerf]

1. With respect to the order: From my side, it's probably no longer strictly necessary to change anything...

With the changes from #325, I can keep my own dts.json largely aligned with the upstream/example one (i.e. I don't need to modify the 1 in the JSON and will have only added (and no modified) lines when I diff mine with the example... any modified lines would mean I need to align something. So I'm happy here.

If you do want to rework this nevertheless, I'd say the order which I gave above sounds mostly reasonable to me, but to make it really perfect, one would probably need a new languagePreferenceOrder setting or so. Not sure whether all this is worth the effort.

2. I'd instead rather suggest to re-define the meaning of default in templates... IMO it's strange to have two different kinds of default

3. As for the case-sensitivity issue of the template names... if you agree that we don't follow up (1) and/or (2) I'd make a separate issue for that... otherwise this gets too messy here.