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 default argument support and some improvements #31

Merged
merged 3 commits into from Mar 10, 2013

Conversation

Projects
None yet
3 participants
@dumganhar
Contributor

dumganhar commented Mar 8, 2013

XXX Please review and test it before merging XXX

Parsing default argument for cxx methods.

Detecting the enumeration type which defined in cxx class.

Renaming the config section:

base_objects ~> classes_have_no_parents, it will be clearer what this section means since i added another section called base_classes_to_skip.

Adding new config sections:

base_classes_to_skip: Like CCObject, lots of classes like CCTextureCache, CCAnimationCache are inheriting from CCObject. I want to make it just skip its parent if it was found. The different between base_classes_to_skip and classes_have_no_parents is that classes_have_no_parents will not to find its parents while base_classes_to_skip will find its parents and if one of its parents is like CCObject, it will skip CCObject just seems it has not found CCObject.

script_control_cpp : Determining whether to use script object(js object) to control the lifecycle of native(cpp) object or the other way around.
This could be useful when developers want to bind their own class and also use the cocos2d-x bindings. Now the template will not produce the marco COCOS2D_JAVASCRIPT, instead, it uses python variable to achieve it.

#ifdef COCOS2D_JAVASCRIPT -----> #if not $generator.script_control_cpp

Module support

If there are different modules need to be bound, it's hard to make the module js bindings glue file in separated files. To achieve that, we should not use make all bindings in just one big file(like cocos2dx.cpp). Developer just registers the module they want. For instance, you could refer to https://github.com/dumganhar/plugin-x/tree/master/jsbindings/auto .
I think in cocos2d-x, we should also separate extension bindings from cocos2dx.cpp.

Any suggestion will be appreciated. Thanks very much.

void js_${generator.prefix}_${current_class.class_name}_finalize(JSFreeOp *fop, JSObject *obj) {
#if $generator.script_control_cpp
js_proxy_t* nproxy;

This comment has been minimized.

@dumganhar

dumganhar Mar 8, 2013

Contributor

If script object controls the lifecycle of native object. We should release the memory of native object when js object is being released.

@dumganhar

dumganhar Mar 8, 2013

Contributor

If script object controls the lifecycle of native object. We should release the memory of native object when js object is being released.

@dumganhar

This comment has been minimized.

Show comment
Hide comment
@dumganhar

dumganhar Mar 9, 2013

Contributor

@funkaster @rohankuruvilla @folecr @minggo
Did anybody review this pull request?

Contributor

dumganhar commented Mar 9, 2013

@funkaster @rohankuruvilla @folecr @minggo
Did anybody review this pull request?

rolandoam added a commit that referenced this pull request Mar 10, 2013

Merge pull request #31 from dumganhar/default-arg-support
Adding default argument support and some improvements

@rolandoam rolandoam merged commit 0cc9a6b into rolandoam:master Mar 10, 2013

@rolandoam

This comment has been minimized.

Show comment
Hide comment
@rolandoam

rolandoam Mar 10, 2013

Owner

thanks!

Owner

rolandoam commented Mar 10, 2013

thanks!

@jdmunro

This comment has been minimized.

Show comment
Hide comment
@jdmunro

jdmunro Mar 12, 2013

With the script_control_cpp variable - this seems to work in terms of allocating non-CCObject derived classes, however what is there to determine when this object should be deleted? You mentioned that the JS object should determine the lifetime of the native object and this is the behaviour I'm after, however I didn't notice anywhere in the generated bindings where the native object is deleted.

jdmunro commented Mar 12, 2013

With the script_control_cpp variable - this seems to work in terms of allocating non-CCObject derived classes, however what is there to determine when this object should be deleted? You mentioned that the JS object should determine the lifetime of the native object and this is the behaviour I'm after, however I didn't notice anywhere in the generated bindings where the native object is deleted.

@jdmunro

This comment has been minimized.

Show comment
Hide comment
@jdmunro

jdmunro Mar 12, 2013

Worked out the issue - in the generated "finalize" function, the native object isn't explicitly deleted and so it is never destroyed.

Here's an example of what it should look like:

void js_rendering_Shader_finalize(JSFreeOp *fop, JSObject *obj) {
    js_proxy_t* nproxy;
    js_proxy_t* jsproxy;
    JS_GET_NATIVE_PROXY(jsproxy, obj);

    if (jsproxy) {
        JS_GET_PROXY(nproxy, jsproxy->ptr);

        rendering::Shader* nobj = static_cast<rendering::Shader *>(nproxy->ptr);
        if (nobj)
            delete nobj;

        JS_REMOVE_PROXY(nproxy, jsproxy);
    }
}

Will submit a patch shortly.

jdmunro commented Mar 12, 2013

Worked out the issue - in the generated "finalize" function, the native object isn't explicitly deleted and so it is never destroyed.

Here's an example of what it should look like:

void js_rendering_Shader_finalize(JSFreeOp *fop, JSObject *obj) {
    js_proxy_t* nproxy;
    js_proxy_t* jsproxy;
    JS_GET_NATIVE_PROXY(jsproxy, obj);

    if (jsproxy) {
        JS_GET_PROXY(nproxy, jsproxy->ptr);

        rendering::Shader* nobj = static_cast<rendering::Shader *>(nproxy->ptr);
        if (nobj)
            delete nobj;

        JS_REMOVE_PROXY(nproxy, jsproxy);
    }
}

Will submit a patch shortly.

@dumganhar

This comment has been minimized.

Show comment
Hide comment
@dumganhar

dumganhar Mar 12, 2013

Contributor

@jdmunro
I think you are right. I forgot to add delete stuff. Thanks.

Contributor

dumganhar commented Mar 12, 2013

@jdmunro
I think you are right. I forgot to add delete stuff. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment