From 6dd1cec600efd3b9f44f67968a23c88e05e19ec8 Mon Sep 17 00:00:00 2001 From: John McCardle Date: Thu, 3 Jul 2025 20:27:32 -0400 Subject: [PATCH] Fix Entity property setters and PyVector implementation Fixed the 'new style getargs format' error in Entity property setters by: - Implementing PyObject_to_sfVector2f/2i using PyVector::from_arg - Adding proper error checking in Entity::set_position - Implementing PyVector get_member/set_member for x/y properties - Fixing PyVector::from_arg to handle non-tuple arguments correctly Now Entity.pos and Entity.sprite_number setters work correctly with proper type validation. --- src/PyVector.cpp | 58 +++++++++++-- src/UIEntity.cpp | 50 ++++++++---- tests/.automation_screenshot_test.py.swp | Bin 12288 -> 0 bytes tests/entity_property_setters_test.py | 99 +++++++++++++++++++++++ tests/entity_setter_simple_test.py | 61 ++++++++++++++ 5 files changed, 247 insertions(+), 21 deletions(-) delete mode 100644 tests/.automation_screenshot_test.py.swp create mode 100644 tests/entity_property_setters_test.py create mode 100644 tests/entity_setter_simple_test.py diff --git a/src/PyVector.cpp b/src/PyVector.cpp index f1143cb..83c243e 100644 --- a/src/PyVector.cpp +++ b/src/PyVector.cpp @@ -106,13 +106,37 @@ PyObject* PyVector::pynew(PyTypeObject* type, PyObject* args, PyObject* kwds) PyObject* PyVector::get_member(PyObject* obj, void* closure) { - // TODO - return Py_None; + PyVectorObject* self = (PyVectorObject*)obj; + if (reinterpret_cast(closure) == 0) { + // x + return PyFloat_FromDouble(self->data.x); + } else { + // y + return PyFloat_FromDouble(self->data.y); + } } int PyVector::set_member(PyObject* obj, PyObject* value, void* closure) { - // TODO + PyVectorObject* self = (PyVectorObject*)obj; + float val; + + if (PyFloat_Check(value)) { + val = PyFloat_AsDouble(value); + } else if (PyLong_Check(value)) { + val = PyLong_AsDouble(value); + } else { + PyErr_SetString(PyExc_TypeError, "Vector members must be numeric"); + return -1; + } + + if (reinterpret_cast(closure) == 0) { + // x + self->data.x = val; + } else { + // y + self->data.y = val; + } return 0; } @@ -120,11 +144,31 @@ PyVectorObject* PyVector::from_arg(PyObject* args) { auto type = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Vector"); if (PyObject_IsInstance(args, (PyObject*)type)) return (PyVectorObject*)args; + auto obj = (PyVectorObject*)type->tp_alloc(type, 0); - int err = init(obj, args, NULL); - if (err) { - Py_DECREF(obj); - return NULL; + + // Handle different input types + if (PyTuple_Check(args)) { + // It's already a tuple, pass it directly to init + int err = init(obj, args, NULL); + if (err) { + Py_DECREF(obj); + return NULL; + } + } else { + // Wrap single argument in a tuple for init + PyObject* tuple = PyTuple_Pack(1, args); + if (!tuple) { + Py_DECREF(obj); + return NULL; + } + int err = init(obj, tuple, NULL); + Py_DECREF(tuple); + if (err) { + Py_DECREF(obj); + return NULL; + } } + return obj; } diff --git a/src/UIEntity.cpp b/src/UIEntity.cpp index 32fd3e7..3b8950c 100644 --- a/src/UIEntity.cpp +++ b/src/UIEntity.cpp @@ -2,6 +2,8 @@ #include "UIGrid.h" #include "McRFPy_API.h" #include "PyObjectUtils.h" +#include "PyVector.h" + UIEntity::UIEntity() {} // this will not work lol. TODO remove default constructor by finding the shared pointer inits that use it @@ -104,28 +106,40 @@ PyObject* UIEntity::get_spritenumber(PyUIEntityObject* self, void* closure) { return PyLong_FromDouble(self->data->sprite.getSpriteIndex()); } -PyObject* sfVector2f_to_PyObject(sf::Vector2f vector) { - return Py_BuildValue("(ff)", vector.x, vector.y); +PyObject* sfVector2f_to_PyObject(sf::Vector2f vec) { + auto type = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Vector"); + auto obj = (PyVectorObject*)type->tp_alloc(type, 0); + if (obj) { + obj->data = vec; + } + return (PyObject*)obj; } -PyObject* sfVector2i_to_PyObject(sf::Vector2i vector) { - return Py_BuildValue("(ii)", vector.x, vector.y); +PyObject* sfVector2i_to_PyObject(sf::Vector2i vec) { + auto type = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Vector"); + auto obj = (PyVectorObject*)type->tp_alloc(type, 0); + if (obj) { + obj->data = sf::Vector2f(static_cast(vec.x), static_cast(vec.y)); + } + return (PyObject*)obj; } sf::Vector2f PyObject_to_sfVector2f(PyObject* obj) { - float x, y; - if (!PyArg_ParseTuple(obj, "ff", &x, &y)) { - return sf::Vector2f(); // TODO / reconsider this default: Return default vector on parse error + PyVectorObject* vec = PyVector::from_arg(obj); + if (!vec) { + // PyVector::from_arg already set the error + return sf::Vector2f(0, 0); } - return sf::Vector2f(x, y); + return vec->data; } sf::Vector2i PyObject_to_sfVector2i(PyObject* obj) { - int x, y; - if (!PyArg_ParseTuple(obj, "ii", &x, &y)) { - return sf::Vector2i(); // TODO / reconsider this default: Return default vector on parse error + PyVectorObject* vec = PyVector::from_arg(obj); + if (!vec) { + // PyVector::from_arg already set the error + return sf::Vector2i(0, 0); } - return sf::Vector2i(x, y); + return sf::Vector2i(static_cast(vec->data.x), static_cast(vec->data.y)); } // TODO - deprecate / remove this helper @@ -161,9 +175,17 @@ PyObject* UIEntity::get_position(PyUIEntityObject* self, void* closure) { int UIEntity::set_position(PyUIEntityObject* self, PyObject* value, void* closure) { if (reinterpret_cast(closure) == 0) { - self->data->position = PyObject_to_sfVector2f(value); + sf::Vector2f vec = PyObject_to_sfVector2f(value); + if (PyErr_Occurred()) { + return -1; // Error already set by PyObject_to_sfVector2f + } + self->data->position = vec; } else { - self->data->collision_pos = PyObject_to_sfVector2i(value); + sf::Vector2i vec = PyObject_to_sfVector2i(value); + if (PyErr_Occurred()) { + return -1; // Error already set by PyObject_to_sfVector2i + } + self->data->collision_pos = vec; } return 0; } diff --git a/tests/.automation_screenshot_test.py.swp b/tests/.automation_screenshot_test.py.swp deleted file mode 100644 index 114022417ff1a32f390b9273bc04f423fe15fdae..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 12288 zcmeI2ONbmr7{@CQH9lg3qMi(u9#(qdIy*DF!MHew7_(%-)sRhskI1B_r+TN8p6<4* zdXver(Ys&{0VCou2nvee#UKIkB0@k#51xY;gO_-Sh#>x}x_Y|zm6gRyq?-JCx~r?c z`o4cXmsF>Ie0qjHQ>znPI|-TCHrJate`E8vC+g%>)C*dhqkvJsC}0#Y3fzGTI9)Tj4zZ z|No?p;ny96d;ap>%?Am&4!#Fhz(?R+a0VO#yTBjY3AqNo z0Uv^MpauSXfRG=-RqzFP8@vd1f*bb}@)P(Dybn%-U0@ryc^@IafotF*cnd6m6JQoR z3Vyklkgos-FM|=V1N^m(kXzsi_zZYp0_+5z-$TfI;8k!8JOLgDzu!&B$KV1u0%kxJ zJOXarMaU1}GH}5Z*ax9~zuB^7!k4XNlcD!);`-)CLl5iA&xzMFXQpvIXv>i1bh*e+ z#EBmY+nQy9x=s*u9QU+UC4}mW4|t88^@ZK24%2jzsr}P0Agj#C0?sCf--l&jUVfFA zE^~PQ?5r;1d(`s-#@k+$hF#if(SU`vObVSopcS+O+eQqS=N+DQehi!=$9^@Q;>5P22Szbpk>7Dvu%#gQ}H(!z^mkO@+uN99l$ zpQO{_yc77SNB1OZLu@CG1K)Kz0n6He$U5Xz_IEbMNjWDdN){C=uQw>fMLIh?D@V`x zqDT3Pikx|6JomY|NbO7kNUy~y_q9A9uOToNc89&ES#+^=6wFPON(_@AyJsz%cG}wM zw%cRN3Kvd4UexFR!a0jbMVB{8b8^l2r6~s;EaBxYuB@n=G&h_~ZaV?VoaLr` z&f&gG|Eu;}VT~_quf|5(s}$6~Y_C#@yuBK$wpT|=P;RGMyjBx6Ch77JiBweG@>)1* zdF^$TG;Dh$b+PWq_gWa8v8y#M5>LL`*2pU({gG~aNIvK5QQF5~_hZc8MFVkV zSlv=T@71mo%d@6gpVlumJ;ydDRA#!y1+Rm)8wF9)(o5n2`C>Qs?j5D-R<9OgWi=y$ zofR#sG<4MjGwZUDS);0M?GZ%CtH}Jd_3Lp=o~W`>$6EVzy_ywLC5sTf1QUE|46g7b zN=4v@tktZQR@1d|trjtAm;ZG2D@w|QwZuX>Emh`w=4=t~lyu&1Hf4Y^Z(Tm@t*;$u zsq?OIBoB0aqk$Gpi>Hl*o5fmarNWhiOSfkhu4>K|(N^gGZkO^1tF`KNUK(Kc!Q?sW zC0)C)#$``BHf>wQAW$=@UfFb=*vP3aw@x-JFc^oo&57-S%0wLuJ5;TgBI)tG&r_^> zOZ=^92f8dcDz}ison$s@D%osniAe1iWouUCEnC!yd{pmwHowS*_eD z;cl!l7epPnBU6