Advertisement

Removing a callback

Started by April 08, 2014 07:28 PM
7 comments, last by BlackMoons 10 years, 7 months ago

Hi. I have the following Angelscript code:


class ElectricLightSource
{
    void Interact(User @ user)
    {
        if (mOwner.GetObject().mState > 0)
        {
            gObjects.RemovePowerDraw(PowerFunction(this.UpdatePower));
            mOwner.GetObject().mState = 0;
        }
        else
        {
            gObjects.AddPowerDraw(PowerFunction(this.UpdatePower));
            mOwner.GetObject().mState = 2; // Start with power off;
        }
    }
}

And the C++ functions called are:


...
    r = scriptEngine->RegisterFuncdef("void PowerFunction()"); assert( r >= 0 );
    r = scriptEngine->RegisterObjectMethod("ObjectManager", "void AddPowerDraw(PowerFunction@)", asMETHOD(ObjectManager,AddPowerDraw), asCALL_THISCALL); assert( r >= 0 );
    r = scriptEngine->RegisterObjectMethod("ObjectManager", "void RemovePowerDraw(PowerFunction@)", asMETHOD(ObjectManager,RemovePowerDraw), asCALL_THISCALL); assert( r >= 0 );
...
std::vector<asIScriptFunction*> mPowerCallbacks;
...

void ObjectManager::AddPowerDraw(asIScriptFunction* function)
{
	mPowerCallbacks.push_back(function);
}
void ObjectManager::RemovePowerDraw(asIScriptFunction* function)
{
	std::vector<asIScriptFunction*>::iterator cur = mPowerCallbacks.begin(), end = mPowerCallbacks.end();
	for (; cur != end; cur++)
	{
		if (*cur == function)
		{
		--end;
		if (cur != end)
			*cur = *end;
		mPowerCallbacks.pop_back();
		return;
		}
	}
}

My problem is that the remove call, "if (*cur == function)" never passes. What am I doing wrong?

Lead Coder/Game Designer for Brutal Nature: http://BrutalNature.com


	PowerFunction @powerCallback;
	void Interact(User @ user)
	{
		if (mOwner.GetObject().mState > 0)
		{
			
			gObjects.RemovePowerDraw(powerCallback);
			mOwner.GetObject().mState = 0;
		}
		else
		{
			powerCallback = PowerFunction(this.UpdatePower); // Must pass same callback object to add and remove
			gObjects.AddPowerDraw(powerCallback);
			mOwner.GetObject().mState = 2; // Start with power off;
		}
	}

Seems to have fixed it, but that seems kinda messy to have to store the callback in the object to remove it (And I add/remove callbacks elsewhere), is there any way to change my C++ code to compare the callback objects how I want?

Lead Coder/Game Designer for Brutal Nature: http://BrutalNature.com

Advertisement

Each time the script calls PowerFunction(this.UpdatePower) a new delegate object instance will be created, so if you compare the pointers/handles to these instances directly they will obviously not match.

Instead you can query the object and method that is being delegated by calling the methods GetDelegateObject and GetDelegateFunction on the asIScriptFunction interface representing the delegate object.

Further information on callbacks and delegates can be found here: http://www.angelcode.com/angelscript/sdk/docs/manual/doc_callbacks.html

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

Ah thanks. Is making a new delegate object very expensive?

Hmm, Now I have the problem where my script classes destructor won't get called to free up any remaining callbacks since the callbacks hold a ref to the object. Hmmmm.

Now I have to decide between breaking up the delegate and not holding a ref to the object (potentially dangerous if a ref is not removed properly) or have a separate 'clean up' function to be called when a script object needs to be destroyed. (Technically I already have one I call from the destructor because mixins can't override the destructor, but I would rather not have to call a named function from C++ to cleanup a script class)

This does seem kind of like a task for weak references as I dislike making the scripting side more complex and would like to be able to report errors rather then crash.

Do I just (C++ side)


CScriptWeakRef ref;
asIScriptFunction *callback;


void SetCallback(asIScriptFunction *cb)
{
...
callback = cb->GetDelegateFunction();
CScriptWeakRef ref(cb->GetDelegateObject(),cb->GetDelegateObjectType());
cb->Release()
...
}

void CallCallback()
{
void* object = ref.Get();
if (object == NULL) return;
// Call callback with 'object'
}

And its all good? I know ref being a local object instead of being a pointer and using ->Release() is a bit naughty, but it should be OK as long as I never give ref back to the script engine right?

Lead Coder/Game Designer for Brutal Nature: http://BrutalNature.com

Oh, CScriptWeakRef has a private destructor. that is a new one for me. Guess I will be using new/Release() then.

Lead Coder/Game Designer for Brutal Nature: http://BrutalNature.com

Hmmm, Run into a roadblock


void ObjectManager::AddPowerDraw(asIScriptFunction* function)
{
	mPowerCallbacks.push_back(PowerCallback());
	mPowerCallbacks.back().mFunction = function->GetDelegateFunction();
	void * object = function->GetDelegateObject();
	asIObjectType * type = function->GetDelegateObjectType();
	Engine::mScript->mScriptEngine->AddRefScriptObject(object, type); // Add ref to object, as CScriptWeakRef releases an object.
	mPowerCallbacks.back().mRef = new CScriptWeakRef(object,type);
	function->Release();
}

It crashes when it calls 'mPowerCallbacks.back().mRef = new CScriptWeakRef(object,type);'

Inside the constructor of CScriptWeakRef

it crashes at: 'm_type->GetEngine()->ReleaseScriptObject(m_ref, m_type->GetSubType());'

the crash in the ReleaseScriptObject function happens at

if( objType->flags & asOBJ_REF )

Hence why I added the 'Engine::mScript->mScriptEngine->AddRefScriptObject(object, type);' line to my own code. but that does not seem to help.

Further debugging shows m_type->GetSubType() returns NULL as m_type is not a template type.

What am I doing wrong in constructing the weakRef?

I think I will update Angelscript next as I seem to be using 2.27.1

Lead Coder/Game Designer for Brutal Nature: http://BrutalNature.com

Advertisement

Ok updated to 2.28.2.

Now its crashing at


// The given type should be the weakref template instance
	assert( strcmp(type->GetName(), "weakref") == 0 ||
	        strcmp(type->GetName(), "const_weakref") == 0 );

Thanks for that assert btw.

So I assume I am passing the wrong type altogether. How do I get the type I want to construct a WeakRef?

<edit>

Ahhhh. Found this http://www.gamedev.net/topic/647835-ownership-problem-of-funcdefs/ that also had the same problem and fixed it.

Again, thanks for the assert it REALLY helped me figure this out!!!

PS: GetObjectTypeByDecl still does not exist.

For anyone who was wondering or later finds this post with the same problem, the correct code was:


void ObjectManager::AddPowerDraw(asIScriptFunction* function)
{
	asIScriptEngine * engine = Engine::mScript->mScriptEngine;
	mPowerCallbacks.push_back(PowerCallback());
	// Todo: Check for function->GetFuncType() == asFUNC_DELEGATE
	mPowerCallbacks.back().mFunction = function->GetDelegateFunction();
	void * object = function->GetDelegateObject();
	asIObjectType * type = function->GetDelegateObjectType();
	std::string weakRefDecl = std::string("weakref<") + type->GetName() + ">";
	asIObjectType *weakRefType = engine->GetObjectTypeById(type->GetModule()->GetTypeIdByDecl(weakRefDecl.c_str()));
	mPowerCallbacks.back().mRef = new CScriptWeakRef(object,weakRefType);
	function->Release();
}

Lead Coder/Game Designer for Brutal Nature: http://BrutalNature.com

Congratulations for figuring it all out by yourself so quickly. ph34r.png

There is obviously room for improvements to make the process smoother. I'll work on this for future releases.

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

Yea, Not too sure what could make it smoother.

Maybe moving


asIObjectType * type = function->GetDelegateObjectType();
std::string weakRefDecl = std::string("weakref<") + type->GetName() + ">";
asIObjectType *weakRefType = engine->GetObjectTypeById(type->GetModule()->GetTypeIdByDecl(weakRefDecl.c_str()));

inside the constructor of CScriptWeakRef

Of course, that breaks existing code, but a new assert should at least alert people to the break. (Yea. I dislike breaking existing peoples code too so, bad idea I guess)

Another option might be some ability to set a CScriptWeakRef via function instead of just by constructor.

CScriptWeakRef * foo = new CScriptWeakRef(); foo->SetRef(object,type);

Of course that makes the interface a little weird as constructor and setting method no longer match in usage.

Another, cleaner option for delegates, might be to just have a new (C++ only?) class like WeakRefDelegate that has an assignment/copy operator for a asIScriptFunction* (And back again if passing back to the script engine is needed?).

Ideally it would also be smart enough to handle non delegate functions transparently (Just leave the internal WeakRef set to null and set a bool to say the function can be called without a valid WeakRef)

Lead Coder/Game Designer for Brutal Nature: http://BrutalNature.com

This topic is closed to new replies.

Advertisement