Skip to content

Choice voice acting and image for localization#1009

Merged
BenRQ merged 7 commits into
NPBruce:development_2.2from
Ssid113:development_2.2
Feb 3, 2019
Merged

Choice voice acting and image for localization#1009
BenRQ merged 7 commits into
NPBruce:development_2.2from
Ssid113:development_2.2

Conversation

@Ssid113

@Ssid113 Ssid113 commented Jan 19, 2019

Copy link
Copy Markdown
Contributor

Added check when parsing for voice files and images. Do not need to change the file *.ini. Old scenarios works. Save, Load and Editor work as before.

How use:

  • add folders to the scenario folder by language name
  • then add the translated file with the name of the original file.

Example:

Choise file for localization.
For the editor, turn off the choice of localization
@Ssid113 Ssid113 changed the title Choise voice acting and image for localization Choice voice acting and image for localization Jan 19, 2019
@BenRQ BenRQ requested review from redwolf2 and scrubbless January 20, 2019 00:34
@BenRQ BenRQ added this to the 2.2 milestone Jan 20, 2019
@BenRQ

BenRQ commented Jan 20, 2019

Copy link
Copy Markdown
Collaborator

Hi @Ssid113,
thanks for this, this is an important feature.

@scrubbless @redwolf2 what do you think about this ?
You think it would be easy to explain in tutorial and use for authors ?

In term of implementation, it looks good to me. Simple and efficient.
We will need to test it of course.

Can you just rename the function 'ChoiseLocalizedSource' to 'FindLocalisedMultimediaFile'
Description should be "Find and return audio or picture file for localization, if it exists. Otherwise return default filename"

Also I don't understand this change:

-                 AddData(section.Key, section.Value, f); 
+                AddData(section.Key, section.Value, Path.GetDirectoryName(questPath));

Can you please explain why you need to do this ? It is just filename there.

@BenRQ BenRQ self-requested a review January 20, 2019 00:51
@redwolf2

Copy link
Copy Markdown
Collaborator

It's a great idea.

@Ssid113

Ssid113 commented Jan 20, 2019

Copy link
Copy Markdown
Contributor Author

Also I don't understand this change:

-                 AddData(section.Key, section.Value, f); 
+                AddData(section.Key, section.Value, Path.GetDirectoryName(questPath));

Can you please explain why you need to do this ? It is just filename there.

Hi @BenRQ
Thanks, I will make a change to the name.
Classes UI, Event, QuestComponent doesn't have info about path to quest.
I find path to quest. And Path.GetDirectoryName(questPath) the best I could find. But questPath (no static) belong class QuestData and if i want use it need reference. I do not know how to do better, but only editor use class QuestComponent.source after void AddData and i write in QuestComponent.source path to quest if editMode = false. Finally i use AddData(section.Key, section.Value, game.editMode ? f : Path.GetDirectoryName(questPath));

Other solutions

  • If somewhere there is information about path to the quest can use it instead QuestData.questPath
  • or add reference in AddData and class constructor public UI(string name, Dictionary<string, string> data, Game game, string path, string questPath) : base(name, data, path, Quest.currentFormat)

I apologize in advance for my English)

rename the function. Add description
@BenRQ

BenRQ commented Jan 20, 2019

Copy link
Copy Markdown
Collaborator

Thanks for the explanation @Ssid113 , and don't worry for the English, I'm not fluent either :)

I'm a bit worried that "source" is used for something else, and your solution may just break something.
It's better to find a way to do this without changing the existing data structure.

My proposal: you could check for existing localization when loading the content itself (audio or image), not when loading content description. So you are sure you have access to path.

Example for pictures :

Function:
public UI(QuestData.UI questUI, Game gameObject) : base(gameObject)

Code:

            if (game.cd.images.ContainsKey(qUI.imageName))
            {
                Vector2 texPos = new Vector2(game.cd.images[qUI.imageName].x, game.cd.images[qUI.imageName].y);
                Vector2 texSize = new Vector2(game.cd.images[qUI.imageName].width, game.cd.images[qUI.imageName].height);
                newTex = ContentData.FileToTexture(game.cd.images[qUI.imageName].image, texPos, texSize);
            }
            else if (qUI.imageName.Length > 0)
            {
>>>>>>>>>  Add a call to check for localization content here, and use it in the function FileToTexture below <<<<<<<<<<<<<
                newTex = ContentData.FileToTexture(Path.Combine(Path.GetDirectoryName(game.quest.qd.questPath), qUI.imageName));
            }

Сheck for existing localization when loading, not when loading content description
@Ssid113

Ssid113 commented Jan 21, 2019

Copy link
Copy Markdown
Contributor Author

Thanks for the tip. Сheck for existing localization when loading a lot better.
Disabled for the editor scenario. !Game.game.editMode

@BenRQ

BenRQ commented Jan 21, 2019

Copy link
Copy Markdown
Collaborator

Looks good !
The WWW request to get a local file, I did not see this one before, I like it :P

I will merge after the merge for new scenario explorer (no conflict expected).

@redwolf2

redwolf2 commented Jan 21, 2019

Copy link
Copy Markdown
Collaborator

@Ssid113

Ssid113 commented Jan 22, 2019

Copy link
Copy Markdown
Contributor Author

WWW is marked as obsolete:
https://docs.unity3d.com/ScriptReference/WWW.html

Might be replaced with:
https://docs.unity3d.com/ScriptReference/Networking.UnityWebRequest.html

Hi, @redwolf2
I didn't change method. I tried to make a minimum of changes. But I can update audio.cs, where the in it hid #1006
If you do not mind)

@redwolf2

redwolf2 commented Jan 22, 2019

Copy link
Copy Markdown
Collaborator

No biggie. I think WWW is used on serveral other places. Those should all be replaced. I am ok with keeping it to WWW for now. I'd just want to stress, that WWW is not the golden hammer for pathing, since it is obsolete.

@BenRQ

BenRQ commented Jan 22, 2019

Copy link
Copy Markdown
Collaborator

Yes let's keep this change simple :)
My remark for WWW was more a funny note on the existing implementation, nothing to do with this patch.

And good point @redwolf2 we will need to update this at some point, not urgent clearly.

@scrubbless

scrubbless commented Jan 22, 2019

Copy link
Copy Markdown
Collaborator

Late to the party here, but yes. Sounds like a very efficient way to deal with this.

How will it work with default language? Just as before?

Will it still work if we put the default language files in a folder?

E.g.

-- [Scenario1]
----- quest.ini (default language - English)
----- events.ini
----- UI.ini
----- [English]
--------- sound1.ogg
--------- image1.jog
----- [French]
--------- sound1.ogg
--------- image1.jog

@Ssid113

Ssid113 commented Jan 22, 2019

Copy link
Copy Markdown
Contributor Author

The default will be exactly as in the *.ini files.
Example:
you create scenario in your native language (this language default for scenario)

-- [Scenario1]
----- quest.ini (default language - your native language)
----- events.ini (path to file)
----- UI.ini (path to file)
----- sound1.ogg (your native language)
----- image1.jpg (your native language)
next you want add translate
-- [Scenario1]
----- quest.ini (default language - your native language, add new language)
----- events.ini (path to file)
----- UI.ini (path to file)
----- sound1.ogg (your native language)
----- image1.jpg (your native language)
----- [New language] (example Spanish, create new folder "Spanish")
--------- sound1.ogg (add file)
--------- image1.jpg (add file)

and you need copy path from root folder scenario to file
Example:
Scenario default language - English

-- [Scenario1]
----- quest.ini (default language - English)
----- events.ini (path to file)
----- UI.ini (path to file)
----- [English]
--------- sound1.ogg (default language - English)
--------- image1.jpg (default language - English)
----- [French]
--------- sound1.ogg (not work)
--------- image1.jpg (not work)
--------- [English]
------------- sound1.ogg (work) (becouse in *.ini recorded "English/sound1.ogg")
------------- image1.jpg (work) (becouse in *.ini recorded "English/sound1.ogg")

In the second case, it is not very beautiful, but the old scripts do not change and work as before.

Upd.
but can do so, and old scenario work too

-- [Scenario1]
----- quest.ini (default language - English)
----- events.ini (path to file)
----- UI.ini (path to file)
----- [English]
--------- sound1.ogg (default language - English)
--------- image1.jpg (default language - English)
----- [French]
--------- sound1.ogg (work) (in *.ini recorded "English/sound1.ogg", get only name "sound1.ogg")
--------- image1.jpg (work) (in *.ini recorded "English/sound1.ogg", get only name "image1.jpg")

Upd.
I remembered why choose the option to copy the path.

-- [Scenario1]
----- quest.ini (default language - English)
----- events.ini (path to file)
----- UI.ini (path to file)
----- sound1.ogg (default language - English)
----- [English]
--------- sound1.ogg (default language - English)
--------- image1.jpg (default language - English)
----- [French]
--------- sound1.ogg (work) (conflict, "English/sound1.ogg" and "sound1.ogg")
--------- image1.jpg (work) (in *.ini recorded "English/sound1.ogg", get only name "image1.jpg")

Disabled for imput files from MoM app.
Disabled for MainMenu.
Fixed problem if source not equal questPath
@scrubbless

Copy link
Copy Markdown
Collaborator

Ok that's fine, as long as we know to document it.

The important thing is to keep the base functionality working as it did.

The ability to move files into folders is very new so not many people use it.

@BenRQ

BenRQ commented Jan 23, 2019

Copy link
Copy Markdown
Collaborator

One point : we would need to support this for audio effect (your patch just change for music I think).
Audio effect can be external files too, so no reason we should not support them here.

Second point:
Regarding your latest commit, I guess you had an issue with audio playback being also used in Valkyrie code (outside of quests).
The new test and path combinations are maybe a bit difficult to read though.

What we can do to avoid all those is to do the call in the quest.cs and eventManager.cs code only.
I see 3 places where we do " game.audioControl.Music(music); "
in Quest.ChangeQuest()
twice in Quest.LoadQuest()
in EventManager.TriggerEvent()

In the 3 places we have access to quest info including the path.

It's probably better to only change code related to quests for this, and not the code related to the audio playback itself.

Tell me what you think about these two points

@Ssid113

Ssid113 commented Jan 23, 2019

Copy link
Copy Markdown
Contributor Author

FindLocalisedMultimediaFile works only with the file path. The goal is to add a language folder to the path. "[folder][folder][folder][Scenario][folder][folder]\filename" replace by "[folder][folder][folder][Scenario][you language][folder][folder]\filename" if File.Exists == true.

One point : we would need to support this for audio effect (your patch just change for music I think).
Audio effect can be external files too, so no reason we should not support them here.

I add FindLocalisedMultimediaFile in public IEnumerator PlayMusic for music and in public IEnumerator PlayEffect for Audio effect before getting the file. Now the search for external files occurs only in the scenario folder.

Regarding your latest commit, I guess you had an issue with audio playback being also used in Valkyrie code (outside of quests).

Yes, I had two problems. The first is the function searched localized multimedia file near the original.
it looked like that: "%AppData%/Valkyrie/MoM/import/.../FileName" replace by "%AppData%/Valkyrie/MoM/import/.../[you language]/FileName" (outside of quests and inside of quests). string.Contains() solved this problem.
And second problem. Previously, all calls gave me the following format strings: source = ...[folder][folder][Scenario] and FileName = [folder][folder]\filename (how in *.ini files). it's easy to add a folder here: source + [you language] + FileName. But in audio.cs differently. Had to look for the necessary folder in string and lead to the general format source + [you language] + FileName.

What we can do to avoid all those is to do the call in the quest.cs and eventManager.cs code only.

Upd.
looked Quest.ChangeQuest(), Quest.LoadQuest(), EventManager.TriggerEvent(). Yes, there is more convenient. And calls FindLocalisedMultimediaFile it is more convenient here. I have not fully figured out how everything works. Are you sure that there will be enough?

Upd.
For audio effect i find Assets.Scripts.UI.UIWindowSelectionListAudio.Play(),EventManager.TriggerEvent(). Didn't miss anything?

@BenRQ

BenRQ commented Jan 23, 2019

Copy link
Copy Markdown
Collaborator

Assets.Scripts.UI.UIWindowSelectionListAudio.Play() is used only in Editor, so you don't need to change it.

So I think it should be enough with the others.
It would be good to have a scenario to test with pictures, music and sound effects.

@BenRQ BenRQ mentioned this pull request Jan 25, 2019
Got rid of unnecessary checks
Tested on a scenario TestChoiseVoiceAndImage.valkyrie
@Ssid113

Ssid113 commented Jan 25, 2019

Copy link
Copy Markdown
Contributor Author

for UI: Quest.UI
for music: Quest.LoadQuest(), EventManager.TriggerEvent()
for audio: EventManager.TriggerEvent()

This option is really better. Allowed to get rid of unnecessary calls and unnecessary checks.

Looked Quest.ChangeQuest(), don't need to change it.

For Tests:

@Ssid113

Ssid113 commented Feb 2, 2019

Copy link
Copy Markdown
Contributor Author

Build test:
https://drive.google.com/open?id=17sPK8_6sLy056kt0YymdGXP1qM5Pg6Xh for Windows
Scenario test:
https://drive.google.com/open?id=1C2x_lnVFHzRpt9JmsKC6c1B8XNCIcT3l

@BenRQ

BenRQ commented Feb 2, 2019

Copy link
Copy Markdown
Collaborator

I did not forget this, I will test.

@BenRQ

BenRQ commented Feb 3, 2019

Copy link
Copy Markdown
Collaborator

@scrubbless maybe you want to test this too ?
You will probably be the first to use it anyway :)

@scrubbless

Copy link
Copy Markdown
Collaborator

I can give it a go, sure. But I don't have any translated images or voice files as of yet.

@BenRQ

BenRQ commented Feb 3, 2019

Copy link
Copy Markdown
Collaborator

Works perfectly for me.
Also the test scenario is very nice, I think we should include it in the documentation.

I will merge now for next test release.

@BenRQ BenRQ merged commit 1b4cadb into NPBruce:development_2.2 Feb 3, 2019
@BenRQ

BenRQ commented Feb 3, 2019

Copy link
Copy Markdown
Collaborator

Wiki has been updated :
https://github.com/NPBruce/valkyrie/wiki/Translations

BenRQ pushed a commit that referenced this pull request Feb 20, 2019
Tested on a scenario TestChoiseVoiceAndImage.valkyrie

Example:
You create scenario in your native language (this language default for scenario)

    -- [Scenario1]
    ----- quest.ini (default language - your native language)
    ----- events.ini (path to file)
    ----- UI.ini (path to file)
    ----- sound1.ogg (your native language)
    ----- image1.jpg (your native language)
    next you want add translate
    -- [Scenario1]
    ----- quest.ini (default language - your native language, add new language)
    ----- events.ini (path to file)
    ----- UI.ini (path to file)
    ----- sound1.ogg (your native language)
    ----- image1.jpg (your native language)
    ----- [New language] (example Spanish, create new folder "Spanish")
    --------- sound1.ogg (add file)
    --------- image1.jpg (add file)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants