Skip to content

fix(pathfinder): Implement robust forward insertion sort for PathfindCell::putOnSortedOpenList() #2432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
xezon merged 2 commits into from
Mar 12, 2026
Merged

fix(pathfinder): Implement robust forward insertion sort for PathfindCell::putOnSortedOpenList() #2432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6abe245
chore(pathfinder): Extract retail compatible insertion sort code into…
Mauller Mar 9, 2026
cdce1d3
fix(pathfinder): Implement new robust forward insertion sort code
Mauller Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Core/GameEngine/Include/GameLogic/AIPathfind.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,14 @@ class PathfindCell

UnsignedInt costSoFar( PathfindCell *parent );

#if RETAIL_COMPATIBLE_PATHFINDING
// Forward insertion sort that is 100% retail compatible
void forwardInsertionSortRetailCompatible(PathfindCellList& list);
#endif

// Forward insertion sort, in ascending cost order
void forwardInsertionSort(PathfindCellList& list);

/// put self on "open" list in ascending cost order
void putOnSortedOpenList( PathfindCellList &list );

Expand Down
68 changes: 58 additions & 10 deletions Core/GameEngine/Source/GameLogic/AI/AIPathfind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1680,8 +1680,9 @@ Bool PathfindCell::removeObstacle( Object *obstacle )
return true;
}

/// put self on "open" list in ascending cost order, return new list
void PathfindCell::putOnSortedOpenList( PathfindCellList &list )
#if RETAIL_COMPATIBLE_PATHFINDING
// Retail compatible insertion sort
void PathfindCell::forwardInsertionSortRetailCompatible(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"));
Expand All @@ -1701,18 +1702,10 @@ void PathfindCell::putOnSortedOpenList( PathfindCellList &list )
// 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;
while (currentCell && cellCount < PATHFIND_CELLS_PER_FRAME && currentCell->m_info->m_totalCost <= m_info->m_totalCost)
{
cellCount++;
#else
while (currentCell && currentCell->m_info->m_totalCost <= m_info->m_totalCost)
{
#endif
previousCell = currentCell;
currentCell = currentCell->getNextOpen();
}
Expand All @@ -1739,6 +1732,61 @@ void PathfindCell::putOnSortedOpenList( PathfindCellList &list )
m_info->m_nextOpen = nullptr;
}
}
#endif

// Forward insertion sort, returns early if the list is being initialised or we are prepending the list
void PathfindCell::forwardInsertionSort(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"));

// mark the new cell as being on the open list
m_info->m_open = true;
m_info->m_closed = false;

if (list.m_head == nullptr) {
m_info->m_prevOpen = nullptr;
m_info->m_nextOpen = nullptr;
list.m_head = this;
return;
}

// If the node needs inserting before the current list head
if (m_info->m_totalCost < list.m_head->m_info->m_totalCost) {
m_info->m_prevOpen = nullptr;
list.m_head->m_info->m_prevOpen = this->m_info;
m_info->m_nextOpen = list.m_head->m_info;
list.m_head = this;
return;
}

// Traverse the list to find correct position
PathfindCell* current = list.m_head;
while (current->m_info->m_nextOpen && current->m_info->m_nextOpen->m_totalCost <= m_info->m_totalCost) {
Copy link

@Skyaero42 Skyaero42 Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This is a bit of a hard read, but it is correct.
It is somewhat confusing with the retail-version, but basically previousCell (retail) = current and currentCell (retail) = current->m_nextOpen

Copy link
Author

@Mauller Mauller Mar 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes compared to the retail it seems more confusing, but it is also algorithmically more stable and doesn't require the second variable to be tracked.

head insertion is handled before the loop and any subsequent insertions are handled inside the loop.

Copy link
Author

@Mauller Mauller Mar 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This design also makes extending this into a skip list easier as well without needing the multiple loop bodged earlier variants i created

current = current->getNextOpen();
}

// Insert the new node in the correct position
m_info->m_nextOpen = current->m_info->m_nextOpen;
if (current->m_info->m_nextOpen != nullptr) {
current->m_info->m_nextOpen->m_prevOpen = this->m_info;
}
current->m_info->m_nextOpen = this->m_info;
m_info->m_prevOpen = current->m_info;
}

/// put self on "open" list in ascending cost order, return new list
void PathfindCell::putOnSortedOpenList( PathfindCellList &list )
{
#if RETAIL_COMPATIBLE_PATHFINDING
if (!s_useFixedPathfinding) {
forwardInsertionSortRetailCompatible(list);
return;
}
#endif

forwardInsertionSort(list);
}

/// remove self from "open" list
void PathfindCell::removeFromOpenList( PathfindCellList &list )
Expand Down
Loading