Jump to content
  • Advertisement
Sign in to follow this  
BlackMoons

Removing a callback

This topic is 2021 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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?

Share this post


Link to post
Share on other sites
Advertisement
	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?

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

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?

Edited by BlackMoons

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

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();
}
Edited by BlackMoons

Share this post


Link to post
Share on other sites

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.

Edited by Andreas Jonsson

Share this post


Link to post
Share on other sites

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)

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!