Add input validation and fix Entity repr
- #212: Add GRID_MAX (8192) validation to Grid, ColorLayer, TileLayer - #213: Validate color components are in 0-255 range - #214: Add null pointer checks before HeightMap operations - #216: Change entities_in_radius(x, y, radius) to (pos, radius) - #217: Fix Entity __repr__ to show actual draw_pos float values Closes #212, closes #213, closes #214, closes #216, closes #217 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
a1b692bb1f
commit
baa7ee354b
4 changed files with 228 additions and 13 deletions
|
|
@ -118,6 +118,14 @@ static bool ParseColorArg(PyObject* obj, sf::Color& out_color, const char* arg_n
|
|||
|
||||
if (PyErr_Occurred()) return false;
|
||||
|
||||
// #213 - Validate color component range (0-255)
|
||||
if (r < 0 || r > 255 || g < 0 || g > 255 || b < 0 || b > 255 || a < 0 || a > 255) {
|
||||
PyErr_Format(PyExc_ValueError,
|
||||
"%s color components must be in range 0-255, got (%d, %d, %d, %d)",
|
||||
arg_name, r, g, b, a);
|
||||
return false;
|
||||
}
|
||||
|
||||
out_color = sf::Color(r, g, b, a);
|
||||
return true;
|
||||
}
|
||||
|
|
@ -818,6 +826,14 @@ int PyGridLayerAPI::ColorLayer_init(PyColorLayerObject* self, PyObject* args, Py
|
|||
grid_x = PyLong_AsLong(PyTuple_GetItem(grid_size_obj, 0));
|
||||
grid_y = PyLong_AsLong(PyTuple_GetItem(grid_size_obj, 1));
|
||||
if (PyErr_Occurred()) return -1;
|
||||
|
||||
// #212 - Validate against GRID_MAX
|
||||
if (grid_x > GRID_MAX || grid_y > GRID_MAX) {
|
||||
PyErr_Format(PyExc_ValueError,
|
||||
"ColorLayer dimensions cannot exceed %d (got %dx%d)",
|
||||
GRID_MAX, grid_x, grid_y);
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
|
||||
// Create the layer (will be attached to grid via add_layer)
|
||||
|
|
@ -897,6 +913,14 @@ PyObject* PyGridLayerAPI::ColorLayer_set(PyColorLayerObject* self, PyObject* arg
|
|||
Py_DECREF(color_type);
|
||||
return NULL;
|
||||
}
|
||||
// #213 - Validate color component range
|
||||
if (r < 0 || r > 255 || g < 0 || g > 255 || b < 0 || b > 255 || a < 0 || a > 255) {
|
||||
Py_DECREF(color_type);
|
||||
PyErr_Format(PyExc_ValueError,
|
||||
"color components must be in range 0-255, got (%d, %d, %d, %d)",
|
||||
r, g, b, a);
|
||||
return NULL;
|
||||
}
|
||||
color = sf::Color(r, g, b, a);
|
||||
} else {
|
||||
Py_DECREF(color_type);
|
||||
|
|
@ -938,6 +962,14 @@ PyObject* PyGridLayerAPI::ColorLayer_fill(PyColorLayerObject* self, PyObject* ar
|
|||
Py_DECREF(color_type);
|
||||
return NULL;
|
||||
}
|
||||
// #213 - Validate color component range
|
||||
if (r < 0 || r > 255 || g < 0 || g > 255 || b < 0 || b > 255 || a < 0 || a > 255) {
|
||||
Py_DECREF(color_type);
|
||||
PyErr_Format(PyExc_ValueError,
|
||||
"color components must be in range 0-255, got (%d, %d, %d, %d)",
|
||||
r, g, b, a);
|
||||
return NULL;
|
||||
}
|
||||
color = sf::Color(r, g, b, a);
|
||||
} else {
|
||||
Py_DECREF(color_type);
|
||||
|
|
@ -1005,6 +1037,14 @@ PyObject* PyGridLayerAPI::ColorLayer_fill_rect(PyColorLayerObject* self, PyObjec
|
|||
Py_DECREF(color_type);
|
||||
return NULL;
|
||||
}
|
||||
// #213 - Validate color component range
|
||||
if (r < 0 || r > 255 || g < 0 || g > 255 || b < 0 || b > 255 || a < 0 || a > 255) {
|
||||
Py_DECREF(color_type);
|
||||
PyErr_Format(PyExc_ValueError,
|
||||
"color components must be in range 0-255, got (%d, %d, %d, %d)",
|
||||
r, g, b, a);
|
||||
return NULL;
|
||||
}
|
||||
color = sf::Color(r, g, b, a);
|
||||
} else {
|
||||
Py_DECREF(color_type);
|
||||
|
|
@ -1253,6 +1293,12 @@ PyObject* PyGridLayerAPI::ColorLayer_apply_hmap_threshold(PyColorLayerObject* se
|
|||
return NULL;
|
||||
}
|
||||
|
||||
// #214 - Check for null heightmap pointer
|
||||
if (!hmap->heightmap) {
|
||||
PyErr_SetString(PyExc_RuntimeError, "HeightMap is not initialized");
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (!ValidateHeightMapSize(hmap, self->data->grid_x, self->data->grid_y)) {
|
||||
return NULL;
|
||||
}
|
||||
|
|
@ -1313,6 +1359,12 @@ PyObject* PyGridLayerAPI::ColorLayer_apply_gradient(PyColorLayerObject* self, Py
|
|||
return NULL;
|
||||
}
|
||||
|
||||
// #214 - Check for null heightmap pointer
|
||||
if (!hmap->heightmap) {
|
||||
PyErr_SetString(PyExc_RuntimeError, "HeightMap is not initialized");
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (!ValidateHeightMapSize(hmap, self->data->grid_x, self->data->grid_y)) {
|
||||
return NULL;
|
||||
}
|
||||
|
|
@ -1375,6 +1427,12 @@ PyObject* PyGridLayerAPI::ColorLayer_apply_ranges(PyColorLayerObject* self, PyOb
|
|||
return NULL;
|
||||
}
|
||||
|
||||
// #214 - Check for null heightmap pointer
|
||||
if (!hmap->heightmap) {
|
||||
PyErr_SetString(PyExc_RuntimeError, "HeightMap is not initialized");
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (!ValidateHeightMapSize(hmap, self->data->grid_x, self->data->grid_y)) {
|
||||
return NULL;
|
||||
}
|
||||
|
|
@ -1666,6 +1724,14 @@ int PyGridLayerAPI::TileLayer_init(PyTileLayerObject* self, PyObject* args, PyOb
|
|||
grid_x = PyLong_AsLong(PyTuple_GetItem(grid_size_obj, 0));
|
||||
grid_y = PyLong_AsLong(PyTuple_GetItem(grid_size_obj, 1));
|
||||
if (PyErr_Occurred()) return -1;
|
||||
|
||||
// #212 - Validate against GRID_MAX
|
||||
if (grid_x > GRID_MAX || grid_y > GRID_MAX) {
|
||||
PyErr_Format(PyExc_ValueError,
|
||||
"TileLayer dimensions cannot exceed %d (got %dx%d)",
|
||||
GRID_MAX, grid_x, grid_y);
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
|
||||
// Create the layer
|
||||
|
|
@ -1801,6 +1867,12 @@ PyObject* PyGridLayerAPI::TileLayer_apply_threshold(PyTileLayerObject* self, PyO
|
|||
return NULL;
|
||||
}
|
||||
|
||||
// #214 - Check for null heightmap pointer
|
||||
if (!hmap->heightmap) {
|
||||
PyErr_SetString(PyExc_RuntimeError, "HeightMap is not initialized");
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (!ValidateHeightMapSize(hmap, self->data->grid_x, self->data->grid_y)) {
|
||||
return NULL;
|
||||
}
|
||||
|
|
@ -1851,6 +1923,12 @@ PyObject* PyGridLayerAPI::TileLayer_apply_ranges(PyTileLayerObject* self, PyObje
|
|||
return NULL;
|
||||
}
|
||||
|
||||
// #214 - Check for null heightmap pointer
|
||||
if (!hmap->heightmap) {
|
||||
PyErr_SetString(PyExc_RuntimeError, "HeightMap is not initialized");
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (!ValidateHeightMapSize(hmap, self->data->grid_x, self->data->grid_y)) {
|
||||
return NULL;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1042,9 +1042,10 @@ PyObject* UIEntity::repr(PyUIEntityObject* self) {
|
|||
std::ostringstream ss;
|
||||
if (!self->data) ss << "<Entity (invalid internal object)>";
|
||||
else {
|
||||
// #176 - Use grid_x/grid_y naming to reflect tile coordinates
|
||||
ss << "<Entity (grid_x=" << static_cast<int>(self->data->position.x)
|
||||
<< ", grid_y=" << static_cast<int>(self->data->position.y)
|
||||
// #217 - Show actual float position (draw_pos) to avoid confusion
|
||||
// Position is stored in tile coordinates; use draw_pos for float values
|
||||
ss << "<Entity (draw_pos=(" << self->data->position.x
|
||||
<< ", " << self->data->position.y << ")"
|
||||
<< ", sprite_index=" << self->data->sprite.getSpriteIndex() << ")>";
|
||||
}
|
||||
std::string repr_str = ss.str();
|
||||
|
|
|
|||
|
|
@ -780,6 +780,14 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
|
|||
return -1;
|
||||
}
|
||||
|
||||
// #212 - Validate against GRID_MAX
|
||||
if (grid_w > GRID_MAX || grid_h > GRID_MAX) {
|
||||
PyErr_Format(PyExc_ValueError,
|
||||
"Grid dimensions cannot exceed %d (got %dx%d)",
|
||||
GRID_MAX, grid_w, grid_h);
|
||||
return -1;
|
||||
}
|
||||
|
||||
// Handle texture argument
|
||||
std::shared_ptr<PyTexture> texture_ptr = nullptr;
|
||||
if (textureObj && textureObj != Py_None) {
|
||||
|
|
@ -1588,16 +1596,24 @@ PyObject* UIGrid::py_layer(PyUIGridObject* self, PyObject* args) {
|
|||
}
|
||||
|
||||
// #115 - Spatial hash query for entities in radius
|
||||
// #216 - Updated to use position tuple/Vector instead of x, y
|
||||
PyObject* UIGrid::py_entities_in_radius(PyUIGridObject* self, PyObject* args, PyObject* kwds)
|
||||
{
|
||||
static const char* kwlist[] = {"x", "y", "radius", NULL};
|
||||
float x, y, radius;
|
||||
static const char* kwlist[] = {"pos", "radius", NULL};
|
||||
PyObject* pos_obj;
|
||||
float radius;
|
||||
|
||||
if (!PyArg_ParseTupleAndKeywords(args, kwds, "fff", const_cast<char**>(kwlist),
|
||||
&x, &y, &radius)) {
|
||||
if (!PyArg_ParseTupleAndKeywords(args, kwds, "Of", const_cast<char**>(kwlist),
|
||||
&pos_obj, &radius)) {
|
||||
return NULL;
|
||||
}
|
||||
|
||||
// Parse position from tuple, Vector, or other 2-element sequence
|
||||
float x, y;
|
||||
if (!PyPosition_FromObject(pos_obj, &x, &y)) {
|
||||
return NULL; // Error already set by helper
|
||||
}
|
||||
|
||||
if (radius < 0) {
|
||||
PyErr_SetString(PyExc_ValueError, "radius must be non-negative");
|
||||
return NULL;
|
||||
|
|
@ -1985,11 +2001,10 @@ PyMethodDef UIGrid::methods[] = {
|
|||
{"layer", (PyCFunction)UIGrid::py_layer, METH_VARARGS,
|
||||
"layer(z_index: int) -> ColorLayer | TileLayer | None"},
|
||||
{"entities_in_radius", (PyCFunction)UIGrid::py_entities_in_radius, METH_VARARGS | METH_KEYWORDS,
|
||||
"entities_in_radius(x: float, y: float, radius: float) -> list[Entity]\n\n"
|
||||
"entities_in_radius(pos: tuple|Vector, radius: float) -> list[Entity]\n\n"
|
||||
"Query entities within radius using spatial hash (O(k) where k = nearby entities).\n\n"
|
||||
"Args:\n"
|
||||
" x: Center X coordinate\n"
|
||||
" y: Center Y coordinate\n"
|
||||
" pos: Center position as (x, y) tuple, Vector, or other 2-element sequence\n"
|
||||
" radius: Search radius\n\n"
|
||||
"Returns:\n"
|
||||
" List of Entity objects within the radius."},
|
||||
|
|
@ -2106,11 +2121,10 @@ PyMethodDef UIGrid_all_methods[] = {
|
|||
"Returns:\n"
|
||||
" The layer with the specified z_index, or None if not found."},
|
||||
{"entities_in_radius", (PyCFunction)UIGrid::py_entities_in_radius, METH_VARARGS | METH_KEYWORDS,
|
||||
"entities_in_radius(x: float, y: float, radius: float) -> list[Entity]\n\n"
|
||||
"entities_in_radius(pos: tuple|Vector, radius: float) -> list[Entity]\n\n"
|
||||
"Query entities within radius using spatial hash (O(k) where k = nearby entities).\n\n"
|
||||
"Args:\n"
|
||||
" x: Center X coordinate\n"
|
||||
" y: Center Y coordinate\n"
|
||||
" pos: Center position as (x, y) tuple, Vector, or other 2-element sequence\n"
|
||||
" radius: Search radius\n\n"
|
||||
"Returns:\n"
|
||||
" List of Entity objects within the radius."},
|
||||
|
|
|
|||
122
tests/regression/issue_212_to_217_test.py
Normal file
122
tests/regression/issue_212_to_217_test.py
Normal file
|
|
@ -0,0 +1,122 @@
|
|||
"""Regression tests for issues #212, #213, #214, #216, #217"""
|
||||
import mcrfpy
|
||||
import sys
|
||||
|
||||
passed = 0
|
||||
failed = 0
|
||||
|
||||
def test(name, condition, expected=True):
|
||||
global passed, failed
|
||||
if condition == expected:
|
||||
print(f" PASS: {name}")
|
||||
passed += 1
|
||||
else:
|
||||
print(f" FAIL: {name}")
|
||||
failed += 1
|
||||
|
||||
print("Testing #212: GRID_MAX validation")
|
||||
# Grid dimensions cannot exceed 8192
|
||||
try:
|
||||
g = mcrfpy.Grid(grid_size=(10000, 10000))
|
||||
test("Grid rejects oversized dimensions", False) # Should have raised
|
||||
except ValueError as e:
|
||||
test("Grid rejects oversized dimensions", "8192" in str(e))
|
||||
|
||||
# ColorLayer dimensions cannot exceed 8192
|
||||
try:
|
||||
cl = mcrfpy.ColorLayer(z_index=0, grid_size=(10000, 100))
|
||||
test("ColorLayer rejects oversized dimensions", False)
|
||||
except ValueError as e:
|
||||
test("ColorLayer rejects oversized dimensions", "8192" in str(e))
|
||||
|
||||
# TileLayer dimensions cannot exceed 8192
|
||||
try:
|
||||
tl = mcrfpy.TileLayer(z_index=0, grid_size=(100, 10000))
|
||||
test("TileLayer rejects oversized dimensions", False)
|
||||
except ValueError as e:
|
||||
test("TileLayer rejects oversized dimensions", "8192" in str(e))
|
||||
|
||||
# Valid dimensions should work
|
||||
try:
|
||||
g = mcrfpy.Grid(grid_size=(100, 100))
|
||||
test("Grid accepts valid dimensions", True)
|
||||
except:
|
||||
test("Grid accepts valid dimensions", False)
|
||||
|
||||
|
||||
print("\nTesting #213: Color component validation (0-255)")
|
||||
# ColorLayer.fill with invalid color
|
||||
try:
|
||||
cl = mcrfpy.ColorLayer(z_index=0, grid_size=(10, 10))
|
||||
cl.fill((300, 0, 0))
|
||||
test("ColorLayer.fill rejects color > 255", False)
|
||||
except ValueError as e:
|
||||
test("ColorLayer.fill rejects color > 255", "0-255" in str(e))
|
||||
|
||||
try:
|
||||
cl = mcrfpy.ColorLayer(z_index=0, grid_size=(10, 10))
|
||||
cl.fill((-10, 0, 0))
|
||||
test("ColorLayer.fill rejects color < 0", False)
|
||||
except ValueError as e:
|
||||
test("ColorLayer.fill rejects color < 0", "0-255" in str(e))
|
||||
|
||||
# Valid color should work
|
||||
try:
|
||||
cl = mcrfpy.ColorLayer(z_index=0, grid_size=(10, 10))
|
||||
cl.fill((128, 64, 200, 255))
|
||||
test("ColorLayer.fill accepts valid color", True)
|
||||
except:
|
||||
test("ColorLayer.fill accepts valid color", False)
|
||||
|
||||
|
||||
print("\nTesting #216: entities_in_radius uses pos tuple/Vector")
|
||||
try:
|
||||
g = mcrfpy.Grid(grid_size=(50, 50), pos=(0, 0), size=(500, 500))
|
||||
e = mcrfpy.Entity()
|
||||
e.draw_pos = (10, 10)
|
||||
g.entities.append(e)
|
||||
|
||||
# New API: pos as tuple + radius
|
||||
result = g.entities_in_radius((10, 10), 5.0)
|
||||
test("entities_in_radius accepts (pos_tuple, radius)", len(result) == 1)
|
||||
|
||||
# Also works with Vector
|
||||
result = g.entities_in_radius(mcrfpy.Vector(10, 10), 5.0)
|
||||
test("entities_in_radius accepts (Vector, radius)", len(result) == 1)
|
||||
|
||||
# Old API should fail
|
||||
try:
|
||||
result = g.entities_in_radius(10, 10, 5.0)
|
||||
test("entities_in_radius rejects old (x, y, radius) API", False)
|
||||
except TypeError:
|
||||
test("entities_in_radius rejects old (x, y, radius) API", True)
|
||||
except Exception as e:
|
||||
print(f" ERROR in #216 tests: {e}")
|
||||
failed += 3
|
||||
|
||||
|
||||
print("\nTesting #217: Entity __repr__ shows actual position")
|
||||
try:
|
||||
e = mcrfpy.Entity()
|
||||
e.draw_pos = (5.5, 3.25)
|
||||
repr_str = repr(e)
|
||||
test("Entity repr shows draw_pos", "draw_pos=" in repr_str)
|
||||
test("Entity repr shows actual float x", "5.5" in repr_str)
|
||||
test("Entity repr shows actual float y", "3.25" in repr_str)
|
||||
|
||||
# draw_pos should be accessible without grid
|
||||
pos = e.draw_pos
|
||||
test("draw_pos accessible without grid", abs(pos.x - 5.5) < 0.01)
|
||||
except Exception as e:
|
||||
print(f" ERROR in #217 tests: {e}")
|
||||
failed += 4
|
||||
|
||||
|
||||
print(f"\n{'='*50}")
|
||||
print(f"Results: {passed} passed, {failed} failed")
|
||||
|
||||
if failed > 0:
|
||||
sys.exit(1)
|
||||
else:
|
||||
print("All tests passed!")
|
||||
sys.exit(0)
|
||||
Loading…
Add table
Add a link
Reference in a new issue