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

[Mauller]

This PR implements a new and more robust forward insertion sort function.

This code is only retail compatible up to the point where a pathfinding crash would normally occur.
This is why the original code is maintained as a retail compatible pathway.

This code builds part of the foundation of the skip list that will be coming in further changes.

[greptile-apps]

Greptile Summary

This PR refactors PathfindCell::putOnSortedOpenList to split its logic into two explicit helpers — forwardInsertionSortRetailCompatible (retail-compatible, bounded loop) and forwardInsertionSort (fixed, unbounded) — and routes between them based on the s_useFixedPathfinding flag (when RETAIL_COMPATIBLE_PATHFINDING is defined). The change is described as groundwork for a future skip-list optimisation.

Key changes:

• The old inline #if RETAIL_COMPATIBLE_PATHFINDING / #else block inside putOnSortedOpenList is replaced by two clearly-named free methods.
• forwardInsertionSort is a clean doubly-linked insertion sort; the pointer updates for all three cases (empty list, prepend, mid/tail insert) are logically correct and equivalent in ordering behaviour to the retail-compatible path.
• Both helpers are exposed as public members, which is intentional per the PR description (skip-list foundation).

Style issues found:

• forwardInsertionSort uses K&R brace placement while forwardInsertionSortRetailCompatible and the surrounding file region use Allman style — these should be consistent.
• The @bugfix comment that explained why the PATHFIND_CELLS_PER_FRAME iteration cap exists in the retail-compatible path was dropped during the refactor; restoring it would preserve important safety context for future readers.

Confidence Score: 4/5

• Safe to merge with minor style clean-up; the core insertion sort logic is correct and well-structured.
• The doubly-linked list manipulation in forwardInsertionSort is verified correct across all cases (empty list, prepend, equal-cost, mid/tail insert), matching the ordering semantics of the retail-compatible variant. No logic bugs, null-pointer risks, or nullptr/NULL violations were found. The deduction from a perfect 5 reflects two style-only issues: inconsistent brace placement in the new function and the loss of an explanatory comment during refactoring.
• No files require special attention — both style issues are confined to Core/GameEngine/Source/GameLogic/AI/AIPathfind.cpp.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameLogic/AIPathfind.h	Adds public declarations for forwardInsertionSortRetailCompatible (guarded by RETAIL_COMPATIBLE_PATHFINDING) and forwardInsertionSort. Both are exposed as public methods, which the PR justifies as groundwork for the upcoming skip list. No license, macro-guard, or nullptr issues found.
Core/GameEngine/Source/GameLogic/AI/AIPathfind.cpp	Refactors the old putOnSortedOpenList into two clearly-separated helpers. The new forwardInsertionSort is logically correct (all linked-list pointer updates are sound, equal-cost ordering matches the retail path). Two style issues: brace placement in forwardInsertionSort differs from the Allman style used in forwardInsertionSortRetailCompatible, and the explanatory @bugfix comment about the infinite-loop guard was dropped during the refactor.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["putOnSortedOpenList(list)"] --> B{RETAIL_COMPATIBLE\n_PATHFINDING?}
    B -- No --> F["forwardInsertionSort(list)"]
    B -- Yes --> C{s_useFixedPathfinding?}
    C -- false --> D["forwardInsertionSortRetailCompatible(list)\n(with PATHFIND_CELLS_PER_FRAME guard)"]
    C -- true --> F

    F --> G{list.m_head\n== nullptr?}
    G -- Yes --> H["Set as sole node\n(prevOpen=nullptr, nextOpen=nullptr)\nlist.m_head = this"]
    G -- No --> I{totalCost <\nhead->totalCost?}
    I -- Yes --> J["Prepend:\nupdate head->prevOpen,\nset nextOpen=old head,\nlist.m_head = this"]
    I -- No --> K["Traverse: advance while\nnextOpen != null &&\nnextOpen->totalCost <= totalCost"]
    K --> L["Insert after 'current':\nlink nextOpen / prevOpen\npointers"]

Loading

_{Last reviewed commit: cdce1d3}

[Skyaero42]

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

[Mauller]

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.

[Mauller]

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

[Mauller]

Tweaked based on feedback

[Skyaero42]

This is good to go

[xezon]

Looks plausible.