Skip to content

chore(pathfinder): Cleanup formatting and fix an incomplete optimization in PathfindZoneManager:calculateZones()#2360

Merged
xezon merged 5 commits into
TheSuperHackers:mainfrom
Mauller:Mauller/fix-pathfindzonemanager-formatting
Feb 28, 2026
Merged

chore(pathfinder): Cleanup formatting and fix an incomplete optimization in PathfindZoneManager:calculateZones()#2360
xezon merged 5 commits into
TheSuperHackers:mainfrom
Mauller:Mauller/fix-pathfindzonemanager-formatting

Conversation

@Mauller

@Mauller Mauller commented Feb 27, 2026

Copy link
Copy Markdown

merge by rebase

This PR is split out from the AiPathfind unification PR to reduce the difference in the merge between Generals and Zero Hours AiPathfind.cpp files.

It also contains a fix for an optimization that was missed in the zero hour code, this fix does not cause any mismatching and is why this PR should be merged by rebase.

There was extensive changes made to PathfindZoneManager:calculateZones() which introduced a large number of formatting differences relative to the rest of the code.

This PR more closely aligns Zero Hours PathfindZoneManager:calculateZones() with Generals implementation by doing the following:

  • Fixes leading spaces by changing them for leading tabs
  • Changes while loops back to for loops to match the original implementaitons in generals ( makes the code much cleaner too )
  • Fixes style of open braces to match the rest of the code by placing them inline with their conditional/logical statement ( K&R style )
  • Reverts a few small refactors in the zero hour code to their Generals implementation since they were cleaner in style

@Mauller Mauller self-assigned this Feb 27, 2026
@Mauller Mauller added Major Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour labels Feb 27, 2026
m_groundRubbleZones[i] = i;
m_terrainZones[i] = i;
m_crusherZones[i] = i;
m_hierarchicalZones[i] = i;

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.

Note - This change is how it was implemented in generals, it makes comparissons in winmerge easier and is far cleaner than the refactor zero hour introduced

}
for (i=1; i<m_maxZone; i++) {
Int zone = m_hierarchicalZones[i];
m_hierarchicalZones[i] = m_hierarchicalZones[zone];

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.

Note - This change is how it was implemented in generals, minus the comment being removed. I left the zero hour comment above the for loop instead to keep the code block clearer.

@greptile-apps

greptile-apps Bot commented Feb 27, 2026

Copy link
Copy Markdown

Greptile Summary

Cleaned up formatting in PathfindZoneManager::calculateZones() by converting spaces to tabs, changing while loops to for loops, and standardizing brace placement to K&R style. Also fixed an incomplete optimization where the top cell comparison (j > globalBounds.lo.y) was missing the notTerrainOrCrusher conditional guard that the left cell comparison already had, making both branches now consistent.

  • Converted 4 while loops to for loops for better readability
  • Standardized indentation from spaces to tabs
  • Moved opening braces inline with conditions (K&R style)
  • Removed obsolete REGISTER keyword
  • Fixed logic bug where water/ground/cliff zone checks for top cell weren't properly guarded by notTerrainOrCrusher flag
  • Added clarifying comment for zone initialization loop

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The changes are entirely formatting improvements and a single logic fix that makes the code more consistent. The logic fix adds a missing conditional guard that was already present in an equivalent code path, reducing the chance of incorrect zone calculations. All custom style rules are followed, and the changes align the Zero Hour implementation more closely with the Generals version.
  • No files require special attention

Important Files Changed

Filename Overview
GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp Formatting cleanup (tabs, for loops, K&R braces) and logic fix to add missing notTerrainOrCrusher check for top cell comparisons

Last reviewed commit: 2b8e7fd

applyZone(r_thisCell, r_topCell, m_groundRubbleZones, m_maxZone);
else if (groundCliff(r_thisCell, r_topCell))
applyZone(r_thisCell, r_topCell, m_groundCliffZones, m_maxZone);
}

@Mauller Mauller Feb 27, 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.

Note - This change is why the PR wants to be merged by rebase. It will make the dif cleaner in the unification PR as well.

This is a small optimisation overall and wants to be seperate in case we needed to revert it.

m_hierarchicalZones[i] = i;
}

REGISTER UnsignedInt maxZone = m_maxZone;

@Mauller Mauller Feb 27, 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.

Note - This was removed to make the code more in line with the generals implementation. Helping make the Diff clearer in the unification.

The register keyword is also deprecated in c++17 and beyond and using the class variable is of no issue. There should be little to no performance difference.

@Skyaero42 Skyaero42 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This looks good to me

@Mauller Mauller changed the title chore(pathfinder): Cleanup formatting and fix an incomplete optimization in PathfindZoneMalanger:calculateZones() chore(pathfinder): Cleanup formatting and fix an incomplete optimization in PathfindZoneManager:calculateZones() Feb 27, 2026
@Mauller Mauller force-pushed the Mauller/fix-pathfindzonemanager-formatting branch from 71e94a9 to 1910557 Compare February 28, 2026 09:36
Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp
Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp
@Mauller Mauller force-pushed the Mauller/fix-pathfindzonemanager-formatting branch from 1910557 to 2b8e7fd Compare February 28, 2026 13:46
@xezon xezon merged commit 83a51f8 into TheSuperHackers:main Feb 28, 2026
16 checks passed
xezon pushed a commit that referenced this pull request Feb 28, 2026
xezon pushed a commit that referenced this pull request Feb 28, 2026
xezon pushed a commit that referenced this pull request Feb 28, 2026
xezon pushed a commit that referenced this pull request Feb 28, 2026
…irectly in PathfindZoneManager::calculateZones() (#2360)
@xezon xezon deleted the Mauller/fix-pathfindzonemanager-formatting branch February 28, 2026 17:04
Okladnoj pushed a commit to Okladnoj/GeneralsGameCode that referenced this pull request Mar 1, 2026
Okladnoj pushed a commit to Okladnoj/GeneralsGameCode that referenced this pull request Mar 1, 2026
Okladnoj pushed a commit to Okladnoj/GeneralsGameCode that referenced this pull request Mar 1, 2026
Okladnoj pushed a commit to Okladnoj/GeneralsGameCode that referenced this pull request Mar 1, 2026
Okladnoj pushed a commit to Okladnoj/GeneralsGameCode that referenced this pull request Mar 1, 2026
stephanmeesters pushed a commit to stephanmeesters/GeneralsGameCode that referenced this pull request Mar 4, 2026
… in PathfindZoneManager::calculateZones() (TheSuperHackers#2360)

fix(logic): Validate playerindex for retaliation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Major Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants