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

XmlConfigParser.parseVo bug using ASC2.0 #15

Open
ianmcgregor opened this issue Jan 30, 2013 · 2 comments
Open

XmlConfigParser.parseVo bug using ASC2.0 #15

ianmcgregor opened this issue Jan 30, 2013 · 2 comments

Comments

@ianmcgregor
Copy link

Hi Matan,

First off, thanks for creating such a great loading library :)

I started a new project today using ASC2.0 and found that AssetLoader is throwing errors due to invalid config values (in my case ConfigVO.type and ConfigVO.blendMode).

I've tracked it down to the XmlConfigParser.parseVo method. What is happening is that using the old compiler the XML attribute queries return null where the attribute does not exist, allowing the OR expression to select the value from the 'inheritFrom' VO. Using the new compiler, the attribute queries appear to be returning an empty XMLList. When assigned to the string properties in ConfigVO, you end up with empty strings for properties such as 'type', which breaks things later on.

child.type = xml.@type || inheritFrom.type;

So the above code will assign xml.@type even when it doesn't exist.

Now, I don't know whether to consider this a bug with ASC2.0 or AssetLoader, but I've found a couple of potential solutions I thought I'd run by you before submitting any pull request:

child.type = xml.hasOwnProperty("@type") ? xml.@type : inheritFrom.type;

child.type = xml.attribute("@type").length() > 0 ? xml.@type : inheritFrom.type;

I'm sure there are several other ways to approach it too. Shame to lose the simple OR assignment, I'm a bit fan of its simplicity, but I really want to start using ASC2.0 and AssetLoader!

Thanks for your time,
Ian

@Matan
Copy link
Owner

Matan commented Jan 31, 2013

Hi Ian,

Thanks for pointing this out. I starting to use the ASC2.0 compiler as well and I will be fixing up Assetloader to support it sometime soon. As you may have noticed I promised v3 quite a while back, but due to workload, getting married, etc. I just didn't get round to it. I've got a half written version which I need to pick up on. But we should squeeze ASC2.0 compiler support in first. :)

@ianmcgregor
Copy link
Author

Hi, Good stuff! I'll carry on working this project with my patched version and I'll let you know if I find anything else. Congratulations BTW!

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

2 participants