Sign in to follow this  
Rakkar2

Explicit derefence

Recommended Posts

Unless I misunderstand things, it seems like a design flaw that both of these expressions are equally valid
SceneNode parent = this.getSceneNode();



SceneNode @parent = this.getSceneNode();



The first form improperly creates an instance of scene node, dereferences the pointer, and makes a copy of the object. The second form is correct. In my opinion, this is a design flaw for 4 reasons: 1. This syntax is rarely needed. While technically I may want either of these, 95% of the time if someone calls a function that returns an object pointer, they use that pointer, rather than immediately dereference and copy the pointer. 2. Causes very difficult to find bugs. For example, I have about 10 objects I deal with only through pointers, and never locally instantiate or copy. Doing so just does a shallow copy, and crashes in some weird place later. I can see myself and others constantly forgetting the @ and losing a lot of time debugging this. 3. Unintuitive AngelScript is like C in every way except this. This mistake is practically begging to happen. I made it on my very first script, and it cost me 15 minutes to figure out why it was crashing in std::map, and then only because it was what I was immediately working on. 4. Used constantly Every time I use the pointer, I need to remember @. I use it all over the place. Thus, I'm going to forget @ all over the place. My only solution at this point is to purposely crash, so at least it crashes at the source of the problem, rather than somewhere unrelated later on.
void ScriptSceneNode::AddRef() {}
void ScriptSceneNode::ReleaseRef() {}
void ScriptSceneNodeConstructAssert(void *ptr)
{
	ScriptManager::ScriptLog(std::string("Attempted to create instance of ScriptSceneNode!  Use @ reference only."));

	// Crash!
	int *a=0;
	*a=10;
}

...


ScriptManager::engine->RegisterObjectType("SceneNode", sizeof(Ogre::SceneNode), asOBJ_CLASS_CD);
ScriptManager::engine->RegisterObjectBehaviour("SceneNode", asBEHAVE_ADDREF, "void f()", asMETHOD(ScriptSceneNode, AddRef),asCALL_THISCALL);
ScriptManager::engine->RegisterObjectBehaviour("SceneNode", asBEHAVE_RELEASE, "void f()", asMETHOD(ScriptSceneNode, ReleaseRef),asCALL_THISCALL);

// You can't construct this class
ScriptManager::engine->RegisterObjectBehaviour("SceneNode", asBEHAVE_CONSTRUCT, "void f()", asFUNCTION(ScriptSceneNodeConstructAssert),asCALL_CDECL_OBJLAST);
ScriptManager::engine->RegisterObjectBehaviour("SceneNode", asBEHAVE_ASSIGNMENT, "SceneNode &f(const SceneNode &in)", asFUNCTION(ScriptSceneNodeConstructAssert),asCALL_CDECL_OBJLAST);



Share this post


Link to post
Share on other sites
Quote:
1. This syntax is rarely needed.
I use it all the time.

Quote:
2. Causes very difficult to find bugs.
Just bind the classes with a size of 0 so that you can't make instances of them yourself.

Quote:
3. Unintuitive
No, AngelScript is exactly like C in this case. Type* and Type are two different types, in C and in AngelScript.

Quote:
4. Used constantly
I never use handles at all. Ever.

Share this post


Link to post
Share on other sites
I agree that ambigous code is not very good. By trying to make AngelScript as easy as possible to use I may have gone a bit too far in some instances. But this can be corrected in future versions.

What do you suggest as the solution?

Should AngelScript require an explicit dereference for assignments? Something like:


SceneNode parent = *this.getSceneNode();


Or should I require an explicit handle to do a handle assignment?


SceneNode @parent = @this.getSceneNode();


I think I like the second option best, and goes well with how the object handle has been designed. It still doesn't stop the script writer from writing the value assignment, but at least it will be a bit more explicit when a handle assignment is being made and when not.

Instead of crashing the application. I suggest you raise a script exception. It's a lot friendlier, and can be treated by the application if you wish.


void ScriptSceneNodeConstructAssert(void *ptr)
{
ScriptManager::ScriptLog(std::string("Attempted to create instance of ScriptSceneNode! Use @ reference only."));

asGetActiveContext()->SetException("Error");
}

Share this post


Link to post
Share on other sites
If I may give my 2c... I use handles a lot as well, and I think I'd like it best like it is in C, so that you need explitly dereference things if you want to go from handle=>"instance". I.e. I think it should be a script compile error to try to do a:
Blah blah = getBlahHandle();
or:
Blah @blah = getBlah();

The former should (in my opinion) need a:
Blah blah = *getBlahHandle();
In C you of course got the other way as well (&), but I don't know if that's used in AngelScript.

That's my thoughts. Ignore if it didn't make sense. :)

Share this post


Link to post
Share on other sites
It makes sense, but with the handle concept I wanted to make it as transparent to the script writer as possible (admittedly I went too far), so I think I'll just do what I agreed with Rakkar2.

However, when I add support for pointers again, I'll do those exactly the way C++ does it, with * for dereferencing and & for taking the address of something. And also the -> for member access through pointer. But this is quite far away though.

Regards,
Andreas

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this