bugfix(pathfinder): Fix uninitialized variable in Pathfinder::tightenPathCallback to prevent mismatches

[Caball009]

Another uninitialized variable rears its ugly head; this time in the path finding code. Causes mismatches, which can be reproduced by driving on the terrain around certain bridges.

Reproduction in Sand Serpent:

bridge_mm.mp4

Replay for VC6:

00-04-16_1v1_adapte_Anthony1.zip This replay does not mismatch with retail executable, and first started to mismatch with 4bbf0c0

Map and replay for VS22:

bridge_mm.zip This replay was recorded with a release build, and the mismatch shows when replay is played back with a debug build.

Notes:

I could not see any noticeable issues in-game because of this bug during my testing. The fix cannot be applied unconditionally, because it breaks the golden replay 1 and probably others. It's currently tied to the path-finding fail-over, because that's better than nothing. It needs to be replaced with a proper opt-in mechanism. Adding this fix should be good for GeneralsOnline.

Edit: maybe some other values will work well enough that we can use them for retail.

TODO:

• Maybe test different values for retail fix.
• Replicate in Generals.

Thanks to @Mauller and @DrGoldFish1 for their help.

[greptile-apps]

Greptile Overview

Greptile Summary

Fixed an uninitialized variable bug in Pathfinder::tightenPathCallback that caused mismatches when units drove on terrain near certain bridges. The pos variable is now properly initialized by calling adjustCoordToCell before being passed to checkForAdjust.

• Made adjustCoordToCell static to allow calling from the static tightenPathCallback function
• Added initialization of the pos variable with conditional compilation guard RETAIL_COMPATIBLE_PATHFINDING and runtime check s_useFixedPathfinding
• The fix is gated behind the pathfinding fail-over mechanism to avoid breaking golden replays

The workaround is documented as incomplete and needs replacement with a proper opt-in mechanism in the future.

Confidence Score: 4/5

• This PR is safe to merge with low risk - fixes a legitimate uninitialized variable bug
• The fix properly addresses an uninitialized variable that was causing deterministic mismatches. The implementation is conservative with conditional compilation guards and runtime checks to preserve retail compatibility. The workaround nature is well-documented and the static method change is necessary and safe.
• No files require special attention

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Include/GameLogic/AIPathfind.h	Made adjustCoordToCell static to allow calling from static callback function
Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp	Initialized uninitialized pos variable in tightenPathCallback using conditional compilation guard, implemented function as static

_{Last reviewed commit: 1d78179}

[helmutbuhler]

Interesting, that commit you mentioned already caused problems earlier in #1061. I predicted there that it might cause further mismatches, and this seems like confirmation of that. Maybe we should revert #1009 and #1013, at least for VC6 builds?

[stephanmeesters]

Did you mean to do the fix in Generals/ not GeneralsMD/?

[Caball009]

Did you mean to do the fix in Generals/ not GeneralsMD/?

Oops, yeah, that's silly of me. Not sure how that happened.

[Caball009]

Interesting, that commit you mentioned already caused problems earlier in #1061. I predicted there that it might cause further mismatches, and this seems like confirmation of that. Maybe we should revert #1009 and #1013, at least for VC6 builds?

I'm not sure if that solves anything. I tried the current main branch with the wwmath.h header from 2856a23 ( the commit before 4bbf0c0 ) and the VC6 replay still mismatches.

Harmless changes anywhere can change stack layouts and introduce mismatches that hadn't been noticeable before because of uninitialized variables.