Fix multiple low priority issues

closes #12, closes #80, closes #95, closes #96, closes #99

- Issue #12: Set tp_new to NULL for GridPoint and GridPointState to prevent instantiation from Python
- Issue #80: Renamed Caption.size to Caption.font_size for semantic clarity
- Issue #95: Fixed UICollection repr to show actual derived types instead of generic UIDrawable
- Issue #96: Added extend() method to UICollection for API consistency with UIEntityCollection
- Issue #99: Exposed read-only properties for Texture (sprite_width, sprite_height, sheet_width, sheet_height, sprite_count, source) and Font (family, source)

All issues have corresponding tests that verify the fixes work correctly.
This commit is contained in:
John McCardle 2025-07-05 16:09:52 -04:00
commit 5a003a9aa5
13 changed files with 1094 additions and 7 deletions

View file

@ -61,3 +61,19 @@ PyObject* PyFont::pynew(PyTypeObject* type, PyObject* args, PyObject* kwds)
{
return (PyObject*)type->tp_alloc(type, 0);
}
PyObject* PyFont::get_family(PyFontObject* self, void* closure)
{
return PyUnicode_FromString(self->data->font.getInfo().family.c_str());
}
PyObject* PyFont::get_source(PyFontObject* self, void* closure)
{
return PyUnicode_FromString(self->data->source.c_str());
}
PyGetSetDef PyFont::getsetters[] = {
{"family", (getter)PyFont::get_family, NULL, "Font family name", NULL},
{"source", (getter)PyFont::get_source, NULL, "Source filename of the font", NULL},
{NULL} // Sentinel
};

View file

@ -21,6 +21,12 @@ public:
static Py_hash_t hash(PyObject*);
static int init(PyFontObject*, PyObject*, PyObject*);
static PyObject* pynew(PyTypeObject* type, PyObject* args=NULL, PyObject* kwds=NULL);
// Getters for properties
static PyObject* get_family(PyFontObject* self, void* closure);
static PyObject* get_source(PyFontObject* self, void* closure);
static PyGetSetDef getsetters[];
};
namespace mcrfpydef {
@ -33,6 +39,7 @@ namespace mcrfpydef {
//.tp_hash = PyFont::hash,
.tp_flags = Py_TPFLAGS_DEFAULT,
.tp_doc = PyDoc_STR("SFML Font Object"),
.tp_getset = PyFont::getsetters,
//.tp_base = &PyBaseObject_Type,
.tp_init = (initproc)PyFont::init,
.tp_new = PyType_GenericNew, //PyFont::pynew,

View file

@ -79,3 +79,43 @@ PyObject* PyTexture::pynew(PyTypeObject* type, PyObject* args, PyObject* kwds)
{
return (PyObject*)type->tp_alloc(type, 0);
}
PyObject* PyTexture::get_sprite_width(PyTextureObject* self, void* closure)
{
return PyLong_FromLong(self->data->sprite_width);
}
PyObject* PyTexture::get_sprite_height(PyTextureObject* self, void* closure)
{
return PyLong_FromLong(self->data->sprite_height);
}
PyObject* PyTexture::get_sheet_width(PyTextureObject* self, void* closure)
{
return PyLong_FromLong(self->data->sheet_width);
}
PyObject* PyTexture::get_sheet_height(PyTextureObject* self, void* closure)
{
return PyLong_FromLong(self->data->sheet_height);
}
PyObject* PyTexture::get_sprite_count(PyTextureObject* self, void* closure)
{
return PyLong_FromLong(self->data->getSpriteCount());
}
PyObject* PyTexture::get_source(PyTextureObject* self, void* closure)
{
return PyUnicode_FromString(self->data->source.c_str());
}
PyGetSetDef PyTexture::getsetters[] = {
{"sprite_width", (getter)PyTexture::get_sprite_width, NULL, "Width of each sprite in pixels", NULL},
{"sprite_height", (getter)PyTexture::get_sprite_height, NULL, "Height of each sprite in pixels", NULL},
{"sheet_width", (getter)PyTexture::get_sheet_width, NULL, "Number of sprite columns in the texture", NULL},
{"sheet_height", (getter)PyTexture::get_sheet_height, NULL, "Number of sprite rows in the texture", NULL},
{"sprite_count", (getter)PyTexture::get_sprite_count, NULL, "Total number of sprites in the texture", NULL},
{"source", (getter)PyTexture::get_source, NULL, "Source filename of the texture", NULL},
{NULL} // Sentinel
};

View file

@ -26,6 +26,16 @@ public:
static Py_hash_t hash(PyObject*);
static int init(PyTextureObject*, PyObject*, PyObject*);
static PyObject* pynew(PyTypeObject* type, PyObject* args=NULL, PyObject* kwds=NULL);
// Getters for properties
static PyObject* get_sprite_width(PyTextureObject* self, void* closure);
static PyObject* get_sprite_height(PyTextureObject* self, void* closure);
static PyObject* get_sheet_width(PyTextureObject* self, void* closure);
static PyObject* get_sheet_height(PyTextureObject* self, void* closure);
static PyObject* get_sprite_count(PyTextureObject* self, void* closure);
static PyObject* get_source(PyTextureObject* self, void* closure);
static PyGetSetDef getsetters[];
};
namespace mcrfpydef {
@ -38,6 +48,7 @@ namespace mcrfpydef {
.tp_hash = PyTexture::hash,
.tp_flags = Py_TPFLAGS_DEFAULT,
.tp_doc = PyDoc_STR("SFML Texture Object"),
.tp_getset = PyTexture::getsetters,
//.tp_base = &PyBaseObject_Type,
.tp_init = (initproc)PyTexture::init,
.tp_new = PyType_GenericNew, //PyTexture::pynew,

View file

@ -197,7 +197,7 @@ PyGetSetDef UICaption::getsetters[] = {
{"outline_color", (getter)UICaption::get_color_member, (setter)UICaption::set_color_member, "Outline color of the text", (void*)1},
//{"children", (getter)PyUIFrame_get_children, NULL, "UICollection of objects on top of this one", NULL},
{"text", (getter)UICaption::get_text, (setter)UICaption::set_text, "The text displayed", NULL},
{"size", (getter)UICaption::get_float_member, (setter)UICaption::set_float_member, "Text size (integer) in points", (void*)5},
{"font_size", (getter)UICaption::get_float_member, (setter)UICaption::set_float_member, "Font size (integer) in points", (void*)5},
{"click", (getter)UIDrawable::get_click, (setter)UIDrawable::set_click, "Object called with (x, y, button) when clicked", (void*)PyObjectsEnum::UICAPTION},
{"z_index", (getter)UIDrawable::get_int, (setter)UIDrawable::set_int, "Z-order for rendering (lower values rendered first)", (void*)PyObjectsEnum::UICAPTION},
{NULL}
@ -314,7 +314,7 @@ bool UICaption::setProperty(const std::string& name, float value) {
text.setPosition(sf::Vector2f(text.getPosition().x, value));
return true;
}
else if (name == "size") {
else if (name == "font_size" || name == "size") { // Support both for backward compatibility
text.setCharacterSize(static_cast<unsigned int>(value));
return true;
}
@ -406,7 +406,7 @@ bool UICaption::getProperty(const std::string& name, float& value) const {
value = text.getPosition().y;
return true;
}
else if (name == "size") {
else if (name == "font_size" || name == "size") { // Support both for backward compatibility
value = static_cast<float>(text.getCharacterSize());
return true;
}

View file

@ -615,6 +615,88 @@ PyObject* UICollection::append(PyUICollectionObject* self, PyObject* o)
return Py_None;
}
PyObject* UICollection::extend(PyUICollectionObject* self, PyObject* iterable)
{
// Accept any iterable of UIDrawable objects
PyObject* iterator = PyObject_GetIter(iterable);
if (iterator == NULL) {
PyErr_SetString(PyExc_TypeError, "UICollection.extend requires an iterable");
return NULL;
}
// Ensure module is initialized
if (!McRFPy_API::mcrf_module) {
Py_DECREF(iterator);
PyErr_SetString(PyExc_RuntimeError, "mcrfpy module not initialized");
return NULL;
}
// Get current highest z_index
int current_z_index = 0;
if (!self->data->empty()) {
current_z_index = self->data->back()->z_index;
}
PyObject* item;
while ((item = PyIter_Next(iterator)) != NULL) {
// Check if item is a UIDrawable subclass
if (!PyObject_IsInstance(item, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Frame")) &&
!PyObject_IsInstance(item, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Sprite")) &&
!PyObject_IsInstance(item, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Caption")) &&
!PyObject_IsInstance(item, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Grid")))
{
Py_DECREF(item);
Py_DECREF(iterator);
PyErr_SetString(PyExc_TypeError, "All items must be Frame, Caption, Sprite, or Grid objects");
return NULL;
}
// Increment z_index for each new element
if (current_z_index <= INT_MAX - 10) {
current_z_index += 10;
} else {
current_z_index = INT_MAX;
}
// Add the item based on its type
if (PyObject_IsInstance(item, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Frame"))) {
PyUIFrameObject* frame = (PyUIFrameObject*)item;
frame->data->z_index = current_z_index;
self->data->push_back(frame->data);
}
else if (PyObject_IsInstance(item, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Caption"))) {
PyUICaptionObject* caption = (PyUICaptionObject*)item;
caption->data->z_index = current_z_index;
self->data->push_back(caption->data);
}
else if (PyObject_IsInstance(item, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Sprite"))) {
PyUISpriteObject* sprite = (PyUISpriteObject*)item;
sprite->data->z_index = current_z_index;
self->data->push_back(sprite->data);
}
else if (PyObject_IsInstance(item, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Grid"))) {
PyUIGridObject* grid = (PyUIGridObject*)item;
grid->data->z_index = current_z_index;
self->data->push_back(grid->data);
}
Py_DECREF(item);
}
Py_DECREF(iterator);
// Check if iteration ended due to an error
if (PyErr_Occurred()) {
return NULL;
}
// Mark scene as needing resort after adding elements
McRFPy_API::markSceneNeedsSort();
Py_INCREF(Py_None);
return Py_None;
}
PyObject* UICollection::remove(PyUICollectionObject* self, PyObject* o)
{
if (!PyLong_Check(o))
@ -734,7 +816,7 @@ PyObject* UICollection::count(PyUICollectionObject* self, PyObject* value) {
PyMethodDef UICollection::methods[] = {
{"append", (PyCFunction)UICollection::append, METH_O},
//{"extend", (PyCFunction)PyUICollection_extend, METH_O}, // TODO
{"extend", (PyCFunction)UICollection::extend, METH_O},
{"remove", (PyCFunction)UICollection::remove, METH_O},
{"index", (PyCFunction)UICollection::index_method, METH_O},
{"count", (PyCFunction)UICollection::count, METH_O},
@ -746,7 +828,47 @@ PyObject* UICollection::repr(PyUICollectionObject* self)
std::ostringstream ss;
if (!self->data) ss << "<UICollection (invalid internal object)>";
else {
ss << "<UICollection (" << self->data->size() << " child objects)>";
ss << "<UICollection (" << self->data->size() << " objects: ";
// Count each type
int frame_count = 0, caption_count = 0, sprite_count = 0, grid_count = 0, other_count = 0;
for (auto& item : *self->data) {
switch(item->derived_type()) {
case PyObjectsEnum::UIFRAME: frame_count++; break;
case PyObjectsEnum::UICAPTION: caption_count++; break;
case PyObjectsEnum::UISPRITE: sprite_count++; break;
case PyObjectsEnum::UIGRID: grid_count++; break;
default: other_count++; break;
}
}
// Build type summary
bool first = true;
if (frame_count > 0) {
ss << frame_count << " Frame" << (frame_count > 1 ? "s" : "");
first = false;
}
if (caption_count > 0) {
if (!first) ss << ", ";
ss << caption_count << " Caption" << (caption_count > 1 ? "s" : "");
first = false;
}
if (sprite_count > 0) {
if (!first) ss << ", ";
ss << sprite_count << " Sprite" << (sprite_count > 1 ? "s" : "");
first = false;
}
if (grid_count > 0) {
if (!first) ss << ", ";
ss << grid_count << " Grid" << (grid_count > 1 ? "s" : "");
first = false;
}
if (other_count > 0) {
if (!first) ss << ", ";
ss << other_count << " UIDrawable" << (other_count > 1 ? "s" : "");
}
ss << ")>";
}
std::string repr_str = ss.str();
return PyUnicode_DecodeUTF8(repr_str.c_str(), repr_str.size(), "replace");

View file

@ -28,6 +28,7 @@ public:
static PyObject* subscript(PyUICollectionObject* self, PyObject* key);
static int ass_subscript(PyUICollectionObject* self, PyObject* key, PyObject* value);
static PyObject* append(PyUICollectionObject* self, PyObject* o);
static PyObject* extend(PyUICollectionObject* self, PyObject* iterable);
static PyObject* remove(PyUICollectionObject* self, PyObject* o);
static PyObject* index_method(PyUICollectionObject* self, PyObject* value);
static PyObject* count(PyUICollectionObject* self, PyObject* value);

View file

@ -75,7 +75,7 @@ namespace mcrfpydef {
.tp_doc = "UIGridPoint object",
.tp_getset = UIGridPoint::getsetters,
//.tp_init = (initproc)PyUIGridPoint_init, // TODO Define the init function
.tp_new = PyType_GenericNew,
.tp_new = NULL, // Prevent instantiation from Python - Issue #12
};
static PyTypeObject PyUIGridPointStateType = {
@ -87,6 +87,6 @@ namespace mcrfpydef {
.tp_flags = Py_TPFLAGS_DEFAULT,
.tp_doc = "UIGridPointState object", // TODO: Add PyUIGridPointState tp_init
.tp_getset = UIGridPointState::getsetters,
.tp_new = PyType_GenericNew,
.tp_new = NULL, // Prevent instantiation from Python - Issue #12
};
}