feat: add tables to facilitate translation of weapontypes and categories

[LordMidas]

The issue is that because the Key inside ::Const.Items.WeaponType is what is used to check for strings inside weapon.m.Categories to assign weapontype or vice versa, it creates a hurdle for translators as translated strings are not matched and therefore automatic weapontype assignment fails.

We change this by decoupling those strings and adding new tables where relevant strings can be pushed by translators.

[Enduriel]

Conceptually lgtm

[Dimon485]

Everything's fine now! Thanks guys, you're the best!

[Suor]

I see 3 issues with this implementation:

1. We change behavior, comparing to prev MSU. We used to match strings "Throwing" and "Musical" and we not doing it anymore. Say Hextech Rifle from Fantasy Bros have this.m.Categories = "Throwing/Crossbow/Bow, Two-Handed"; and Barbarian Drum in Legends has this.m.Categories = "Musical, Staff, Two-Handed";
2. It's mixes two purposes into one thing again - representation string and matching strings. Which is not the same when translation is used, because we want to retain english matching strings but not represenation ones.
3. The nested structure is harder to extend when a flat one. Since strings are unique we can still use them as keys.

[LordMidas]

1. The point is that certain strings in the Categories should be matched to a certain WeaponType. The question is what should be these strings? In my opinion the answer is determined by the fact that when MSU does buildCategoriesFromWeaponType it puts the name there i.e. getWeaponTypeName() and not any other representation string e.g. Throwing. Therefore, to keep things consistent, if someone defines Categories manually and wants WeaponType to be assigned based on that then these strings in the Categories should be WeaponTypeName. So one is not expected to put Throwing/Crossbow/Bow but rather it should be Throwing Weapon/Crossbow/Bow.

However, your point is valid that this changes behavior. Previously Throwing in Categories was enough and now it will require Throwing Weapon. I don't know if this should be considered a fix of behavior or change.

2. I think my first paragraph above is relevant for this. There is no concept of "Representation strings", but rather it should be that the string that buildCategoriesFromWeaponType puts in m.Categories when building categories automatically from defined WeaponTypes should be the same string that buildWeaponTypeFromCategories detects when building WeaponType from defined Categories. This table is created at the start and is static, so translators can simply push to this table, no?

3. I did not understand this point, could you elaborate please? It's meant to be an array that people can just push to.

[Suor]

2. The whole thing with translation is that we want to separate what we are matching and what we are presenting. I.e. match both, present in a target language. I.e. matching strings should not be used wherever else, like you do in build categories for One-Handed and Two-Handed.
3. The simple extend from this turns into some more tricky code. One-Handed and Two-Handed thing is even more error prone. I.e. this is not an array it's a table of arrays, which is unnecessary for matching but using it for dual purpose makes it like that.

[Suor]

I appreciate the try to extract One-Handed and Two-Handed. Not sure this would be useful for anyone though. For the guys translating strings inplace it will make it worse, i.e. they will translate it and loose matching for english or not translate and then they will be left as is. For Rosetta it just doesn't matter.

[LordMidas]

Let's say that a certain WeaponType has a different "string" in the matching than its WeaponTypeName. Let's say this WeaponType is called Throwing. It's matching string is Throwing but its WeaponTypeName is changed to Jumping Weapon.

Now consider this situation:

• Someone uses buildCategoriesFromWeaponType. When this is used MSU uses getWeaponTypeName() to generate the m.Categories string.
• Then someone adds a new WeaponType to the weapon by addWeaponType(::Const.Items.WeaponType.Throwing).
• This should then again update the categories with step 1 above. This now adds Jumping Weapon to the m.Categories string.
• Then someone clears the this.m.WeaponType.
• Then someone calls buildWeaponTypeFromCategories to rebuild the WeaponType.
• Remember that our matching string was Throwing .. but because of the function above, MSU already put Jumping Weapon in the Categories. So it doesn't get matched, and the WeaponType isn't correctly assigned.

Therefore, in my opinion, it is essential that the two strings are not different i.e. the string that buildCategoriesFromWeaponType puts in m.Categories should exactly match the string that buildWeaponTypeFromCategories expects to be there. And because the former function uses getWeaponTypeName() the latter also has to use the same as the detection string.

However, we allow translators to push new strings for WeaponTypeName into the WeaponTypeCategoriesStrings table's arrays. This basically makes the buildWeaponTypeFromCategories function to detect both the original getWeaponTypeName() i.e. the English hard-coded one and the one from translators (as long as they are modifying the WeaponTypeName table in a hook that runs after MSU and are not overwriting MSU files).

In summary:
The translators should queue after MSU and do the following:

• Overwrite the WeaponTypeName table with their own strings. This allows those strings to be displayed by getWeaponTypeName().
• Push their strings to WeaponTypeCategoriesStrings. This allows hard-coded English weapons to be properly detected, while also translated ones to be properly detected by buildWeaponTypeFromCategories.

What are your thoughts about this?

[Suor]

The first thought is that this is overcomplicated, same as this implementation. Second one - is it even supposed by anyone to call these build* functions or clear weaponType? Because if someone starts to do such things your code will break anyway because you initWeaponType once.

So certain symmetry between matching and representation strings might be needed, like ones included into another so that statically translated weapons will work properly. But that's it and it should be translator responsibility.

[Suor]

So actually representation is only for One/Two-Handed. And pushing to the end of the arrays will work for both, even if that feels awkward. If pushing more than one item to those will need to be sure to push the canonical one later.

For weapon types there is no clash, representation strings go to ::Const.Items.WeaponTypeName and matching strings to this new nested structure.

Still using a flat table {string = type} makes more sense to me. And One/Two-Handed is not even a weapon types, so I would just use a separate thing for those. Will make for simpler code.

[Suor]

The implementation I had in mind is just:

::Consts.Items.CategoryToWeaponType <- clone ::Const.Items.WeaponType;

and then use that in the matching code. Nothing fancy, pure decoupling.

Maybe add One/Two-Handed to that new table and use it in the code. Leave the string building as is.

[Suor]

There is already a one broken translation of MSU based on this PR:

I.e. ::Const.Items.WeaponTypeName translated inplace and so ::Const.Items.WeaponTypeCategoriesStrings will not have english strings inside.

[LordMidas]

@Suor I see. Yes that's a valid point. I have pushed a commit that should resolve this. Also that translation is overwriting the WeaponHandedCategoriesStrings table... why? It should simply push its value to it and not replace the English one!

I still don't understand how a simple CategoriesToWeaponType table will fix the situation. Because we need to pull Categories strings inside buildCategoriesFromWeaponType so we need a WeaponType to Categories strings map as well. And this one needs to use WeaponTypeName, not just a representation string.

And similarly we need buildWeaponTypeFromCategories which needs to translate the WeaponTypeName back into WeaponType.

One-Handed and Two-Handed strings are used in Categories so we need to have support for them in order to correctly utilize both the above functions to assign the correct categories or weapontype+itemtype.

[Suor]

This example won't work because key is not "Axe" here. Probably should be still Axe for consistency.

Also if ::Const.Items.WeaponTypeName is translated above then pushing is not needed for the most part.

[Suor]

Also, the guys translating mod itself not Rosetta like can just add , "..." to arrays below

[Suor]

These "Throwing Weapon" and "Musical Instrument" entries are in fact noop. Make things a little bit slower.

[Suor]

This version should work 👍

I still prefer flat to nested, heh :)

[Suor]

Copied from Discord:

Instead of exposing structures may expose "official API" via functions. I.e.:

::MSU.useStrings({
    WeaponTypeName = {
        "Axe": "..."
        "Throwing weapon": "..."
    },
    WeaponCategories = {
        "One-Handed": "..."
        ...
    } 
})

Then it will be easier for you to change internals later.

Here strings work best as keys, because it's more universal and easier to understand for translators.