unify(pathfinder): Sanitize and merge AIPathfind and dependencies by Mauller · Pull Request #2341 · TheSuperHackers/GeneralsGameCode

Mauller · 2026-02-22T17:08:52Z

This PR merged Aipathfind.h and Aipathfind.cpp

The majority of the merge has required conditionals being placed throughout the code, this is due to significant changes between zero hour and generals making the function incompatible.

The majority of the merges are safe and distinct, either directly inherited from zero hour and into generals, or cleaned up from generals into zero hour.

The most involved the majority merge and refactor involves the functions within the PathfindZoneManager, as zero hour changes large portions of this code. The code itself was a mess and i reverted the implementation of parts of it from while back to for loops as used within generals to make the code easier to understand.

Generals inherited some optimisations from zero hour within PathfindZoneManager but had to retain the original code in other places due to mismatches occuring.

Generals gained the boolean returning versions of the terrain object handling functions, although the generals code does not specifically use them. This was done as a cleaner merge.

Generals also gained some zero hour functions that now check for nulls.
Generals also gained some functional name changes from zero hour which required updating call sites.

Zero hour gained some debugging that was lost from generals and some cleaned up implementation and formatting.

EDIT: I also had to extend an enum in generals' AI.h due to debug changes merged back from zero hour.

I split the unification into commits to make reviewing it easier, the most strenuous commit is the third commit of the PathfindZoneManager. This is where a considerable amount of satinising and refactoring occurred between both implementations.

…risky merge

…ghboringCells, safe merge

…dge, safe merge

…State, safe merge

… merge

greptile-apps · 2026-02-22T17:12:20Z

Greptile Summary

This PR unifies AIPathfind implementation between Generals and Zero Hour codebases through extensive conditional compilation. The merge adds 1,208 lines while removing 373 across 10 files.

Major Changes:

Merged AIPathfind.cpp and AIPathfind.h with RTS_GENERALS/RTS_ZEROHOUR conditionals
Refactored PathfindZoneManager functions (reverted while to for loops for clarity)
Renamed quickDoesPathExist to clientSafeQuickDoesPathExist (updated 6 call sites)
Changed setTypeAsObstacle to return Bool instead of void
Added AI_DEBUG_ZONES enum to AI.h
Generals inherited Zero Hour optimizations and null-checking functions
Zero Hour gained debugging capabilities and cleaner formatting from Generals

Critical Issue:

Compilation failure: Orphaned else statement at line 2810 in Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp will break RTS_ZEROHOUR builds

Assessment:
The merge strategy is sound with clear separation via preprocessor conditionals. Most changes are mechanical (function renames, call site updates). The PathfindZoneManager refactoring improves readability. However, the orphaned else bug must be fixed before merge.

Confidence Score: 1/5

This PR cannot be merged due to a critical compilation error in the RTS_ZEROHOUR build
The orphaned else statement at line 2810 will cause immediate compilation failure when building with RTS_ZEROHOUR defined. While the overall merge strategy is sound and most changes are safe mechanical updates, this single syntax error blocks the PR completely until fixed
Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp requires immediate fix for the orphaned else statement at line 2810 before merge

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp	Major merge with conditionals, contains critical orphaned `else` compilation error at line 2810
GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp	Cleaned up implementation with formatting improvements, safe merges from Generals
Generals/Code/GameEngine/Include/GameLogic/AIPathfind.h	Header merge with function signature changes, returns boolean from `setTypeAsObstacle`

_{Last reviewed commit: 35a0d0a}

greptile-apps

_{9 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

greptile-apps

_{9 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-22T17:18:11Z

Generals/Code/GameEngine/Include/GameLogic/AIPathfind.h


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


typo: onlyk should be only

Suggested change

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

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: Generals/Code/GameEngine/Include/GameLogic/AIPathfind.h Line: 667 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 · 2026-02-22T17:18:13Z

Generals/Code/GameEngine/Include/GameLogic/AIPathfind.h

+#if RTS_GENERALS
+	Bool			m_needToCalculateZones;		///< True if terrain has changed.
+#else
+	UnsignedInt		m_nextFrameToCalculateZones;		///< WHen should I recalculate, next?.


typo: WHen should be When

Suggested change

UnsignedInt m_nextFrameToCalculateZones; ///< WHen should I recalculate, next?.

UnsignedInt m_nextFrameToCalculateZones; ///< When should I recalculate, next?.

Prompt To Fix With AI

This is a comment left during a code review. Path: Generals/Code/GameEngine/Include/GameLogic/AIPathfind.h Line: 585 Comment: typo: `WHen` should be `When` ```suggestion UnsignedInt m_nextFrameToCalculateZones; ///< When should I recalculate, next?. ``` How can I resolve this? If you propose a fix, please make it concise.

Generals/Code/GameEngine/Include/GameLogic/AIPathfind.h

GeneralsMD/Code/GameEngine/Include/GameLogic/AIPathfind.h

Mauller · 2026-02-22T17:31:44Z

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

Skyaero42 · 2026-02-23T08:04:28Z

Generals/Code/GameEngine/Include/GameLogic/AIPathfind.h

+		CELL_OBSTACLE	= 0x04,									///< Occupied by a structure
+		CELL_BRIDGE_IMPASSABLE = 0x05,				///< Piece of a bridge that is impassable.
+#if RTS_ZEROHOUR
+		CELL_IMPASSABLE = 0x06								///< Just plain impassable except for aircraft.


These are not xfered I think. Would it be an issue if Generals uses 0x06 anyways?

It's more of a map data issue, the map files also contain data on types of tiles that have been painted onto the map manually.

Skyaero42 · 2026-02-23T08:10:26Z

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

 {
+#if RTS_GENERALS
 	DEBUG_ASSERTCRASH(m_info, ("Has to have info."));
+#else


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.

Skyaero42 · 2026-02-23T11:47:54Z

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

+			  return;
+		}
+	}
+#endif


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 · 2026-02-23T11:57:26Z

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

 				// See if the goal is a valid destination.  If not, accept closest cell.
 				if (closesetCell!=nullptr && !canPathThroughUnits && !checkDestination(obj, parentCell->getXIndex(), parentCell->getYIndex(), parentCell->getLayer(), radius, centerInCell)) {
+#if RTS_GENERALS
 					break;


If you set foundGoal = true before the break then line 8767

Bool foundGoal = false;

doesn't have to be in RTS_ZEROHOUR

and lines 8875-8882

// If we haven't already found the goal cell, continue examining. [8/25/2003] if (!foundGoal) { // Check to see if we can change layers in this cell. checkChangeLayers(parentCell); count += examineNeighboringCells(parentCell, goalCell, locomotorSet, isHuman, centerInCell, radius, startCellNdx, obj, NO_ATTACK); }

can also do without guards, simplifying the merge.

Did you mean if i don't set foundGoal before the break, so it remains false? that way the code enters into the block with checkChangeLayers in the generals code like it original would have?

made the change and no mismatching

Skyaero42 · 2026-02-23T16:44:50Z

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

 		crusher, scanCell.x, scanCell.y, m_map)) {
 		PathfindCell *newCell = getCell(LAYER_GROUND, scanCell.x, scanCell.y);
+#if RTS_GENERALS
 		if (newCell->hasInfo() && (newCell->getOpen() || newCell->getClosed())) return; // already looked at this one.


Is this a bug in Generals, where it should have been !newCell->hasInfo?

There can be a behavioural difference here, since if newcell does not have info we won't return early in the generals version and instead we will allocate data for it further down before accessing it

The call to GetZone also does not make use of m_info so it won't crash at this point.

Skyaero42 · 2026-02-23T20:12:52Z

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

+	}
+#endif
+#if RTS_GENERALS
+	m_needToCalculateZones = false;


See earlier comments

Skyaero42 · 2026-02-23T20:13:58Z

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

+#else
+	if (zone >= m_maxZone) {
+		DEBUG_CRASH(("Invalid zone."));
+		return UNINITIALIZED_ZONE;


UNINITIALIZED_ZONE equals 0. Code can be unified without guards.

Skyaero42 · 2026-02-23T20:19:09Z

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

+#else
+	m_zoneManager.updateZonesForModify(m_map, m_layers, cellBounds, m_extent);
+
+	Int i, j;


Move Int i, j outside of guards

Skyaero42 · 2026-02-23T20:20:34Z

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

 							empty = false;
 							break;
+#if RTS_ZEROHOUR
+						case PathfindCell::CELL_BRIDGE_IMPASSABLE:


CELL_BRIDGE_IMPASSABLE is a ZH state that does not get set in generals. It is safe to add this case without guards.

yeah this should be fine, i left other code in some places in too for generals that considers CELL_BRIDGE_IMPASSABLE since i already merged the enumerations

Skyaero42 · 2026-02-23T20:23:40Z

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

 				if (newObstacle != nullptr && newObstacle->isKindOf(KINDOF_AIRFIELD))
+#else
+				if (newObstacle != nullptr && newObstacle->isKindOf(KINDOF_FS_AIRFIELD))
+#endif


Probably cannot be unified due to enumeration order changes and the enumeration be within xfer?

yes theres significant differences with KINDOF_AIRFIELD removed from zero hour

i don't think these can be merged as i think they are baked into data

Mauller · 2026-02-23T21:56:56Z

updated based on feedback

greptile-apps

_{10 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-23T21:59:59Z

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

+					applyZone(r_thisCell, r_topCell, m_crusherZones, m_maxZone);
+				}
+#else
+				else


orphaned else statement will cause compilation failure in RTS_ZEROHOUR build

The #if RTS_GENERALS branch (lines 2788-2808) contains independent if statements, not an if-else chain. When preprocessing switches to #else, this else keyword has no matching if statement.

Fix: change this to match the earlier pattern (line 2748) where the else comes after the closing brace of the type check if statement on line 2786

Prompt To Fix With AI

This is a comment left during a code review. Path: Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp Line: 2810 Comment: orphaned `else` statement will cause compilation failure in RTS_ZEROHOUR build The `#if RTS_GENERALS` branch (lines 2788-2808) contains independent `if` statements, not an if-else chain. When preprocessing switches to `#else`, this `else` keyword has no matching `if` statement. Fix: change this to match the earlier pattern (line 2748) where the `else` comes after the closing brace of the type check `if` statement on line 2786 How can I resolve this? If you propose a fix, please make it concise.

greptile-apps

_{10 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-23T22:13:08Z

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

+					applyZone(r_thisCell, r_topCell, m_crusherZones, m_maxZone);
+				}
+#else
+				else


orphaned else - will cause RTS_ZEROHOUR build to fail

The #if RTS_GENERALS block (lines 2788-2808) contains independent if statements, not an if-else chain. When preprocessing removes this block for RTS_ZEROHOUR, this else has no matching if.

Fix: Move else to line 2787 after the closing brace, matching the pattern at line 2748

Suggested change

else

}

else

{

#else

Prompt To Fix With AI

This is a comment left during a code review. Path: Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp Line: 2810 Comment: orphaned `else` - will cause RTS_ZEROHOUR build to fail The `#if RTS_GENERALS` block (lines 2788-2808) contains independent `if` statements, not an if-else chain. When preprocessing removes this block for RTS_ZEROHOUR, this `else` has no matching `if`. Fix: Move `else` to line 2787 after the closing brace, matching the pattern at line 2748 ```suggestion } else { #else ``` How can I resolve this? If you propose a fix, please make it concise.

Mauller added 7 commits February 22, 2026 12:15

unify(pathfinder): Merge Headers safe merges

f1e7716

unify(pathfinder) Merge IS_PASSABLE to costToHierGoal, safe merges

bce1b72

unify(pathfinder): Merge and sanatise PathfindZoneManager functions, …

9c927c3

…risky merge

unify(pathfinder): Merge classify functions, safe merges

067311e

unify(pathfinder): Merge chooseBestLocomotorForPosition to examineNei…

9a0d3af

…ghboringCells, safe merge

unify(pathfinder): Merge Hierarchical path functions to findBrokenBri…

f52559d

…dge, safe merge

unify(pathfinder): merge clientSafeQuickDoesPathExist to changeBridge…

7997e35

…State, safe merge

Mauller self-assigned this Feb 22, 2026

Mauller added Critical Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Unify Unifies code between Generals and Zero Hour labels Feb 22, 2026

unify(pathfinder): Merge functions updateGoal to Findattackpath, safe…

ca72e33

… merge

Mauller force-pushed the Mauller/unify-aipathfind-easyparts branch from 60286bd to e20892a Compare February 22, 2026 17:09

greptile-apps bot reviewed Feb 22, 2026

View reviewed changes

Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp Show resolved Hide resolved

Unify(pathfinder): merge copyright header

180cc40

Mauller force-pushed the Mauller/unify-aipathfind-easyparts branch from e20892a to 180cc40 Compare February 22, 2026 17:15

greptile-apps bot reviewed Feb 22, 2026

View reviewed changes

bobtista reviewed Feb 22, 2026

View reviewed changes

Generals/Code/GameEngine/Include/GameLogic/AIPathfind.h Show resolved Hide resolved

bobtista reviewed Feb 22, 2026

View reviewed changes

GeneralsMD/Code/GameEngine/Include/GameLogic/AIPathfind.h Show resolved Hide resolved

bobtista reviewed Feb 22, 2026

View reviewed changes

GeneralsMD/Code/GameEngine/Include/GameLogic/AIPathfind.h Show resolved Hide resolved

unify(pathfinder): Add AI.h enumeration dependency for merge

2b407e0

Skyaero42 changed the title ~~unify(pathfinder): Sanatize and merge AIPathfind and dependencies~~ unify(pathfinder): Sanitize and merge AIPathfind and dependencies Feb 23, 2026

Skyaero42 requested changes Feb 23, 2026

View reviewed changes

greptile-apps bot reviewed Feb 23, 2026

View reviewed changes

Feedback Update 1

35a0d0a

Mauller force-pushed the Mauller/unify-aipathfind-easyparts branch from 75d3750 to 35a0d0a Compare February 23, 2026 22:09

greptile-apps bot reviewed Feb 23, 2026

View reviewed changes

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

	UnsignedInt m_nextFrameToCalculateZones; ///< WHen should I recalculate, next?.
	UnsignedInt m_nextFrameToCalculateZones; ///< When should I recalculate, next?.

-				else
+				}
+				else
+				{
+#else

Comments

Conversation

Mauller commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 1/5

Important Files Changed

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mauller commented Feb 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mauller Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mauller commented Feb 23, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 23, 2026

Choose a reason for hiding this comment

Mauller commented Feb 22, 2026 •

edited

Loading

greptile-apps bot commented Feb 22, 2026 •

edited

Loading

Mauller Feb 23, 2026 •

edited

Loading