From a3463cdc704976d7a381a0a5752d9f47c025ee61 Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Thu, 5 Nov 2026 02:33:48 -0700 Subject: [PATCH 1/2] Expect removal to take out every pointer to an entity Test 66 pinned the leftover pointer as a known quirk; it now asserts the intended result, and Tests 69-71 cover ordering, moves, and same-id twins. These fail until the fix lands, which records the red half of the regression check on CI. Co-Authored-By: Claude Opus 5.5 --- src/tests.cpp | 84 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 72 insertions(+), 12 deletions(-) diff --git a/src/tests.cpp b/src/tests.cpp index 8d1ca2d..be13f1c 100644 --- a/src/tests.cpp +++ b/src/tests.cpp @@ -1089,26 +1089,20 @@ void testMovingEntityInSingleLocationGridAlwaysReturnsFalse() { std::cout << " --- " << "Success" << std::endl; } -void testRemovingEntityAddedTwiceToOneLocationLeavesOnePointer() { - std::cout << "Test 66 - Removing an entity added twice to one location leaves one pointer behind"; +void testRemovingEntityAddedTwiceToOneLocationRemovesBothPointers() { + std::cout << "Test 66 - Removing an entity added twice to one location removes both pointers"; Environment environment(0, "Earth", 2); Entity entity(0, "Daniel"); Location& location = environment.getGrid()->getLocationByCoordinates(0, 0); environment.addEntityToLocation(entity, location); environment.addEntityToLocation(entity, location); environment.removeEntity(entity); - assert(location.getNumEntities() == 1); - assert(environment.isEntityPresent(entity) == true); + assert(location.getNumEntities() == 0); + assert(environment.isEntityPresent(entity) == false); + assert(environment.getNumEntities() == 0); assert(entity.getEnvironmentId() == -1); assert(entity.getGridId() == -1); assert(entity.getLocationId() == "N/S"); - bool threw = false; - try { - environment.moveEntityUp(0); - } catch (const std::runtime_error& e) { - threw = true; - } - assert(threw == true); std::cout << " --- " << "Success" << std::endl; } @@ -1139,6 +1133,69 @@ void testRemovingEntityViaSameIdTwinLeavesOriginalIdsStale() { std::cout << " --- " << "Success" << std::endl; } +void testRemovingEntityFromLocationRemovesEveryPointerAndKeepsOrder() { + std::cout << "Test 69 - Removing an entity from a location removes every pointer and keeps the others in order"; + Location location("test-location", 0, 0); + Entity first(1, "Daniel"); + Entity repeated(2, "Bob"); + Entity last(3, "Alice"); + location.addEntity(&repeated); + location.addEntity(&first); + location.addEntity(&repeated); + location.addEntity(&last); + location.removeEntity(&repeated); + assert(location.getNumEntities() == 2); + assert(location.isEntityPresent(&repeated) == false); + assert(location.getEntities()[0] == &first); + assert(location.getEntities()[1] == &last); + assert(repeated.getLocationId() == "N/S"); + assert(first.getLocationId() == "test-location"); + assert(last.getLocationId() == "test-location"); + std::cout << " --- " << "Success" << std::endl; +} + +void testMovingEntityAddedTwiceToOneLocationLeavesASinglePointer() { + std::cout << "Test 70 - Moving an entity added twice to one location leaves a single pointer at the destination"; + Environment environment(0, "Earth", 2); + Entity entity(0, "Daniel"); + Location& start = environment.getGrid()->getLocationByCoordinates(0, 0); + Location& right = environment.getGrid()->getLocationByCoordinates(1, 0); + Location& corner = environment.getGrid()->getLocationByCoordinates(1, 1); + environment.addEntityToLocation(entity, start); + environment.addEntityToLocation(entity, start); + assert(environment.moveEntityRight(0) == true); + assert(start.getNumEntities() == 0); + assert(right.getNumEntities() == 1); + assert(environment.getNumEntities() == 1); + assert(entity.getLocationId() == right.getId()); + + environment.addEntityToLocation(entity, right); + environment.moveEntityToNewLocation(0, corner.getId()); + assert(right.getNumEntities() == 0); + assert(corner.getNumEntities() == 1); + assert(environment.getNumEntities() == 1); + assert(entity.getLocationId() == corner.getId()); + std::cout << " --- " << "Success" << std::endl; +} + +void testRemovingEntityAlsoRemovesSameIdTwinInItsLocation() { + std::cout << "Test 71 - Removing an entity also removes a same-id twin stored in its location"; + Environment environment(0, "Earth", 2); + Entity original(5, "Daniel"); + Entity twin(5, "Bob"); + Location& location = environment.getGrid()->getLocationByCoordinates(1, 0); + environment.addEntityToLocation(original, location); + environment.addEntityToLocation(twin, location); + environment.removeEntity(original); + assert(location.getNumEntities() == 0); + assert(environment.getNumEntities() == 0); + assert(original.getLocationId() == "N/S"); + assert(twin.getEnvironmentId() == 0); + assert(twin.getGridId() == 0); + assert(twin.getLocationId() == "0-1-0"); + std::cout << " --- " << "Success" << std::endl; +} + void seedRandomNumberGenerator() { srand (time (NULL)); } @@ -1213,8 +1270,11 @@ int main() { testRemovingLocationLeavesEntityEnvironmentIdUnchanged(); testAddingLocationToGridStoresACopy(); testMovingEntityInSingleLocationGridAlwaysReturnsFalse(); - testRemovingEntityAddedTwiceToOneLocationLeavesOnePointer(); + testRemovingEntityAddedTwiceToOneLocationRemovesBothPointers(); testRemovingEntityAddedToTwoLocationsRemovesBoth(); testRemovingEntityViaSameIdTwinLeavesOriginalIdsStale(); + testRemovingEntityFromLocationRemovesEveryPointerAndKeepsOrder(); + testMovingEntityAddedTwiceToOneLocationLeavesASinglePointer(); + testRemovingEntityAlsoRemovesSameIdTwinInItsLocation(); return 0; } \ No newline at end of file From 0d69d1191782d3b4c5c3ca67db7dd5844ac760a1 Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Thu, 5 Nov 2026 02:35:53 -0700 Subject: [PATCH 2/2] Remove every pointer to an entity from its location Location::removeEntity stopped at the first id match, so an entity added twice to one location stayed present with its ids already reset, and a directional move then threw on location id "N/S". Erase every match instead, which also leaves a single pointer after a move. Closes #70 Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- src/location.cpp | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 0b1a2e8..8b9d8fe 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ A newly constructed entity has an environment id and grid id of `-1` and a locat Adding is likewise unchecked. `addEntity` and `addEntityToLocation` do not test whether the entity is already present, so adding it a second time stores a second pointer to it (in another location, or in the same one), its location id names only the most recent location, and `getNumEntities` counts it twice. To relocate an entity that is already placed, use `moveEntityToNewLocation` or one of the directional move methods, which remove it before adding it again. -Removal takes out at most one pointer per location. An entity added to two different locations is removed from both, but one added twice to the same location keeps a pointer there after `removeEntity`: it is still present and still counted, while its ids have already been reset, so a directional move then throws `std::runtime_error` because its location id `"N/S"` names no location. +Removal takes out every pointer with a matching id. An entity added to two different locations is removed from both, and one added twice to the same location is removed entirely, so a move leaves a single pointer at the destination. Since matching is by id, a distinct same-id twin stored in that location is taken out too, and that twin keeps its ids. `Grid::removeLocation` also detaches any entities in the removed location: it resets their location id to `"N/S"` and their grid id to `-1`. It leaves their environment id unchanged, since the grid has no knowledge of its environment. diff --git a/src/location.cpp b/src/location.cpp index 4d25098..245741c 100644 --- a/src/location.cpp +++ b/src/location.cpp @@ -1,5 +1,6 @@ #include "header/location.h" +#include #include namespace envlibcpp { @@ -31,16 +32,13 @@ namespace envlibcpp { } void Location::removeEntity(Entity* entity) { - int index = -1; - for (size_t i = 0; i < getNumEntities(); i++) { - if (entities[i]->getId() == entity->getId()) { - index = i; - break; - } - } + // every match goes, since an entity added twice is stored twice and must not stay present + auto newEnd = std::remove_if(entities.begin(), entities.end(), [entity](Entity* e) { + return e->getId() == entity->getId(); + }); // an entity that is not in this location keeps its location id - if (index != -1) { - entities.erase(entities.begin() + index); + if (newEnd != entities.end()) { + entities.erase(newEnd, entities.end()); entity->resetLocationId(); } }