NPBruce / valkyrie

Valkyrie GM for Fantasy Flight Board Games
Apache License 2.0
506 stars 105 forks source link

Support for language selection and multiple language scenarios #45

Closed eorahil closed 7 years ago

eorahil commented 7 years ago

MoM2E has a really cool multilanguage system, allowing that some scenarios can be translated to some languages and others to other different ones. Even, there is still translation for almost every elements of the game (tiles, characters, items, events,...). They even created a special Font including expansion icons and special characers from other languages. In order to simplify localization, valkyrie should use same system for it's own translatable strings, and edited scenario files. Valkyrie should allow to select one language from the list of available ones in original App, and scenario editor use the user language. Allowing that, as soon as an scenario is created in english, for example, a Spanish user can edit this scenario in order to edit texts and translate to his language, not overwriting original text, but adding new language texts.

NPBruce commented 7 years ago

Yeah I am aware that I have not built localisation in at all. It would be great but is further down my list than complete game support at the moment. I am happy for you to work on abstracting all the hard coded text if you like.

eorahil commented 7 years ago

In order to include multilanguage texts in a custom scenario, quest.ini should look like this:

[Quest] name=Example Scenario description=[this:EXAMPLE_TEXT_MULTILANG] type=MoM

...

[Localization] EXAMPLE_TEXT_MULTILANG,This is a multilanguage text.,Esto es un texto multilenguaje.,,,,,,,

The localization tag should be exactly like FFG localization format, in order to homogeneize. There could be [ffg:XXX] tags in order to translate elements like Monster names, still translated in ffg files. If there is no translation, the text will be empty, in this case the app will print the default language, allways english, or some defined in the quest default language atribute (new). The MADGaramondPro is an incredible awesome font which includes all foreign languages accutes, special characters, even asian characters. So using this font in text editors, every languages supported in the original app will be supported in Valkyrie. I'm creating a StringI18n class in order to manage this behaviour. It can be used to get/set the current language text or to create a wholeString including all languages in order to write the quest.ini file in localization tag. I didn'd dive enought in code in order to make it works but i think this is the next step in order to make Valkyrie multilanguage. I will pull request as soon as it works

NPBruce commented 7 years ago

Can I suggest that the csv part be in it's own file? You can have localization.csv (or even split to say localization-events.csv, etc).

Then just include this like so:

[QuestData]
file
another file

This is already supported for additional ini data, just need to add csv. To this end all such files must have a clear header, followed by language definitions, ie:

CVS Text
English,Spanish,Other

Next I think it would be good to have a function (maybe {lookup:<>} that tries a quest specific lookup first, and if that fails tries FFG localisation.

The other big thing to think about is that there is a lot of text in Valkyrie itself. One (or more) CSVs will need to be created for this, and a function that tries first to match a key with a local file, and if that fails use the FFG localisation.

NPBruce commented 7 years ago

Also need to think about how the editor works. Perhaps you set an editing language globally.

NPBruce commented 7 years ago

Another thought: don't even have the lookup. Assume that it is one, and display as is when there is no match.

[Quest]
name=EXAMPLE_SCENARIO
description=EXAMPLE_TEXT
type=MoM
eorahil commented 7 years ago

Ok. My first thought was the localization.csv file but then I suposes the default save format for scenarios would need to be .valkyrie. I saw in wiki that .valkirie files can't be edited. But ok, lets use localization.csv. I prefer only one localization file for every texts in the quest. There must be an localization.csv for valkyrie editor itself, in order to translate the app's buttons and messages. Agree with the lookup, I supose the order is first Scenario, second valkyrie files and last the ffg files. For the last comment about lookup, if there is an scenario EXAMPLE_TEXT key, but it isn't in my language, I wont lookup valkyrie files neither ffg files. I will use the default language text. If i'm playin an edited scenario partialy transtlated, I prefer the text in english than the key name. I will resume using google translator, with the KEY que game will be broken.

For the editor my thoughts are:

NPBruce commented 7 years ago

Ok. My first thought was the localization.csv file but then I suposes the default save format for scenarios would need to be .valkyrie. I saw in wiki that .valkirie files can't be edited.

.valkyrie is just a zip file for a quest (or multiple quests) for distribution. It means you can have ini files, cvs files, audio, images, etc. Rename to/from .zip to use.

For the last comment about lookup, if there is an scenario EXAMPLE_TEXT key, but it isn't in my language, I wont lookup valkyrie files neither ffg files

Thinking on this again I don't think it should look up anything but local quest data. quests shouldn't rely of FFG data which could change. If you want quest specific text, include it in your cvs.

For content data (things in content directory) it can look up a Valkyrie csv and fall back to FFG, then fall back to english (for both), then fall back to key only.

eorahil commented 7 years ago

I'm putting my efforts in this task but I feel its too large (the task), assuming that you are working too in same classes I'm afraid my pull request, when it comes, would be too big and tricky... I suggest to divide the task intro few tasks, in order to pull request each change before facing next one. These tasks are based on my first examinations of your code. I think the tasks should be:

Don't know if it's preferable to create these 4 issues individually or if we could continue using this single one.

ADDITIONALLY: Diving into code, I was really confused about the two classes with the same name Quest. There is a Quest class which defines the structure of an scenario (monsters, tiles, events, ...). And there is another Quest class defining the current status of a playing game. I would suggest that this second Quest class should be renamed as GameSession or SessionStatus or something like this in order to be a more understandable code. I think. What you think about it?

NPBruce commented 7 years ago

I completely agree that frequent small pull requests are much better than infrequent big ones. I don't think merging will be too hard though, even if we do edit the same files due to the nature of this change.

On global variables: 'Game' class has one object which can be found from anywhere, so I use that as a reference point for all common data (have a look at the start of this class).

On Quest: Do you mean Quest and QuestData?

eorahil commented 7 years ago

I was refering to the Quest class inside QuestLoader.cs in comparison with the Quest.cs class. I'm used to the rule "One class/ One file" and I sometimes feel lost analyzing the code. But no problem, as soon as I get used to it structure I won't feel lost.

NPBruce commented 7 years ago

Yes, that one is a subclass of QuestLoader. I am OK with you renaming things and splitting things into more files where you think it makes more sense. Not all of the structure is good.

eorahil commented 7 years ago

first phase changes requested #61

NPBruce commented 7 years ago

I am marking this as post 1.0 because I don't plan to hold up 1.0, but I am also happy if this is working for the release of 1.0

eorahil commented 7 years ago

Pulled #91 for ffg strings localization:

Next step is localization of all valkyrie strings

NPBruce commented 7 years ago

Pull request #91 merged (with some changes).

Some comments on future direction:

I would like either a quest specific and a core subclass of StringKey or use settings in StringKey (I have currently just disabled in quests for now).

It would be good to remove {ffg:} entirely. If all ini files never have user facing text the lookup can be assumed. Note that {0}/{1} etc replacement is still required.

eorahil commented 7 years ago

my intention is to use same class for both, StringKey uses LocalizationRead class. Wich contains all dictionaries. And now I will change the FFGLookup method with a generic Lookup who examines the left part of : to know what dictionary will be read. If you want to remove the {ffg:} part, whe can create another constructor for StringKey with two string parameters, one for witch dicionary to use (ffg,val,qst or something like that) and second the key itself, including parameteres for {0} {1}. I think for now we should still be using the {dict:key} notation. introducing now the val (valkyrie) dict and the qst (quest) dict. After completing all localization we could figure out removing dictionary prefix.

I assumed all keys starts with a '{' character. Do you think the preventLookup inside StringKey is needed. If the key doesn't start with '{' there wont be a look. or am I wrong? And if in the app you insert your custom text, the text wont be '{' prefixed and there won't be a lookup try.

NPBruce commented 7 years ago

I like the dictionary specifying, the prevent should probably change to be use quest dictionary instead.

Happy to stick with {dict:key} for now, remove later. Dict should always be known from where the key comes from, so specifying is redundant (and I don't want quests using other dictionaries).

eorahil commented 7 years ago

Where should it be preferable to be the localization.txt file for valkyrie texts?

NPBruce commented 7 years ago

Probably just in Assets/Resources somewhere, unless you want to user to be able to see it (not sure why this would be useful).

NPBruce commented 7 years ago

I am starting to think about #96 and it is strongly connected to this. First to expand on my last comment I think we need a localization for Valkyrie in resources, and additional localization files for all content packs.

Currently for message with a random hero we do this:

{ffg:MESSAGE_KEY:{0}:{rnd:hero}}

We need to use a different key in this situation depending on the hero selected (note that hero can't be selected until the event is raised). I also want to move towards removing the {ffg, so perhaps something like this:

Standard:

MESSAGE_KEY

Replace text:

{repalce:MESSAGE_KEY:{0}:{rnd:hero}}

Pick key on trait:

{trait:{rnd:hero}:male:{replace:MESSAGE_KEY:{0}:{rnd:hero}}:{replace:MESSAGE_KEY_ALT:{0}:{rnd:hero}}

Or is this just over complicating things?

eorahil commented 7 years ago

First of all sorry about my release speed. I got very few free time but hope to get some more from now. The main problem on this is that we consider {trait:{rnd:hero}:male:{replace:MESSAGE_KEY:{0}:{rnd:hero}}:{replace:MESSAGE_KEY_ALT:{0}:{rnd:hero}} a key but its a expression.

In order to not mix concepts. We should separate this into 2 concepts: translation keys and "text expressions". In my opinion whe should keep the StringKey element to get the current language translation of a text. And we should introduce the StringExpression object in order to process these complex messages with a expression. In my personal projects I created something like this in order to create SQL expressions without using strings directly and I think it could be the same. This way we can keep language translation appart from complex message composition. And we can, in near future, add more complexity to our "complex message builder". Put here your ideas for message expressions. I suggest changing: {replace|MESSAGE_TO_REPLACE|param_1|param_1_value|param_2|param_2_value|...} using vertical bar we avoid using ':' wich can be a problem if a text contains it.

NPBruce commented 7 years ago

It's OK spend the time you want to.

I think the problem is we need to have something that picks what key to use, then the key look up, then something to do further on the text after the lookup. These things need to be connected also (ie: based on male/female).

Using : or | is fine (we should escape if needed anyway), but I how do you suggest we pick a different key based on the randomly selected hero?

eorahil commented 7 years ago

I think that what FFG does in his app is:

NPBruce commented 7 years ago

OK, but that means the hero lookup needs to happen before the key lookup, and applied after the lookup.

So case perhaps hero(BASE_KEY, "{0}")? I know this notation is different again, still paying with ideas. I like this better I think. With this we go back to always having a lookup function: maybe key(BASE_KEY) and key(BASE_KEY, dictionary)?

eorahil commented 7 years ago

Requested translation for valkyrie texts. Traits,Tiles and other internal codes are still not translated but it's reasonable understandable even in Spanish. The translation includes Spanish translations and Localization.txt file can de edited in order to translate to other FFG MoM App languages. If non translated language is selected, the texts will be in english

eorahil commented 7 years ago

Ok. So we enter the final part, the Quests translations. My intention is to follow these steps:

NPBruce commented 7 years ago

Back compatibility is easy, there is a format level in the quests now, as long as we don't break the ability to read the existing format (2) then there is no problem.

Given that all user facing text should be in the localisation file, and there is nowhere else it can be (not exposing FFG/Valkyrie text to quest) I propose a more significant change, removing all non functional data from the ini:

quest.ini

[Quest]
format=3
type=MoM
packs=MoM1E

[QuestData]
moredata.ini

[QuestText]
Localization-Events.txt
Localization-Part2.txt

[TokenBasement]
xposition=2
yposition=5
event1=EventClue1A EventClue1B EventClue1C
buttons=1
type=TokenSearch

[EventPrologue]
event1=EventMinCam
buttons=1
audio=AudioDoorCreak1

[EventMinCam]
event1=EventMaxCam
buttons=0
mincam=true
xposition=-8
yposition=-16

Localization.txt

Quest_NAME,The Fall of House Lynch,La caída de la casa Lynch,...
Quest_DESCRIPTION,"Conversion of the Mansions Of Madness first edition scenario.","Conversión del escenario de la primera edición de Mansions Of Madness.",...

EventPrologue_TEXT,"Light rain falls on your face as you ex...","Lluvia ligera cae sobre tu cara cuando","Another translation"
eventPrologue_BUTTON1,{this:CONTINUE}

TokenBasement_TEXT,"Strange vials lie on the bench","Frascos extraños se encuentran en el banco",...
TokenBasement_BUTTON1,"{action} Search","{action} Buscar",....

CONTINUE,Continue,continuar,more...

All of this would be hidden from the editor. We can add a way to add extra keys (like CONTINUE shown above).

I'd probably also prefer to completely drop the ALL CAPS and go for <component>_Text, <component>_Button<num> etc.

Excuse the google translate.

Zranz commented 7 years ago

I have one question/suggestion: why use a single localization file with every supported language for the quest? (and possibly also with the feminine counterparts).

Wouldn't it be easier to have a localization file as "localization.en.txt", "localization.es.txt", "localization.fr.txt," etc. so that depending on the language selection in the app the key parameters are looked up on the corresponding file? And it would default to English if the translation is not available?

I say this because it would be easier for translators to incorporate an additional file rather than take the original quest, editing it and actually having full access to the original story rather than the actual translation only. When the number of quests becomes large having control over translations and original stories could be an issue.

Furthermore, this scheme would possibly allow more languages than just the ones supported in the original app.

What do you think?

Same thoughts would apply for the base app, although it would be less critical.

NPBruce commented 7 years ago

The main reason is that the data we get from FFG is like this and it lets us use the same code. We could always do something different.

Furthermore, this scheme would possibly allow more languages than just the ones supported in the original app.

This is simple enough anyway, as long as the first line in the file is a reference like this:

KEY,english,french,latin,logban

What you suggest would certainly make merging easier with multiple editors, and the files easier to read. As eorahil is doing the actual work on this one I will leave it to him.

NPBruce commented 7 years ago

I am going to revert your phase bar changes. The different font on the buttons looks strange and it no longer fits at 4x3 aspect. I understand this makes translations not work, but it will need another solution. We could abbreviate, or drop font size based on string length, or maybe drop support for 4x3 if we have to.

I have also fixed layout in options and puzzles.

eorahil commented 7 years ago

Ok no problem. I will find a way to support all aspects. but I didn't change the font, not intentionally. Didn't know about autosize the font. T_T.

About Zranz suggestion,multiple localization files is a better solution for quests. My first step will be the FFG localization because it's the simplest solution for now. After this I will try the change.

About NPBruce suggestion removing ffg keys from ini files. Im afraid of:

NPBruce commented 7 years ago

The way I have auto sized some things is to first create a dialog, then use unity's preferredHeight or preferredWidth to recalculate appropriate sizes.

Old adventures won't be valid for the new system. we should implement an importer.

Don't need an importer as such, just need to be able to read the old format (more on this later).

The app is still growing while I'm developing changes and I'm afraid of breaking it while merging. Because new screens and data structures will appear. So I need a way to follow when merging changes in order to adapt. IE: For previous commint, Changing the constructors of TextBox and DialogBox clases, all new strings will fail my build until I convert it.

This change should be easier (I will step through)

Here is how I would do it (this is mergable after every step):

1

2

Repeate 1 for all user facing text. Merges can be made at any time (as long as A and B are done for a particular item). Lookups are <SectionName>.Text etc.

3

Modify editor save to also save the dictionary

4

Populate dictionary from saved file on quest load (also still read ini data)

5

PS: Just realised that my example in step 1 is actually the hardest, because you will need to lookup all quests and have many dictionaries. After that it should be easy.

NPBruce commented 7 years ago

As I don't think the mythos male/female issue will depend on this I have fixed it.

eorahil commented 7 years ago

I will try

NPBruce commented 7 years ago

Did you want me to do some work on this or would you rather I left it to you?

eorahil commented 7 years ago

I did my approach and trying to convert to yours, ie. eliminate from ini files the localization strings. The problem I must solve now is the component renaming. I create localization keys by concatenating nameComponent.element. If I rename the component I need to update all localization references. With my approach I don't have this problem because in the ini file there will still be the key to use. IE: ini file [ActivationShoggoth1] ability={qst:ActivationShoggoth1.ability} master={qst:ActivationShoggoth1.master}

localization file .,English,Spanish,French,German,Italian,Portuguese,Polish,Japanese,Chinese,Czech ActivationShoggoth1.ability,ability text eng,ability text spa,,,,,,,, ActivationShoggoth1.master,master text eng,master text spa,,,,,,,,

after renaming the keys remain the same, and if I edit ability or master, at this point it will be created the new key and automatically unused keys will be deleted.

My other problem is quest.name. In the scenario selection only ini files are loaded, and not its localization. so after translating an scenario. The name is replaced by quest.name.

NPBruce commented 7 years ago

In the first example I would want to rename anyway, otherwise if the keyname is left it would be very confusing.

Yeah I mentioned above the quest name is the hardest. It is also a problem for download, the manifest system will need work. Would you like any help?

eorahil commented 7 years ago

If you want to speed up I can pull request in the branch you want and then we can do:

eorahil commented 7 years ago

For the quest.name problem. the only global solution I see is to create a "foreignname" attibute as a StringKey with the name of the Scenario in multiple languages. The "name" attribute will be the name of the scenario in the language used to create it. I should create too the foreigndescription for the same reason.

NPBruce commented 7 years ago

It can work the same as all other keys. Unless you are worried about performance reading all the dictionaries to get titles? We could always do this in the background as soon as Valkyrie is started if that becomes a problem.

eorahil commented 7 years ago

The editor are not the problem, as you are saying it is fast to open a file and read only 2 keys. The problem is that all scenarios has the same key for the name (quest.name). So if you have 2 scenarios in a list. You must read the first one dictionary, translate, and then read the second one. If you load both quests, quest.name will be overwritten. The only way to fix this is to generate an unique ID foreach scenario. and the key of the quest should be quest-3DG435T.name,"The fall of the House Lynch",,,,,,,, and in the quest.ini

[Quest]
id=3DG435T
name={qst:quest-3DG435T.name}
NPBruce commented 7 years ago

If you save the name and description in QuestData.Quest on load you can dump the dictionary (or even better only do a partial read until you find those two keys).

eorahil commented 7 years ago

Ok finally I didn't add the uid element, as you said, while loading the ini's of the scenarios to create the list imports the localizations too to get the quest.name. I already supported the multilanguage scenarios. I'm translating traits and I will start to remove all the StringKeys from the ini file to be only in the localization file, but It seems to me that It will be harder to be manually edited. There are a lot of things that you need to examine the localization file in order to know it. Could we pull this changes and after decide what keys to remove?

NPBruce commented 7 years ago

I am in favor of merging often, anytime you want to raise a pull request please do so.

NPBruce commented 7 years ago

Are you OK with closing this or are you still changing things? We can open new issues to fix problems.

eorahil commented 7 years ago

Yes. We can Close (EUREKA!!). The translation is complete. I have in mind some things like log traces, but the messages can change during development. We can translate in future.