BGE Python API

CListValue fixes
- Disable changing CValueLists that the BGE uses internally (scene.objects.append(1) would crash when drawing)
- val=clist+list would modify clist in place, now return a new value.
- clist.append([....]), was working like extend.
- clist.append(val) didnt work for most CValue types like KX_GameObjects.

Other changes
- "isValid" was always returning True.
- Set all errors for invalid proxy access to PyExc_SystemError (was using a mix of error types)
- Added PyObjectPlus::InvalidateProxy() to manually invalidate, though if python ever gains access again, it will make a new valid proxy. This is so removing an object from a scene can invalidate the object even if its stored elsewhere in a CValueList for eg.

source/gameengine/Expressions/ListValue.cpp

@@ -39,8 +39,10 @@ Py_ssize_t listvalue_bufferlen(PyObject* self)
 PyObject* listvalue_buffer_item(PyObject* self, Py_ssize_t index)
 {
 	CListValue *list= static_cast<CListValue *>(BGE_PROXY_REF(self));
+	CValue *cval;
+	
 	if (list==NULL) {
-		PyErr_SetString(PyExc_IndexError, BGE_PROXY_ERROR_MSG);
+		PyErr_SetString(PyExc_SystemError, "val = CList[i], "BGE_PROXY_ERROR_MSG);
 		return NULL;
 	}
 	
@@ -49,24 +51,25 @@ PyObject* listvalue_buffer_item(PyObject* self, Py_ssize_t index)
 	if (index < 0)
 		index = count+index;
 	
-	if (index >= 0 && index < count)
-	{
-		PyObject* pyobj = list->GetValue(index)->ConvertValueToPython();
-		if (pyobj)
-			return pyobj;
-		else
-			return list->GetValue(index)->GetProxy();
-
+	if (index < 0 || index >= count) {
+		PyErr_SetString(PyExc_IndexError, "CList[i]: Python ListIndex out of range in CValueList");
+		return NULL;
 	}
-	PyErr_SetString(PyExc_IndexError, "list[i]: Python ListIndex out of range in CValueList");
-	return NULL;
+	
+	cval= list->GetValue(index);
+	
+	PyObject* pyobj = cval->ConvertValueToPython();
+	if (pyobj)
+		return pyobj;
+	else
+		return cval->GetProxy();
 }
 
 PyObject* listvalue_mapping_subscript(PyObject* self, PyObject* pyindex)
 {
 	CListValue *list= static_cast<CListValue *>(BGE_PROXY_REF(self));
 	if (list==NULL) {
-		PyErr_SetString(PyExc_IndexError, BGE_PROXY_ERROR_MSG);
+		PyErr_SetString(PyExc_SystemError, "value = CList[i], "BGE_PROXY_ERROR_MSG);
 		return NULL;
 	}
 	
@@ -85,7 +88,7 @@ PyObject* listvalue_mapping_subscript(PyObject* self, PyObject* pyindex)
 	}
 	
 	PyObject *pyindex_str = PyObject_Repr(pyindex); /* new ref */
-	PyErr_Format(PyExc_KeyError, "list[key]: '%s' key not in list", PyString_AsString(pyindex_str));
+	PyErr_Format(PyExc_KeyError, "CList[key]: '%s' key not in list", PyString_AsString(pyindex_str));
 	Py_DECREF(pyindex_str);
 	return NULL;
 }
@@ -96,7 +99,7 @@ PyObject* listvalue_buffer_slice(PyObject* self,Py_ssize_t ilow, Py_ssize_t ihig
 {
 	CListValue *list= static_cast<CListValue *>(BGE_PROXY_REF(self));
 	if (list==NULL) {
-		PyErr_SetString(PyExc_IndexError, BGE_PROXY_ERROR_MSG);
+		PyErr_SetString(PyExc_SystemError, "val = CList[i:j], "BGE_PROXY_ERROR_MSG);
 		return NULL;
 	}
 	
@@ -127,73 +130,79 @@ PyObject* listvalue_buffer_slice(PyObject* self,Py_ssize_t ilow, Py_ssize_t ihig
 }
 
 
-
-static PyObject *
-listvalue_buffer_concat(PyObject * self, PyObject * other)
+/* clist + list, return a list that python owns */
+static PyObject *listvalue_buffer_concat(PyObject * self, PyObject * other)
 {
 	CListValue *listval= static_cast<CListValue *>(BGE_PROXY_REF(self));
+	int i, numitems, numitems_orig;
+	
 	if (listval==NULL) {
-		PyErr_SetString(PyExc_IndexError, BGE_PROXY_ERROR_MSG);
+		PyErr_SetString(PyExc_SystemError, "CList+other, "BGE_PROXY_ERROR_MSG);
 		return NULL;
 	}
 	
+	numitems_orig= listval->GetCount();
+	
 	// for now, we support CListValue concatenated with items
 	// and CListValue concatenated to Python Lists
 	// and CListValue concatenated with another CListValue
 	
-	listval->AddRef();
-	if (other->ob_type == &PyList_Type)
+	/* Shallow copy, dont use listval->GetReplica(), it will screw up with KX_GameObjects */
+	CListValue* listval_new = new CListValue();
+	
+	if (PyList_Check(other))
 	{
+		CValue* listitemval;
 		bool error = false;
-
-		int i;
-		int numitems = PyList_Size(other);
+		
+		numitems = PyList_Size(other);
+		
+		/* copy the first part of the list */
+		listval_new->Resize(numitems_orig + numitems);
+		for (i=0;i<numitems_orig;i++)
+			listval_new->SetValue(i, listval->GetValue(i)->AddRef());
+		
 		for (i=0;i<numitems;i++)
 		{
-			PyObject* listitem = PyList_GetItem(other,i);
-			CValue* listitemval = listval->ConvertPythonToValue(listitem);
-			if (listitemval)
-			{
-				listval->Add(listitemval);
-			} else
-			{
-				error = true;
+			listitemval = listval->ConvertPythonToValue(PyList_GetItem(other,i), "cList + pyList: CListValue, ");
+			
+			if (listitemval) {
+				listval_new->SetValue(i+numitems_orig, listitemval);
+			} else {
+				error= true;
+				break;
 			}
 		}
-
+		
 		if (error) {
-			PyErr_SetString(PyExc_SystemError, "list.append(val): couldn't add one or more items to this CValueList");
+			listval_new->Resize(numitems_orig+i); /* resize so we dont try release NULL pointers */
+			listval_new->Release();
+			return NULL; /* ConvertPythonToValue above sets the error */ 
+		}
+	
+	}
+	else if (PyObject_TypeCheck(other, &CListValue::Type)) {
+		// add items from otherlist to this list
+		CListValue* otherval = static_cast<CListValue *>(BGE_PROXY_REF(other));
+		if(otherval==NULL) {
+			listval_new->Release();
+			PyErr_SetString(PyExc_SystemError, "CList+other, "BGE_PROXY_ERROR_MSG);
 			return NULL;
 		}
-
-	} else
-	{
-		if (other->ob_type == &CListValue::Type)
-		{
-			// add items from otherlist to this list
-			CListValue* otherval = (CListValue*) other;
-			
-
-			for (int i=0;i<otherval->GetCount();i++)
-			{
-				otherval->Add(listval->GetValue(i)->AddRef());
-			}
-		}
-		else
-		{
-			CValue* objval = listval->ConvertPythonToValue(other);
-			if (objval)
-			{
-				listval->Add(objval);
-			} else
-			{
-				PyErr_SetString(PyExc_SystemError, "list.append(i): couldn't add item to this CValueList");
-				return NULL;
-			}
-		}
+		
+		numitems = otherval->GetCount();
+		
+		/* copy the first part of the list */
+		listval_new->Resize(numitems_orig + numitems); /* resize so we dont try release NULL pointers */
+		for (i=0;i<numitems_orig;i++)
+			listval_new->SetValue(i, listval->GetValue(i)->AddRef());
+		
+		/* now copy the other part of the list */
+		for (i=0;i<numitems;i++)
+			listval_new->SetValue(i+numitems_orig, otherval->GetValue(i)->AddRef());
+		
 	}
-
-	return self;
+	return listval_new->NewProxy(true); /* python owns this list */
 }
 
 
@@ -437,14 +446,24 @@ void CListValue::MergeList(CListValue *otherlist)
 	{
 		SetValue(i+numelements,otherlist->GetValue(i)->AddRef());
 	}
-
 }
 
 
-
 PyObject* CListValue::Pyappend(PyObject* value)
 {
-	return listvalue_buffer_concat(m_proxy, value); /* m_proxy is the same as self */
+	CValue* objval = ConvertPythonToValue(value, "CList.append(i): CValueList, ");
+
+	if (!objval) /* ConvertPythonToValue sets the error */
+		return NULL;
+	
+	if (!BGE_PROXY_PYOWNS(m_proxy)) {
+		PyErr_SetString(PyExc_TypeError, "CList.append(i): this CValueList is used internally for the game engine and can't be modified");
+		return NULL;
+	}
+	
+	Add(objval);
+	
+	Py_RETURN_NONE;
 }
 
 
@@ -482,7 +501,7 @@ PyObject* CListValue::Pyindex(PyObject *value)
 {
 	PyObject* result = NULL;
 
-	CValue* checkobj = ConvertPythonToValue(value);
+	CValue* checkobj = ConvertPythonToValue(value, "val = cList[i]: CValueList, ");
 	if (checkobj==NULL)
 		return NULL; /* ConvertPythonToValue sets the error */
 
@@ -499,7 +518,7 @@ PyObject* CListValue::Pyindex(PyObject *value)
 	checkobj->Release();
 
 	if (result==NULL) {
-		PyErr_SetString(PyExc_ValueError, "list.index(x): x not in CListValue");
+		PyErr_SetString(PyExc_ValueError, "CList.index(x): x not in CListValue");
 	}
 	return result;
 	
@@ -511,7 +530,7 @@ PyObject* CListValue::Pycount(PyObject* value)
 {
 	int numfound = 0;
 
-	CValue* checkobj = ConvertPythonToValue(value);
+	CValue* checkobj = ConvertPythonToValue(value, "cList.count(val): CValueList, ");
 	
 	if (checkobj==NULL) { /* in this case just return that there are no items in the list */
 		PyErr_Clear();

source/gameengine/Expressions/PyObjectPlus.cpp

@@ -138,9 +138,9 @@ PyObject *PyObjectPlus::py_base_getattro(PyObject * self, PyObject *attr)
 	PyObjectPlus *self_plus= BGE_PROXY_REF(self);
 	if(self_plus==NULL) {
 		if(!strcmp("isValid", PyString_AsString(attr))) {
-			Py_RETURN_TRUE;
+			Py_RETURN_FALSE;
 		}
-		PyErr_SetString(PyExc_RuntimeError, BGE_PROXY_ERROR_MSG);
+		PyErr_SetString(PyExc_SystemError, BGE_PROXY_ERROR_MSG);
 		return NULL;
 	}
 	
@@ -171,7 +171,7 @@ int PyObjectPlus::py_base_setattro(PyObject *self, PyObject *attr, PyObject *val
 {
 	PyObjectPlus *self_plus= BGE_PROXY_REF(self);
 	if(self_plus==NULL) {
-		PyErr_SetString(PyExc_RuntimeError, BGE_PROXY_ERROR_MSG);
+		PyErr_SetString(PyExc_SystemError, BGE_PROXY_ERROR_MSG);
 		return -1;
 	}
 	
@@ -186,7 +186,7 @@ PyObject *PyObjectPlus::py_base_repr(PyObject *self)			// This should be the ent
 	
 	PyObjectPlus *self_plus= BGE_PROXY_REF(self);
 	if(self_plus==NULL) {
-		PyErr_SetString(PyExc_RuntimeError, BGE_PROXY_ERROR_MSG);
+		PyErr_SetString(PyExc_SystemError, BGE_PROXY_ERROR_MSG);
 		return NULL;
 	}
 	
@@ -818,6 +818,23 @@ void PyObjectPlus::ProcessReplica()
 	m_proxy= NULL;
 }
 
+/* Sometimes we might want to manually invalidate a BGE type even if
+ * it hasnt been released by the BGE, say for example when an object
+ * is removed from a scene, accessing it may cause problems.
+ * 
+ * In this case the current proxy is made invalid, disowned,
+ * and will raise an error on access. However if python can get access
+ * to this class again it will make a new proxy and work as expected.
+ */
+void PyObjectPlus::InvalidateProxy()		// check typename of each parent
+{
+	if(m_proxy) { 
+		BGE_PROXY_REF(m_proxy)=NULL;
+		Py_DECREF(m_proxy);
+		m_proxy= NULL;
+	}
+}
+
 /* Utility function called by the macro py_getattro_up()
  * for getting ob.__dict__() values from our PyObject
  * this is used by python for doing dir() on an object, so its good

source/gameengine/Expressions/PyObjectPlus.h

@@ -451,6 +451,8 @@ public:
 	static PyObject *GetProxy_Ext(PyObjectPlus *self, PyTypeObject *tp);
 	static PyObject *NewProxy_Ext(PyObjectPlus *self, PyTypeObject *tp, bool py_owns);
 	
+	void	InvalidateProxy();
+	
 	/**
 	 * Makes sure any internal data owned by this class is deep copied.
 	 */

source/gameengine/Expressions/Value.cpp

@@ -604,7 +604,7 @@ PyObject* CValue::py_getattro_dict() {
 	py_getattro_dict_up(PyObjectPlus);
 }
 
-CValue* CValue::ConvertPythonToValue(PyObject* pyobj)
+CValue* CValue::ConvertPythonToValue(PyObject* pyobj, const char *error_prefix)
 {
 
 	CValue* vallie = NULL;
@@ -620,7 +620,7 @@ CValue* CValue::ConvertPythonToValue(PyObject* pyobj)
 		for (i=0;i<numitems;i++)
 		{
 			PyObject* listitem = PyList_GetItem(pyobj,i); /* borrowed ref */
-			CValue* listitemval = ConvertPythonToValue(listitem);
+			CValue* listitemval = ConvertPythonToValue(listitem, error_prefix);
 			if (listitemval)
 			{
 				listval->Add(listitemval);
@@ -657,13 +657,22 @@ CValue* CValue::ConvertPythonToValue(PyObject* pyobj)
 	{
 		vallie = new CStringValue(PyString_AsString(pyobj),"");
 	} else
-	if (pyobj->ob_type==&CValue::Type || pyobj->ob_type==&CListValue::Type)
+	if (BGE_PROXY_CHECK_TYPE(pyobj)) /* Note, dont let these get assigned to GameObject props, must check elsewhere */
 	{
-		vallie = ((CValue*) pyobj)->AddRef();
+		if (BGE_PROXY_REF(pyobj) && (BGE_PROXY_REF(pyobj))->isA(&CValue::Type))
+		{
+			vallie = (static_cast<CValue *>(BGE_PROXY_REF(pyobj)))->AddRef();
+		} else {
+			
+			if(BGE_PROXY_REF(pyobj))	/* this is not a CValue */
+				PyErr_Format(PyExc_TypeError, "%sgame engine python type cannot be used as a property", error_prefix);
+			else						/* PyObjectPlus_Proxy has been removed, cant use */
+				PyErr_Format(PyExc_SystemError, "%s"BGE_PROXY_ERROR_MSG, error_prefix);
+		}
 	} else
 	{
 		/* return an error value from the caller */
-		PyErr_SetString(PyExc_TypeError, "This python type could not be converted to a to a game engine property");
+		PyErr_Format(PyExc_TypeError, "%scould convert python value to a game engine property", error_prefix);
 	}
 	return vallie;
 
@@ -682,10 +691,11 @@ int	CValue::py_delattro(PyObject *attr)
 int	CValue::py_setattro(PyObject *attr, PyObject* pyobj)
 {
 	char *attr_str= PyString_AsString(attr);
-	CValue* oldprop = GetProperty(attr_str);
-	
-	CValue* vallie = ConvertPythonToValue(pyobj);
-	if (vallie)
+	CValue* oldprop = GetProperty(attr_str);	
+	CValue* vallie;
+
+	/* Dissallow python to assign GameObjects, Scenes etc as values */
+	if ((BGE_PROXY_CHECK_TYPE(pyobj)==0) && (vallie = ConvertPythonToValue(pyobj, "cvalue.attr = value: ")))
 	{
 		if (oldprop)
 			oldprop->SetValue(vallie);

source/gameengine/Expressions/Value.h

@@ -230,7 +230,7 @@ public:
 		return NULL;
 	}
 
-	virtual CValue*	ConvertPythonToValue(PyObject* pyobj);
+	virtual CValue*	ConvertPythonToValue(PyObject* pyobj, const char *error_prefix);
 
 
 	virtual int				py_delattro(PyObject *attr);

source/gameengine/Ketsji/KX_GameObject.cpp

@@ -1200,7 +1200,7 @@ PyObject *KX_GameObject::Map_GetItem(PyObject *self_v, PyObject *item)
 	PyObject* pyconvert;
 	
 	if (self==NULL) {
-		PyErr_SetString(PyExc_RuntimeError, BGE_PROXY_ERROR_MSG);
+		PyErr_SetString(PyExc_SystemError, BGE_PROXY_ERROR_MSG);
 		return NULL;
 	}
 	
@@ -1234,7 +1234,7 @@ int KX_GameObject::Map_SetItem(PyObject *self_v, PyObject *key, PyObject *val)
 		PyErr_Clear();
 	
 	if (self==NULL) {
-		PyErr_SetString(PyExc_RuntimeError, BGE_PROXY_ERROR_MSG);
+		PyErr_SetString(PyExc_SystemError, BGE_PROXY_ERROR_MSG);
 		return -1;
 	}
 	
@@ -1261,9 +1261,9 @@ int KX_GameObject::Map_SetItem(PyObject *self_v, PyObject *key, PyObject *val)
 		int set= 0;
 		
 		/* as CValue */
-		if(attr_str)
+		if(attr_str && BGE_PROXY_CHECK_TYPE(val)==0) /* dont allow GameObjects for eg to be assigned to CValue props */
 		{
-			CValue* vallie = self->ConvertPythonToValue(val);
+			CValue* vallie = self->ConvertPythonToValue(val, ""); /* error unused */
 			
 			if(vallie)
 			{
@@ -2643,7 +2643,7 @@ bool ConvertPythonToGameObject(PyObject * value, KX_GameObject **object, bool py
 		
 		/* sets the error */
 		if (*object==NULL) {
-			PyErr_Format(PyExc_RuntimeError, "%s, " BGE_PROXY_ERROR_MSG, error_prefix);
+			PyErr_Format(PyExc_SystemError, "%s, " BGE_PROXY_ERROR_MSG, error_prefix);
 			return false;
 		}
 		

source/gameengine/Ketsji/KX_MeshProxy.cpp

@@ -348,7 +348,7 @@ bool ConvertPythonToMesh(PyObject * value, RAS_MeshObject **object, bool py_none
 		
 		/* sets the error */
 		if (*object==NULL) {
-			PyErr_Format(PyExc_RuntimeError, "%s, " BGE_PROXY_ERROR_MSG, error_prefix);
+			PyErr_Format(PyExc_SystemError, "%s, " BGE_PROXY_ERROR_MSG, error_prefix);
 			return false;
 		}		
 		

source/gameengine/Ketsji/KX_Scene.cpp

@@ -891,7 +891,15 @@ void KX_Scene::RemoveObject(class CValue* gameobj)
 {
 	KX_GameObject* newobj = (KX_GameObject*) gameobj;
 
-	// first disconnect child from parent
+	/* Invalidate the python reference, since the object may exist in script lists
+	 * its possible that it wont be automatically invalidated, so do it manually here,
+	 * 
+	 * if for some reason the object is added back into the scene python can always get a new Proxy
+	 */
+	gameobj->InvalidateProxy();
+	
+	
+	// disconnect child from parent
 	SG_Node* node = newobj->GetSGNode();
 
 	if (node)

source/gameengine/Ketsji/KX_SceneActuator.cpp

@@ -316,7 +316,7 @@ int KX_SceneActuator::pyattr_set_camera(void *self, const struct KX_PYATTRIBUTE_
 		
 		if(camOb==NULL)
 		{
-			PyErr_SetString(PyExc_RuntimeError, BGE_PROXY_ERROR_MSG);
+			PyErr_SetString(PyExc_SystemError, BGE_PROXY_ERROR_MSG);
 			return 1;
 		}
 		
@@ -435,7 +435,7 @@ PyObject* KX_SceneActuator::PySetCamera(PyObject* args)
 		new_camera = static_cast<KX_Camera*>BGE_PROXY_REF(cam);
 		if(new_camera==NULL)
 		{
-			PyErr_SetString(PyExc_RuntimeError, BGE_PROXY_ERROR_MSG);
+			PyErr_SetString(PyExc_SystemError, BGE_PROXY_ERROR_MSG);
 			return NULL;
 		}