unify(pathfinder): Merge AIPathfind and update dependencies

[Mauller]

Merge by rebase

This PR merges the remainder of Aipathfind.h and Aipathfind.cpp

I split the original version of this unification into seperate PR's to make the merging easier, this now deals with the remaining parts of the merger.

The following functions are merged here:

Pathfinder::clientSafeQuickDoesPathExist <- A Safe reordering adds an earlier escape for generals
Pathfinder::clientSafeQuickDoesPathExistForUI <- Entirely new from Zero Hour
These two also required code changes at call sites in generals due to the change of function name

Pathfinder::checkForPossible
Pathfinder::processPathfindQueue
Pathfinder::examineNeighboringCells
Pathfinder::buildHierarchicalPath

Pathfinder::moveAlliesDestinationCallback
Pathfinder::processHierarchicalCell <- Generals gains a m_info Null check which would prevent a crash
Pathfinder::findClosestPath
Pathfinder::updateGoal <- Some formatting cleanup on the Zero Hour side when merging

Pathfinder::moveAllies
Pathfinder::findAttackPath

[greptile-apps]

Greptile Summary

This PR successfully merges the remaining pathfinding functions from AIPathfind, unifying the codebase and porting Zero Hour improvements where appropriate. Key changes include:

• Function renames + call-site updates: quickDoesPathExist → clientSafeQuickDoesPathExist across 6 Generals call sites (AIUpdate, AIPlayer, AISkirmishPlayer, PartitionManager, GameLogicDispatch, and AIPathfind itself), with consistent name changes throughout.
• New UI path function: clientSafeQuickDoesPathExistForUI for terrain-only path checks (ignoring stealthed structures) for cursor feedback.
• Well-structured conditional merging: Zero Hour pathfinding improvements (Black Lotus exploit fix, idle-unit ally-movement guards, vehicle attack-path stripping, hierarchical-path zone expansion) are cleanly isolated behind #if !(RTS_GENERALS && RETAIL_COMPATIBLE_PATHFINDING) guards.
• Consistent call-site routing: Both Generals and GeneralsMD AIUpdate.cpp correctly route UI queries to clientSafeQuickDoesPathExistForUI and AI queries to clientSafeQuickDoesPathExist, with build conditionals applied appropriately.

The merge maintains backward compatibility for retail Generals builds while enabling improved pathfinding for non-retail configurations, and all function renames have been applied consistently across the codebase.

Confidence Score: 5/5

• PR is safe to merge with all mechanical call-site renames applied correctly and conditional logic properly isolated.
• The bulk of the changes are low-risk mechanical transformations: function renames applied consistently across 6 call sites, straightforward header declarations, and clear conditional compilation guards on ported features. All verified changes compile correctly and follow the project's established patterns for unifying Generals and Zero Hour code paths.
• No files require special attention.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Caller needs path check] --> B{UI query?\nisQuickPathAvailable}
    A --> C{AI/Logic query\nisPathAvailable / findPath / etc.}

    B --> D{RTS_GENERALS &&\nRETAIL_COMPATIBLE_PATHFINDING}
    D -- Yes --> E[clientSafeQuickDoesPathExist\nterrain + buildings check]
    D -- No --> F[clientSafeQuickDoesPathExistForUI\nterrain-only check\nignores stealthed structures]

    C --> E

    E --> G{validMovementPosition\nearly escape}
    G -- false --> H[return false]
    G -- true --> I{Zones equal?}
    I -- Yes --> J[return true]
    I -- No --> K[return FALSE]

    F --> L{Initial zone\nUNINITIALIZED?}
    L -- Yes --> M[return true\nfalse-positive safe]
    L -- No --> N{Terrain zone\nrecalculation}
    N --> O{zone1 UNINITIALIZED\nafter recalc?}
    O -- Yes --> M
    O -- No --> P{zone1 == zone2?}
    P -- Yes --> J
    P -- No --> K

    style H fill:#f88,stroke:#c00
    style K fill:#f88,stroke:#c00
    style J fill:#8f8,stroke:#0a0
    style M fill:#8f8,stroke:#0a0

Loading

_{Last reviewed commit: 3fdd461}

[greptile-apps]

_{10 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{9 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[Mauller]

Had to fix a small dependency that i missed when building debug in generals

[Skyaero42]

If m_info is not initialized, this crashes in Generals when m_info->m_pos.x is called.
Therefore the 1.01 patch can be ported to Generals. Game would have crashed at this point anyways, so it will not mismatch.

[xezon]

Perhaps just take Zero Hour code for both games?

[Skyaero42]

I think this can be simplified to:

	if (!ai->isDoingGroundMovement())
	{
#if RTS_GENERALS
		Bool isUnmannedHelicopter = false;
#else
		// exception:sniped choppers are on ground
		Bool isUnmannedHelicopter = ( obj->isKindOf( KINDOF_PRODUCED_AT_HELIPAD ) && obj->isDisabledByType( DISABLED_UNMANNED  ) ) ;
#endif
		if ( ! isUnmannedHelicopter )
		{
			  updateAircraftGoal(obj, newGoalPos);
			  return;
		}
	}

[Skyaero42]

See earlier comments

[Skyaero42]

Move Int i, j outside of guards

[Mauller]

updated based on feedback

[greptile-apps]

_{10 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{10 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{10 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

GeneralsMD/Code/GameEngine/Include/GameLogic/AIPathfind.h
typo: onlyk should be only

	Bool clientSafeQuickDoesPathExistForUI( const LocomotorSet& locomotorSet, const Coord3D *from, const Coord3D *to );  ///< Can we build any path at all between the locations	(terrain only - fast)

Prompt To Fix With AI

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Include/GameLogic/AIPathfind.h
Line: 659

Comment:
typo: `onlyk` should be `only`

```suggestion
	Bool clientSafeQuickDoesPathExistForUI( const LocomotorSet& locomotorSet, const Coord3D *from, const Coord3D *to );  ///< Can we build any path at all between the locations	(terrain only - fast)
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

_{10 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[xezon]

Is this meant to be Merged with Rebase? The title says "Sanitize" and "Merge" which implies 2 distinct steps and commits.

[xezon]

I find this review incredibly difficult. Why is the diff so big?

I do wonder if the merge needs to happen in steps, for example first add those variable r_thisCell etc, and then afterwards do the #ifdef RTS_GENERALS #else etc.

[xezon]

Perhaps just take Zero Hour code for both games?

[Mauller]

I find this review incredibly difficult. Why is the diff so big?

I do wonder if the merge needs to happen in steps, for example first add those variable r_thisCell etc, and then afterwards do the #ifdef RTS_GENERALS #else etc.

The dif is quite big because i had to change around large blocks of the zero hour and generals code within the pathfind zone manager to figure out what functionality matched. This was due to zero hour having large blocks of badly formatted refactored code, which changed parts from the generals implementation that were unecessary.

So from the zero hour code i reverted while blocks back to for blocks and cleaned up a lot of the formatting. Some of these optimisations then made their back back into generals with the r_thisCell changes since they didn't alter the functionality or mismatch.

[Mauller]

Is this meant to be Merged with Rebase? The title says "Sanitize" and "Merge" which implies 2 distinct steps and commits.

It can be squash merged, the title can be changed, i just considered it a bit of a sanitising as well due to the fixing of a fair bit of formatting from refactored zero hour code.

I split it into commits to make it easier to review changes

[xezon]

I do prefer committing small reviewable chunks to main branch. Makes it also easier to test later if something breaks. I think reviewability can be improved by doing merge in steps if some code has changed in more than one way. For example, if variables were renamed, formatting was changed, and code was added and removed, then edit these things in 3 separate commits. This will make the diffs much cleaner.

[Mauller]

I do prefer committing small reviewable chunks to main branch. Makes it also easier to test later if something breaks. I think reviewability can be improved by doing merge in steps if some code has changed in more than one way. For example, if variables were renamed, formatting was changed, and code was added and removed, then edit these things in 3 separate commits. This will make the diffs much cleaner.

well the biggest changes are due to pathfind zone manager, i could make a seperate PR just to cleanup the mess in that first before other unification and changes.

[xezon]

This change looks risky.

---

Do Generals 1.08 Replays pass without mismatching? I think some Generals replays will already fail before this change because we do not test and fix it much.

---

I am skeptical of the use of #if RTS_ZEROHOUR instead of #if !RTS_GENERALS.

---

Some #if block are difficult to read and maintain when constructed like so:

  if (...)
  {
  }
#if
  else
  {
    ...
  }
#else
  else
  {
    ...
  }
#endif

Consider a cleaner separation of #if blocks with no sneaky branch carry overs.

---

The commits would be better organized as logical units, as opposed to fixed regions of the code. Do we need so many individual commits? Do all commits compile in succession?

[Mauller]

The commits would be better organized as logical units, as opposed to fixed regions of the code. Do we need so many individual commits? Do all commits compile in succession?

I think i need to make one or two tweaks but they should compile and work when compiled in sequence.

I considered doing this as individual commits in case something did mismatch and it could be tracked back. But it can always be merged as a single commit if i just make sure any possible blocks that could mismatch are kept for generals

[Mauller]

Updated with the last of the Mergers needed for the Pathfinder

[xezon]

Looks okay but a few questions.

[xezon]

In Replay CRC we trust

[Mauller]

In Replay CRC we trust

I grabbed a Dozen more replays from GameReplays for Generals to test this, they all ran fine.
I made sure to add Twilight Flame to get bridges involved too.