Hello,

I am experiencing an issue with passing string properties as "const string& inout" references in AngelScript. On CPP side the string properties are defined as "std::string" instances. On AngelScript side I want to use the "CScriptString" add-on to pass strings by reference and to have the extended numeric conversion support. Therefore I have defined the following wrapper templates, which allow to register the string properties with the AngelScript engine. The wrappers take care of the conversion between "CScriptString" and "std::string".

// Accessor wrapper for string properties.template <class Type, std::string Type::* Member>struct StringPropertyWrapper{    // Retrieves the string property as a cscriptstring handle    static CScriptString* get(Type* object)    {        return new CScriptString(object->*Member);    }    // Assignes the string property from a cscriptstring handle    static void set(const CScriptString* string, Type* object)    {        object->*Member = string->buffer;    }};// Accessor wrapper for string properties.template <class Type, const std::string& (Type::*Getter)() const, void (Type::*Setter)(const std::string&)>struct StringPropertyAccessorWrapper{    // Retrieves the string property as a cscriptstring handle    static CScriptString* get(Type* object)    {        return new CScriptString((object->*Getter)());    }    // Assignes the string property from a cscriptstring handle    static void set(const CScriptString* string, Type* object)    {        (object->*Setter)(string->buffer);    }};

To improve the registration workflow I have additionally defined the following two macros. The macros allow to register the respecitve string properties with one short line as compared to two long lines of code.

// Register the getter and setter wrapper#define REGISTER_STRING_PROPERTY(engine, type, name, class, member)    assert(mEngine->RegisterObjectMethod(type, std::string("const string& get_").append(name).append("() const").c_str(), asFunctionPtr(StringPropertyWrapper<class, &class::member>::get), asCALL_CDECL_OBJLAST) >= 0);    assert(mEngine->RegisterObjectMethod(type, std::string("void set_").append(name).append("(const string& in)").c_str(), asFunctionPtr(StringPropertyWrapper<class, &class::member>::set), asCALL_CDECL_OBJLAST) >= 0);// Register the getter and setter wrapper#define REGISTER_STRING_PROPERTY_ACCESSORS(engine, type, name, class, getter, setter)    assert(mEngine->RegisterObjectMethod(type, std::string("const string& get_").append(name).append("() const").c_str(), asFunctionPtr(StringPropertyAccessorWrapper<class, &class::getter, &class::setter>::get), asCALL_CDECL_OBJLAST) >= 0);    assert(mEngine->RegisterObjectMethod(type, std::string("void set_").append(name).append("(const string& in)").c_str(), asFunctionPtr(StringPropertyAccessorWrapper<class, &class::getter, &class::setter>::set), asCALL_CDECL_OBJLAST) >= 0);

So far everything seems to work fine, the accessor wrappers are called with the correct values and the effects are reflected properly. Next I defined a function with a "const string& inout" parameter

void test(const string&)

and issued the call

test(object.property)

where the access to "object.property" is actually handled by the wrapper functions. The behavior that I encounter here is that the function parameter is not initialized with the object property. Actually the getter of the property isn't executed at all. I was wondering if this behavior is intended. Originally I was expecting first a get-call, the the test function call and finally a set call.

Thanks for your help.

Cheers,
Georg

[Edited by - ghackenberg on July 15, 2010 5:38:38 AM]

Hi,

The main drawback with the stdstring is the call-by-value restriction as it is stated in the manual. Is there any way to adjust the class to work as reference?

xad

Quote:Original post by WitchLord
1. There's a memory leak with the get accessor. The C++ implementation is returning a pointer to a newly allocated object. But the method is registered as returning a reference, rather than a handle. AngelScript will not release this object, since it assumes the application side still holds the reference.

Okay, I can confirm this problem. My next idea was to replace the reference types by actual handles. The updated code looks like this (note the release call in the setters).

// Accessor wrapper for string properties.template <class Type, std::string Type::* Member>struct StringPropertyWrapper{    // Retrieves the string property as a cscriptstring handle    static CScriptString* get(Type* object)    {        return new CScriptString(object->*Member);    }    // Assignes the string property from a cscriptstring handle    static void set(CScriptString* string, Type* object)    {        object->*Member = string->buffer;        string->Release();    }};// Accessor wrapper for string properties.template <class Type, const std::string& (Type::*Getter)() const, void (Type::*Setter)(const std::string&)>struct StringPropertyAccessorWrapper{    // Retrieves the string property as a cscriptstring handle    static CScriptString* get(Type* object)    {        return new CScriptString((object->*Getter)());    }    // Assignes the string property from a cscriptstring handle    static void set(CScriptString* string, Type* object)    {        (object->*Setter)(string->buffer);        string->Release();    }};

Quote:
2. Your macros REGISTER_STRING_PROPERTY and REGISTER_STRING_PROPERTY_ACCESSORS put actual logic in the arguments to the assert function. When compiling the code in release mode these arguments will never evaluated, as the whole function call is removed by the preprocessor.

That's also true, actually I noticed already yesterday after sending my initial post. Therefore I updated the macros as follows:

// Register the getter and setter wrapper#define REGISTER_STRING_PROPERTY(engine, type, name, class, member)     {        int r;        r = mEngine->RegisterObjectMethod(type, std::string("string@ get_").append(name).append("() const").c_str(), asFunctionPtr(StringPropertyWrapper<class, &class::member>::get), asCALL_CDECL_OBJLAST); assert(r >= 0);        r = mEngine->RegisterObjectMethod(type, std::string("void set_").append(name).append("(string@)").c_str(), asFunctionPtr(StringPropertyWrapper<class, &class::member>::set), asCALL_CDECL_OBJLAST); assert(r >= 0);    }// Register the getter and setter wrapper#define REGISTER_STRING_PROPERTY_ACCESSORS(engine, type, name, class, getter, setter)    {        int r;        r = mEngine->RegisterObjectMethod(type, std::string("string@ get_").append(name).append("() const").c_str(), asFunctionPtr(StringPropertyAccessorWrapper<class, &class::getter, &class::setter>::get), asCALL_CDECL_OBJLAST); assert(r >= 0);        r = mEngine->RegisterObjectMethod(type, std::string("void set_").append(name).append("(string@)").c_str(), asFunctionPtr(StringPropertyAccessorWrapper<class, &class::getter, &class::setter>::set), asCALL_CDECL_OBJLAST); assert(r >= 0);    }

Now, the problem with the updated code is that the setters are never really called. My guess is that AngelScript just updates the handle returned by the getter instead, right?

Quote:
Also, if your application code uses std::string extensively, maybe you should consider registering the std::string directly (see RegisterStdString).

Actually, my colleague and I have been working with "RegisterStdString" previously. However, some days ago we decided to switch to "CScriptString" to support call-by-reference between AngelScript and CPP. To avoid populating our CPP classes with "CScriptString" we came up with these generic accessor templates.

Cheers,
Georg

[Edited by - ghackenberg on July 15, 2010 5:34:21 AM]

Quote:Original post by WitchLord
The expression for the function argument is only evaluated once. In your case the expression is evaluated as the get property accessor to get the handle, then that handle is passed to the function which updates the string it points to directly.

Actually, I was testing something like this:

object.property = "new value";

where "object.property" was returning a string handle through the above defined get accessor. In this case it also seems that the set accessor is not called (i.e. if the get accessor already returns a handle, then the set accessor seems obsolete, right?).