collide: Error checking for CollisionPolygon::setup/verify_points()

panda/src/collide/collisionPolygon_ext.cxx

@@ -29,7 +29,11 @@ extern struct Dtool_PyTypedObject Dtool_LPoint3f;
  */
 bool Extension<CollisionPolygon>::
 verify_points(PyObject *points) {
-  const pvector<LPoint3> vec = convert_points(points);
+  pvector<LPoint3> vec;
+  if (!convert_points(vec, points)) {
+    return false;
+  }
+
   const LPoint3 *verts_begin = &vec[0];
   const LPoint3 *verts_end = verts_begin + vec.size();
 
@@ -42,7 +46,16 @@ verify_points(PyObject *points) {
  */
 void Extension<CollisionPolygon>::
 setup_points(PyObject *points) {
-  const pvector<LPoint3> vec = convert_points(points);
+  pvector<LPoint3> vec;
+  if (!convert_points(vec, points)) {
+    return;
+  }
+
+  if (vec.size() < 3) {
+    PyErr_SetString(PyExc_ValueError, "expected at least 3 points");
+    return;
+  }
+
   const LPoint3 *verts_begin = &vec[0];
   const LPoint3 *verts_end = verts_begin + vec.size();
 
@@ -52,13 +65,11 @@ setup_points(PyObject *points) {
 /**
  * Converts a Python sequence to a list of LPoint3 objects.
  */
-pvector<LPoint3> Extension<CollisionPolygon>::
+bool Extension<CollisionPolygon>::
-convert_points(PyObject *points) {
+convert_points(pvector<LPoint3> &vec, PyObject *points) {
-  pvector<LPoint3> vec;
   PyObject *seq = PySequence_Fast(points, "function expects a sequence");
-
   if (!seq) {
-    return vec;
+    return false;
   }
 
   PyObject **items = PySequence_Fast_ITEMS(seq);
@@ -69,18 +80,23 @@ convert_points(PyObject *points) {
 
   for (Py_ssize_t i = 0; i < len; ++i) {
 #ifdef STDFLOAT_DOUBLE
-    if (ptr = DtoolInstance_UPCAST(items[i], Dtool_LPoint3d)) {
+    if (DtoolInstance_Check(itemts[i]) &&
+        (ptr = DtoolInstance_UPCAST(items[i], Dtool_LPoint3d))) {
 #else
-    if (ptr = DtoolInstance_UPCAST(items[i], Dtool_LPoint3f)) {
+    if (DtoolInstance_Check(items[i]) &&
+        (ptr = DtoolInstance_UPCAST(items[i], Dtool_LPoint3f))) {
 #endif
       vec.push_back(*(LPoint3 *)ptr);
-    } else {
+    }
-      collide_cat.warning() << "Argument must be of LPoint3 type.\n";
+    else {
+      Dtool_Raise_TypeError("Argument must be of LPoint3 type.");
+      Py_DECREF(seq);
+      return false;
     }
   }
 
   Py_DECREF(seq);
-  return vec;
+  return true;
 }
 
 #endif

panda/src/collide/collisionPolygon_ext.h

@@ -35,7 +35,7 @@ public:
   void setup_points(PyObject *points);
 
 private:
-  static pvector<LPoint3> convert_points(PyObject *points);
+  static bool convert_points(pvector<LPoint3> &vec, PyObject *points);
 };
 
 #endif  // HAVE_PYTHON

tests/collide/test_collision_polygon.py

@@ -1,4 +1,5 @@
 from panda3d import core
+import pytest
 
 
 def test_collision_polygon_verify_not_enough_points():
@@ -27,6 +28,9 @@ def test_collision_polygon_verify_points():
     assert core.CollisionPolygon.verify_points([core.LPoint3(3, -8, -7), core.LPoint3(9, 10, 8), core.LPoint3(7, 0, 10), core.LPoint3(-6, -2, 3)])
     assert core.CollisionPolygon.verify_points([core.LPoint3(-1, -3, -5), core.LPoint3(10, 3, -10), core.LPoint3(-10, 10, -4), core.LPoint3(0, 1, -4), core.LPoint3(-9, -2, 0)])
 
+    with pytest.raises(TypeError):
+        core.CollisionPolygon.verify_points([core.LPoint3(0, 0, 0), None])
+
 
 def test_collision_polygon_setup_points():
     # Create empty collision polygon
@@ -43,3 +47,9 @@ def test_collision_polygon_setup_points():
         polygon.setup_points(points)
         assert polygon.is_valid()
         assert polygon.get_num_points() == len(points)
+
+    with pytest.raises(TypeError):
+        polygon.setup_points([core.LPoint3(0, 0, 0), None, 1])
+
+    with pytest.raises(ValueError):
+        polygon.setup_points([core.LPoint3(0, 0, 0)])