multilang: strange behaviour when selecting dts templates #317

philcerf · 2021-02-08T13:46:30Z

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.

The text was updated successfully, but these errors were encountered:

jfberry · 2021-02-08T13:54:59Z

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 · 2021-02-08T19:15:12Z

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:…

language, ID, default=true (i.e. default gives even more precedence, than just an exact match, but one could also omit this)
language, ID, default=false (i.e. exact match)

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

language, default=true (i.e. if no matching ID, take the default of the selected lang)
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:)

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

last but not least:

default=true
any

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

jfberry · 2021-02-08T19:23:39Z

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 · 2021-02-08T20:28:24Z

Well, AFAIU your code it's the following:

type, platform, language, ID, default=* (I guess it would take whatever comes first?)
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

type, platform, language, ID=*, default=yes
type, platform, language="", ID=*, default=yes
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 · 2021-02-08T21:20:32Z

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 · 2021-02-08T23:56:29Z

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 · 2021-02-08T23:57:15Z

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 · 2021-02-09T16:55:31Z

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

When no template name is specified in a user’s command, a default of “1” was used so far. This makes the default name configurable, thereby working around the issues described in: KartulUdus#317

philcerf · 2021-02-10T18:58:04Z

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.

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
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.

jfberry mentioned this issue Feb 9, 2021

template names with _ not correctly parsed #313

Closed

philcerf mentioned this issue Feb 9, 2021

make default template name configurable #325

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

multilang: strange behaviour when selecting dts templates #317

multilang: strange behaviour when selecting dts templates #317

philcerf commented Feb 8, 2021

jfberry commented Feb 8, 2021

philcerf commented Feb 8, 2021

jfberry commented Feb 8, 2021

philcerf commented Feb 8, 2021

jfberry commented Feb 8, 2021 •

edited

Loading

philcerf commented Feb 8, 2021

philcerf commented Feb 8, 2021

jfberry commented Feb 9, 2021

philcerf commented Feb 10, 2021

multilang: strange behaviour when selecting dts templates #317

multilang: strange behaviour when selecting dts templates #317

Comments

philcerf commented Feb 8, 2021

jfberry commented Feb 8, 2021

philcerf commented Feb 8, 2021

jfberry commented Feb 8, 2021

philcerf commented Feb 8, 2021

jfberry commented Feb 8, 2021 • edited Loading

philcerf commented Feb 8, 2021

philcerf commented Feb 8, 2021

jfberry commented Feb 9, 2021

philcerf commented Feb 10, 2021

jfberry commented Feb 8, 2021 •

edited

Loading