Phase 1: Safety & performance foundation for Grid/Entity overhaul
- Fix Entity3D self-reference cycle: replace raw `self` pointer with `pyobject` strong-ref pattern matching UIEntity (closes #266) - TileLayer inherits Grid texture when none set, in all three attachment paths: constructor, add_layer(), and .grid property (closes #254) - Add SpatialHash::queryCell() for O(1) entity-at-cell lookup; fix missing spatial_hash.insert() in Entity.__init__ grid= kwarg path; use queryCell in GridPoint.entities (closes #253) - Add FOV dirty flag and parameter cache to skip redundant computeFOV calls when map unchanged and params match (closes #292) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
836a0584df
commit
94f5f5a3fd
13 changed files with 436 additions and 47 deletions
|
|
@ -59,8 +59,8 @@ Entity3D::Entity3D(int grid_x, int grid_z)
|
||||||
|
|
||||||
Entity3D::~Entity3D()
|
Entity3D::~Entity3D()
|
||||||
{
|
{
|
||||||
// Cleanup cube geometry when last entity is destroyed?
|
// Release Python identity reference (handles viewport destruction edge case)
|
||||||
// For now, leave it - it's shared static data
|
releasePyIdentity();
|
||||||
|
|
||||||
// Clean up Python animation callback
|
// Clean up Python animation callback
|
||||||
Py_XDECREF(py_anim_callback_);
|
Py_XDECREF(py_anim_callback_);
|
||||||
|
|
@ -468,7 +468,7 @@ void Entity3D::updateAnimation(float dt)
|
||||||
// Fire Python callback
|
// Fire Python callback
|
||||||
if (py_anim_callback_) {
|
if (py_anim_callback_) {
|
||||||
PyObject* result = PyObject_CallFunction(py_anim_callback_, "(Os)",
|
PyObject* result = PyObject_CallFunction(py_anim_callback_, "(Os)",
|
||||||
self, anim_clip_.c_str());
|
pyobject, anim_clip_.c_str());
|
||||||
if (result) {
|
if (result) {
|
||||||
Py_DECREF(result);
|
Py_DECREF(result);
|
||||||
} else {
|
} else {
|
||||||
|
|
@ -722,7 +722,9 @@ int Entity3D::init(PyEntity3DObject* self, PyObject* args, PyObject* kwds)
|
||||||
|
|
||||||
// Register in object cache
|
// Register in object cache
|
||||||
self->data->serial_number = PythonObjectCache::getInstance().assignSerial();
|
self->data->serial_number = PythonObjectCache::getInstance().assignSerial();
|
||||||
self->data->self = (PyObject*)self;
|
// Set strong ref for Python subclass identity preservation
|
||||||
|
self->data->pyobject = (PyObject*)self;
|
||||||
|
Py_INCREF((PyObject*)self);
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -42,9 +42,18 @@ struct VoxelPointState {
|
||||||
class Entity3D : public std::enable_shared_from_this<Entity3D> {
|
class Entity3D : public std::enable_shared_from_this<Entity3D> {
|
||||||
public:
|
public:
|
||||||
// Python integration
|
// Python integration
|
||||||
PyObject* self = nullptr; // Reference to Python object
|
PyObject* pyobject = nullptr; // Strong ref: preserves Python subclass identity while in viewport
|
||||||
uint64_t serial_number = 0; // For object cache
|
uint64_t serial_number = 0; // For object cache
|
||||||
|
|
||||||
|
/// Release Python identity reference (call at all viewport exit points)
|
||||||
|
void releasePyIdentity() {
|
||||||
|
if (pyobject) {
|
||||||
|
PyObject* tmp = pyobject;
|
||||||
|
pyobject = nullptr;
|
||||||
|
Py_DECREF(tmp);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
Entity3D();
|
Entity3D();
|
||||||
Entity3D(int grid_x, int grid_z);
|
Entity3D(int grid_x, int grid_z);
|
||||||
~Entity3D();
|
~Entity3D();
|
||||||
|
|
@ -383,6 +392,8 @@ inline PyTypeObject PyEntity3DType = {
|
||||||
{
|
{
|
||||||
PyEntity3DObject* obj = (PyEntity3DObject*)self;
|
PyEntity3DObject* obj = (PyEntity3DObject*)self;
|
||||||
PyObject_GC_UnTrack(self);
|
PyObject_GC_UnTrack(self);
|
||||||
|
// Clear the identity ref without DECREF - we ARE this object
|
||||||
|
if (obj->data) obj->data->pyobject = nullptr;
|
||||||
if (obj->weakreflist != NULL) {
|
if (obj->weakreflist != NULL) {
|
||||||
PyObject_ClearWeakRefs(self);
|
PyObject_ClearWeakRefs(self);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -631,10 +631,10 @@ static PyObject* convertEntity3DToPython(std::shared_ptr<mcrf::Entity3D> entity)
|
||||||
Py_RETURN_NONE;
|
Py_RETURN_NONE;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Use the entity's cached Python self pointer if available
|
// Use the entity's cached Python pyobject pointer if available
|
||||||
if (entity->self) {
|
if (entity->pyobject) {
|
||||||
Py_INCREF(entity->self);
|
Py_INCREF(entity->pyobject);
|
||||||
return entity->self;
|
return entity->pyobject;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create a new wrapper
|
// Create a new wrapper
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,7 @@
|
||||||
#include "PyFOV.h"
|
#include "PyFOV.h"
|
||||||
#include "PyPositionHelper.h"
|
#include "PyPositionHelper.h"
|
||||||
#include "PyHeightMap.h"
|
#include "PyHeightMap.h"
|
||||||
|
#include "PythonObjectCache.h"
|
||||||
#include <sstream>
|
#include <sstream>
|
||||||
|
|
||||||
// =============================================================================
|
// =============================================================================
|
||||||
|
|
@ -1632,17 +1633,31 @@ PyObject* PyGridLayerAPI::ColorLayer_get_grid(PyColorLayerObject* self, void* cl
|
||||||
Py_RETURN_NONE;
|
Py_RETURN_NONE;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create Python Grid wrapper for the parent grid
|
// Check cache first — preserves identity (layer.grid is layer.grid)
|
||||||
auto* grid_type = (PyTypeObject*)PyObject_GetAttrString(
|
if (self->grid->serial_number != 0) {
|
||||||
PyImport_ImportModule("mcrfpy"), "Grid");
|
PyObject* cached = PythonObjectCache::getInstance().lookup(self->grid->serial_number);
|
||||||
if (!grid_type) return NULL;
|
if (cached) {
|
||||||
|
return cached;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// No cached wrapper — allocate a new one
|
||||||
|
auto* grid_type = &mcrfpydef::PyUIGridType;
|
||||||
PyUIGridObject* py_grid = (PyUIGridObject*)grid_type->tp_alloc(grid_type, 0);
|
PyUIGridObject* py_grid = (PyUIGridObject*)grid_type->tp_alloc(grid_type, 0);
|
||||||
Py_DECREF(grid_type);
|
|
||||||
if (!py_grid) return NULL;
|
if (!py_grid) return NULL;
|
||||||
|
|
||||||
py_grid->data = self->grid;
|
py_grid->data = self->grid;
|
||||||
py_grid->weakreflist = NULL;
|
py_grid->weakreflist = NULL;
|
||||||
|
|
||||||
|
// Register in cache
|
||||||
|
if (self->grid->serial_number == 0) {
|
||||||
|
self->grid->serial_number = PythonObjectCache::getInstance().assignSerial();
|
||||||
|
}
|
||||||
|
PyObject* weakref = PyWeakref_NewRef((PyObject*)py_grid, NULL);
|
||||||
|
if (weakref) {
|
||||||
|
PythonObjectCache::getInstance().registerObject(self->grid->serial_number, weakref);
|
||||||
|
Py_DECREF(weakref);
|
||||||
|
}
|
||||||
return (PyObject*)py_grid;
|
return (PyObject*)py_grid;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -2267,17 +2282,31 @@ PyObject* PyGridLayerAPI::TileLayer_get_grid(PyTileLayerObject* self, void* clos
|
||||||
Py_RETURN_NONE;
|
Py_RETURN_NONE;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create Python Grid wrapper for the parent grid
|
// Check cache first — preserves identity (layer.grid is layer.grid)
|
||||||
auto* grid_type = (PyTypeObject*)PyObject_GetAttrString(
|
if (self->grid->serial_number != 0) {
|
||||||
PyImport_ImportModule("mcrfpy"), "Grid");
|
PyObject* cached = PythonObjectCache::getInstance().lookup(self->grid->serial_number);
|
||||||
if (!grid_type) return NULL;
|
if (cached) {
|
||||||
|
return cached;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// No cached wrapper — allocate a new one
|
||||||
|
auto* grid_type = &mcrfpydef::PyUIGridType;
|
||||||
PyUIGridObject* py_grid = (PyUIGridObject*)grid_type->tp_alloc(grid_type, 0);
|
PyUIGridObject* py_grid = (PyUIGridObject*)grid_type->tp_alloc(grid_type, 0);
|
||||||
Py_DECREF(grid_type);
|
|
||||||
if (!py_grid) return NULL;
|
if (!py_grid) return NULL;
|
||||||
|
|
||||||
py_grid->data = self->grid;
|
py_grid->data = self->grid;
|
||||||
py_grid->weakreflist = NULL;
|
py_grid->weakreflist = NULL;
|
||||||
|
|
||||||
|
// Register in cache
|
||||||
|
if (self->grid->serial_number == 0) {
|
||||||
|
self->grid->serial_number = PythonObjectCache::getInstance().assignSerial();
|
||||||
|
}
|
||||||
|
PyObject* weakref = PyWeakref_NewRef((PyObject*)py_grid, NULL);
|
||||||
|
if (weakref) {
|
||||||
|
PythonObjectCache::getInstance().registerObject(self->grid->serial_number, weakref);
|
||||||
|
Py_DECREF(weakref);
|
||||||
|
}
|
||||||
return (PyObject*)py_grid;
|
return (PyObject*)py_grid;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -2360,6 +2389,11 @@ int PyGridLayerAPI::TileLayer_set_grid(PyTileLayerObject* self, PyObject* value,
|
||||||
py_grid->data->layers_need_sort = true;
|
py_grid->data->layers_need_sort = true;
|
||||||
self->grid = py_grid->data;
|
self->grid = py_grid->data;
|
||||||
|
|
||||||
|
// Inherit grid texture if TileLayer has none (#254)
|
||||||
|
if (!self->data->texture) {
|
||||||
|
self->data->texture = py_grid->data->getTexture();
|
||||||
|
}
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -72,6 +72,28 @@ void SpatialHash::update(std::shared_ptr<UIEntity> entity, float old_x, float ol
|
||||||
buckets[new_bucket].push_back(entity);
|
buckets[new_bucket].push_back(entity);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
std::vector<std::shared_ptr<UIEntity>> SpatialHash::queryCell(int x, int y) const
|
||||||
|
{
|
||||||
|
std::vector<std::shared_ptr<UIEntity>> result;
|
||||||
|
|
||||||
|
auto bucket_coord = getBucket(static_cast<float>(x), static_cast<float>(y));
|
||||||
|
auto it = buckets.find(bucket_coord);
|
||||||
|
if (it == buckets.end()) return result;
|
||||||
|
|
||||||
|
for (const auto& wp : it->second) {
|
||||||
|
auto entity = wp.lock();
|
||||||
|
if (!entity) continue;
|
||||||
|
|
||||||
|
// Exact integer position match
|
||||||
|
if (static_cast<int>(entity->position.x) == x &&
|
||||||
|
static_cast<int>(entity->position.y) == y) {
|
||||||
|
result.push_back(entity);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
std::vector<std::pair<int, int>> SpatialHash::getBucketsInRadius(float x, float y, float radius) const
|
std::vector<std::pair<int, int>> SpatialHash::getBucketsInRadius(float x, float y, float radius) const
|
||||||
{
|
{
|
||||||
std::vector<std::pair<int, int>> result;
|
std::vector<std::pair<int, int>> result;
|
||||||
|
|
|
||||||
|
|
@ -33,6 +33,10 @@ public:
|
||||||
// This removes from old bucket and inserts into new bucket if needed
|
// This removes from old bucket and inserts into new bucket if needed
|
||||||
void update(std::shared_ptr<UIEntity> entity, float old_x, float old_y);
|
void update(std::shared_ptr<UIEntity> entity, float old_x, float old_y);
|
||||||
|
|
||||||
|
// Query all entities at a specific cell (exact integer position match)
|
||||||
|
// O(n) where n = entities in the bucket containing this cell
|
||||||
|
std::vector<std::shared_ptr<UIEntity>> queryCell(int x, int y) const;
|
||||||
|
|
||||||
// Query all entities within radius of a point
|
// Query all entities within radius of a point
|
||||||
// Returns entities whose positions are within the circular radius
|
// Returns entities whose positions are within the circular radius
|
||||||
std::vector<std::shared_ptr<UIEntity>> queryRadius(float x, float y, float radius) const;
|
std::vector<std::shared_ptr<UIEntity>> queryRadius(float x, float y, float radius) const;
|
||||||
|
|
|
||||||
|
|
@ -281,7 +281,9 @@ int UIEntity::init(PyUIEntityObject* self, PyObject* args, PyObject* kwds) {
|
||||||
self->data->grid = pygrid->data;
|
self->data->grid = pygrid->data;
|
||||||
// Append entity to grid's entity list
|
// Append entity to grid's entity list
|
||||||
pygrid->data->entities->push_back(self->data);
|
pygrid->data->entities->push_back(self->data);
|
||||||
|
// Insert into spatial hash for O(1) cell queries (#253)
|
||||||
|
pygrid->data->spatial_hash.insert(self->data);
|
||||||
|
|
||||||
// Don't initialize gridstate here - lazy initialization to support large numbers of entities
|
// Don't initialize gridstate here - lazy initialization to support large numbers of entities
|
||||||
// gridstate will be initialized when visibility is updated or accessed
|
// gridstate will be initialized when visibility is updated or accessed
|
||||||
}
|
}
|
||||||
|
|
@ -634,14 +636,33 @@ PyObject* UIEntity::get_grid(PyUIEntityObject* self, void* closure)
|
||||||
Py_RETURN_NONE;
|
Py_RETURN_NONE;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Return a Python Grid object wrapping the C++ grid
|
auto& grid = self->data->grid;
|
||||||
auto grid_type = &mcrfpydef::PyUIGridType;
|
|
||||||
|
|
||||||
|
// Check cache first — preserves identity (entity.grid is entity.grid)
|
||||||
|
if (grid->serial_number != 0) {
|
||||||
|
PyObject* cached = PythonObjectCache::getInstance().lookup(grid->serial_number);
|
||||||
|
if (cached) {
|
||||||
|
return cached; // Already INCREF'd by lookup
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// No cached wrapper — allocate a new one
|
||||||
|
auto grid_type = &mcrfpydef::PyUIGridType;
|
||||||
auto pyGrid = (PyUIGridObject*)grid_type->tp_alloc(grid_type, 0);
|
auto pyGrid = (PyUIGridObject*)grid_type->tp_alloc(grid_type, 0);
|
||||||
|
|
||||||
if (pyGrid) {
|
if (pyGrid) {
|
||||||
pyGrid->data = self->data->grid;
|
pyGrid->data = grid;
|
||||||
pyGrid->weakreflist = NULL;
|
pyGrid->weakreflist = NULL;
|
||||||
|
|
||||||
|
// Register in cache so future accesses return the same wrapper
|
||||||
|
if (grid->serial_number == 0) {
|
||||||
|
grid->serial_number = PythonObjectCache::getInstance().assignSerial();
|
||||||
|
}
|
||||||
|
PyObject* weakref = PyWeakref_NewRef((PyObject*)pyGrid, NULL);
|
||||||
|
if (weakref) {
|
||||||
|
PythonObjectCache::getInstance().registerObject(grid->serial_number, weakref);
|
||||||
|
Py_DECREF(weakref);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return (PyObject*)pyGrid;
|
return (PyObject*)pyGrid;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -568,13 +568,14 @@ void UIGrid::sortLayers() {
|
||||||
void UIGrid::syncTCODMap()
|
void UIGrid::syncTCODMap()
|
||||||
{
|
{
|
||||||
if (!tcod_map) return;
|
if (!tcod_map) return;
|
||||||
|
|
||||||
for (int y = 0; y < grid_h; y++) {
|
for (int y = 0; y < grid_h; y++) {
|
||||||
for (int x = 0; x < grid_w; x++) {
|
for (int x = 0; x < grid_w; x++) {
|
||||||
const UIGridPoint& point = at(x, y);
|
const UIGridPoint& point = at(x, y);
|
||||||
tcod_map->setProperties(x, y, point.transparent, point.walkable);
|
tcod_map->setProperties(x, y, point.transparent, point.walkable);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
fov_dirty = true; // #292: map changed, FOV needs recomputation
|
||||||
}
|
}
|
||||||
|
|
||||||
void UIGrid::syncTCODMapCell(int x, int y)
|
void UIGrid::syncTCODMapCell(int x, int y)
|
||||||
|
|
@ -583,14 +584,32 @@ void UIGrid::syncTCODMapCell(int x, int y)
|
||||||
|
|
||||||
const UIGridPoint& point = at(x, y);
|
const UIGridPoint& point = at(x, y);
|
||||||
tcod_map->setProperties(x, y, point.transparent, point.walkable);
|
tcod_map->setProperties(x, y, point.transparent, point.walkable);
|
||||||
|
fov_dirty = true; // #292: cell changed, FOV needs recomputation
|
||||||
}
|
}
|
||||||
|
|
||||||
void UIGrid::computeFOV(int x, int y, int radius, bool light_walls, TCOD_fov_algorithm_t algo)
|
void UIGrid::computeFOV(int x, int y, int radius, bool light_walls, TCOD_fov_algorithm_t algo)
|
||||||
{
|
{
|
||||||
if (!tcod_map || x < 0 || x >= grid_w || y < 0 || y >= grid_h) return;
|
if (!tcod_map || x < 0 || x >= grid_w || y < 0 || y >= grid_h) return;
|
||||||
|
|
||||||
|
// #292: Skip redundant FOV computation if map hasn't changed and params match
|
||||||
|
if (!fov_dirty &&
|
||||||
|
x == fov_last_x && y == fov_last_y &&
|
||||||
|
radius == fov_last_radius &&
|
||||||
|
light_walls == fov_last_light_walls &&
|
||||||
|
algo == fov_last_algo) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
std::lock_guard<std::mutex> lock(fov_mutex);
|
std::lock_guard<std::mutex> lock(fov_mutex);
|
||||||
tcod_map->computeFov(x, y, radius, light_walls, algo);
|
tcod_map->computeFov(x, y, radius, light_walls, algo);
|
||||||
|
|
||||||
|
// Cache parameters for deduplication
|
||||||
|
fov_dirty = false;
|
||||||
|
fov_last_x = x;
|
||||||
|
fov_last_y = y;
|
||||||
|
fov_last_radius = radius;
|
||||||
|
fov_last_light_walls = light_walls;
|
||||||
|
fov_last_algo = algo;
|
||||||
}
|
}
|
||||||
|
|
||||||
bool UIGrid::isInFOV(int x, int y) const
|
bool UIGrid::isInFOV(int x, int y) const
|
||||||
|
|
@ -1101,6 +1120,12 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
|
||||||
self->data->layers.push_back(layer);
|
self->data->layers.push_back(layer);
|
||||||
py_layer->grid = self->data;
|
py_layer->grid = self->data;
|
||||||
|
|
||||||
|
// Inherit grid texture if TileLayer has none (#254)
|
||||||
|
auto tile_layer_ptr = std::static_pointer_cast<TileLayer>(layer);
|
||||||
|
if (!tile_layer_ptr->texture) {
|
||||||
|
tile_layer_ptr->texture = self->data->getTexture();
|
||||||
|
}
|
||||||
|
|
||||||
} else {
|
} else {
|
||||||
Py_DECREF(item);
|
Py_DECREF(item);
|
||||||
Py_DECREF(iterator);
|
Py_DECREF(iterator);
|
||||||
|
|
@ -1720,6 +1745,12 @@ PyObject* UIGrid::py_add_layer(PyUIGridObject* self, PyObject* args) {
|
||||||
self->data->layers_need_sort = true;
|
self->data->layers_need_sort = true;
|
||||||
py_layer->grid = self->data;
|
py_layer->grid = self->data;
|
||||||
|
|
||||||
|
// Inherit grid texture if TileLayer has none (#254)
|
||||||
|
auto tile_layer = std::static_pointer_cast<TileLayer>(layer);
|
||||||
|
if (!tile_layer->texture) {
|
||||||
|
tile_layer->texture = self->data->getTexture();
|
||||||
|
}
|
||||||
|
|
||||||
} else {
|
} else {
|
||||||
Py_DECREF(color_layer_type);
|
Py_DECREF(color_layer_type);
|
||||||
Py_DECREF(tile_layer_type);
|
Py_DECREF(tile_layer_type);
|
||||||
|
|
|
||||||
|
|
@ -135,6 +135,13 @@ public:
|
||||||
TCOD_fov_algorithm_t fov_algorithm; // Default FOV algorithm (from mcrfpy.default_fov)
|
TCOD_fov_algorithm_t fov_algorithm; // Default FOV algorithm (from mcrfpy.default_fov)
|
||||||
int fov_radius; // Default FOV radius
|
int fov_radius; // Default FOV radius
|
||||||
|
|
||||||
|
// #292 - FOV deduplication: skip redundant computations
|
||||||
|
bool fov_dirty = true; // Set true when TCOD map changes
|
||||||
|
int fov_last_x = -1, fov_last_y = -1; // Last FOV computation parameters
|
||||||
|
int fov_last_radius = -1;
|
||||||
|
bool fov_last_light_walls = true;
|
||||||
|
TCOD_fov_algorithm_t fov_last_algo = FOV_BASIC;
|
||||||
|
|
||||||
// #142, #230 - Grid cell mouse events
|
// #142, #230 - Grid cell mouse events
|
||||||
// Cell hover callbacks take only (cell_pos); cell click still takes (cell_pos, button, action)
|
// Cell hover callbacks take only (cell_pos); cell click still takes (cell_pos, button, action)
|
||||||
std::unique_ptr<PyCellHoverCallable> on_cell_enter_callable;
|
std::unique_ptr<PyCellHoverCallable> on_cell_enter_callable;
|
||||||
|
|
|
||||||
|
|
@ -41,11 +41,12 @@ sf::Color PyObject_to_sfColor(PyObject* obj) {
|
||||||
// #150 - Removed get_color/set_color - now handled by layers
|
// #150 - Removed get_color/set_color - now handled by layers
|
||||||
|
|
||||||
// Helper to safely get the GridPoint data from coordinates
|
// Helper to safely get the GridPoint data from coordinates
|
||||||
|
// Routes through UIGrid::at() which handles both flat and chunked storage
|
||||||
static UIGridPoint* getGridPointData(PyUIGridPointObject* self) {
|
static UIGridPoint* getGridPointData(PyUIGridPointObject* self) {
|
||||||
if (!self->grid) return nullptr;
|
if (!self->grid) return nullptr;
|
||||||
int idx = self->y * self->grid->grid_w + self->x;
|
if (self->x < 0 || self->x >= self->grid->grid_w ||
|
||||||
if (idx < 0 || idx >= static_cast<int>(self->grid->points.size())) return nullptr;
|
self->y < 0 || self->y >= self->grid->grid_h) return nullptr;
|
||||||
return &self->grid->points[idx];
|
return &self->grid->at(self->x, self->y);
|
||||||
}
|
}
|
||||||
|
|
||||||
PyObject* UIGridPoint::get_bool_member(PyUIGridPointObject* self, void* closure) {
|
PyObject* UIGridPoint::get_bool_member(PyUIGridPointObject* self, void* closure) {
|
||||||
|
|
@ -94,7 +95,7 @@ int UIGridPoint::set_bool_member(PyUIGridPointObject* self, PyObject* value, voi
|
||||||
|
|
||||||
// #150 - Removed get_int_member/set_int_member - now handled by layers
|
// #150 - Removed get_int_member/set_int_member - now handled by layers
|
||||||
|
|
||||||
// #114 - Get list of entities at this grid cell
|
// #114, #253 - Get list of entities at this grid cell (uses spatial hash for O(1) lookup)
|
||||||
PyObject* UIGridPoint::get_entities(PyUIGridPointObject* self, void* closure) {
|
PyObject* UIGridPoint::get_entities(PyUIGridPointObject* self, void* closure) {
|
||||||
if (!self->grid) {
|
if (!self->grid) {
|
||||||
PyErr_SetString(PyExc_RuntimeError, "GridPoint has no parent grid");
|
PyErr_SetString(PyExc_RuntimeError, "GridPoint has no parent grid");
|
||||||
|
|
@ -107,25 +108,23 @@ PyObject* UIGridPoint::get_entities(PyUIGridPointObject* self, void* closure) {
|
||||||
PyObject* list = PyList_New(0);
|
PyObject* list = PyList_New(0);
|
||||||
if (!list) return NULL;
|
if (!list) return NULL;
|
||||||
|
|
||||||
// Iterate through grid's entities and find those at this position
|
// Use spatial hash for O(bucket_size) lookup instead of O(n) iteration
|
||||||
for (auto& entity : *(self->grid->entities)) {
|
auto entities = self->grid->spatial_hash.queryCell(target_x, target_y);
|
||||||
if (static_cast<int>(entity->position.x) == target_x &&
|
for (auto& entity : entities) {
|
||||||
static_cast<int>(entity->position.y) == target_y) {
|
// Create Python Entity object for this entity
|
||||||
// Create Python Entity object for this entity
|
auto type = &mcrfpydef::PyUIEntityType;
|
||||||
auto type = &mcrfpydef::PyUIEntityType;
|
auto obj = (PyUIEntityObject*)type->tp_alloc(type, 0);
|
||||||
auto obj = (PyUIEntityObject*)type->tp_alloc(type, 0);
|
if (!obj) {
|
||||||
if (!obj) {
|
Py_DECREF(list);
|
||||||
Py_DECREF(list);
|
return NULL;
|
||||||
return NULL;
|
|
||||||
}
|
|
||||||
obj->data = entity;
|
|
||||||
if (PyList_Append(list, (PyObject*)obj) < 0) {
|
|
||||||
Py_DECREF(obj);
|
|
||||||
Py_DECREF(list);
|
|
||||||
return NULL;
|
|
||||||
}
|
|
||||||
Py_DECREF(obj); // List now owns the reference
|
|
||||||
}
|
}
|
||||||
|
obj->data = entity;
|
||||||
|
if (PyList_Append(list, (PyObject*)obj) < 0) {
|
||||||
|
Py_DECREF(obj);
|
||||||
|
Py_DECREF(list);
|
||||||
|
return NULL;
|
||||||
|
}
|
||||||
|
Py_DECREF(obj); // List now owns the reference
|
||||||
}
|
}
|
||||||
|
|
||||||
return list;
|
return list;
|
||||||
|
|
|
||||||
73
tests/regression/issue_253_spatial_hash_test.py
Normal file
73
tests/regression/issue_253_spatial_hash_test.py
Normal file
|
|
@ -0,0 +1,73 @@
|
||||||
|
"""Regression test for #253: GridPoint.entities uses spatial hash for O(1) lookup."""
|
||||||
|
import mcrfpy
|
||||||
|
import sys
|
||||||
|
|
||||||
|
def test_gridpoint_entities_basic():
|
||||||
|
"""Entities at known positions are returned correctly."""
|
||||||
|
scene = mcrfpy.Scene("test253")
|
||||||
|
mcrfpy.current_scene = scene
|
||||||
|
|
||||||
|
tex = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||||
|
grid = mcrfpy.Grid(grid_size=(20, 20), texture=tex, pos=(0, 0), size=(320, 320))
|
||||||
|
scene.children.append(grid)
|
||||||
|
|
||||||
|
# Place entities at specific cells
|
||||||
|
e1 = mcrfpy.Entity((5, 5), grid=grid)
|
||||||
|
e2 = mcrfpy.Entity((5, 5), grid=grid)
|
||||||
|
e3 = mcrfpy.Entity((10, 10), grid=grid)
|
||||||
|
|
||||||
|
# Query cell (5, 5) - should have 2 entities
|
||||||
|
cell_5_5 = grid.at(5, 5)
|
||||||
|
ents = cell_5_5.entities
|
||||||
|
assert len(ents) == 2, f"Expected 2 entities at (5,5), got {len(ents)}"
|
||||||
|
print("PASS: 2 entities at (5,5)")
|
||||||
|
|
||||||
|
# Query cell (10, 10) - should have 1 entity
|
||||||
|
cell_10_10 = grid.at(10, 10)
|
||||||
|
ents = cell_10_10.entities
|
||||||
|
assert len(ents) == 1, f"Expected 1 entity at (10,10), got {len(ents)}"
|
||||||
|
print("PASS: 1 entity at (10,10)")
|
||||||
|
|
||||||
|
def test_gridpoint_entities_empty():
|
||||||
|
"""Empty cells return empty list."""
|
||||||
|
scene = mcrfpy.Scene("test253b")
|
||||||
|
mcrfpy.current_scene = scene
|
||||||
|
|
||||||
|
tex = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||||
|
grid = mcrfpy.Grid(grid_size=(20, 20), texture=tex, pos=(0, 0), size=(320, 320))
|
||||||
|
scene.children.append(grid)
|
||||||
|
|
||||||
|
# No entities placed - empty cell should return empty list
|
||||||
|
cell = grid.at(0, 0)
|
||||||
|
ents = cell.entities
|
||||||
|
assert len(ents) == 0, f"Expected 0 entities, got {len(ents)}"
|
||||||
|
print("PASS: empty cell returns empty list")
|
||||||
|
|
||||||
|
def test_gridpoint_entities_after_move():
|
||||||
|
"""Moving an entity updates spatial hash so GridPoint.entities reflects new position."""
|
||||||
|
scene = mcrfpy.Scene("test253c")
|
||||||
|
mcrfpy.current_scene = scene
|
||||||
|
|
||||||
|
tex = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||||
|
grid = mcrfpy.Grid(grid_size=(20, 20), texture=tex, pos=(0, 0), size=(320, 320))
|
||||||
|
scene.children.append(grid)
|
||||||
|
|
||||||
|
e = mcrfpy.Entity((3, 3), grid=grid)
|
||||||
|
|
||||||
|
# Verify entity is at (3, 3)
|
||||||
|
assert len(grid.at(3, 3).entities) == 1, "Entity should be at (3,3)"
|
||||||
|
|
||||||
|
# Move entity to (7, 7)
|
||||||
|
e.grid_pos = (7, 7)
|
||||||
|
|
||||||
|
# Old cell should be empty, new cell should have the entity
|
||||||
|
assert len(grid.at(3, 3).entities) == 0, "Old cell should be empty after move"
|
||||||
|
assert len(grid.at(7, 7).entities) == 1, "New cell should have entity after move"
|
||||||
|
print("PASS: entity move updates spatial hash correctly")
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
test_gridpoint_entities_basic()
|
||||||
|
test_gridpoint_entities_empty()
|
||||||
|
test_gridpoint_entities_after_move()
|
||||||
|
print("All #253 tests passed")
|
||||||
|
sys.exit(0)
|
||||||
67
tests/regression/issue_254_tilelayer_texture_test.py
Normal file
67
tests/regression/issue_254_tilelayer_texture_test.py
Normal file
|
|
@ -0,0 +1,67 @@
|
||||||
|
"""Regression test for #254: TileLayer inherits Grid texture when none set."""
|
||||||
|
import mcrfpy
|
||||||
|
import sys
|
||||||
|
|
||||||
|
def test_tilelayer_texture_inheritance():
|
||||||
|
"""TileLayer without texture should inherit grid's texture on attachment."""
|
||||||
|
scene = mcrfpy.Scene("test254")
|
||||||
|
mcrfpy.current_scene = scene
|
||||||
|
|
||||||
|
# Create grid with texture
|
||||||
|
tex = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||||
|
grid = mcrfpy.Grid(grid_size=(10, 10), texture=tex, pos=(0, 0), size=(160, 160))
|
||||||
|
scene.children.append(grid)
|
||||||
|
|
||||||
|
# Create TileLayer WITHOUT texture
|
||||||
|
layer_no_tex = mcrfpy.TileLayer(name="terrain", z_index=0)
|
||||||
|
grid.add_layer(layer_no_tex)
|
||||||
|
|
||||||
|
# Verify it inherited the grid's texture
|
||||||
|
assert layer_no_tex.texture is not None, "TileLayer should inherit grid texture"
|
||||||
|
print("PASS: TileLayer without texture inherits grid texture")
|
||||||
|
|
||||||
|
# Create TileLayer WITH explicit texture
|
||||||
|
tex2 = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||||
|
layer_with_tex = mcrfpy.TileLayer(name="overlay", z_index=1, texture=tex2)
|
||||||
|
grid.add_layer(layer_with_tex)
|
||||||
|
|
||||||
|
# Verify it kept its own texture
|
||||||
|
assert layer_with_tex.texture is not None, "TileLayer with texture should keep it"
|
||||||
|
print("PASS: TileLayer with explicit texture keeps its own")
|
||||||
|
|
||||||
|
def test_tilelayer_texture_via_constructor():
|
||||||
|
"""TileLayer passed in Grid constructor should also inherit texture."""
|
||||||
|
scene = mcrfpy.Scene("test254b")
|
||||||
|
mcrfpy.current_scene = scene
|
||||||
|
|
||||||
|
tex = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||||
|
layer = mcrfpy.TileLayer(name="base", z_index=0)
|
||||||
|
|
||||||
|
grid = mcrfpy.Grid(grid_size=(10, 10), texture=tex, pos=(0, 0), size=(160, 160),
|
||||||
|
layers=[layer])
|
||||||
|
scene.children.append(grid)
|
||||||
|
|
||||||
|
assert layer.texture is not None, "TileLayer in constructor should inherit grid texture"
|
||||||
|
print("PASS: TileLayer in constructor inherits grid texture")
|
||||||
|
|
||||||
|
def test_tilelayer_texture_via_grid_property():
|
||||||
|
"""TileLayer attached via layer.grid = grid should inherit texture."""
|
||||||
|
scene = mcrfpy.Scene("test254c")
|
||||||
|
mcrfpy.current_scene = scene
|
||||||
|
|
||||||
|
tex = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||||
|
grid = mcrfpy.Grid(grid_size=(10, 10), texture=tex, pos=(0, 0), size=(160, 160))
|
||||||
|
scene.children.append(grid)
|
||||||
|
|
||||||
|
layer = mcrfpy.TileLayer(name="via_prop", z_index=0)
|
||||||
|
layer.grid = grid
|
||||||
|
|
||||||
|
assert layer.texture is not None, "TileLayer attached via .grid should inherit texture"
|
||||||
|
print("PASS: TileLayer via .grid property inherits grid texture")
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
test_tilelayer_texture_inheritance()
|
||||||
|
test_tilelayer_texture_via_constructor()
|
||||||
|
test_tilelayer_texture_via_grid_property()
|
||||||
|
print("All #254 tests passed")
|
||||||
|
sys.exit(0)
|
||||||
118
tests/regression/issue_292_fov_dedup_test.py
Normal file
118
tests/regression/issue_292_fov_dedup_test.py
Normal file
|
|
@ -0,0 +1,118 @@
|
||||||
|
"""Regression test for #292: Deduplicate FOV computation via dirty flag."""
|
||||||
|
import mcrfpy
|
||||||
|
import sys
|
||||||
|
|
||||||
|
def test_fov_basic_correctness():
|
||||||
|
"""FOV computation still works correctly with dirty flag."""
|
||||||
|
scene = mcrfpy.Scene("test292")
|
||||||
|
mcrfpy.current_scene = scene
|
||||||
|
|
||||||
|
tex = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||||
|
grid = mcrfpy.Grid(grid_size=(20, 20), texture=tex, pos=(0, 0), size=(320, 320))
|
||||||
|
scene.children.append(grid)
|
||||||
|
|
||||||
|
# Make all cells transparent and walkable
|
||||||
|
for y in range(20):
|
||||||
|
for x in range(20):
|
||||||
|
pt = grid.at(x, y)
|
||||||
|
pt.walkable = True
|
||||||
|
pt.transparent = True
|
||||||
|
|
||||||
|
# Compute FOV from center
|
||||||
|
grid.compute_fov((10, 10), radius=5)
|
||||||
|
|
||||||
|
# Center should be visible
|
||||||
|
assert grid.is_in_fov((10, 10)), "Center should be in FOV"
|
||||||
|
# Nearby cell should be visible
|
||||||
|
assert grid.is_in_fov((10, 11)), "Adjacent cell should be in FOV"
|
||||||
|
# Far cell should NOT be visible
|
||||||
|
assert not grid.is_in_fov((0, 0)), "Far cell should not be in FOV"
|
||||||
|
print("PASS: FOV basic correctness")
|
||||||
|
|
||||||
|
def test_fov_duplicate_call():
|
||||||
|
"""Calling computeFOV twice with same params should still give correct results."""
|
||||||
|
scene = mcrfpy.Scene("test292b")
|
||||||
|
mcrfpy.current_scene = scene
|
||||||
|
|
||||||
|
tex = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||||
|
grid = mcrfpy.Grid(grid_size=(20, 20), texture=tex, pos=(0, 0), size=(320, 320))
|
||||||
|
scene.children.append(grid)
|
||||||
|
|
||||||
|
for y in range(20):
|
||||||
|
for x in range(20):
|
||||||
|
pt = grid.at(x, y)
|
||||||
|
pt.walkable = True
|
||||||
|
pt.transparent = True
|
||||||
|
|
||||||
|
# Compute FOV twice with same params (second should be skipped internally)
|
||||||
|
grid.compute_fov((10, 10), radius=5)
|
||||||
|
result1 = grid.is_in_fov((10, 11))
|
||||||
|
|
||||||
|
grid.compute_fov((10, 10), radius=5)
|
||||||
|
result2 = grid.is_in_fov((10, 11))
|
||||||
|
|
||||||
|
assert result1 == result2, "Duplicate FOV call should give same result"
|
||||||
|
print("PASS: Duplicate FOV call gives same result")
|
||||||
|
|
||||||
|
def test_fov_updates_after_map_change():
|
||||||
|
"""FOV should recompute after a cell's walkable/transparent changes."""
|
||||||
|
scene = mcrfpy.Scene("test292c")
|
||||||
|
mcrfpy.current_scene = scene
|
||||||
|
|
||||||
|
tex = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||||
|
grid = mcrfpy.Grid(grid_size=(20, 20), texture=tex, pos=(0, 0), size=(320, 320))
|
||||||
|
scene.children.append(grid)
|
||||||
|
|
||||||
|
for y in range(20):
|
||||||
|
for x in range(20):
|
||||||
|
pt = grid.at(x, y)
|
||||||
|
pt.walkable = True
|
||||||
|
pt.transparent = True
|
||||||
|
|
||||||
|
# Compute FOV - cell (10, 12) should be visible
|
||||||
|
grid.compute_fov((10, 10), radius=5)
|
||||||
|
assert grid.is_in_fov((10, 12)), "Cell should be visible initially"
|
||||||
|
|
||||||
|
# Block line of sight by making (10, 11) opaque
|
||||||
|
grid.at(10, 11).transparent = False
|
||||||
|
|
||||||
|
# Recompute FOV with same params - dirty flag should force recomputation
|
||||||
|
grid.compute_fov((10, 10), radius=5)
|
||||||
|
assert not grid.is_in_fov((10, 12)), "Cell behind wall should not be visible after map change"
|
||||||
|
print("PASS: FOV updates correctly after map change")
|
||||||
|
|
||||||
|
def test_fov_different_params_recompute():
|
||||||
|
"""FOV should recompute when params change even if map hasn't."""
|
||||||
|
scene = mcrfpy.Scene("test292d")
|
||||||
|
mcrfpy.current_scene = scene
|
||||||
|
|
||||||
|
tex = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||||
|
grid = mcrfpy.Grid(grid_size=(20, 20), texture=tex, pos=(0, 0), size=(320, 320))
|
||||||
|
scene.children.append(grid)
|
||||||
|
|
||||||
|
for y in range(20):
|
||||||
|
for x in range(20):
|
||||||
|
pt = grid.at(x, y)
|
||||||
|
pt.walkable = True
|
||||||
|
pt.transparent = True
|
||||||
|
|
||||||
|
# Compute from (10, 10) with radius 3
|
||||||
|
grid.compute_fov((10, 10), radius=3)
|
||||||
|
visible_3 = grid.is_in_fov((10, 14))
|
||||||
|
|
||||||
|
# Compute from (10, 10) with radius 5 - different params should recompute
|
||||||
|
grid.compute_fov((10, 10), radius=5)
|
||||||
|
visible_5 = grid.is_in_fov((10, 14))
|
||||||
|
|
||||||
|
# Radius 3 shouldn't see (10, 14), but radius 5 should
|
||||||
|
assert not visible_3, "(10,14) should not be visible with radius 3"
|
||||||
|
assert visible_5, "(10,14) should be visible with radius 5"
|
||||||
|
print("PASS: Different FOV params force recomputation")
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
test_fov_basic_correctness()
|
||||||
|
test_fov_duplicate_call()
|
||||||
|
test_fov_updates_after_map_change()
|
||||||
|
test_fov_different_params_recompute()
|
||||||
|
print("All #292 tests passed")
|
||||||
|
sys.exit(0)
|
||||||
Loading…
Add table
Add a link
Reference in a new issue