Rework Fluent file path and yaml references#21622
Conversation
PunkPun
left a comment
PunkPun
left a comment
There was a problem hiding this comment.
UserInterface.String() is definitely weird, we should do something different. At the minimum UserInterface.GetString()
I recon the best might be FluentProvider.GetString()?
PunkPun
left a comment
PunkPun
left a comment
There was a problem hiding this comment.
while FluentStrings is a passable name, I feel like strings is the wrong term here. We are expecting .ftl files, not strings
FTL stands for Fluent Translation List, and it's the expected format of the field. Perhaps a better name would be FluentTranslationFiles, following the already existing convention.
|
Actually, the various nodes in map.yaml do expect raw file contents as child nodes, with the ability to reference external files being an odd implementation detail. The fluent docs define that the basic unit of translation as a "message", so we should probably be replacing all uses of GetString etc with GetMessage. |
The terminology in Linguini and our wrapping of that is to use "Bundle" to refer to a collection of messages. So would |
pchote
force-pushed
the
remove-magic-translation-paths
branch
from
4f9f121 to
4794b5e
Compare
|
Rebased on #21623 and reworked based on feedback above and in Discord. This now also fixes #21618 by updating the and |
pchote
force-pushed
the
remove-magic-translation-paths
branch
from
4794b5e to
52ef68b
Compare
| { | ||
| { "Rules", map.RuleDefinitions }, | ||
| { "Translations", map.TranslationDefinitions }, | ||
| { "FluentMessages", map.FluentMessageDefinitions }, |
There was a problem hiding this comment.
This is also worse. We don't call it MiniYamlRules but just Rules. Please keep this as is for consistency.
There was a problem hiding this comment.
It is definitely confusing. I almost feel like just having it be Fluent would be better
There was a problem hiding this comment.
In Fluent, the basic unit of translation is called a message. Messages are containers for information. You use messages to identify, store, and recall translation information to be used in the product.
This node is listing filenames containing/inline definitions of fluent messages. This is the same as "Sequences" listing filenames containing/inline definitions of sequences, etc.
There is no logic here to handle multiple languages, which I feel is a basic requirement for calling something "Translations".
There was a problem hiding this comment.
The definition already says that it is TranslationMessages. People who don't know that @projectfluent is a translation framework will be irritated. Ideally you should revert the renamings from #21602 as well.
There was a problem hiding this comment.
That would conflict with the naming of things in the actual translation system that manages the sets of language-specific fluent / sound / sprite files.
There was a problem hiding this comment.
On a yaml level I think this is the only place it is called fluent? So for non-power users both the concept of fluent and messages might be foreign
There was a problem hiding this comment.
for more advanced modders the other place will be #21622 (comment). I think we should make sure these are named well.
| // TODO: This should be controlled by a user-selected translation bundle! | ||
| public readonly string FluentCulture = "en"; |
There was a problem hiding this comment.
No, it should be controlled by a user settings like it already is.
There was a problem hiding this comment.
it should be controlled by the bundles. The user should be able to select the language from installed languages eventually yeah. It should give the pop-up window to reload the game same way changing display size does
pchote
force-pushed
the
remove-magic-translation-paths
branch
3 times, most recently
from
36d5ded to
bf4a684
Compare
This enables the RC to parse and share custom messages as part of the map's custom rules without any additional API changes.
pchote
force-pushed
the
remove-magic-translation-paths
branch
from
bf4a684 to
30e4228
Compare
PunkPun
dismissed
Mailaender’s stale review
It should be exposed to settings menu, but a ton of work is yet to be done to properly implement it into setting menu
PunkPun
left a comment
PunkPun
left a comment
There was a problem hiding this comment.
This changes done here should be good enough for playtest
3 participants
Followup to #21602 and implements most of the remaining points from the previous Discord discussion.
The motivation for these changes is to remove the magic file path replacements and the specific references to "en" in the file paths. The exact shape of the future ingame translation system is still up for debate, but i'm fairly confident that it will end up as some variation of one of the following approaches (likely the first):
Dynamic file overrides for mod-defined paths - e.g. via a downloadable translation bundle that contains a manifest saying "remap file
ra|fluent/rules.ftltolangbundle|rules.ftl. In this case it would be incorrect to specifyenin the mod-defined paths because the files that they load may not be in english.Explicitly defined file paths for each language at the point of usage - in which case we will need to update all of these file references when future languages are supported, meaning that nothing is lost by removing "en" from the paths in the meantime.
The incomplete user-level language selection is replaced by a mod-level language key. This enables modders and downstream repackagers to ship non-english languages and not have Fluent misbehave due to assuming the "en" culture.
For the time being, this PR updates the logic added in #21601 for passing strings to the content installer, but I intend to file a third PR in this series that will remove a lot of that magic in favour of an explicit mod.yaml definition for each content installer mod.