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]

I'll have to make some tests to see what may be going wrong.

However, you have a couple of errors in your code:

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.

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.



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


Regards,
Andreas

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

Well, the CScriptString is a wrapper on the std::string to allow it to be handled by reference. But it has it's own drawbacks if the application relies heavily on std::string usage.

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]

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.


Why do you want the setter to be called in this case? The function is registered as 'const &' so the function shouldn't really be modifying the object in any case. By the way, registering the function as 'const ∈' is probably better in this case.


Call by reference is allowed for value types as well, but you need to specify either in or out, i.e. you will not be able to register functions that take a reference to a string that first use the value contained in it, and then modify it before returning.

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?).

If the property accessors are registered like this

string@ get_prop()void set_prop(string@)´

then AngelScript will treat the property as if it was a handle, i.e.

string@ prop;

In this case the expression

// Value assignmentobject.prop = "new value";

is rewritten as:

object.get_prop() = "new value";

And the expression

// Handle assignment@object.prop = @"new value";

is rewritten as:

object.set_prop(@"new value");

This is a limitation of the current way the property accessors work. I'm trying to think of a better way to declare property accessors that would allow a bit more flexibility in the way they are implemented. You would want some way of telling AngelScript to treat the property as

string prop;

but still have the accessors return and receive a handle rather than a reference to the string object.