From 0c9969cc034733b5ecd86df7b283c1430b76a50c Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Sun, 1 Jun 2025 19:05:34 +0100 Subject: [PATCH 1/7] [ZH] Reduce crashing within AIPathfind due to inadequate pathing resource cleanup and null pathing info within some cells. Co-authored-by: Bart Roossien --- .../Source/GameLogic/AI/AIPathfind.cpp | 70 +++++++++++-------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index c32a35d0b7..be2009d670 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -6650,6 +6650,7 @@ Path *Pathfinder::internalFindPath( Object *obj, const LocomotorSet& locomotorSe Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), from, goalCell, centerInCell, false ); parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return path; } @@ -6726,8 +6727,9 @@ Path *Pathfinder::internalFindPath( Object *obj, const LocomotorSet& locomotorSe #endif m_isTunneling = false; - goalCell->releaseInfo(); + parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return NULL; } @@ -7167,6 +7169,7 @@ Path *Pathfinder::findGroundPath( const Coord3D *from, Path *path = buildGroundPath(crusher, from, goalCell, centerInCell, pathDiameter ); parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return path; } @@ -7371,8 +7374,9 @@ Path *Pathfinder::findGroundPath( const Coord3D *from, TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - goalCell->releaseInfo(); + parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return NULL; } @@ -7750,11 +7754,9 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu #endif } #endif - if (goalCell->hasInfo() && !goalCell->getClosed() && !goalCell->getOpen()) { - goalCell->releaseInfo(); - } parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return path; } @@ -7951,10 +7953,9 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu } #endif #endif - if (goalCell->hasInfo() && !goalCell->getClosed() && !goalCell->getOpen()) { - goalCell->releaseInfo(); - } + parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return path; } @@ -8001,8 +8002,9 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - goalCell->releaseInfo(); + parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return NULL; } @@ -8437,6 +8439,7 @@ Bool Pathfinder::pathDestination( Object *obj, const LocomotorSet& locomotorSet } #endif m_isTunneling = false; + parentCell->releaseInfo(); cleanOpenAndClosedLists(); goalCell->releaseInfo(); return false; @@ -8570,7 +8573,9 @@ Int Pathfinder::checkPathCost(Object *obj, const LocomotorSet& locomotorSet, con if (parentCell==goalCell) { Int cost = parentCell->getTotalCost(); m_isTunneling = false; + parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return cost; } @@ -8682,10 +8687,9 @@ Int Pathfinder::checkPathCost(Object *obj, const LocomotorSet& locomotorSet, con } m_isTunneling = false; - if (goalCell->hasInfo() && !goalCell->getClosed() && !goalCell->getOpen()) { - goalCell->releaseInfo(); - } + parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return MAX_COST; } @@ -8888,8 +8892,8 @@ Path *Pathfinder::findClosestPath( Object *obj, const LocomotorSet& locomotorSet // construct and return path Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), from, goalCell, centerInCell, blocked); parentCell->releaseInfo(); - goalCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return path; } // put parent cell onto closed list - its evaluation is finished @@ -8969,8 +8973,9 @@ Path *Pathfinder::findClosestPath( Object *obj, const LocomotorSet& locomotorSet rawTo->y = closesetCell->getYIndex()*PATHFIND_CELL_SIZE_F + PATHFIND_CELL_SIZE_F/2.0f; // construct and return path Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), from, closesetCell, centerInCell, blocked ); - goalCell->releaseInfo(); + parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return path; } @@ -8991,8 +8996,9 @@ Path *Pathfinder::findClosestPath( Object *obj, const LocomotorSet& locomotorSet TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - goalCell->releaseInfo(); + parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return NULL; } @@ -9114,12 +9120,16 @@ void Pathfinder::prependCells( Path *path, const Coord3D *fromPos, } prevCell = cell; } - m_zoneManager.setPassable(cell->getXIndex(), cell->getYIndex(), true); - if (goalCellNull) { - // Very short path. - adjustCoordToCell(cell->getXIndex(), cell->getYIndex(), center, pos, cell->getLayer()); - path->prependNode( &pos, cell->getLayer() ); + + if (cell->hasInfo()) { + m_zoneManager.setPassable(cell->getXIndex(), cell->getYIndex(), true); + if (goalCellNull) { + // Very short path. + adjustCoordToCell(cell->getXIndex(), cell->getYIndex(), center, pos, cell->getLayer()); + path->prependNode( &pos, cell->getLayer() ); + } } + // put actual start position as first node on the path, so it begins right at the unit's feet if (fromPos->x != path->getFirstNode()->getPosition()->x || fromPos->y != path->getFirstNode()->getPosition()->y) { path->prependNode( fromPos, cell->getLayer() ); @@ -10400,6 +10410,7 @@ Path *Pathfinder::getMoveAwayFromPath(Object* obj, Object *otherObj, DEBUG_LOG(("Unit '%s', time %f\n", obj->getTemplate()->getName().str(), (::GetTickCount()-startTimeMS)/1000.0f)); m_isTunneling = false; + parentCell->releaseInfo(); cleanOpenAndClosedLists(); return NULL; } @@ -10516,6 +10527,7 @@ Path *Pathfinder::patchPath( const Object *obj, const LocomotorSet& locomotorSet } if (startNode == originalPath->getLastNode()) { + parentCell->releaseInfo(); cleanOpenAndClosedLists(); return NULL; // no open nodes. } @@ -10584,12 +10596,9 @@ Path *Pathfinder::patchPath( const Object *obj, const LocomotorSet& locomotorSet } #endif m_isTunneling = false; - if (!candidateGoal->getOpen() && !candidateGoal->getClosed()) - { - // Not on one of the lists - candidateGoal->releaseInfo(); - } + parentCell->releaseInfo(); cleanOpenAndClosedLists(); + candidateGoal->releaseInfo(); return NULL; } @@ -10882,10 +10891,9 @@ Path *Pathfinder::findAttackPath( const Object *obj, const LocomotorSet& locomot } } Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), obj->getPosition(), parentCell, centerInCell, false); - if (goalCell->hasInfo() && !goalCell->getClosed() && !goalCell->getOpen()) { - goalCell->releaseInfo(); - } + parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return path; } } @@ -10942,10 +10950,9 @@ Path *Pathfinder::findAttackPath( const Object *obj, const LocomotorSet& locomot TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - if (goalCell->hasInfo() && !goalCell->getClosed() && !goalCell->getOpen()) { - goalCell->releaseInfo(); - } + parentCell->releaseInfo(); cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return NULL; } @@ -11103,6 +11110,7 @@ Path *Pathfinder::findSafePath( const Object *obj, const LocomotorSet& locomotor TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; + parentCell->releaseInfo(); cleanOpenAndClosedLists(); return NULL; } From 75a3bb63b69a74b8da72b4c5e228ede54fb6357e Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Mon, 2 Jun 2025 21:54:40 +0100 Subject: [PATCH 2/7] [ZH] Add more places to prevent cells leaking pathing info if they fail some conditions Co-authored-by: Bart Roossien --- .../Source/GameLogic/AI/AIPathfind.cpp | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index be2009d670..1757be4d1f 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -6565,10 +6565,14 @@ Path *Pathfinder::internalFindPath( Object *obj, const LocomotorSet& locomotorSe zone2 = m_zoneManager.getEffectiveZone(locomotorSet.getValidSurfaces(), isCrusher, goalCell->getZone()); if (layer==LAYER_WALL && zone1 == 0) { + goalCell->releaseInfo(); + parentCell->releaseInfo(); return NULL; } if (destinationLayer==LAYER_WALL && zone2 == 0) { + goalCell->releaseInfo(); + parentCell->releaseInfo(); return NULL; } @@ -6650,7 +6654,6 @@ Path *Pathfinder::internalFindPath( Object *obj, const LocomotorSet& locomotorSe Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), from, goalCell, centerInCell, false ); parentCell->releaseInfo(); cleanOpenAndClosedLists(); - goalCell->releaseInfo(); return path; } @@ -7105,6 +7108,7 @@ Path *Pathfinder::findGroundPath( const Coord3D *from, PathfindLayerEnum layer = TheTerrainLogic->getLayerForDestination(from); PathfindCell *parentCell = getClippedCell( layer,&clipFrom ); if (parentCell == NULL) { + goalCell->releaseInfo(); return NULL; } if (parentCell!=goalCell) { @@ -7169,7 +7173,6 @@ Path *Pathfinder::findGroundPath( const Coord3D *from, Path *path = buildGroundPath(crusher, from, goalCell, centerInCell, pathDiameter ); parentCell->releaseInfo(); cleanOpenAndClosedLists(); - goalCell->releaseInfo(); return path; } @@ -8268,7 +8271,6 @@ Bool Pathfinder::pathDestination( Object *obj, const LocomotorSet& locomotorSet if (parentCell!=goalCell) { if (!parentCell->allocateInfo(startCellNdx)) { - desiredCell->releaseInfo(); goalCell->releaseInfo(); return FALSE; } @@ -8383,6 +8385,8 @@ Bool Pathfinder::pathDestination( Object *obj, const LocomotorSet& locomotorSet if (!newCell->allocateInfo(newCellCoord)) { // Out of cells for pathing... + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return cellCount; } cellCount++; @@ -8567,18 +8571,17 @@ Int Pathfinder::checkPathCost(Object *obj, const LocomotorSet& locomotorSet, con parentCell = m_openList; m_openList = parentCell->removeFromOpenList(m_openList); - // put parent cell onto closed list - its evaluation is finished - m_closedList = parentCell->putOnClosedList( m_closedList ); - if (parentCell==goalCell) { Int cost = parentCell->getTotalCost(); m_isTunneling = false; parentCell->releaseInfo(); cleanOpenAndClosedLists(); - goalCell->releaseInfo(); return cost; } + // put parent cell onto closed list - its evaluation is finished + m_closedList = parentCell->putOnClosedList( m_closedList ); + if (cellCount > MAX_CELL_COUNT) { continue; } @@ -8639,8 +8642,11 @@ Int Pathfinder::checkPathCost(Object *obj, const LocomotorSet& locomotorSet, con if (!newCell->allocateInfo(newCellCoord)) { // Out of cells for pathing... + parentCell->releaseInfo(); + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); return cellCount; - } + } cellCount++; newCostSoFar = newCell->costSoFar( parentCell ); @@ -8826,6 +8832,7 @@ Path *Pathfinder::findClosestPath( Object *obj, const LocomotorSet& locomotorSet if (parentCell!=goalCell) { worldToCell(&clipFrom, &pos2d); if (!parentCell->allocateInfo(pos2d)) { + goalCell->releaseInfo(); return NULL; } } @@ -9121,13 +9128,11 @@ void Pathfinder::prependCells( Path *path, const Coord3D *fromPos, prevCell = cell; } - if (cell->hasInfo()) { - m_zoneManager.setPassable(cell->getXIndex(), cell->getYIndex(), true); - if (goalCellNull) { - // Very short path. - adjustCoordToCell(cell->getXIndex(), cell->getYIndex(), center, pos, cell->getLayer()); - path->prependNode( &pos, cell->getLayer() ); - } + m_zoneManager.setPassable(cell->getXIndex(), cell->getYIndex(), true); + if (goalCellNull) { + // Very short path. + adjustCoordToCell(cell->getXIndex(), cell->getYIndex(), center, pos, cell->getLayer()); + path->prependNode( &pos, cell->getLayer() ); } // put actual start position as first node on the path, so it begins right at the unit's feet @@ -10528,7 +10533,6 @@ Path *Pathfinder::patchPath( const Object *obj, const LocomotorSet& locomotorSet } if (startNode == originalPath->getLastNode()) { parentCell->releaseInfo(); - cleanOpenAndClosedLists(); return NULL; // no open nodes. } PathfindCell *candidateGoal; @@ -10536,6 +10540,7 @@ Path *Pathfinder::patchPath( const Object *obj, const LocomotorSet& locomotorSet ICoord2D goalCellNdx; worldToCell(&goalPos, &goalCellNdx); if (!candidateGoal->allocateInfo(goalCellNdx)) { + parentCell->releaseInfo(); return NULL; } From 956b0b6e84da5753e9c368a151e9f9694714378e Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Thu, 5 Jun 2025 20:41:31 +0100 Subject: [PATCH 3/7] [ZH] Reorder cells cleanup so cell lists are cleared before the individual cells and add clearing of parent cell pointers when releasing pathfind cell info Co-authored-by: Bart Roossien --- .../Source/GameLogic/AI/AIPathfind.cpp | 52 ++++++++++--------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index 1757be4d1f..fecec140ae 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -1297,6 +1297,12 @@ Bool PathfindCell::allocateInfo( const ICoord2D &pos ) */ void PathfindCell::releaseInfo( void ) { + // TheSuperHackers @bugfix Mauller/SkyAero 05/06/2025 Parent cell links need clearing to prevent dangling pointers on starting points that can link them to an invalid parent cell. + // Parent cells are only cleared within Pathfinder::prependCells, so cells that do not make it onto the final path do not get their parent cell cleared. + // Cells with a special flags also do not get their pathfindinfo cleared and therefore can leave a parent cell set on a starting cell. + if (m_info) { + m_info->m_pathParent = NULL; + } if (m_type==PathfindCell::CELL_OBSTACLE) { return; } @@ -6652,8 +6658,8 @@ Path *Pathfinder::internalFindPath( Object *obj, const LocomotorSet& locomotorSe m_isTunneling = false; // construct and return path Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), from, goalCell, centerInCell, false ); - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); return path; } @@ -6730,8 +6736,8 @@ Path *Pathfinder::internalFindPath( Object *obj, const LocomotorSet& locomotorSe #endif m_isTunneling = false; - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return NULL; } @@ -7171,8 +7177,8 @@ Path *Pathfinder::findGroundPath( const Coord3D *from, m_isTunneling = false; // construct and return path Path *path = buildGroundPath(crusher, from, goalCell, centerInCell, pathDiameter ); - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); return path; } @@ -7377,8 +7383,8 @@ Path *Pathfinder::findGroundPath( const Coord3D *from, TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return NULL; } @@ -7757,8 +7763,8 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu #endif } #endif - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return path; } @@ -7956,8 +7962,8 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu } #endif #endif - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return path; } @@ -8005,8 +8011,8 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return NULL; } @@ -8443,8 +8449,8 @@ Bool Pathfinder::pathDestination( Object *obj, const LocomotorSet& locomotorSet } #endif m_isTunneling = false; - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return false; } @@ -8574,8 +8580,8 @@ Int Pathfinder::checkPathCost(Object *obj, const LocomotorSet& locomotorSet, con if (parentCell==goalCell) { Int cost = parentCell->getTotalCost(); m_isTunneling = false; - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); return cost; } @@ -8642,8 +8648,8 @@ Int Pathfinder::checkPathCost(Object *obj, const LocomotorSet& locomotorSet, con if (!newCell->allocateInfo(newCellCoord)) { // Out of cells for pathing... - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return cellCount; } @@ -8693,8 +8699,8 @@ Int Pathfinder::checkPathCost(Object *obj, const LocomotorSet& locomotorSet, con } m_isTunneling = false; - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return MAX_COST; } @@ -8898,8 +8904,8 @@ Path *Pathfinder::findClosestPath( Object *obj, const LocomotorSet& locomotorSet m_isTunneling = false; // construct and return path Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), from, goalCell, centerInCell, blocked); - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return path; } @@ -8980,8 +8986,8 @@ Path *Pathfinder::findClosestPath( Object *obj, const LocomotorSet& locomotorSet rawTo->y = closesetCell->getYIndex()*PATHFIND_CELL_SIZE_F + PATHFIND_CELL_SIZE_F/2.0f; // construct and return path Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), from, closesetCell, centerInCell, blocked ); - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return path; } @@ -9003,8 +9009,8 @@ Path *Pathfinder::findClosestPath( Object *obj, const LocomotorSet& locomotorSet TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return NULL; } @@ -9127,14 +9133,12 @@ void Pathfinder::prependCells( Path *path, const Coord3D *fromPos, } prevCell = cell; } - m_zoneManager.setPassable(cell->getXIndex(), cell->getYIndex(), true); if (goalCellNull) { // Very short path. adjustCoordToCell(cell->getXIndex(), cell->getYIndex(), center, pos, cell->getLayer()); path->prependNode( &pos, cell->getLayer() ); } - // put actual start position as first node on the path, so it begins right at the unit's feet if (fromPos->x != path->getFirstNode()->getPosition()->x || fromPos->y != path->getFirstNode()->getPosition()->y) { path->prependNode( fromPos, cell->getLayer() ); @@ -10392,8 +10396,8 @@ Path *Pathfinder::getMoveAwayFromPath(Object* obj, Object *otherObj, m_isTunneling = false; // construct and return path Path *newPath = buildActualPath( obj, locomotorSet.getValidSurfaces(), obj->getPosition(), parentCell, centerInCell, false); - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); return newPath; } // put parent cell onto closed list - its evaluation is finished @@ -10415,8 +10419,8 @@ Path *Pathfinder::getMoveAwayFromPath(Object* obj, Object *otherObj, DEBUG_LOG(("Unit '%s', time %f\n", obj->getTemplate()->getName().str(), (::GetTickCount()-startTimeMS)/1000.0f)); m_isTunneling = false; - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); return NULL; } @@ -10575,8 +10579,8 @@ Path *Pathfinder::patchPath( const Object *obj, const LocomotorSet& locomotorSet // cleanup the path by checking line of sight path->optimize(obj, locomotorSet.getValidSurfaces(), blocked); - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); candidateGoal->releaseInfo(); return path; @@ -10601,8 +10605,8 @@ Path *Pathfinder::patchPath( const Object *obj, const LocomotorSet& locomotorSet } #endif m_isTunneling = false; - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); candidateGoal->releaseInfo(); return NULL; } @@ -10896,8 +10900,8 @@ Path *Pathfinder::findAttackPath( const Object *obj, const LocomotorSet& locomot } } Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), obj->getPosition(), parentCell, centerInCell, false); - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return path; } @@ -10955,8 +10959,8 @@ Path *Pathfinder::findAttackPath( const Object *obj, const LocomotorSet& locomot TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); goalCell->releaseInfo(); return NULL; } @@ -11079,8 +11083,8 @@ Path *Pathfinder::findSafePath( const Object *obj, const LocomotorSet& locomotor #endif // construct and return path Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), obj->getPosition(), parentCell, centerInCell, false); - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); return path; } @@ -11115,8 +11119,8 @@ Path *Pathfinder::findSafePath( const Object *obj, const LocomotorSet& locomotor TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - parentCell->releaseInfo(); cleanOpenAndClosedLists(); + parentCell->releaseInfo(); return NULL; } From 6536b761e915a4296290f53e4b66ef966cc15738 Mon Sep 17 00:00:00 2001 From: Bart Roossien Date: Sat, 7 Jun 2025 20:35:02 +0200 Subject: [PATCH 4/7] Fix m_open incorrectly being initialized to true in startPathFind --- GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index fecec140ae..cf06ccc51d 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -1243,7 +1243,7 @@ Bool PathfindCell::startPathfind( PathfindCell *goalCell ) if (goalCell) { m_info->m_totalCost = costToGoal( goalCell ); } - m_info->m_open = TRUE; + m_info->m_open = FALSE; m_info->m_closed = FALSE; return true; } From ce5518d492f444f3849f0e5a2b0c0b1c784e1b80 Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Sun, 8 Jun 2025 20:15:59 +0100 Subject: [PATCH 5/7] [ZH] Add extra allocateInfo checks within internal_findHierarchicalPath() and processHierarchicalCell() Co-authored-by: Bart Roossien --- .../Source/GameLogic/AI/AIPathfind.cpp | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index cf06ccc51d..5f3ab09592 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -7449,7 +7449,7 @@ void Pathfinder::processHierarchicalCell( const ICoord2D &scanCell, const ICoord } newCell->allocateInfo(scanCell); - if (!newCell->getClosed() && !newCell->getOpen()) { + if (newCell->hasInfo() && !newCell->getClosed() && !newCell->getOpen()) { m_closedList = newCell->putOnClosedList(m_closedList); } @@ -7607,7 +7607,11 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu // Close parent cell; m_openList = parentCell->removeFromOpenList(m_openList); m_closedList = parentCell->putOnClosedList(m_closedList); - startCell->allocateInfo(ndx); + if (!startCell->allocateInfo(ndx)) { + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); + return NULL; + } startCell->setParentCellHierarchical(parentCell); cellCount++; Int curCost = startCell->costToHierGoal(parentCell); @@ -7619,7 +7623,11 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu m_openList = startCell->putOnSortedOpenList( m_openList ); cellCount++; - cell->allocateInfo(toNdx); + if(!cell->allocateInfo(toNdx)) { + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); + return NULL; + } curCost = cell->costToHierGoal(parentCell); remCost = cell->costToHierGoal(goalCell); cell->setCostSoFar(curCost); @@ -7703,13 +7711,21 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu PathfindCell *startCell = getCell(LAYER_GROUND, ndx.x, ndx.y); if (startCell==NULL) continue; if (startCell != parentCell) { - startCell->allocateInfo(ndx); + if(!startCell->allocateInfo(ndx)) { + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); + return NULL; + } startCell->setParentCellHierarchical(parentCell); if (!startCell->getClosed() && !startCell->getOpen()) { m_closedList = startCell->putOnClosedList(m_closedList); } } - cell->allocateInfo(toNdx); + if(!cell->allocateInfo(toNdx)) { + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); + return NULL; + } cell->setParentCellHierarchical(startCell); cellCount++; From 92eed40b5cb4675a01f236881dfb0f4726761662 Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Tue, 10 Jun 2025 21:43:56 +0100 Subject: [PATCH 6/7] [ZH] Scuff the Pathing so it runs like retail before getting caught in crashing areas and resetting to use fixed pathfinding --- .../GameEngine/Include/GameLogic/AIPathfind.h | 11 + .../Source/GameLogic/AI/AIPathfind.cpp | 502 ++++++++++++++---- 2 files changed, 424 insertions(+), 89 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/GameLogic/AIPathfind.h b/GeneralsMD/Code/GameEngine/Include/GameLogic/AIPathfind.h index 44d275df88..5206c912c4 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameLogic/AIPathfind.h +++ b/GeneralsMD/Code/GameEngine/Include/GameLogic/AIPathfind.h @@ -37,6 +37,7 @@ //#include "GameLogic/Locomotor.h" // no, do not include this, unless you like long recompiles #include "GameLogic/LocomotorSet.h" #include "GameLogic/GameLogic.h" +#include "Common/GameDefines.h" class Bridge; class Object; @@ -50,6 +51,10 @@ class PathfindZoneManager; #define INFANTRY_MOVES_THROUGH_INFANTRY +// TheSuperHackers @info This is here to easily toggle the retail compatible hacks for the pathfinding +#if RETAIL_COMPATIBLE_CRC +#define RETAIL_COMPATIBLE_PATHFINDING (1) +#endif typedef UnsignedShort zoneStorageType; @@ -208,6 +213,9 @@ class PathfindCellInfo { friend class PathfindCell; public: +#if RETAIL_COMPATIBLE_PATHFINDING + static void forceCleanPathFindInfoCells(void); +#endif static void allocateCellInfos(void); static void releaseCellInfos(void); @@ -699,6 +707,9 @@ class Pathfinder : PathfindServicesInterface, public Snapshot Path *getDebugPath( void ); void setDebugPath( Path *debugpath ); +#if RETAIL_COMPATIBLE_PATHFINDING + void forceCleanCells(void); +#endif void cleanOpenAndClosedLists(void); // Adjusts the destination to a spot near dest that is not occupied by other units. diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index 5f3ab09592..c0a77478d8 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -1104,7 +1104,40 @@ Real Path::computeFlightDistToGoal( const Coord3D *pos, Coord3D& goalPos ) enum { PATHFIND_CELLS_PER_FRAME=5000}; // Number of cells we will search pathfinding per frame. enum {CELL_INFOS_TO_ALLOCATE = 30000}; PathfindCellInfo *PathfindCellInfo::s_infoArray = NULL; -PathfindCellInfo *PathfindCellInfo::s_firstFree = NULL; +PathfindCellInfo *PathfindCellInfo::s_firstFree = NULL; + +#if RETAIL_COMPATIBLE_PATHFINDING +// TheSuperHackers @info This variable is here so the code will run down the retail compatible path till a failure mode is hit +// The pathfinding will then switch over to the corrected pathfinding code for SH clients +Bool s_useFixedPathfinding = false; +Bool s_forceCleanCells = false; + +void PathfindCellInfo::forceCleanPathFindInfoCells() { + + for (Int i = 0; i < CELL_INFOS_TO_ALLOCATE - 1; i++) { + s_infoArray[i].m_nextOpen = NULL; + s_infoArray[i].m_prevOpen = NULL; + s_infoArray[i].m_open = FALSE; + s_infoArray[i].m_closed = FALSE; + } +} + +void Pathfinder::forceCleanCells() { + + for (int j = 0; j <= m_extent.hi.y; ++j) { + for (int i = 0; i <= m_extent.hi.x; ++i) { + if (m_map[ i ][ j ].hasInfo()) { + m_map[i][j].releaseInfo(); + } + } + } + +} +#else +#define s_useFixedPathfinding 1 +#define s_forcedCleanup 0 +#endif + /** * Allocates a pool of pathfind cell infos. */ @@ -1243,7 +1276,11 @@ Bool PathfindCell::startPathfind( PathfindCell *goalCell ) if (goalCell) { m_info->m_totalCost = costToGoal( goalCell ); } - m_info->m_open = FALSE; + if (s_useFixedPathfinding) { + m_info->m_open = FALSE; + } else { + m_info->m_open = TRUE; + } m_info->m_closed = FALSE; return true; } @@ -1300,8 +1337,10 @@ void PathfindCell::releaseInfo( void ) // TheSuperHackers @bugfix Mauller/SkyAero 05/06/2025 Parent cell links need clearing to prevent dangling pointers on starting points that can link them to an invalid parent cell. // Parent cells are only cleared within Pathfinder::prependCells, so cells that do not make it onto the final path do not get their parent cell cleared. // Cells with a special flags also do not get their pathfindinfo cleared and therefore can leave a parent cell set on a starting cell. - if (m_info) { - m_info->m_pathParent = NULL; + if (s_useFixedPathfinding) { + if (m_info) { + m_info->m_pathParent = NULL; + } } if (m_type==PathfindCell::CELL_OBSTACLE) { return; @@ -1597,9 +1636,22 @@ Int PathfindCell::releaseOpenList( PathfindCell *list ) DEBUG_ASSERTCRASH(list->m_info->m_closed==FALSE && list->m_info->m_open==TRUE, ("Serious error - Invalid flags. jba")); PathfindCell *cur = list; PathfindCellInfo *curInfo = list->m_info; + +#if RETAIL_COMPATIBLE_PATHFINDING + // TheSuperHackers @info This is only here to catch a crash point in the retail compatible pathfinding + // One crash mode is where a cell has no pathfindinfo, resulting in a nullptr access and a crash. + // Therefore we signal that we need to clean the maps pathing cell and the pathfindinfo cells + if(!curInfo && !s_useFixedPathfinding) { + list = NULL; + s_useFixedPathfinding = true; + s_forceCleanCells = true; + return count; + } +#endif //RETAIL_COMPATIBLE_PATHFINDING + if (curInfo->m_nextOpen) { list = curInfo->m_nextOpen->m_cell; - } else { + } else { list = NULL; } DEBUG_ASSERTCRASH(cur == curInfo->m_cell, ("Bad backpointer in PathfindCellInfo")); @@ -1621,9 +1673,22 @@ Int PathfindCell::releaseClosedList( PathfindCell *list ) DEBUG_ASSERTCRASH(list->m_info->m_closed==TRUE && list->m_info->m_open==FALSE, ("Serious error - Invalid flags. jba")); PathfindCell *cur = list; PathfindCellInfo *curInfo = list->m_info; + +#if RETAIL_COMPATIBLE_PATHFINDING + // TheSuperHackers @info This is only here to catch a crash point in the retail compatible pathfinding + // One crash mode is where a cell has no pathfindinfo, resulting in a nullptr access and a crash. + // Therefore we signal that we need to clean the maps pathing cell and the pathfindinfo cells + if(!curInfo && !s_useFixedPathfinding) { + list = NULL; + s_useFixedPathfinding = true; + s_forceCleanCells = true; + return count; + } +#endif //RETAIL_COMPATIBLE_PATHFINDING + if (curInfo->m_nextOpen) { list = curInfo->m_nextOpen->m_cell; - } else { + } else { list = NULL; } DEBUG_ASSERTCRASH(cur == curInfo->m_cell, ("Bad backpointer in PathfindCellInfo")); @@ -3908,6 +3973,11 @@ void Pathfinder::reset( void ) m_wallHeight = 0.0f; } m_zoneManager.reset(); + +#ifdef RETAIL_COMPATIBLE_PATHFINDING + s_useFixedPathfinding = false; + s_forceCleanCells = false; +#endif } /** @@ -4827,11 +4897,35 @@ void Pathfinder::cleanOpenAndClosedLists(void) { if (m_openList) { count += PathfindCell::releaseOpenList(m_openList); m_openList = NULL; - } + } + +#if RETAIL_COMPATIBLE_PATHFINDING + // TheSuperHackers @info this is here as the map cells are contained within the pathfinder and cannot be cleaned externally. + // If the crash mode within PathfinCell::releaseOpenList is hit, it will set s_forceCleanCells to allow the system to cleanly recover. + if (s_forceCleanCells) { + PathfindCellInfo::forceCleanPathFindInfoCells(); + forceCleanCells(); + // TheSuperHackers @info cells on the closed list are forcefully cleaned up by this point + m_closedList = NULL; + s_forceCleanCells = false; + } +#endif + if (m_closedList) { count += PathfindCell::releaseClosedList(m_closedList); m_closedList = NULL; - } + } + +#if RETAIL_COMPATIBLE_PATHFINDING + // TheSuperHackers @info this is here and performs the same function as the above block, but for when the crash occurs within the closed list. + // If the crash mode within PathfinCell::releaseClosedList is hit, it will set s_forceCleanCells to allow the system to cleanly recover. + if (s_forceCleanCells) { + PathfindCellInfo::forceCleanPathFindInfoCells(); + forceCleanCells(); + s_forceCleanCells = false; + } +#endif + m_cumulativeCellsAllocated += count; //#ifdef RTS_DEBUG #if 0 @@ -6571,14 +6665,18 @@ Path *Pathfinder::internalFindPath( Object *obj, const LocomotorSet& locomotorSe zone2 = m_zoneManager.getEffectiveZone(locomotorSet.getValidSurfaces(), isCrusher, goalCell->getZone()); if (layer==LAYER_WALL && zone1 == 0) { - goalCell->releaseInfo(); - parentCell->releaseInfo(); + if (s_useFixedPathfinding) { + goalCell->releaseInfo(); + parentCell->releaseInfo(); + } return NULL; } if (destinationLayer==LAYER_WALL && zone2 == 0) { - goalCell->releaseInfo(); - parentCell->releaseInfo(); + if (s_useFixedPathfinding) { + goalCell->releaseInfo(); + parentCell->releaseInfo(); + } return NULL; } @@ -6658,8 +6756,14 @@ Path *Pathfinder::internalFindPath( Object *obj, const LocomotorSet& locomotorSe m_isTunneling = false; // construct and return path Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), from, goalCell, centerInCell, false ); - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + } + else { + parentCell->releaseInfo(); + cleanOpenAndClosedLists(); + } return path; } @@ -6736,9 +6840,15 @@ Path *Pathfinder::internalFindPath( Object *obj, const LocomotorSet& locomotorSe #endif m_isTunneling = false; - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } + else { + goalCell->releaseInfo(); + cleanOpenAndClosedLists(); + } return NULL; } @@ -7114,7 +7224,9 @@ Path *Pathfinder::findGroundPath( const Coord3D *from, PathfindLayerEnum layer = TheTerrainLogic->getLayerForDestination(from); PathfindCell *parentCell = getClippedCell( layer,&clipFrom ); if (parentCell == NULL) { - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + goalCell->releaseInfo(); + } return NULL; } if (parentCell!=goalCell) { @@ -7177,8 +7289,14 @@ Path *Pathfinder::findGroundPath( const Coord3D *from, m_isTunneling = false; // construct and return path Path *path = buildGroundPath(crusher, from, goalCell, centerInCell, pathDiameter ); - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + } + else { + parentCell->releaseInfo(); + cleanOpenAndClosedLists(); + } return path; } @@ -7383,9 +7501,15 @@ Path *Pathfinder::findGroundPath( const Coord3D *from, TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } + else { + goalCell->releaseInfo(); + cleanOpenAndClosedLists(); + } return NULL; } @@ -7449,8 +7573,14 @@ void Pathfinder::processHierarchicalCell( const ICoord2D &scanCell, const ICoord } newCell->allocateInfo(scanCell); - if (newCell->hasInfo() && !newCell->getClosed() && !newCell->getOpen()) { - m_closedList = newCell->putOnClosedList(m_closedList); + if (s_useFixedPathfinding) { + if (newCell->hasInfo() && !newCell->getClosed() && !newCell->getOpen()) { + m_closedList = newCell->putOnClosedList(m_closedList); + } + } else { + if (!newCell->getClosed() && !newCell->getOpen()) { + m_closedList = newCell->putOnClosedList(m_closedList); + } } adjNewCell->allocateInfo(adjacentCell); @@ -7608,8 +7738,19 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu m_openList = parentCell->removeFromOpenList(m_openList); m_closedList = parentCell->putOnClosedList(m_closedList); if (!startCell->allocateInfo(ndx)) { - cleanOpenAndClosedLists(); - goalCell->releaseInfo(); + //TheSuperHackers @info We need to forcefully cleanup dangling pathfinding cells if this failure condition is hit in retail + //Retail clients will crash beyond this point, but we attempt to recover by performing a full cleanup then enabling the fixed pathfinding codepath + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); + } + else { + s_useFixedPathfinding = true; + PathfindCellInfo::forceCleanPathFindInfoCells(); + forceCleanCells(); + m_openList = NULL; + m_closedList = NULL; + } return NULL; } startCell->setParentCellHierarchical(parentCell); @@ -7624,8 +7765,19 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu cellCount++; if(!cell->allocateInfo(toNdx)) { - cleanOpenAndClosedLists(); - goalCell->releaseInfo(); + //TheSuperHackers @info We need to forcefully cleanup dangling pathfinding cells if this failure condition is hit in retail + //Retail clients will crash beyond this point, but we attempt to recover by performing a full cleanup then enabling the fixed pathfinding codepath + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); + } + else { + s_useFixedPathfinding = true; + PathfindCellInfo::forceCleanPathFindInfoCells(); + forceCleanCells(); + m_openList = NULL; + m_closedList = NULL; + } return NULL; } curCost = cell->costToHierGoal(parentCell); @@ -7712,8 +7864,19 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu if (startCell==NULL) continue; if (startCell != parentCell) { if(!startCell->allocateInfo(ndx)) { - cleanOpenAndClosedLists(); - goalCell->releaseInfo(); + //TheSuperHackers @info We need to forcefully cleanup dangling pathfinding cells if this failure condition is hit in retail + //Retail clients will crash beyond this point, but we attempt to recover by performing a full cleanup then enabling the fixed pathfinding codepath + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); + } + else { + s_useFixedPathfinding = true; + PathfindCellInfo::forceCleanPathFindInfoCells(); + forceCleanCells(); + m_openList = NULL; + m_closedList = NULL; + } return NULL; } startCell->setParentCellHierarchical(parentCell); @@ -7722,8 +7885,19 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu } } if(!cell->allocateInfo(toNdx)) { - cleanOpenAndClosedLists(); - goalCell->releaseInfo(); + //TheSuperHackers @info We need to forcefully cleanup dangling pathfinding cells if this failure condition is hit in retail + //Retail clients will crash beyond this point, but we attempt to recover by performing a full cleanup then enabling the fixed pathfinding codepath + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); + } + else { + s_useFixedPathfinding = true; + PathfindCellInfo::forceCleanPathFindInfoCells(); + forceCleanCells(); + m_openList = NULL; + m_closedList = NULL; + } return NULL; } cell->setParentCellHierarchical(startCell); @@ -7779,9 +7953,17 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu #endif } #endif - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } else { + if (goalCell->hasInfo() && !goalCell->getClosed() && !goalCell->getOpen()) { + goalCell->releaseInfo(); + } + parentCell->releaseInfo(); + cleanOpenAndClosedLists(); + } return path; } @@ -7978,9 +8160,16 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu } #endif #endif - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } else { + if (goalCell->hasInfo() && !goalCell->getClosed() && !goalCell->getOpen()) { + goalCell->releaseInfo(); + } + cleanOpenAndClosedLists(); + } return path; } @@ -8027,9 +8216,15 @@ Path *Pathfinder::internal_findHierarchicalPath( Bool isHuman, const LocomotorSu TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } else { + goalCell->releaseInfo(); + cleanOpenAndClosedLists(); + } + return NULL; } @@ -8293,6 +8488,9 @@ Bool Pathfinder::pathDestination( Object *obj, const LocomotorSet& locomotorSet if (parentCell!=goalCell) { if (!parentCell->allocateInfo(startCellNdx)) { + if (!s_useFixedPathfinding) { + desiredCell->releaseInfo(); + } goalCell->releaseInfo(); return FALSE; } @@ -8407,8 +8605,10 @@ Bool Pathfinder::pathDestination( Object *obj, const LocomotorSet& locomotorSet if (!newCell->allocateInfo(newCellCoord)) { // Out of cells for pathing... - cleanOpenAndClosedLists(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); + } return cellCount; } cellCount++; @@ -8465,9 +8665,15 @@ Bool Pathfinder::pathDestination( Object *obj, const LocomotorSet& locomotorSet } #endif m_isTunneling = false; - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } else { + cleanOpenAndClosedLists(); + goalCell->releaseInfo(); + } + return false; } @@ -8593,16 +8799,25 @@ Int Pathfinder::checkPathCost(Object *obj, const LocomotorSet& locomotorSet, con parentCell = m_openList; m_openList = parentCell->removeFromOpenList(m_openList); + // put parent cell onto closed list - its evaluation is finished - Retail compatible behaviour + if (!s_useFixedPathfinding) { + m_closedList = parentCell->putOnClosedList( m_closedList ); + } + if (parentCell==goalCell) { Int cost = parentCell->getTotalCost(); m_isTunneling = false; cleanOpenAndClosedLists(); - parentCell->releaseInfo(); + if (s_useFixedPathfinding) { + parentCell->releaseInfo(); + } return cost; } - // put parent cell onto closed list - its evaluation is finished - m_closedList = parentCell->putOnClosedList( m_closedList ); + // put parent cell onto closed list - its evaluation is finished - Fixed behaviour + if (s_useFixedPathfinding) { + m_closedList = parentCell->putOnClosedList( m_closedList ); + } if (cellCount > MAX_CELL_COUNT) { continue; @@ -8664,11 +8879,13 @@ Int Pathfinder::checkPathCost(Object *obj, const LocomotorSet& locomotorSet, con if (!newCell->allocateInfo(newCellCoord)) { // Out of cells for pathing... - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } return cellCount; - } + } cellCount++; newCostSoFar = newCell->costSoFar( parentCell ); @@ -8715,9 +8932,16 @@ Int Pathfinder::checkPathCost(Object *obj, const LocomotorSet& locomotorSet, con } m_isTunneling = false; - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } else { + if (goalCell->hasInfo() && !goalCell->getClosed() && !goalCell->getOpen()) { + goalCell->releaseInfo(); + } + cleanOpenAndClosedLists(); + } return MAX_COST; } @@ -8854,7 +9078,9 @@ Path *Pathfinder::findClosestPath( Object *obj, const LocomotorSet& locomotorSet if (parentCell!=goalCell) { worldToCell(&clipFrom, &pos2d); if (!parentCell->allocateInfo(pos2d)) { - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + goalCell->releaseInfo(); + } return NULL; } } @@ -8920,9 +9146,16 @@ Path *Pathfinder::findClosestPath( Object *obj, const LocomotorSet& locomotorSet m_isTunneling = false; // construct and return path Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), from, goalCell, centerInCell, blocked); - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } else { + parentCell->releaseInfo(); + goalCell->releaseInfo(); + cleanOpenAndClosedLists(); + } + return path; } // put parent cell onto closed list - its evaluation is finished @@ -9002,9 +9235,14 @@ Path *Pathfinder::findClosestPath( Object *obj, const LocomotorSet& locomotorSet rawTo->y = closesetCell->getYIndex()*PATHFIND_CELL_SIZE_F + PATHFIND_CELL_SIZE_F/2.0f; // construct and return path Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), from, closesetCell, centerInCell, blocked ); - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } else { + goalCell->releaseInfo(); + cleanOpenAndClosedLists(); + } return path; } @@ -9025,9 +9263,14 @@ Path *Pathfinder::findClosestPath( Object *obj, const LocomotorSet& locomotorSet TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } else { + goalCell->releaseInfo(); + cleanOpenAndClosedLists(); + } return NULL; } @@ -9149,6 +9392,34 @@ void Pathfinder::prependCells( Path *path, const Coord3D *fromPos, } prevCell = cell; } + + // TheSuperHackers @info This pathway is here for retail compatibility, it is to catch when a starting cell has a dangling parent that contains no pathing information + // Beyond this point a retail client will crash due to a null pointer access within cell->getXIndex() + // To recover from this we set the cell to the previous cell, which should be the actual starting cell then set to use the fixed pathing and perform a forced cleanup + if (!s_useFixedPathfinding) { + if (!cell->hasInfo()) { + cell = prevCell; + + m_zoneManager.setPassable(cell->getXIndex(), cell->getYIndex(), true); + if (goalCellNull) { + // Very short path. + adjustCoordToCell(cell->getXIndex(), cell->getYIndex(), center, pos, cell->getLayer()); + path->prependNode( &pos, cell->getLayer() ); + } + // put actual start position as first node on the path, so it begins right at the unit's feet + if (fromPos->x != path->getFirstNode()->getPosition()->x || fromPos->y != path->getFirstNode()->getPosition()->y) { + path->prependNode( fromPos, cell->getLayer() ); + } + + s_useFixedPathfinding = true; + PathfindCellInfo::forceCleanPathFindInfoCells(); + forceCleanCells(); + m_openList = NULL; + m_closedList = NULL; + return; + } + } + m_zoneManager.setPassable(cell->getXIndex(), cell->getYIndex(), true); if (goalCellNull) { // Very short path. @@ -10412,8 +10683,13 @@ Path *Pathfinder::getMoveAwayFromPath(Object* obj, Object *otherObj, m_isTunneling = false; // construct and return path Path *newPath = buildActualPath( obj, locomotorSet.getValidSurfaces(), obj->getPosition(), parentCell, centerInCell, false); - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + } else { + parentCell->releaseInfo(); + cleanOpenAndClosedLists(); + } return newPath; } // put parent cell onto closed list - its evaluation is finished @@ -10435,8 +10711,12 @@ Path *Pathfinder::getMoveAwayFromPath(Object* obj, Object *otherObj, DEBUG_LOG(("Unit '%s', time %f\n", obj->getTemplate()->getName().str(), (::GetTickCount()-startTimeMS)/1000.0f)); m_isTunneling = false; - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + } else { + cleanOpenAndClosedLists(); + } return NULL; } @@ -10552,7 +10832,11 @@ Path *Pathfinder::patchPath( const Object *obj, const LocomotorSet& locomotorSet } if (startNode == originalPath->getLastNode()) { - parentCell->releaseInfo(); + if (s_useFixedPathfinding) { + parentCell->releaseInfo(); + } else { + cleanOpenAndClosedLists(); + } return NULL; // no open nodes. } PathfindCell *candidateGoal; @@ -10560,7 +10844,9 @@ Path *Pathfinder::patchPath( const Object *obj, const LocomotorSet& locomotorSet ICoord2D goalCellNdx; worldToCell(&goalPos, &goalCellNdx); if (!candidateGoal->allocateInfo(goalCellNdx)) { - parentCell->releaseInfo(); + if (s_useFixedPathfinding) { + parentCell->releaseInfo(); + } return NULL; } @@ -10595,9 +10881,15 @@ Path *Pathfinder::patchPath( const Object *obj, const LocomotorSet& locomotorSet // cleanup the path by checking line of sight path->optimize(obj, locomotorSet.getValidSurfaces(), blocked); - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - candidateGoal->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + candidateGoal->releaseInfo(); + } else { + parentCell->releaseInfo(); + cleanOpenAndClosedLists(); + candidateGoal->releaseInfo(); + } return path; } @@ -10621,9 +10913,18 @@ Path *Pathfinder::patchPath( const Object *obj, const LocomotorSet& locomotorSet } #endif m_isTunneling = false; - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - candidateGoal->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + candidateGoal->releaseInfo(); + } else { + if (!candidateGoal->getOpen() && !candidateGoal->getClosed()) + { + // Not on one of the lists + candidateGoal->releaseInfo(); + } + cleanOpenAndClosedLists(); + } return NULL; } @@ -10916,9 +11217,16 @@ Path *Pathfinder::findAttackPath( const Object *obj, const LocomotorSet& locomot } } Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), obj->getPosition(), parentCell, centerInCell, false); - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } else { + if (goalCell->hasInfo() && !goalCell->getClosed() && !goalCell->getOpen()) { + goalCell->releaseInfo(); + } + cleanOpenAndClosedLists(); + } return path; } } @@ -10975,9 +11283,16 @@ Path *Pathfinder::findAttackPath( const Object *obj, const LocomotorSet& locomot TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); - goalCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + goalCell->releaseInfo(); + } else { + if (goalCell->hasInfo() && !goalCell->getClosed() && !goalCell->getOpen()) { + goalCell->releaseInfo(); + } + cleanOpenAndClosedLists(); + } return NULL; } @@ -11099,8 +11414,13 @@ Path *Pathfinder::findSafePath( const Object *obj, const LocomotorSet& locomotor #endif // construct and return path Path *path = buildActualPath( obj, locomotorSet.getValidSurfaces(), obj->getPosition(), parentCell, centerInCell, false); - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + } else { + parentCell->releaseInfo(); + cleanOpenAndClosedLists(); + } return path; } @@ -11135,8 +11455,12 @@ Path *Pathfinder::findSafePath( const Object *obj, const LocomotorSet& locomotor TheGameLogic->incrementOverallFailedPathfinds(); #endif m_isTunneling = false; - cleanOpenAndClosedLists(); - parentCell->releaseInfo(); + if (s_useFixedPathfinding) { + cleanOpenAndClosedLists(); + parentCell->releaseInfo(); + } else { + cleanOpenAndClosedLists(); + } return NULL; } From b016746d232e9dd3954c9d63b54ed309301b43ed Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Sun, 22 Jun 2025 15:45:06 +0100 Subject: [PATCH 7/7] [ZH] Catch dangling parent cell crash in PathfindCell::costSoFar() --- .../Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index c0a77478d8..c3150dfb0c 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -1812,6 +1812,16 @@ UnsignedInt PathfindCell::costSoFar( PathfindCell *parent ) Int numTurns = 0; PathfindCell *prevCell = parent->getParentCell(); if (prevCell) { + +#if RETAIL_COMPATIBLE_PATHFINDING + // TheSuperHackers @info this is a possible crash point in the retail pathfinding, we just prevent the crash at this point + // External code should catch the issue in another block and cleanup the pathfinding before switching to the fixed pathfinding. + if (!prevCell->hasInfo()) + { + return cost; + } +#endif + ICoord2D dir; dir.x = prevCell->getXIndex() - parent->getXIndex(); dir.y = prevCell->getYIndex() - parent->getYIndex();