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.

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

Greptile Summary

This PR successfully unifies the AIPathfind implementation between Generals and Zero Hour through conditional compilation (RTS_GENERALS/RTS_ZEROHOUR). The unification includes:

PathfindZoneManager refactoring: Converted while loops to for loops for improved readability and merged zone calculation logic with version-specific optimizations
Function signature updates: Changed setTypeAsObstacle and removeObstacle to return Bool values, and renamed quickDoesPathExist to clientSafeQuickDoesPathExist across all call sites
Zone management improvements: Generals inherited zone update frequency logic from Zero Hour, while Zero Hour gained debugging capabilities from Generals
Call site updates: Updated 6 call sites across the codebase to use the renamed pathfinding function

The changes are well-structured with clear conditional compilation boundaries. Previously raised concerns about "orphaned else statements" are not valid - the preprocessor logic is correct and the else at line 2802 properly pairs with the if at line 2777 when RTS_ZEROHOUR is defined.

Confidence Score: 4/5

Safe to merge with minor considerations - the unification is well-executed with proper conditional compilation
The code is architecturally sound with proper conditional compilation. The main AIPathfind.cpp files warrant extra attention due to complexity, but the refactoring from while to for loops improves clarity. Function renames are consistently applied. The previously flagged "orphaned else" issue is not a bug - the preprocessor logic is correct.
Pay close attention to Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp and GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp during testing, particularly the PathfindZoneManager zone calculation logic around lines 2720-2830

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Include/GameLogic/AIPathfind.h	Updated function signatures and added conditional compilation support for unification
Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp	Major refactoring of PathfindZoneManager with extensive conditional compilation for unification
GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp	Refactored PathfindZoneManager zone calculation from while loops to for loops for clarity

_{Last reviewed commit: 2ea0f8b}

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}

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

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

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; } }

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

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

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

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


See earlier comments

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

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

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

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

updated based on feedback

greptile-apps

_{10 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

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

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.

line 2785 - 2787 has the if block that links to the else when the #if RTS_GENERALS conditional block is not used

…risky merge

…ghboringCells, safe merge

…dge, safe merge

…State, safe merge

… merge

greptile-apps

_{10 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-25T17:44:44Z

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 compilation failure

The #if RTS_GENERALS block (lines 2788-2808) contains independent if statements, not an if-else chain. When the preprocessor evaluates this for RTS_ZEROHOUR builds, it removes lines 2788-2808 and keeps the #else block starting at line 2809. This leaves the else at line 2810 with no matching if statement (the matching if at line 2785 has already been closed at line 2787).

Move this else to line 2787 (after the closing brace), matching the pattern at line 2748.

Suggested change

else

}

else

#if RTS_GENERALS

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 compilation failure The `#if RTS_GENERALS` block (lines 2788-2808) contains independent `if` statements, not an if-else chain. When the preprocessor evaluates this for RTS_ZEROHOUR builds, it removes lines 2788-2808 and keeps the `#else` block starting at line 2809. This leaves the `else` at line 2810 with no matching `if` statement (the matching `if` at line 2785 has already been closed at line 2787). Move this `else` to line 2787 (after the closing brace), matching the pattern at line 2748. ```suggestion } else #if RTS_GENERALS ``` How can I resolve this? If you propose a fix, please make it concise.

line 2785 - 2787 has the if block that links to the else when the #if RTS_GENERALS conditional block is not used

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

greptile-apps · 2026-02-25T17:44:50Z

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


 	UnsignedShort m_maxZone;								///< Max zone used.
-	Bool					m_needToCalculateZones;		///< True if terrain has changed.
+	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: 578 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.

greptile-apps · 2026-02-25T17:44:51Z

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

@@ -576,6 +576,7 @@ class PathfindZoneManager

 	UnsignedShort m_maxZone;								///< Max zone used.
 	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: GeneralsMD/Code/GameEngine/Include/GameLogic/AIPathfind.h Line: 578 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.

greptile-apps · 2026-02-25T17:44:54Z

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}

greptile-apps · 2026-02-25T21:22:23Z

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

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


Critical compilation error: orphaned else statement when RTS_ZEROHOUR is defined.

When the preprocessor processes this for RTS_ZEROHOUR builds, it removes the #if RTS_GENERALS block (lines 2788-2808), leaving this else without a matching if statement. The if statement on line 2785 has already closed at line 2787.

The developer's claim that "line 2785 - 2787 has the if block that links to the else" is incorrect. Line 2787 closes the if block from line 2785, so when the #if RTS_GENERALS block is removed by the preprocessor, line 2810's else becomes orphaned.

Move the else to line 2787 (after the closing brace), matching the working pattern at lines 2745-2748.

Suggested change

else

if (r_thisCell.getType() == r_topCell.getType()) {

applyZone(r_thisCell, r_topCell, m_hierarchicalZones, m_maxZone);

}

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: Critical compilation error: orphaned `else` statement when `RTS_ZEROHOUR` is defined. When the preprocessor processes this for `RTS_ZEROHOUR` builds, it removes the `#if RTS_GENERALS` block (lines 2788-2808), leaving this `else` without a matching `if` statement. The `if` statement on line 2785 has already closed at line 2787. The developer's claim that "line 2785 - 2787 has the if block that links to the else" is incorrect. Line 2787 closes the `if` block from line 2785, so when the `#if RTS_GENERALS` block is removed by the preprocessor, line 2810's `else` becomes orphaned. Move the `else` to line 2787 (after the closing brace), matching the working pattern at lines 2745-2748. ```suggestion if (r_thisCell.getType() == r_topCell.getType()) { applyZone(r_thisCell, r_topCell, m_hierarchicalZones, m_maxZone); } else ``` How can I resolve this? If you propose a fix, please make it concise.

this is not an issue you keep reading the statements wrong

greptile-apps · 2026-02-25T21:22:24Z

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

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


Critical compilation error: orphaned else statement when RTS_ZEROHOUR is defined (same issue as in Generals version).

Move the else to line 2787 after the closing brace to match the working pattern at lines 2745-2748.

Suggested change

else

if (r_thisCell.getType() == r_topCell.getType()) {

applyZone(r_thisCell, r_topCell, m_hierarchicalZones, m_maxZone);

}

else

Prompt To Fix With AI

This is a comment left during a code review. Path: GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp Line: 2810 Comment: Critical compilation error: orphaned `else` statement when `RTS_ZEROHOUR` is defined (same issue as in Generals version). Move the `else` to line 2787 after the closing brace to match the working pattern at lines 2745-2748. ```suggestion if (r_thisCell.getType() == r_topCell.getType()) { applyZone(r_thisCell, r_topCell, m_hierarchicalZones, m_maxZone); } else ``` How can I resolve this? If you propose a fix, please make it concise.

This is not an issue, you keep reading the statements wrong

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

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

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

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

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

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

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

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 Outdated Show resolved Hide resolved

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

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

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

Mauller added 10 commits February 25, 2026 16:47

unify(pathfinder): Merge Headers safe merges

d666041

unify(pathfinder) Merge IS_PASSABLE to costToHierGoal, safe merges

ef895ef

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

16e33c0

…risky merge

unify(pathfinder): Merge classify functions, safe merges

748a956

unify(pathfinder): Merge chooseBestLocomotorForPosition to examineNei…

0712f4e

…ghboringCells, safe merge

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

5862464

…dge, safe merge

unify(pathfinder): merge clientSafeQuickDoesPathExist to changeBridge…

d1a0e4e

…State, safe merge

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

94044f4

… merge

Unify(pathfinder): merge copyright header

4c6b6c1

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

9429a98

Mauller force-pushed the Mauller/unify-aipathfind-easyparts branch from 35a0d0a to dc18e3d Compare February 25, 2026 17:40

greptile-apps bot reviewed Feb 25, 2026

View reviewed changes

Feedback Update 1

87f4e50

Mauller force-pushed the Mauller/unify-aipathfind-easyparts branch from dc18e3d to 87f4e50 Compare February 25, 2026 21:18

greptile-apps bot reviewed Feb 25, 2026

View reviewed changes

Removed extra useless debug

2ea0f8b

-				else
+				}
+				else
+				{
+#else

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

		@@ -576,6 +576,7 @@ class PathfindZoneManager

		UnsignedShort m_maxZone; ///< Max zone used.
		UnsignedInt m_nextFrameToCalculateZones; ///< WHen should I recalculate, next?.

-				else
+				if (r_thisCell.getType() == r_topCell.getType()) {
+					applyZone(r_thisCell, r_topCell, m_hierarchicalZones, m_maxZone);
+				}
+				else

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: 4/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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mauller commented Feb 22, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Mauller commented Feb 23, 2026

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 23, 2026

Choose a reason for hiding this comment

Uh oh!

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 25, 2026

Choose a reason for hiding this comment

Uh oh!

Mauller Feb 25, 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!

Uh oh!

greptile-apps bot Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Feb 25, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 25, 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 25, 2026 •

edited

Loading