bugfix(pathfinder): Improve initialization of uninitialized variable in Pathfinder::classifyFence

[Caball009]

• Follow up for https://github.com/TheSuperHackers/GeneralsGameCode/ pull/1748
  ( the link is broken on purpose so it doesn't link, because I may close this draft PR if it doesn't work out )

This PR makes a small change to the initialization of the previously uninitialized variable in Pathfinder::classifyFence. I saw 4 places where the game eventually calls Pathfinder::classifyFence for maps with fences:

• map start: GameLogic::startNewGame
• map end: GameLogic::processDestroyList
• fence is toppled, e.g. by a unit that drives over it: ToppleUpdate::applyTopplingForce
• fence is destroyed, e.g. by flames or explosions: GameLogic::processDestroyList

I noticed there's an initialization pattern that the game appears to follow for the most part. The uninitialized values tend to be either very large (> 100'000) or 0. GameLogic::processDestroyList runs directly after PartitionManager::update which zero initializes > 20K of memory on the stack. This sets the uninitialized variables that are used in Pathfinder::classifyFence to 0, and they usually stay 0.

I also noticed that the destruction of an object drawable can modify the values as well, which is why the zero initialization is disabled after the first destroyed object. Naturally, there's no way to come up with a complete fix due to the nature of the issue, but the new initialization does fix the mismatches in these two replays:

1v1 (25.05.17) [rank] arctic arena zh v1 BoYcaH^(nuke) vs NGE_Killer(gla).rep
1v1 (25.09.20) [rank] snowy drought zh v5 SR_MaD(stealth) vs Majesta(laser).rep

fixed_replays.zip

I have four replays in total that mismatch because of the uninitialized variable; two are fixed with this change, and two continue to mismatch:

00-43-03_2v2_player_fh_abed_HardAI.rep
1v1 (25.11.28) [rank] farmlands of the fallen zh v1 SR_MaD^(sw) vs Penguin(gla).rep

mm_replays.zip

TODO:

• Use Zero Hour only macros.

[greptile-apps]

Greptile Summary

This PR refines the retail-CRC-compatibility workaround in Pathfinder::classifyFence (originally introduced in PR #1748) by replacing the single hardcoded uninitialized-stack approximation with a two-mode initialisation strategy: zero-init for the first object destroyed in GameLogic::processDestroyList (mirroring the zero'd stack state left by PartitionManager::update), and large-value init (1 000 000) for all subsequent objects (where drawable destruction can have modified the stack). This fixes replay CRC mismatches for 2 of 4 known affected replays; the remaining 2 are acknowledged as still mismatching.

• AIPathfind.h — adds Bool m_classifyFenceZeroInit behind #if RTS_ZEROHOUR && RETAIL_COMPATIBLE_CRC. Member is exposed via an inserted public: section rather than a setter, which is a minor encapsulation concern.
• AIPathfind.cpp — replaces the single fixed values (253961804 / 4202797) with a conditional branch driven by the new flag; initialises the flag to false in Pathfinder::reset().
• GameLogic.cpp — sets the flag to true before the destroy loop starts, then resets it to false after each individual object deletion so only the first deleted object gets zero-init behaviour.

Confidence Score: 4/5

• Safe to merge; the change is narrowly scoped behind compile-time guards, makes measurable progress on a known replay desync, and introduces no regressions in the default build configurations.
• The logic is sound and well-justified by the PR description. The only non-blocking concern is the use of a public: access specifier inserted mid-class to expose an otherwise internal flag; a setter would be cleaner but is not a blocking issue. Two of four known mismatching replays are fixed; the partial nature of the fix is honestly documented.
• Pay attention to Core/GameEngine/Include/GameLogic/AIPathfind.h — the public: insertion exposes implementation state that should ideally be accessed through a setter.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameLogic/AIPathfind.h	Adds m_classifyFenceZeroInit member (guarded by RTS_ZEROHOUR && RETAIL_COMPATIBLE_CRC) exposed via an inserted public: section; uses #pragma once correctly; license header intact.
Core/GameEngine/Source/GameLogic/AI/AIPathfind.cpp	Replaces the previous single-value hardcoded initialization (253961804 / 4202797) with a conditional branch that uses 0,0 (zero-init path) or 1000000,1000000 (large-value path) depending on m_classifyFenceZeroInit; resets the flag to false in Pathfinder::reset().
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Sets m_classifyFenceZeroInit = true before the first object is destroyed (mirroring the zero-initialised stack state from PartitionManager::update) and resets it to false after each deletion so subsequent objects use the large-value path.

Sequence Diagram

sequenceDiagram
    participant PU as PartitionManager::update
    participant GL as GameLogic::processDestroyList
    participant PF as Pathfinder
    participant CF as classifyFence

    PU->>PU: zero-initialises >20K of stack memory
    GL->>PF: m_classifyFenceZeroInit = true (if list non-empty)
    loop For each object in destroy list
        GL->>GL: Object::friend_deleteInstance(currentObject)
        Note over GL: Object destructor may trigger classifyFence
        GL-->>CF: (indirect) classifyFence(obj, insert)
        alt m_classifyFenceZeroInit == true (first object)
            CF->>CF: cellBounds.hi = {0, 0}
        else m_classifyFenceZeroInit == false (subsequent objects)
            CF->>CF: cellBounds.hi = {1000000, 1000000}
        end
        GL->>PF: m_classifyFenceZeroInit = false
    end
    GL->>GL: m_objectsToDestroy.clear()

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: Core/GameEngine/Include/GameLogic/AIPathfind.h
Line: 908-911

Comment:
**Public member breaks class encapsulation**

Inserting a `public:` specifier inside what is otherwise a `private:` section to expose `m_classifyFenceZeroInit` is a workaround that breaks encapsulation. Any code (not just `GameLogic::processDestroyList`) can freely read/write this state, and the naming convention (`m_` prefix) signals it should be private implementation detail.

Consider providing a dedicated setter (or a friend declaration) instead:

```cpp
#if RTS_ZEROHOUR && RETAIL_COMPATIBLE_CRC
	void setClassifyFenceZeroInit(Bool value) { m_classifyFenceZeroInit = value; }
private:
	Bool m_classifyFenceZeroInit;
#endif
```

This keeps the data private while still allowing `GameLogic::processDestroyList` to call the setter.

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

---

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
Line: 2493

Comment:
**Comment slightly misrepresents when the flag is reset**

The comment says "It's set to false when this function exits," but `m_classifyFenceZeroInit` is actually reset to `false` *inside the loop* — immediately after each object is deleted (line 2558). This means all objects after the first are processed with `m_classifyFenceZeroInit = false` (large-value mode), not just at exit. The comment could be clearer about this behaviour to prevent future readers from assuming the flag stays `true` for the entire loop.

```suggestion
	// TheSuperHackers @info Set m_classifyFenceZeroInit to true for the first object. It's reset to false after each object is deleted.
```

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

_{Last reviewed commit: "Tweaked comments."}