Skip to content

refactor(pathfinder): Cleanup retail compatible insertion sort code #2331

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 1 commit into from
Feb 21, 2026
Merged

refactor(pathfinder): Cleanup retail compatible insertion sort code #2331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
82 changes: 40 additions & 42 deletions Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
82 changes: 40 additions & 42 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

@xezon xezon Feb 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If I am not mistaken you can write it like so:

#if RETAIL_COMPATIBLE_PATHFINDING
UnsignedInt cellCount = 0;
#endif

while (currentCell && currentCell->m_info->m_totalCost <= m_info->m_totalCost)
{
#if RETAIL_COMPATIBLE_PATHFINDING
	if (++cellCount >= PATHFIND_CELLS_PER_FRAME)
		break;
#endif

	previousCell = currentCell;
	currentCell = currentCell->getNextOpen();
}

Looks a bit cleaner.

@Mauller Mauller Feb 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, this will get cleaned up in a subsequent PR.

All of the functionality in putOnOpenSortedList is going to get extracted into a different function, which will then be wrapped in a retail compatible conditional entirely.

This PR was just to cleanup the implementation a bit before the bigger refactor where i do the above and add the not fully retail but more robust insertion sort code.

i will esentially be doing something like

putOnSortedList() <<--- This may even get renamed to just insert() and be a function on the new PathfindCellList in #2327
{ 
#if retail_compatible
if(!s_useFixedPathFinding) {
forwardInsertionSortRetailCompatible()
}
#endif

forwardInsertionSort()

}

But even this is an intermediate refactor before further changes.

( The hint here is that theres going to be a reverseInsertionSort coming up and the code within putOnSortedList / Insert will choose the relevant function etc )

@Mauller Mauller Feb 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I am just trying to split out these changes into stages due to the complexity being built up into the final implementation with the skip list.

{
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
Expand Down
Loading