Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vector{2,3}.__delattr__ messaging fixed when deleting x, y, {z} and removed Vector4 zombies #3069

Merged
merged 2 commits into from
Aug 30, 2024
Merged

Vector{2,3}.__delattr__ messaging fixed when deleting x, y, {z} and removed Vector4 zombies #3069

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c94273
Fixed __delattr__ messaging when trying to delete vector properties, …
oddbookworm Aug 20, 2024
2e0800b
Removed vector3-specific check because vector2 shouldn't ever hit it
oddbookworm Aug 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 25 additions & 60 deletions src_c/math.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,20 +195,12 @@ static PyObject *
vector_gety(pgVector *self, void *closure);
static PyObject *
vector_getz(pgVector *self, void *closure);
#ifdef PYGAME_MATH_VECTOR_HAVE_W
static PyObject *
vector_getw(pgVector *self, void *closure);
#endif
static int
vector_setx(pgVector *self, PyObject *value, void *closure);
static int
vector_sety(pgVector *self, PyObject *value, void *closure);
static int
vector_setz(pgVector *self, PyObject *value, void *closure);
#ifdef PYGAME_MATH_VECTOR_HAVE_W
static int
vector_setw(pgVector *self, PyObject *value, void *closure);
#endif
static PyObject *
vector_richcompare(PyObject *o1, PyObject *o2, int op);
static PyObject *
Expand Down Expand Up @@ -435,13 +427,6 @@ pgVectorCompatible_Check(PyObject *obj, Py_ssize_t dim)
return 1;
}
break;
/*
case 4:
if (pgVector4_Check(obj)) {
return 1;
}
break;
*/
default:
PyErr_SetString(
PyExc_SystemError,
Expand Down Expand Up @@ -634,10 +619,6 @@ pgVector_NEW(Py_ssize_t dim)
return vector2_new(&pgVector2_Type, NULL, NULL);
case 3:
return vector3_new(&pgVector3_Type, NULL, NULL);
/*
case 4:
return vector4_new(&pgVector4_Type, NULL, NULL);
*/
default:
return RAISE(PyExc_SystemError,
"Wrong internal call to pgVector_NEW.\n");
Expand Down Expand Up @@ -1219,11 +1200,26 @@ static int
vector_set_component(pgVector *self, PyObject *value, int component)
{
if (value == NULL) {
PyErr_SetString(PyExc_TypeError, "Cannot delete the x attribute");
return -1;
}
if (component >= self->dim) {
PyErr_BadInternalCall();
switch (component) {
case 0: {
PyErr_SetString(PyExc_TypeError,
"Cannot delete the x attribute");
break;
}
case 1: {
PyErr_SetString(PyExc_TypeError,
"Cannot delete the y attribute");
break;
}
case 2: {
PyErr_SetString(PyExc_TypeError,
"Cannot delete the z attribute");
break;
}
default: {
PyErr_BadInternalCall();
}
}
return -1;
}

Expand Down Expand Up @@ -1269,20 +1265,6 @@ vector_setz(pgVector *self, PyObject *value, void *closure)
return vector_set_component(self, value, 2);
}

#ifdef PYGAME_MATH_VECTOR_HAVE_W
static PyObject *
vector_getw(pgVector *self, void *closure)
{
return PyFloat_FromDouble(self->coords[3]);
}

static int
vector_setw(pgVector *self, PyObject *value, void *closure)
{
return vector_set_component(self, value, 3);
}
#endif

static PyObject *
vector_richcompare(PyObject *o1, PyObject *o2, int op)
{
Expand Down Expand Up @@ -1944,8 +1926,7 @@ vector_getAttr_swizzle(pgVector *self, PyObject *attr_name)
if (attr == NULL)
goto internal_error;
/* If we are not a swizzle, go straight to GenericGetAttr. */
if ((attr[0] != 'x') && (attr[0] != 'y') && (attr[0] != 'z') &&
(attr[0] != 'w')) {
if ((attr[0] != 'x') && (attr[0] != 'y') && (attr[0] != 'z')) {
goto swizzle_failed;
}

Expand All @@ -1965,8 +1946,6 @@ vector_getAttr_swizzle(pgVector *self, PyObject *attr_name)
case 'z':
idx = attr[i] - 'x';
goto swizzle_idx;
case 'w':
idx = 3;

swizzle_idx:
if (idx >= self->dim) {
Expand Down Expand Up @@ -2045,9 +2024,6 @@ vector_setAttr_swizzle(pgVector *self, PyObject *attr_name, PyObject *val)
case 'z':
idx = attr[i] - 'x';
break;
case 'w':
idx = 3;
break;
default:
/* swizzle failed. attempt generic attribute setting */
Py_DECREF(attr_unicode);
Expand Down Expand Up @@ -4459,8 +4435,7 @@ MODINIT_DEFINE(math)
if ((PyType_Ready(&pgVector2_Type) < 0) ||
(PyType_Ready(&pgVector3_Type) < 0) ||
(PyType_Ready(&pgVectorIter_Type) < 0) ||
(PyType_Ready(&pgVectorElementwiseProxy_Type) < 0) /*||
(PyType_Ready(&pgVector4_Type) < 0)*/) {
(PyType_Ready(&pgVectorElementwiseProxy_Type) < 0)) {
return NULL;
}

Expand All @@ -4476,9 +4451,6 @@ MODINIT_DEFINE(math)
Py_INCREF(&pgVector3_Type);
Py_INCREF(&pgVectorIter_Type);
Py_INCREF(&pgVectorElementwiseProxy_Type);
/*
Py_INCREF(&pgVector4_Type);
*/
if ((PyModule_AddObject(module, "Vector2", (PyObject *)&pgVector2_Type) !=
0) ||
(PyModule_AddObject(module, "Vector3", (PyObject *)&pgVector3_Type) !=
Expand All @@ -4487,9 +4459,7 @@ MODINIT_DEFINE(math)
(PyObject *)&pgVectorElementwiseProxy_Type) !=
0) ||
(PyModule_AddObject(module, "VectorIterator",
(PyObject *)&pgVectorIter_Type) != 0) /*||
(PyModule_AddObject(module, "Vector4", (PyObject *)&pgVector4_Type) !=
0)*/) {
(PyObject *)&pgVectorIter_Type) != 0)) {
if (!PyObject_HasAttrString(module, "Vector2"))
Py_DECREF(&pgVector2_Type);
if (!PyObject_HasAttrString(module, "Vector3"))
Expand All @@ -4498,10 +4468,6 @@ MODINIT_DEFINE(math)
Py_DECREF(&pgVectorElementwiseProxy_Type);
if (!PyObject_HasAttrString(module, "VectorIterator"))
Py_DECREF(&pgVectorIter_Type);
/*
if (!PyObject_HasAttrString(module, "Vector4"))
Py_DECREF(&pgVector4_Type);
*/
Py_DECREF(module);
return NULL;
}
Expand All @@ -4510,9 +4476,8 @@ MODINIT_DEFINE(math)
c_api[0] = &pgVector2_Type;
c_api[1] = &pgVector3_Type;
/*
c_api[2] = &pgVector4_Type;
c_api[3] = pgVector_NEW;
c_api[4] = pgVectorCompatible_Check;
c_api[2] = pgVector_NEW;
c_api[3] = pgVectorCompatible_Check;
*/
apiobj = encapsulate_api(c_api, "math");
if (PyModule_AddObject(module, PYGAMEAPI_LOCAL_ENTRY, apiobj)) {
Expand Down
40 changes: 40 additions & 0 deletions test/math_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1347,6 +1347,22 @@ def supermariobrosiscool(self):
self.assertEqual(type(other / 3), TestVector)
self.assertEqual(type(other.elementwise() ** 3), TestVector)

def test_del_x(self):
"""Verify that the correct error message gets spit out when trying to delete x"""
with self.assertRaises(TypeError) as ctx:
del Vector2().x

exception = ctx.exception
self.assertEqual(str(exception), "Cannot delete the x attribute")

def test_del_y(self):
"""Verify that the correct error message gets spit out when trying to delete y"""
with self.assertRaises(TypeError) as ctx:
del Vector2().y

exception = ctx.exception
self.assertEqual(str(exception), "Cannot delete the y attribute")


class Vector3TypeTest(unittest.TestCase):
def setUp(self):
Expand Down Expand Up @@ -3072,6 +3088,30 @@ def test_move_towards_errors(self):
self.assertRaises(TypeError, origin.move_towards, "c", 3)
self.assertRaises(TypeError, origin.move_towards_ip, "d", 3)

def test_del_x(self):
"""Verify that the correct error message gets spit out when trying to delete x"""
with self.assertRaises(TypeError) as ctx:
del Vector3().x

exception = ctx.exception
self.assertEqual(str(exception), "Cannot delete the x attribute")

def test_del_y(self):
"""Verify that the correct error message gets spit out when trying to delete y"""
with self.assertRaises(TypeError) as ctx:
del Vector3().y

exception = ctx.exception
self.assertEqual(str(exception), "Cannot delete the y attribute")

def test_del_z(self):
"""Verify that the correct error message gets spit out when trying to delete z"""
with self.assertRaises(TypeError) as ctx:
del Vector3().z

exception = ctx.exception
self.assertEqual(str(exception), "Cannot delete the z attribute")


if __name__ == "__main__":
unittest.main()
Loading