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

Adding CCDataVisitor and CCPrettyPrinter #2406

Merged
merged 9 commits into from May 10, 2013

Conversation

dumganhar
Copy link

The usage of CCPrettyPrinter is:

    CCPrettyPrinter vistor;
    CCDictionary* pDict = CCDictionary::createWithContentsOfFile("animations/animations.plist");
    pDict->acceptVisitor(vistor);
    CCLog("%s", vistor.getResult().c_str());

@sergey-shambir
Copy link
Contributor

Thanks! I think it better to rename it to CCPrettyPrinter and create method getResult() which returns resulting string. Default visitor can be good example of usage, but there are no need in intermediate visitor since CCDataVisitor calls visitObject() for all unhandled classes.

I.e. class that serializes cocos2d-x data structs to plist will override visit() method for CCDictionary, CCArray, CCString, CCFloat/CCBool/CCDouble/CCInteger and override visitObject() method to print error message in log (maybe with correct type name received from typeid(*obj).name.

@dumganhar
Copy link
Author

I think it better to rename it to CCPrettyPrinter and create method getResult() which returns resulting string.

Make sense. So if i understand clearly, do you mean that we don't need to CCLog in CCPrettyPrinter, just save all output string in a buffer, then offer a getResult() method?

Default visitor can be good example of usage, but there are no need in intermediate visitor since CCDataVisitor calls visitObject() for all unhandled classes.

Do you mean that CCPrettyPrinter doesn't need to override visitObject method? But visitObject is a pure method. It should be implemented in CCDataVisitor's subclass.

(maybe with correct type name received from typeid(*obj).name.

Agree, I'll do it like this. Thanks.

@sergey-shambir
Copy link
Contributor

Make sense. So if i understand clearly, do you mean that we don't need to CCLog in CCPrettyPrinter, just save all output string in a buffer, then offer a getResult() method?

Yes.

Do you mean that CCPrettyPrinter doesn't need to override visitObject method? But visitObject is a pure method. It should be implemented in CCDataVisitor's subclass

No, just mean that word 'default' in CCDefaultVisitor misleads user: s/he thinks that CCDefaultVisitor should be subclassed instead of subclassing CCDataVisitor directly.

@dumganhar
Copy link
Author

Got it, Thank you. I will update this Pull Request soon.

@sergey-shambir
Copy link
Contributor

Are there any troubles, or your are busy after release day ;) ?

Actually change will show benefits only when extensions developers will start use it. Currently I see 2 candidates: CCDictionary::writeToFile() and 3rd-party extensions for JSON serialization; I will help to update them.

P.S. JSON also can have own class CCNull, which complicates things a bit.

@minggo
Copy link
Contributor

minggo commented May 9, 2013

I have told to @dumganhar about this PR.
He will handle it quickly.

dumganhar pushed a commit that referenced this pull request May 10, 2013
fixed #2142: Adding CCDataVisitor and Implementing CCPrettyPrinter.
@dumganhar dumganhar merged commit 306ff0a into cocos2d:master May 10, 2013
@dumganhar dumganhar deleted the data-visitor branch May 10, 2013 08:29
@dumganhar
Copy link
Author

@sergey-shambir
It was merged. :)

@walzer
Copy link
Contributor

walzer commented May 12, 2013

OMG, you shouldn't add acceptVisitor(CCDataVisitor &visitor) into CCObject.
CCObject was designed only for calculate reference count. Maybe I should rename CCObject to CCReference in the future.

@dumganhar
Copy link
Author

I agree. Maybe we should rename it to Ref in cocos2d 3.0.
The member variables of CCObject like

unsigned int        m_uID;
int                 m_nLuaID;

need to be moved to ScriptTarget class like which in GamePlay.

@ricardoquesada
Copy link
Contributor

Putting the Script data/methods in a new object is a good idea.

Having a way to dump objects to string is also useful.

@ricardoquesada
Copy link
Contributor

@dumganhar

Two quick questions:

a)
currently I need to dump the contents of a CCDictionary into a string... could you post a little sample that shows how to do it ?

b)
And also, how can we improve the CCDictionary # valueForKey to return a string when the value is a CCinteger ? Currently it is using a dynamic_cast to check if it is string and it doesn't look that good:
( eg: https://github.com/cocos2d/cocos2d-x/blob/cocos2d-2.1rc0-x-2.1.3/cocos2dx/cocoa/CCDictionary.cpp#L190 )

Thanks!

@dumganhar
Copy link
Author

@ricardoquesada

a)
currently I need to dump the contents of a CCDictionary into a string... could you post a little sample that shows how to do it ?

What's the format of the string dumped from CCDictionary? If you just want to show the content of CCDictionary when debugging, CCPrettyPrinter could do that job. If that couldn't meet your requirement, you could implement a new subclass of CCDataVisitor, maybe CCJsonVisitor to dump CCDictionary to JSON format.

b)
And also, how can we improve the CCDictionary # valueForKey to return a string when the value is a CCinteger ? Currently it is using a dynamic_cast to check if it is string and it doesn't look that good:

Alternatively, base type like CCInteger, CCBool should implement a virtual method like toString. I don't find a better way to achieve that.

dumganhar pushed a commit that referenced this pull request Jul 16, 2013
issue #2406: Override updateDisplayedColor and updateDisplayedOpacity method in Scale9Sprite class.
Resolved issue: Color and Opacity of Scale9Sprite will not be changed when it's added to NodeRGBA and run with FadeIn/Out actions.
angeltown pushed a commit to angeltown/cocos2d-x that referenced this pull request Apr 28, 2014
fixed cocos2d#2142: Adding CCDataVisitor and Implementing CCPrettyPrinter.
angeltown pushed a commit to angeltown/cocos2d-x that referenced this pull request Apr 28, 2014
issue cocos2d#2406: Override updateDisplayedColor and updateDisplayedOpacity method in Scale9Sprite class.
Resolved issue: Color and Opacity of Scale9Sprite will not be changed when it's added to NodeRGBA and run with FadeIn/Out actions.
angeltown pushed a commit to angeltown/cocos2d-x that referenced this pull request Apr 29, 2014
fixed cocos2d#2142: Adding CCDataVisitor and Implementing CCPrettyPrinter.
angeltown pushed a commit to angeltown/cocos2d-x that referenced this pull request Apr 29, 2014
issue cocos2d#2406: Override updateDisplayedColor and updateDisplayedOpacity method in Scale9Sprite class.
Resolved issue: Color and Opacity of Scale9Sprite will not be changed when it's added to NodeRGBA and run with FadeIn/Out actions.
angeltown pushed a commit to angeltown/cocos2d-x that referenced this pull request Apr 30, 2014
issue cocos2d#2406: Override updateDisplayedColor and updateDisplayedOpacity method in Scale9Sprite class.
Resolved issue: Color and Opacity of Scale9Sprite will not be changed when it's added to NodeRGBA and run with FadeIn/Out actions.
angeltown pushed a commit to angeltown/cocos2d-x that referenced this pull request Apr 30, 2014
issue cocos2d#2406: Override updateDisplayedColor and updateDisplayedOpacity method in Scale9Sprite class.
Resolved issue: Color and Opacity of Scale9Sprite will not be changed when it's added to NodeRGBA and run with FadeIn/Out actions.
dumganhar pushed a commit that referenced this pull request May 4, 2014
fixed #2142: Adding CCDataVisitor and Implementing CCPrettyPrinter.
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

Successfully merging this pull request may close these issues.

None yet

5 participants