From 5312fb0a4dfac67ae2e6869cb84780860393e7ce Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Fri, 20 Feb 2026 18:20:46 +0000 Subject: [PATCH] reafactor(pathfinder): Cleanup retail compatible insertion sort code (#2331) --- .../Source/GameLogic/AI/AIPathfind.cpp | 82 +++++++++---------- .../Source/GameLogic/AI/AIPathfind.cpp | 82 +++++++++---------- 2 files changed, 80 insertions(+), 84 deletions(-) diff --git a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index f74387efb0e..8268fe9cbeb 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -1668,62 +1668,60 @@ void PathfindCell::removeObstacle( Object *obstacle ) void PathfindCell::putOnSortedOpenList( PathfindCellList &list ) { DEBUG_ASSERTCRASH(m_info, ("Has to have info.")); - DEBUG_ASSERTCRASH(m_info->m_closed==FALSE && m_info->m_open==FALSE, ("Serious error - Invalid flags. jba")); + DEBUG_ASSERTCRASH(m_info->m_closed == FALSE && m_info->m_open == FALSE, ("Serious error - Invalid flags. jba")); + + // mark the newCell as being on the open list + m_info->m_open = true; + m_info->m_closed = false; + if (list.m_head == nullptr) { list.m_head = this; m_info->m_prevOpen = nullptr; m_info->m_nextOpen = nullptr; + return; } - else - { - // insertion sort - PathfindCell *c, *lastCell = nullptr; + + // insertion sort + PathfindCell* currentCell = list.m_head; + PathfindCell* previousCell = nullptr; #if RETAIL_COMPATIBLE_PATHFINDING - // TheSuperHackers @bugfix In the retail compatible pathfinding, on rare occasions, we get stuck in an infinite loop - // External code should pickup on the bad behaviour and cleanup properly, but we need to explicitly break out here - // The fixed pathfinding does not have this issue due to the proper cleanup of pathfindCells and their pathfindCellInfos - UnsignedInt cellCount = 0; - for (c = list.m_head; c && cellCount < PATHFIND_CELLS_PER_FRAME; c = c->getNextOpen()) - { - cellCount++; + // TheSuperHackers @bugfix In the retail compatible pathfinding, on rare occasions, we get stuck in an infinite loop + // External code should pickup on the bad behaviour and cleanup properly, but we need to explicitly break out here + // The fixed pathfinding does not have this issue due to the proper cleanup of pathfindCells and their pathfindCellInfos + UnsignedInt cellCount = 0; + while (currentCell && cellCount < PATHFIND_CELLS_PER_FRAME && currentCell->m_info->m_totalCost <= m_info->m_totalCost) + { + cellCount++; #else - for (c = list.m_head; c; c = c->getNextOpen()) - { + while (currentCell && currentCell->m_info->m_totalCost <= m_info->m_totalCost) + { #endif - if (c->m_info->m_totalCost > m_info->m_totalCost) - break; - - lastCell = c; - } + previousCell = currentCell; + currentCell = currentCell->getNextOpen(); + } - if (c) - { - // insert just before "c" - if (c->m_info->m_prevOpen) - c->m_info->m_prevOpen->m_nextOpen = this->m_info; - else - list.m_head = this; + if (currentCell) + { + // insert just before "currentCell" + if (currentCell->m_info->m_prevOpen) + currentCell->m_info->m_prevOpen->m_nextOpen = this->m_info; + else + list.m_head = this; - m_info->m_prevOpen = c->m_info->m_prevOpen; - c->m_info->m_prevOpen = this->m_info; + m_info->m_prevOpen = currentCell->m_info->m_prevOpen; + currentCell->m_info->m_prevOpen = this->m_info; - m_info->m_nextOpen = c->m_info; + m_info->m_nextOpen = currentCell->m_info; - } - else - { - // append after "lastCell" - end of list - lastCell->m_info->m_nextOpen = this->m_info; - m_info->m_prevOpen = lastCell->m_info; - m_info->m_nextOpen = nullptr; - } } - - // mark newCell as being on open list - m_info->m_open = true; - m_info->m_closed = false; - + else + { + // append after "previousCell" - we are at the end of the list + previousCell->m_info->m_nextOpen = this->m_info; + m_info->m_prevOpen = previousCell->m_info; + m_info->m_nextOpen = nullptr; + } } /// remove self from "open" list diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index 564cff7d769..5e97719ee85 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -1685,62 +1685,60 @@ Bool PathfindCell::removeObstacle( Object *obstacle ) void PathfindCell::putOnSortedOpenList( PathfindCellList &list ) { DEBUG_ASSERTCRASH(m_info, ("Has to have info.")); - DEBUG_ASSERTCRASH(m_info->m_closed==FALSE && m_info->m_open==FALSE, ("Serious error - Invalid flags. jba")); + DEBUG_ASSERTCRASH(m_info->m_closed == FALSE && m_info->m_open == FALSE, ("Serious error - Invalid flags. jba")); + + // mark the newCell as being on the open list + m_info->m_open = true; + m_info->m_closed = false; + if (list.m_head == nullptr) { list.m_head = this; m_info->m_prevOpen = nullptr; m_info->m_nextOpen = nullptr; + return; } - else - { - // insertion sort - PathfindCell *c, *lastCell = nullptr; + + // insertion sort + PathfindCell* currentCell = list.m_head; + PathfindCell* previousCell = nullptr; #if RETAIL_COMPATIBLE_PATHFINDING - // TheSuperHackers @bugfix In the retail compatible pathfinding, on rare occasions, we get stuck in an infinite loop - // External code should pickup on the bad behaviour and cleanup properly, but we need to explicitly break out here - // The fixed pathfinding does not have this issue due to the proper cleanup of pathfindCells and their pathfindCellInfos - UnsignedInt cellCount = 0; - for (c = list.m_head; c && cellCount < PATHFIND_CELLS_PER_FRAME; c = c->getNextOpen()) - { - cellCount++; + // TheSuperHackers @bugfix In the retail compatible pathfinding, on rare occasions, we get stuck in an infinite loop + // External code should pickup on the bad behaviour and cleanup properly, but we need to explicitly break out here + // The fixed pathfinding does not have this issue due to the proper cleanup of pathfindCells and their pathfindCellInfos + UnsignedInt cellCount = 0; + while (currentCell && cellCount < PATHFIND_CELLS_PER_FRAME && currentCell->m_info->m_totalCost <= m_info->m_totalCost) + { + cellCount++; #else - for (c = list.m_head; c; c = c->getNextOpen()) - { + while (currentCell && currentCell->m_info->m_totalCost <= m_info->m_totalCost) + { #endif - if (c->m_info->m_totalCost > m_info->m_totalCost) - break; - - lastCell = c; - } + previousCell = currentCell; + currentCell = currentCell->getNextOpen(); + } - if (c) - { - // insert just before "c" - if (c->m_info->m_prevOpen) - c->m_info->m_prevOpen->m_nextOpen = this->m_info; - else - list.m_head = this; + if (currentCell) + { + // insert just before "currentCell" + if (currentCell->m_info->m_prevOpen) + currentCell->m_info->m_prevOpen->m_nextOpen = this->m_info; + else + list.m_head = this; - m_info->m_prevOpen = c->m_info->m_prevOpen; - c->m_info->m_prevOpen = this->m_info; + m_info->m_prevOpen = currentCell->m_info->m_prevOpen; + currentCell->m_info->m_prevOpen = this->m_info; - m_info->m_nextOpen = c->m_info; + m_info->m_nextOpen = currentCell->m_info; - } - else - { - // append after "lastCell" - end of list - lastCell->m_info->m_nextOpen = this->m_info; - m_info->m_prevOpen = lastCell->m_info; - m_info->m_nextOpen = nullptr; - } } - - // mark newCell as being on open list - m_info->m_open = true; - m_info->m_closed = false; - + else + { + // append after "previousCell" - we are at the end of the list + previousCell->m_info->m_nextOpen = this->m_info; + m_info->m_prevOpen = previousCell->m_info; + m_info->m_nextOpen = nullptr; + } } /// remove self from "open" list