larsiusprime / firetongue

A translation/localization framework written in Haxe
MIT License
144 stars 32 forks source link

Crash if files are missing or incomplete #6

Closed AndreiRegiani closed 8 years ago

AndreiRegiani commented 11 years ago

Tested both ways:

lang.init(Capabilities.language, null, true, true);
lang.init(Capabilities.language, null, false, false);

Both ways crash (null reference) if there's no localization folder/files of selected language.

If localization exists but is missing strings, as (true, true) it crashes, but works normally when (false, false), just show the flag string.

larsiusprime commented 11 years ago

I'll get to this as soon as I can.

larsiusprime commented 10 years ago

Geez I've been ignoring this for too long. Promise I'll get it soon!

larsiusprime commented 10 years ago

I apparently fixed this a long time ago and never closed it out. This now prints in the example project if you initialize with Capabilities.language (resulting in "en" on my machine) rather than directly with "en-US":

FireTongue.hx:573: ERROR: default locale(en) not found, searching for closest match...
FireTongue.hx:599: --> looking in: assets/locales
FireTongue.hx:625: --> candidates: [en-CA,en-GB,en-US,nb-NO,yo-DA]
FireTongue.hx:577: --> going with: en-CA
RudolfVonKrugstein commented 10 years ago

Mmh, I seem tp have exactly this Problem. I init firetongue with:

    tongue.init(flash.system.Capabilities.language, onFiretongueLoaded, true, true);

And on android, it crashes with:

I/trace   ( 5462): FireTongue.hx:173: LocaleData.init(de,#sfunction0,true,false,)
I/trace   ( 5462): Lib.hx:316: Null Object Reference
I/trace   ( 5462): Called from *._Function_1_1 (ApplicationMain.hx line 75)
I/trace   ( 5462): Called from Reflect.callMethod (Reflect.hx line 50)
I/trace   ( 5462): Called from Main.main (Main.hx line 57)
I/trace   ( 5462): Called from firetongue.FireTongue.init (firetongue/FireTongue.hx line 198)
I/trace   ( 5462): Called from firetongue.FireTongue.startLoad (firetongue/FireTongue.hx line 502)
I/trace   ( 5462): Called from task.TaskList.addTask (task/TaskList.hx line 34)
I/trace   ( 5462): Called from task.TaskList.processTasks (task/TaskList.hx line 213)
I/trace   ( 5462): Called from task.TaskList.runTask (task/TaskList.hx line 288)
I/trace   ( 5462): Called from Reflect.callMethod (Reflect.hx line 50)
I/trace   ( 5462): Called from firetongue.FireTongue.loadFile (firetongue/FireTongue.hx line 696)
I/trace   ( 5462): Called from firetongue.FireTongue.processCSV (firetongue/FireTongue.hx line 788)
I/trace   ( 5462): Called from firetongue.FireTongue.writeIndex (firetongue/FireTongue.hx line 800)
I/trace   ( 5462): Called from firetongue.FireTongue.logMissingFlag (firetongue/FireTongue.hx line 812)
I/trace   ( 5462): Called from *._Function_1_1 (openfl/display/Stage.hx line 120)
larsiusprime commented 10 years ago

Hmm, thought I had fixed this. Does it crash on not-android with the same setup?

RudolfVonKrugstein commented 10 years ago

I get the same error on linux 64bit

RudolfVonKrugstein commented 10 years ago

It is fixable by this:

--- a/firetongue/FireTongue.hx
+++ b/firetongue/FireTongue.hx
@@ -848,9 +849,6 @@ package firetongue;
                                        for (key in _missing_flags.keys()) {
                                                i++;
                                        }
-                                       if (i == 0) {
-                                               _missing_flags = null;
-                                       }
                                }

                                if (_callback_finished != null) {

but I do not know which implications this fix might have ...

It is also very strange that _missing_flags is empty. There are flags missing in the de locale with respect to the en-US locale and this is my index.xml:

<?xml version="1.0" encoding="utf-8" ?>
<data>
    <file id="data" value="data.csv"/>

    <!--American English-->
    <locale id="en-US" is_default="true" sort="0">  
        <contributors value="Nathan Huesken"/>
        <ui language="Language" region="Region" accept="Okay" />
        <label id="en-US,en-GB,en-CA" language="English" region="United States"/>   
    </locale>    

</data>
RudolfVonKrugstein commented 10 years ago

The problem still persists, but I cannot reopen this issue. Should I make a new?

larsiusprime commented 10 years ago

Here, I'll reopen it. Can you post a link to an example project that reproduces this error?

RudolfVonKrugstein commented 10 years ago

Sorry for taking so extemly long. Here: https://github.com/RudolfVonKrugstein/firetongue_test

For me it works on linux, while it crashes on android.

RudolfVonKrugstein commented 10 years ago

It seems to me, when the locale (provided as a parameter to init) is not found, it does NOT search for the default locale (provided with "is_default" in the index.xml). Why not?

larsiusprime commented 10 years ago

Thanks for this. I'll check it out as soon as I can.

The answer to your question is probably "because it's a bug :)" -- it's probably just assuming that if you provide a locale, that it is there, and makes no further safety checks. Makes sense it should make one last effort to find the default before giving up.

larsiusprime commented 10 years ago

Okay, so sorry about the long time to reply.

Right now the behavior works like this: if it cannot find a particular locale, it will look for the closest match for the locale. So if you offer "en" but all it has is "en-US", then it will try to use en-US. This was done to be more friendly with whatever Capabilities.locale might return, rather than just always defaulting to english or whatever whenever it's not a perfect match.

Here's what happens in the HelloWorlds sample if I try to load the nonexistent locale "ERROR":

FireTongue.hx:161: LocaleData.init(ERROR,#function0,true,true,) Assets.hx:548: [openfl.Assets] There is no String asset with an ID of "assets/locales/ERROR/data.tsv" FireTongue.hx:568: ERROR: default locale(ERROR) not found, searching for closest match... FireTongue.hx:594: --> looking in: assets/locales FireTongue.hx:620: --> candidates: [en-CA,en-GB,en-US,nb-NO,yo-DA] FireTongue.hx:572: --> going with: en-CA Assets.hx:548: [openfl.Assets] There is no String asset with an ID of "assets/locales/en-CA/more_data.tsv"

It works for me on Flash, Windows, and Neko, with both (true, true) and (false, false) as the extra parameters. I sadly don't have any mobile devices to test on.

What might be crashing it on mobile devices is that there might be some unanticipated wrinkle with file handling -- on windows/linux it might just say "sorry that file doesn't exist" and let the program continue, but on android it might just explode. If someone can help me figure out which call this might be and how to fix it I'd be glad to help.

I updated the "HelloWorlds" sample with an example of the "can't find locale X" functionality.

On Tue, Sep 23, 2014 at 6:13 AM, Nathan notifications@github.com wrote:

It seems to me, when the locale (provided as a parameter to init) is not found, it does NOT search for the default locale (provided with "is_default" in the index.xml). Why not?

— Reply to this email directly or view it on GitHub https://github.com/larsiusprime/firetongue/issues/6#issuecomment-56505239 .

www.fortressofdoors.com -- Games, Art, Design

RudolfVonKrugstein commented 10 years ago

Hey,

Well, actually the problem occurs when a locale that does not exist at all is given. This case is not covered by the HelloWorlds example. If I add

locales = tongue.locales.copy();
locales.push("fa-KE");

and add a "fa-KE" flag, it crashes on linux too. Maybe with a different error ... I will find out.

RudolfVonKrugstein commented 10 years ago

When I change the line

tongue.init("en-US", onFinish, true);

to

tongue.init("de-DE", onFinish, true);

it crashes on linux and android with:

FireTongue.hx:173: LocaleData.init(de-DE,#function0,true,false,)
Assets.hx:547: [openfl.Assets] There is no String asset with an ID of "assets/locales/de-DE/data.csv"
Assets.hx:547: [openfl.Assets] There is no String asset with an ID of "assets/locales/de-DE/more_data.csv"
Assets.hx:547: [openfl.Assets] There is no String asset with an ID of "assets/locales/de-DE/fonts.xml"
Failure type not string @ ./Xml.cpp:390
Called from *._Function_1_1 (ApplicationMain.hx line 99)
Called from openfl.display.DisplayObjectContainer.addChild (openfl/display/DisplayObjectContainer.hx line 31)
Called from openfl.display.DisplayObject.__setParent (openfl/display/DisplayObject.hx line 435)
Called from openfl.display.DisplayObjectContainer.__onAdded (openfl/display/DisplayObjectContainer.hx line 373)
Called from openfl.display.DisplayObject.__onAdded (openfl/display/DisplayObject.hx line 374)
Called from openfl.display.DisplayObject.dispatchEvent (openfl/display/DisplayObject.hx line 71)
Called from openfl.display.DisplayObject.__dispatchEvent (openfl/display/DisplayObject.hx line 194)
Called from openfl.events.EventDispatcher.dispatchEvent (openfl/events/EventDispatcher.hx line 98)
Called from openfl.events.Listener.dispatchEvent (openfl/events/EventDispatcher.hx line 268)
Called from Main.onInit (Main.hx line 59)
Called from firetongue.FireTongue.init (firetongue/FireTongue.hx line 198)
Called from firetongue.FireTongue.startLoad (firetongue/FireTongue.hx line 498)
Called from task.TaskList.addTask (task/TaskList.hx line 34)
Called from task.TaskList.processTasks (task/TaskList.hx line 213)
Called from task.TaskList.runTask (task/TaskList.hx line 288)
Called from Reflect.callMethod (Reflect.hx line 50)
Called from firetongue.FireTongue.loadFile (firetongue/FireTongue.hx line 703)
Called from Xml.parse (Xml.hx line 104)
Called from extern.cffi (/usr/lib/haxe/lib/hxcpp/git/src/hx/Lib.cpp line 119)
larsiusprime commented 10 years ago

Thanks for this! I'll try another fix today.

On Thu, Oct 9, 2014 at 8:27 AM, Nathan notifications@github.com wrote:

When I change the line

tongue.init("en-US", onFinish, true);

to

tongue.init("de-DE", onFinish, true);

it crashes on linux and android with:

FireTongue.hx:173: LocaleData.init(de-DE,#function0,true,false,) Assets.hx:547: [openfl.Assets] There is no String asset with an ID of "assets/locales/de-DE/data.csv" Assets.hx:547: [openfl.Assets] There is no String asset with an ID of "assets/locales/de-DE/more_data.csv" Assets.hx:547: [openfl.Assets] There is no String asset with an ID of "assets/locales/de-DE/fonts.xml" Failure type not string @ ./Xml.cpp:390 Called from *._Function_1_1 (ApplicationMain.hx line 99) Called from openfl.display.DisplayObjectContainer.addChild (openfl/display/DisplayObjectContainer.hx line 31) Called from openfl.display.DisplayObject.setParent (openfl/display/DisplayObject.hx line 435) Called from openfl.display.DisplayObjectContainer.onAdded (openfl/display/DisplayObjectContainer.hx line 373) Called from openfl.display.DisplayObject.onAdded (openfl/display/DisplayObject.hx line 374) Called from openfl.display.DisplayObject.dispatchEvent (openfl/display/DisplayObject.hx line 71) Called from openfl.display.DisplayObject.dispatchEvent (openfl/display/DisplayObject.hx line 194) Called from openfl.events.EventDispatcher.dispatchEvent (openfl/events/EventDispatcher.hx line 98) Called from openfl.events.Listener.dispatchEvent (openfl/events/EventDispatcher.hx line 268) Called from Main.onInit (Main.hx line 59) Called from firetongue.FireTongue.init (firetongue/FireTongue.hx line 198) Called from firetongue.FireTongue.startLoad (firetongue/FireTongue.hx line 498) Called from task.TaskList.addTask (task/TaskList.hx line 34) Called from task.TaskList.processTasks (task/TaskList.hx line 213) Called from task.TaskList.runTask (task/TaskList.hx line 288) Called from Reflect.callMethod (Reflect.hx line 50) Called from firetongue.FireTongue.loadFile (firetongue/FireTongue.hx line 703) Called from Xml.parse (Xml.hx line 104) Called from extern.cffi (/usr/lib/haxe/lib/hxcpp/git/src/hx/Lib.cpp line 119)

— Reply to this email directly or view it on GitHub https://github.com/larsiusprime/firetongue/issues/6#issuecomment-58508447 .

www.fortressofdoors.com -- Games, Art, Design

larsiusprime commented 9 years ago

Sorry for the delay.

Just pushed a fix, hopefully this will work? I don't have android so I can't test that specific platform, but I put in some better checks for handling missing files and defaulting to something that does exist.

On Thu, Oct 9, 2014 at 9:09 AM, Lars Doucet lars.doucet@gmail.com wrote:

Thanks for this! I'll try another fix today.

On Thu, Oct 9, 2014 at 8:27 AM, Nathan notifications@github.com wrote:

When I change the line

tongue.init("en-US", onFinish, true);

to

tongue.init("de-DE", onFinish, true);

it crashes on linux and android with:

FireTongue.hx:173: LocaleData.init(de-DE,#function0,true,false,) Assets.hx:547: [openfl.Assets] There is no String asset with an ID of "assets/locales/de-DE/data.csv" Assets.hx:547: [openfl.Assets] There is no String asset with an ID of "assets/locales/de-DE/more_data.csv" Assets.hx:547: [openfl.Assets] There is no String asset with an ID of "assets/locales/de-DE/fonts.xml" Failure type not string @ ./Xml.cpp:390 Called from *._Function_1_1 (ApplicationMain.hx line 99) Called from openfl.display.DisplayObjectContainer.addChild (openfl/display/DisplayObjectContainer.hx line 31) Called from openfl.display.DisplayObject.setParent (openfl/display/DisplayObject.hx line 435) Called from openfl.display.DisplayObjectContainer.onAdded (openfl/display/DisplayObjectContainer.hx line 373) Called from openfl.display.DisplayObject.onAdded (openfl/display/DisplayObject.hx line 374) Called from openfl.display.DisplayObject.dispatchEvent (openfl/display/DisplayObject.hx line 71) Called from openfl.display.DisplayObject.dispatchEvent (openfl/display/DisplayObject.hx line 194) Called from openfl.events.EventDispatcher.dispatchEvent (openfl/events/EventDispatcher.hx line 98) Called from openfl.events.Listener.dispatchEvent (openfl/events/EventDispatcher.hx line 268) Called from Main.onInit (Main.hx line 59) Called from firetongue.FireTongue.init (firetongue/FireTongue.hx line 198) Called from firetongue.FireTongue.startLoad (firetongue/FireTongue.hx line 498) Called from task.TaskList.addTask (task/TaskList.hx line 34) Called from task.TaskList.processTasks (task/TaskList.hx line 213) Called from task.TaskList.runTask (task/TaskList.hx line 288) Called from Reflect.callMethod (Reflect.hx line 50) Called from firetongue.FireTongue.loadFile (firetongue/FireTongue.hx line 703) Called from Xml.parse (Xml.hx line 104) Called from extern.cffi (/usr/lib/haxe/lib/hxcpp/git/src/hx/Lib.cpp line 119)

— Reply to this email directly or view it on GitHub https://github.com/larsiusprime/firetongue/issues/6#issuecomment-58508447 .

www.fortressofdoors.com -- Games, Art, Design

www.fortressofdoors.com -- Games, Art, Design

RudolfVonKrugstein commented 9 years ago

The testapp still fails when using an unkown loacle (trace appended below). So the crash happens in FireTognue.hx:601. The problem is, that paths is null because getDirectoryContents returns null. I am not sure, maybe "readDirectory" does not work on android?

I am also still confused. When I add another

I/trace   (27593): FireTongue.hx:161: LocaleData.init(de,#sfunction0,true,false,)
I/trace   (27593): Assets.hx:548: [openfl.Assets] There is no String asset with an ID of "assets/locales/de/data.csv"
I/trace   (27593): FireTongue.hx:568: ERROR: default locale(de) not found, searching for closest match...
I/trace   (27593): FireTongue.hx:594: --> looking in: assets/locales
I/trace   (27593): Lib.hx:324: Null Object Reference
I/trace   (27593): Called from *._Function_1_1 (ApplicationMain.hx line 81)
I/trace   (27593): Called from Reflect.callMethod (Reflect.hx line 50)
I/trace   (27593): Called from Main.main (Main.hx line 9)
I/trace   (27593): Called from firetongue.FireTongue.init (firetongue/FireTongue.hx line 187)
I/trace   (27593): Called from firetongue.FireTongue.startLoad (firetongue/FireTongue.hx line 477)
I/trace   (27593): Called from firetongue.FireTongue.loadRootDirectory (firetongue/FireTongue.hx line 570)
I/trace   (27593): Called from firetongue.FireTongue.findClosestExistingLocale (firetongue/FireTongue.hx line 601)
I/trace   (27593): Called from *._Function_1_1 (openfl/_v2/display/Stage.hx line 124)
RudolfVonKrugstein commented 9 years ago

I also noticed, on linux: I have a default locale, which is en_US (set by is_default="true"). So if the locale is not found, I want the system to go with en_US!

But this is the output I get on linux:

FireTongue.hx:568: ERROR: default locale(de) not found, searching for closest match...
FireTongue.hx:594: --> looking in: assets/locales
FireTongue.hx:597: assets/locales
FireTongue.hx:602: paths: [index.xml,au,en-US,_flags]
FireTongue.hx:622: --> candidates: [au,en-US]
FireTongue.hx:572: --> going with: au

Why does it choose "au"? When someone in japan (for which I do not have locales) uses the app, I want them to have it in english not in austria-german ...

larsiusprime commented 9 years ago

Sorry about this, I'll try to fix it again... thanks for your persistence. I really don't know what's going on with the Android bug, if you have any insights into why readDirectory() is causing problems I'd be happy to make any fixes based on that information. I guess we can ask around.

Also, I'll look into the default locale thing, I think that's just a stupid oversight on my part.

On Wed, Nov 12, 2014 at 4:03 AM, Nathan notifications@github.com wrote:

I also noticed, on linux: I have a default locale, which is en_US (set by is_default="true"). So if the locale is not found, I want the system to go with en_US!

But this is the output I get on linux:

FireTongue.hx:568: ERROR: default locale(de) not found, searching for closest match... FireTongue.hx:594: --> looking in: assets/locales FireTongue.hx:597: assets/locales FireTongue.hx:602: paths: [index.xml,au,en-US,_flags] FireTongue.hx:622: --> candidates: [au,en-US] FireTongue.hx:572: --> going with: au

Why does it choose "au"? When someone in japan (for which I do not have locales) uses the app, I want them to have it in english not in austria-german ...

— Reply to this email directly or view it on GitHub https://github.com/larsiusprime/firetongue/issues/6#issuecomment-62695261 .

www.fortressofdoors.com -- Games, Art, Design

RudolfVonKrugstein commented 9 years ago

I have no real inside on readDirectory() on android, but when I do "readDirectory(".")" I get:

[sdcard,firmware,persist,storage,config,cache,acct,vendor,d,etc,mnt,ueventd.rc,ueventd.mako.rc,system,sys,sepolicy,seapp_contexts,sbin,res,property_contexts,proc,init.usb.rc,init.trace.rc,init.rc,init.mako.usb.rc,init.mako.rc,init.environ.rc,init,fstab.mako,file_contexts,default.prop,data,charger,root,dev]

which seems to be the root directory of the android system, or something like that. I do not know, where the app directory would be or if there even is an app directory.

Maybe use "openfl.Assets.list" to find all text assets and then filter them? That could be more platform independent ...

larsiusprime commented 9 years ago

I guess the only real solution to this is get some more android testers. On android it is supposed to be prepending all readdirectory calls with the applicationStorageDirectory, because otherwise it will crash. I should see if others have your same problem.

Tiago-Ling commented 9 years ago

Tested here, the result is this, running the unmodified example project from Rudolf:

I/trace   (25097): FireTongue.hx:173: LocaleData.init(de,#sfunction0,true,false,
)
I/trace   (25097): Assets.hx:548: [openfl.Assets] There is no String asset with
an ID of "assets/locales/de/data.csv"
I/trace   (25097): Lib.hx:324: Null Object Reference
I/trace   (25097): Called from *._Function_1_1 (ApplicationMain.hx line 76)
I/trace   (25097): Called from Reflect.callMethod (C:\HaxeToolkit\haxe\std/cpp/_
std/Reflect.hx line 50)
I/trace   (25097): Called from Main.main (Main.hx line 9)
I/trace   (25097): Called from firetongue.FireTongue.init (firetongue/FireTongue
.hx line 198)
I/trace   (25097): Called from firetongue.FireTongue.startLoad (firetongue/FireT
ongue.hx line 502)
I/trace   (25097): Called from task.TaskList.addTask (task/TaskList.hx line 34)
I/trace   (25097): Called from task.TaskList.processTasks (task/TaskList.hx line
 213)
I/trace   (25097): Called from task.TaskList.runTask (task/TaskList.hx line 288)

I/trace   (25097): Called from Reflect.callMethod (C:\HaxeToolkit\haxe\std/cpp/_
std/Reflect.hx line 50)
I/trace   (25097): Called from firetongue.FireTongue.loadFile (firetongue/FireTo
ngue.hx line 696)
I/trace   (25097): Called from firetongue.FireTongue.processCSV (firetongue/Fire
Tongue.hx line 788)
I/trace   (25097): Called from firetongue.FireTongue.writeIndex (firetongue/Fire
Tongue.hx line 800)
I/trace   (25097): Called from firetongue.FireTongue.logMissingFlag (firetongue/
FireTongue.hx line 812)
I/trace   (25097): Called from *._Function_1_1 (openfl/_v2/display/Stag
E/ACDB-LOADER(  196): Error: ACDB AudProc vol returned = -8
E/Metrics:BatchTransmitte(32295): QueuePusher.sendBatches - Batch transmission f
ailed on client error, discarded.;

What is the supposed input for the method logMissingFlag ? I'm getting this when i trace the params:

I/trace   (27951): FireTongue.hx:812: id : 'data' | flag : '$YES'

Another strange thing in this method are these lines, which do nothing (maybe some left over code from debugging in the past?):

var list:Array<String> = _missing_flags.get(id);
list.push(flag);

The program crashes because _missing_flags is null, and checking samples / HelloWorlds / Source / Main.hx lines 120 to 129 it is possible to see that in the sample this null condition is treated externally.

The simplest solution is to check _missing_flags inside the logMissingFlag method:

private function logMissingFlag(id:String, flag:String):Void {
    if (_missing_flags == null) {
        //Either return or instantiate
        //return;
        //_missing_flags = new Map<String, Array<String>>();
    }

    ...
}

Probably not the best solution, but this will at least prevent the crash (it did prevent here).

Another solution would be to instantiate _missing_flags regardless of the _check_missing condition, just to be sure:

if (_check_missing) { //Remove this check
    _missing_files = new Array<String>();
    _missing_flags = new Map<String,Array<String>>();
}

Anyway this has something to do with this check immediately before the condition:

if (_locale != default_locale) {                
    _check_missing = check_missing_;
    _replace_missing = replace_missing_;
}

The _check_missing problem is probably related to the other problem you described (not selecting a default locale intelligently).

Tiago-Ling commented 9 years ago

I created a pull request with this temp solution here: #14

larsiusprime commented 8 years ago

I'm closing since the temp solution was merged a long time ago, and also I just refactored to clean up the loading functionality so that a user can override it with their own if they must.