mono / cocos2d-xna

XNA Port of Cocos2d-X
23 stars 16 forks source link

SpriteTest - Regression #18

Closed kjpou1 closed 11 years ago

kjpou1 commented 11 years ago

There seems to be a regression with the change made when eliminating CCNS https://github.com/totallyevil/cocos2d-xna/pull/78.

There are null values that need to be considered or it is reading incorrectly.

totallyeviljake commented 11 years ago

Is this in CCBReader?

totallyeviljake commented 11 years ago

CCSizeConverter.cs line 28? NULL value there? From CCSize.Parse line 272 while reading the sprite frames from a PList.

totallyeviljake commented 11 years ago

SpriteFrameAliasNameTest (line 24)

totallyeviljake commented 11 years ago

this one is an incorrectly formatted type 3 Plist file.

totallyeviljake commented 11 years ago

This regression was introduced by the new type converters. In the past, the CCSize.parse would ignore the null I think, but the type converters do not. That's good, because this PList is clearly not format 3.

animations/grossini-aliases.plist, it's a format 2 hybrid with aliases in it.

totallyeviljake commented 11 years ago

The original cocos2d-x had the correct plist for this one, so I pulled it in

totallyeviljake commented 11 years ago

I my commit fixes this?

https://github.com/totallyevil/cocos2d-xna/commit/92428734079dfe4ca366bd22c73380d40307dc18

kjpou1 commented 11 years ago

Jake

Thanks for that. I was going to allow null and pass it on through which would not have been good in the end.

I will test you change. Is there anything we really should be checking here instead and send a more informative error?

totallyeviljake commented 11 years ago

The PList reader should handle incorrectly formatted fields better than it does. So if we want to fix this NRE then it should happen at the Plist level instead of the type converter.

kjpou1 commented 11 years ago

That did not work.

I totally agree with what you say it should be fixed at the Plist level instead of the type converter. That means that the Plist is not correctly formatted for some reason.

totallyeviljake commented 11 years ago

All of the sprite tests work for me. Which one is failing for you?

kjpou1 commented 11 years ago

Oh I see I think it is because it was just the .plist that you committed and not the .xnb of the plist. I do not have windows here to create the content.

totallyeviljake commented 11 years ago

oops, sorry!

kjpou1 commented 11 years ago

Here is what I got from git:

Kenneth-Pounceys-iMac:cocos2d-xna Jimmy$ git fetch upstream remote: Counting objects: 21, done. remote: Compressing objects: 100% (6/6), done. remote: Total 10 (delta 7), reused 7 (delta 4) Unpacking objects: 100% (10/10), done. From https://github.com/totallyevil/cocos2d-xna d43515e..9242873 master -> upstream/master Kenneth-Pounceys-iMac:cocos2d-xna Jimmy$ git merge upstream/master Merge made by the 'recursive' strategy. tests/testsContent/animations/grossini-aliases.plist | 500 +++++++++++++++++++++--------------------- 1 file changed, 250 insertions(+), 250 deletions(-)

totallyeviljake commented 11 years ago

https://github.com/totallyevil/cocos2d-xna/commit/f00217c05e9d022a21f74d9b4dc6908ecd074883

kjpou1 commented 11 years ago

Yes!!! And another one bites the dust.

Thanks. That really is better than what I was going to do.