aduros / flambe

Rapidly cook up games for HTML5, Flash, Android, and iOS.
https://github.com/aduros/flambe/wiki
MIT License
743 stars 118 forks source link

manifest.externalBasePath not respected on Android browsers for AssetFormat.Data types. #191

Closed volkipp closed 10 years ago

volkipp commented 10 years ago

I haven't had too much time to investigate any further, but I managed to track down this problem we were having with our Android HTML projects and our localization system. It appears that the externalBasePath property is not respected on Android browsers, and only on text files being loaded. PNGs and other asset types load just fine.

If you load a manifest like this: var manifest :Manifest = new Manifest(); manifest.externalBasePath = "http://your-ip:7000/testpath"; manifest.add("config.xml", "config.xml"); manifest.add("americanCaptain_black_22.fnt", "fonts/americanCaptain_black_22.fnt"); manifest.add("americanCaptain_black_22", "fonts/americanCaptain_black_22.png");

The PNG will load on Android (both stock browser and Chrome), but the .fnt and .xml files will not. You can see the 404's and the paths through the Android Debug Bridge in Chrome. All other platforms work as expected.

Here is the test case I used to check the bug. http://www.seven2.com/FTP/manifest_test.zip

Any help would be appreciated. Thanks! -Kipp

aduros commented 10 years ago

Android CORS is blacklisted due to buggy browser. From Manifest.hx:

    // Whether the environment fully supports loading assets from another domain
    private static var _supportsCrossOrigin :Bool = (function () {
#if html
        // CORS in the stock Android browser is buggy. If your game is contained in an iframe, XHR
        // will work the first time. If the response had an Expires header, on subsequent page loads
        // instead of retrieving it from the cache, it will fail with error code 0.
        // http://stackoverflow.com/questions/6090816/android-cors-requests-work-only-once
        //
        // TODO(bruno): Better UA detection that only blacklists the stock browser, not Chrome or FF
        // for Android
        var blacklist = ~/\b(Android)\b/;
        if (blacklist.match(js.Browser.window.navigator.userAgent)) {
            return false;
        }

I consistently ran into that bug when using iframes, but it may come up under other conditions too...

In any case, you should always host a copy of your assets on the same domain (can customize the location using relativeBasePath), as a fallback for browsers that don't support CORS.

volkipp commented 10 years ago

That all makes sense, the trouble I ran into was that:

So in my example, americanCaptain_black_22.png loads from where it is expected: http://your-ip:7000/testpath/fonts/americanCaptain_black_22.png However, the fnt file attempts to load from here on Android, even though they are in the same directories: http://your-ip:7000/fonts/americanCaptain_black_22.fnt

My workaround was to prepend the fully qualified base path by hand in each entry I add and it works great. Even some kind of helpful error message would save time troubleshooting. Is there any reason why we couldn't use a fully qualified base path if we are part of the same domain?

aduros commented 10 years ago

It turns out web browsers allow loading images and sounds from remote domains, even if CORS isn't supported, since the actual data isn't being read. For Data files though, they need to come from the same domain, or CORS needs to be supported. Flambe tries to be clever and load as many assets from the remote domain as possible, since typically the remote domain is a faster CDN. externalBasePath is really only a suggestion to use that remote domain where available, and otherwise fall back to relativeBasePath.

If you know the base path is on the same domain, it sounds like you should set relativeBasePath instead? I'm pretty sure you can even set it to an absolute path by starting it with a slash (I guess relative is a misnomer, it should be named something else)

var manifest = Manifest.build("somepack");
manifest.relativeBasePath = "/testpath"; // load from /testpath/ instead of assets/
// load the pack as normal
volkipp commented 10 years ago

Unfortunately, we really can't use a relative path because that's not what the CMS passes us. It would be nice though to at least have a warning that using that setting might re-write the path on us.

aduros commented 10 years ago

I simplified/fixed some things related to base paths in the last few commits, could you take a look?

I think the approach for your case would be to take the URL the CMS passes you and strip out the http:/// part (assuming you know that will be on the same domain) and set relativeBasePath to that.

I'm planning on renaming relativeBasePath/externalBasePath to localBase/remoteBase or something, and fleshing out the documentation on when you would use each. Relative is a poor name since you can set it to an absolute path.

markknol commented 10 years ago

It would be nice if you could provide the localBase/remoteBase in the html embed too, as optional setting.

volkipp commented 10 years ago

Thanks, that helps. I still think that the warning should happen on all HTML targets if you're using the externalBasePath feature, not just the unsupported browser. It would certainly help broadcast that you might be running into trouble from Android browsers and save some debugging time (like the texture size warning).

aduros commented 10 years ago

The texture size warning at least gives a call to action to fix the warning (shrink your images). I think warning just because you're using externalBasePath could be spammy. I think the renaming + more documentation will help to explain how this part of asset loading works more than a warning.