[Bugfix] UIEntity.set_grid() missing spatial hash cleanup on grid transfer #274

New issue

Closed

opened 2026-03-07 23:25:06 +00:00 by john · 0 comments

john commented

2026-03-07 23:25:06 +00:00

Owner

Summary

UIEntity::set_grid() (the entity.grid = new_grid property setter) removes the entity from the old grid's entity list but does not remove it from the old grid's spatial hash, and does not insert it into the new grid's spatial hash. This leaves a stale spatial hash entry that can cause incorrect collision/pathfinding results or use-after-free.

Root Cause

UIEntity.cpp:691-717 (set_grid):

// Remove from old grid
if (self->data->grid && self->data->grid != new_grid) {
    auto& old_entities = self->data->grid->entities;
    auto it = std::find_if(old_entities->begin(), old_entities->end(),
        [self](const std::shared_ptr<UIEntity>& e) {
            return e.get() == self->data.get();
        });
    if (it != old_entities->end()) {
        old_entities->erase(it);
        // BUG: No spatial_hash.remove() here!
    }
}

// Add to new grid
if (self->data->grid != new_grid) {
    new_grid->entities->push_back(self->data);
    self->data->grid = new_grid;
    // BUG: No spatial_hash.insert() here!

Working comparison — UIEntityCollection::append() (lines 558-579) does this correctly:

old_grid->spatial_hash.remove(entity->data);   // ✓
// ...
self->grid->spatial_hash.insert(entity->data);  // ✓

Reproduction

import mcrfpy

grid_a = mcrfpy.Grid(grid_size=(10, 10))
grid_b = mcrfpy.Grid(grid_size=(10, 10))

entity = mcrfpy.Entity((5, 5), grid=grid_a)
entity.grid = grid_b  # Stale entry left in grid_a.spatial_hash

# grid_a's spatial hash still thinks entity is at (5,5)
# grid_b's spatial hash doesn't know about entity

Fix

Add spatial hash operations:

if (it != old_entities->end()) {
    self->data->grid->spatial_hash.remove(self->data);  // ADD
    old_entities->erase(it);
}
// ...
new_grid->entities->push_back(self->data);
self->data->grid = new_grid;
new_grid->spatial_hash.insert(self->data);  // ADD

Severity

High — causes incorrect spatial lookups. Stale spatial hash entries can reference positions in the old grid, leading to wrong collision results or index-out-of-bounds in the spatial hash.

## Summary `UIEntity::set_grid()` (the `entity.grid = new_grid` property setter) removes the entity from the old grid's entity list but does **not** remove it from the old grid's spatial hash, and does **not** insert it into the new grid's spatial hash. This leaves a stale spatial hash entry that can cause incorrect collision/pathfinding results or use-after-free. ## Root Cause `UIEntity.cpp:691-717` (set_grid): ```cpp // Remove from old grid if (self->data->grid && self->data->grid != new_grid) { auto& old_entities = self->data->grid->entities; auto it = std::find_if(old_entities->begin(), old_entities->end(), [self](const std::shared_ptr<UIEntity>& e) { return e.get() == self->data.get(); }); if (it != old_entities->end()) { old_entities->erase(it); // BUG: No spatial_hash.remove() here! } } // Add to new grid if (self->data->grid != new_grid) { new_grid->entities->push_back(self->data); self->data->grid = new_grid; // BUG: No spatial_hash.insert() here! ``` **Working comparison** — `UIEntityCollection::append()` (lines 558-579) does this correctly: ```cpp old_grid->spatial_hash.remove(entity->data); // ✓ // ... self->grid->spatial_hash.insert(entity->data); // ✓ ``` ## Reproduction ```python import mcrfpy grid_a = mcrfpy.Grid(grid_size=(10, 10)) grid_b = mcrfpy.Grid(grid_size=(10, 10)) entity = mcrfpy.Entity((5, 5), grid=grid_a) entity.grid = grid_b # Stale entry left in grid_a.spatial_hash # grid_a's spatial hash still thinks entity is at (5,5) # grid_b's spatial hash doesn't know about entity ``` ## Fix Add spatial hash operations: ```cpp if (it != old_entities->end()) { self->data->grid->spatial_hash.remove(self->data); // ADD old_entities->erase(it); } // ... new_grid->entities->push_back(self->data); self->data->grid = new_grid; new_grid->spatial_hash.insert(self->data); // ADD ``` ## Severity **High** — causes incorrect spatial lookups. Stale spatial hash entries can reference positions in the old grid, leading to wrong collision results or index-out-of-bounds in the spatial hash.