Grid::addEntityToLocation (src/grid.cpp:82-89) walks locations looking for an element whose id matches the location argument, but when a match is found the entity is pushed into the argument rather than into the matching grid-owned element:
for (Location& l : locations) {
if (l.getId() == location.getId()) {
location.addEntity(&entity); // `l` is the grid's element; `location` may not be
}
}
The loop variable l is never used for the insertion. Two consequences follow:
-
A caller-owned Location with a matching id receives the entity while the grid's own copy stays empty. Grid::addLocation stores a copy of its argument (locations.push_back(location), src/grid.cpp:54), so the pattern in testAddingLocationToGrid — construct a local Location, pass it to addLocation, keep using the local — produces exactly this situation. After grid.addEntityToLocation(entity, localLocation), entity.getLocationId() and entity.getGridId() both report the grid, yet grid.isEntityPresent(entity) returns false and grid.getNumEntities() is unchanged, because the pointer was stored in the local copy.
-
When no id matches, the entity's grid id is still stamped. entity.setGridId(id) runs on line 83 before the membership check, so an entity that was never added anywhere is left reporting membership of the grid (and, via Environment::addEntityToLocation, of the environment), with getLocationId() still "N/S".
The existing test for this path (testAddingEntityToSpecificLocation, Test 29) does not catch either case because it obtains its Location& from getGrid()->getFirstLocation(), i.e. a reference to the grid's own element, where the argument and l are the same object.
Suggested fix: insert into l instead of location (l.addEntity(&entity);), and move entity.setGridId(id) inside the match branch (or return early / throw when nothing matches, mirroring getLocation's std::runtime_error("Location not found")). A regression test should add a location via addLocation, then call addEntityToLocation with the caller's original object and assert grid.isEntityPresent(entity).
This issue body was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).
drafted by Claude on behalf of Daniel Stephenson
Grid::addEntityToLocation(src/grid.cpp:82-89) walkslocationslooking for an element whose id matches thelocationargument, but when a match is found the entity is pushed into the argument rather than into the matching grid-owned element:The loop variable
lis never used for the insertion. Two consequences follow:A caller-owned
Locationwith a matching id receives the entity while the grid's own copy stays empty.Grid::addLocationstores a copy of its argument (locations.push_back(location),src/grid.cpp:54), so the pattern intestAddingLocationToGrid— construct a localLocation, pass it toaddLocation, keep using the local — produces exactly this situation. Aftergrid.addEntityToLocation(entity, localLocation),entity.getLocationId()andentity.getGridId()both report the grid, yetgrid.isEntityPresent(entity)returnsfalseandgrid.getNumEntities()is unchanged, because the pointer was stored in the local copy.When no id matches, the entity's grid id is still stamped.
entity.setGridId(id)runs on line 83 before the membership check, so an entity that was never added anywhere is left reporting membership of the grid (and, viaEnvironment::addEntityToLocation, of the environment), withgetLocationId()still"N/S".The existing test for this path (
testAddingEntityToSpecificLocation, Test 29) does not catch either case because it obtains itsLocation&fromgetGrid()->getFirstLocation(), i.e. a reference to the grid's own element, where the argument andlare the same object.Suggested fix: insert into
linstead oflocation(l.addEntity(&entity);), and moveentity.setGridId(id)inside the match branch (or return early / throw when nothing matches, mirroringgetLocation'sstd::runtime_error("Location not found")). A regression test should add a location viaaddLocation, then calladdEntityToLocationwith the caller's original object and assertgrid.isEntityPresent(entity).This issue body was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).
drafted by Claude on behalf of Daniel Stephenson