Skip to content

feat: add tables to facilitate translation of weapontypes and categories #463

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.

Sign up for GitHub

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

Open
LordMidas wants to merge 5 commits into
base: development
Choose a base branch
from
Open

feat: add tables to facilitate translation of weapontypes and categories #463

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e41f6a0
feat: add tables to facilitate translation of weapontypes and categories
LordMidas May 5, 2025
258d522
fix: push weapon type name to the strings array in adding new weapontype
LordMidas May 5, 2025
f15d381
refactor: create WeaponTypeCategoriesStrings using WeaponTypeName
LordMidas May 5, 2025
958721a
fix: trying to call function before declaration
LordMidas May 6, 2025
587afc5
fix: add the english strings manually to the categories strings table
LordMidas May 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions msu/hooks/config/items.nut
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,27 @@ foreach (itemType in ::Const.Items.ItemType)
"Musical Instrument"
];

// Map WeaponTypes to relevant strings from weapon.m.Categories.
// Is used during automatic assignment of weapontypes to weapons.
// Translators should push strings to the relevant weaponType here.

// Key = WeaponType
// Value = array of strings from weapon.m.Categories that should match to this weapon type
::Const.Items.WeaponTypeCategoriesStrings <- {};
foreach (w in ::Const.Items.WeaponType)
{
::Const.Items.WeaponTypeCategoriesStrings[w] <- [::Const.Items.getWeaponTypeName(w)];

@Suor Suor May 6, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 LordMidas May 6, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

  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.

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

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

@Suor Suor May 6, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. 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.
  2. 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 Suor May 6, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 LordMidas May 6, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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 Suor May 6, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 Suor May 6, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

}

// Map OneHanded and TwoHanded to relevant strings from weapon.m.Categories
// Is used during weapon.buildCategoriesFromWeaponType.
// Translators should push strings to the relevant handed-ness here.
// The last index string is always used to build the categories string.
::Const.Items.WeaponHandedCategoriesStrings <- {
OneHanded = ["One-Handed"],
TwoHanded = ["Two-Handed"]
};

::Const.Items.getWeaponTypeName <- function( _weaponType )
{
local idx = ::MSU.Math.log2int(_weaponType) + 1;
Expand Down Expand Up @@ -127,5 +148,6 @@ foreach (itemType in ::Const.Items.ItemType)
_weaponTypeName = _weaponType;
}

::Const.Items.WeaponTypeCategoriesStrings[::Const.Items.WeaponType[_weaponType]] <- [_weaponTypeName];
::Const.Items.WeaponTypeName.push(_weaponTypeName);
}
40 changes: 27 additions & 13 deletions msu/hooks/items/weapons/weapon.nut
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,29 +54,41 @@
return;
}

foreach (k, w in ::Const.Items.WeaponType)
foreach (weaponType, strings in ::Const.Items.WeaponTypeCategoriesStrings)
{
if (categories.find(k) != null)
foreach (str in strings)
{
this.m.WeaponType = this.m.WeaponType | w;
if (categories.find(str) != null)
{
this.m.WeaponType = this.m.WeaponType | weaponType;
break;
}
}
}

if (categories.find("One-Handed") != null && !this.isItemType(::Const.Items.ItemType.OneHanded))
foreach (str in ::Const.Items.WeaponHandedCategoriesStrings.OneHanded)
{
this.m.ItemType = this.m.ItemType | ::Const.Items.ItemType.OneHanded;
if (this.isItemType(::Const.Items.ItemType.TwoHanded))
if (categories.find(str) != null && !this.isItemType(::Const.Items.ItemType.OneHanded))
{
this.m.ItemType -= ::Const.Items.ItemType.TwoHanded;
this.m.ItemType = this.m.ItemType | ::Const.Items.ItemType.OneHanded;
if (this.isItemType(::Const.Items.ItemType.TwoHanded))
{
this.m.ItemType -= ::Const.Items.ItemType.TwoHanded;
}
break;
}
}

if (categories.find("Two-Handed") != null && !this.isItemType(::Const.Items.ItemType.TwoHanded))
foreach (str in ::Const.Items.WeaponHandedCategoriesStrings.TwoHanded)
{
this.m.ItemType = this.m.ItemType | ::Const.Items.ItemType.TwoHanded;
if (this.isItemType(::Const.Items.ItemType.OneHanded))
if (categories.find(str) != null && !this.isItemType(::Const.Items.ItemType.TwoHanded))
{
this.m.ItemType -= ::Const.Items.ItemType.OneHanded;
this.m.ItemType = this.m.ItemType | ::Const.Items.ItemType.TwoHanded;
if (this.isItemType(::Const.Items.ItemType.OneHanded))
{
this.m.ItemType -= ::Const.Items.ItemType.OneHanded;
}
break;
}
}
}
Expand Down Expand Up @@ -144,13 +156,15 @@

if (this.m.Categories != "") this.m.Categories = this.m.Categories.slice(0, -1) + ", ";

// We always use the last entry that was pushed to the strings so translators just
// have to push it there and it works.
if (this.isItemType(::Const.Items.ItemType.OneHanded))
{
this.m.Categories += "One-Handed";
this.m.Categories += ::Const.Items.WeaponHandedCategoriesStrings.OneHanded.top();
}
else if (this.isItemType(::Const.Items.ItemType.TwoHanded))
{
this.m.Categories += "Two-Handed";
this.m.Categories += ::Const.Items.WeaponHandedCategoriesStrings.TwoHanded.top();
}
}

Expand Down