minetest-mods / unified_inventory

An extensible inventory mod which allows searching crafting and browsing for recipes in the same dialogue.
Other
50 stars 38 forks source link

Fix invalid use of the NS function. Resolves #187 #188

Closed Poikilos closed 2 years ago

SmallJoker commented 2 years ago

As correct as it may look, there is still a call to S or NS necessary outside of the table. The locale update script does a static analysis:

I hope this clarifies why a plaintext call to this function is necessary, maybe even within an extended comment section (i.e. --[[ ... bla bla bla ]] would be fine.

Poikilos commented 2 years ago

Using S("Result @1 (@2") will set the variable to "Result nil, nil" since there are no params, no? Even if the translation "worked" (left "@1 (@2)" in there) The S function is called on it later, so there wouldn't be anything found in the translation table since the value wasn't an id (was translated at the initialization not still an id). The substitution only didn't break before because NS was set to do nothing. However, the NS function (name suggested by the intllib docs) has an actual meaning in the intllib docs so setting it to do nothing then using it differently than NS makes the code unclear.

SmallJoker commented 2 years ago

unified_inventory uses Minetest's client-side translation feature, and not intllib. Latter was replaced a while ago. Hence NS now seems to used as "do not translate" dummy call in order to get the translation update script working.

For reference, in MTG: https://github.com/minetest/minetest_game/blob/master/mods/dye/init.lua#L107

Poikilos commented 2 years ago

Sure, I get it. However, the dummy call was wrong anyway. Calling NS on the variable then S later makes 0 sense. It being there may actually confuse people trying to learn or trying to work on the mod.

SmallJoker commented 2 years ago

8e9ea34ae8