Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assetUtils.ts is not es6 ready #27

Closed
chessIthaca opened this issue May 16, 2017 · 6 comments
Closed

assetUtils.ts is not es6 ready #27

chessIthaca opened this issue May 16, 2017 · 6 comments

Comments

@chessIthaca
Copy link

chessIthaca commented May 16, 2017

Hi
assetutil is not es6 ready. It uses reflection api that is not available in es6.
I fixed this locally for resources I use ...

The rest of the code seems to be es6 ready (not promising full test coverage :)).

Added two functions that do the same and are es6 ready:

private static loadImages() {
        for (let image in Assets.Images) {
            if (!this.game.cache.checkImageKey(Assets.Images[image].getName())) {
                if (Assets.Images[image].getPNG !== undefined) {
                    this.game.load.image(Assets.Images[image].getName(), Assets.Images[image].getPNG());
                } else if (Assets.Images[image].getJPG !== undefined)  {
                    this.game.load.image(Assets.Images[image].getName(), Assets.Images[image].getJPG());
                } else {
                    throw 'not supported';
                }
            }
        }
    }
    
    private static loadAtlases() {
        for (let atlas in Assets.Atlases) {
            if (!this.game.cache.checkImageKey(Assets.Atlases[atlas].getName())) {
                let imageOption = null;
                let dataOption = null;

                if (Assets.Atlases[atlas].getPNG !== undefined) {
                    imageOption = Assets.Atlases[atlas].getPNG;
                } else if (imageOption = Assets.Atlases[atlas].getJPG !== undefined)  {
                    imageOption = Assets.Atlases[atlas].getJPG;
                } else {
                    throw 'image format not supported';
                }

                if (Assets.Atlases[atlas].getXML !== undefined) {
                    this.game.load.atlasXML(Assets.Atlases[atlas].getName(), imageOption.call(), Assets.Atlases[atlas].getXML());
                } else if (Assets.Atlases[atlas].getJSONArray !== undefined) {
                    this.game.load.atlasJSONArray(Assets.Atlases[atlas].getName(), imageOption.call(), Assets.Atlases[atlas].getJSONArray());
                } else if (Assets.Atlases[atlas].getJSONHash !== undefined) {
                    this.game.load.atlasJSONHash(Assets.Atlases[atlas].getName(), imageOption.call(), Assets.Atlases[atlas].getJSONHash());
                } else {
                    throw 'atlas format not supported';
                }
            }
        }
    }
@rroylance
Copy link
Owner

First; really appreciate your involvement and activeness in reporting issues and suggestions!

Second... hmm, I never considered ES6 readiness. However I don't like this particular solution since it relies on checking for each image type individually (getPNG, getJPG) and there are many more image types that are already supported and I really do not want to have to make sure that this class, the webpack configs, the generateAssetsClass all match the growing lists of supported types.

The way I did it the way I did is so that the loader doesn't care what extensions are available and what is supported, it just attempts to load anything and everything that is available in the asset class.

If you think of a different solution that doesn't involve a giant if else if statement in every loader, please let me know...

@chessIthaca
Copy link
Author

chessIthaca commented May 16, 2017

well, you could also use
js Object.getOwnPropertyNames(Assets.Images[image])

alas it would return more things and so you would have to test for more methods to ignore for example in loadatlases.

in this case:
["length", "name", "prototype", "getName", "getPNG"]

that said, I get it and agree - it is less elegant but I thought I share as I really wanted to use the es6 collection classes for my project. So I had to change this ....

@rroylance
Copy link
Owner

It's definitely good to know and something to keep an eye on; I've never done ES6 explicitly and never needed to write ES6 compliant TS so I never would have come across this.

I'll leave this issue open as it is an issue for sure, just not a pressing one.

Thanks!

@chessIthaca
Copy link
Author

Agree, one thing to consider is that the loop you use right now fails silently in es6 and you end up with a project that just doesn't' load any of the resources. This then will make the sound loading code end up sitting in an endless loop.
It took me quite a while to figure out that it is this code that failed - I was blaming Phaser at first!

Anyways, I mention it just in case others try to use es6.

@MajinBui
Copy link
Contributor

I've made a working change over here :
#33

for (let option of Object.getOwnPropertyNames(Assets.Images[image])) {
                   if (option !== 'getName' && option.includes('get')) {

@rroylance
Copy link
Owner

Merged in #33, so this should be good to go now. Thanks!

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

No branches or pull requests

3 participants