bugfix(aiplayer): Improve initialization of uninitialized variable in AIPlayer::onUnitProduced

[Caball009]

• Closes Initialization of uninitialized variable in AIPlayer::onUnitProduced broke retail compatibility for Generals #3378
• Follow up for [GEN][ZH] Initialize supplyTruck variable #820
• Similar to bugfix(pathfinder): Improve initialization of uninitialized variable in Pathfinder::classifyFence #2460

#820 initialized a previously uninitialized variable supplyTruck in AIPlayer::onUnitProduced. This was tested for Zero Hour and appears to work great there, but for Generals an unconditional initialization is inadequate. This PR aims to improve the initialization to fix the mismatches it causes with some replays.

The 11 replays included in the issue mismatch with the current main branch, but play correctly again with the fix from this PR. I checked ~1000 Generals ai replays and did not see a mismatch introduced by this change.

TODO:

• Replicate to Zero Hour.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d6cf188c-a1f4-4f8b-90d7-a38946437e69

📥 Commits

Reviewing files that changed from the base of the PR and between 4252cd4 and 0d57a77.

📒 Files selected for processing (4)

• GeneralsMD/Code/GameEngine/Include/GameLogic/GameLogic.h
• GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/SpawnBehavior.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

Walkthrough

For Generals builds using MSVC earlier than 1300, spawn creation sets a flag around onUnitCreated. onUnitProduced uses the flag to set supplyTruck to false. GeneralsMD also changes update-module mask checks for non-retail-compatible CRC builds.

Changes

Generals unit production

Layer / File(s)	Summary
Flag declaration and spawn scope Generals/Code/GameEngine/Include/GameLogic/GameLogic.h, Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp, Generals/Code/GameEngine/Source/GameLogic/Object/Behavior/SpawnBehavior.cpp, GeneralsMD/Code/GameEngine/Include/GameLogic/GameLogic.h, GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/SpawnBehavior.cpp	The conditional flag is declared and initialized to false. createSpawn sets it to true before onUnitCreated and resets it to false afterward.
Production initialization and update-module checks Generals/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	onUnitProduced sets supplyTruck to false when the flag is true. Otherwise, its prior initialization remains. For non-retail-compatible CRC builds, normal and sleepy update modules run only when all disabled flags are included in the module mask; retail-compatible CRC builds retain the prior condition.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: xezon

Merge Risk: ⚪ Minimal · up to 0d57a

The changes address legacy Generals unit initialization, with no concrete merge-blocking regression identified. No actionable merge-blocking risk remains beyond normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 0d57a

The change is limited to legacy Generals unit production and does not appear to expose a new interface or weaken an access control. The shared callback flag has a narrow intended lifetime, though unusual nested execution remains a design consideration.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated effect is within compiler-gated game simulation and AI production state; the inspected changes do not establish a new network, credential, or privilege boundary.

Trust Boundaries and Controls

• inferred — No attacker-controlled route to the new flag was established. The observed writer is the compile-gated spawn callback, while the observed consumer is AI production handling.

Resilience and Maintainability Implications

• observed — The transient flag can affect stateful AI production behavior, including supply-truck handling and dozer scheduling, but the inspected normal callback sequence resets it after use.

Hardening Proposals

• proposed — If callback reentrancy becomes possible, preserve and restore the previous flag value rather than always resetting it to false, so nested callbacks retain the outer context.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address issue #3378. For the Generals MSVC compatibility path, GameLogic initializes m_onUnitProducedZeroInit to false. SpawnBehavior::createSpawn sets the flag only during the `on…
Out of Scope Changes check	✅ Passed	The pull request changes the two game variants in the relevant GameLogic, AIPlayer::onUnitProduced, and SpawnBehavior::createSpawn code paths. The added member, constructor initialization, callb…
Title check	✅ Passed	The title clearly identifies the bug fix and the affected function. It accurately summarizes the improved initialization of the uninitialized variable in AIPlayer::onUnitProduced.
Description check	✅ Passed	The description directly explains the Generals replay mismatches, the relation to prior changes, the intended fix, and the reported replay validation.

---

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Adds a workaround flag to game unit initialization logic.

The PR should not merge as a Zero Hour replication until its new behavior is enabled in that build.

Findings

1.  Zero Hour fix is excluded ▶

Summary

The PR adds a temporary spawn-context flag so VS6 Generals builds initialize supplyTruck differently when AI units are created through SpawnBehavior.

• The same changes were added to Zero Hour, but their RTS_GENERALS guards exclude them from its build.

_{Reviews (2) · Last reviewed commit: "Replicated to Zero Hour."}

[Skyaero42]

I remember testing the supplyTruck variable initialization was really finicky. Even adding a single logging line could change the outcome of a test.

@Mauller and I did a lot of testing and exploration and were keep getting issues. So we decided to be conservative. When Helmut introduced #820, we couldn't reproduce our earlier issues.

@Caball009 said that a lot of Generals replays were mismatching due to this and that the given PR resolves it.

supplyTruck is one of most sensitive uninitialized variables around there. If this solutions works as given, I would take it.

[Mauller]

Looks okay to me, just need replicating to zero hour.

[xezon]

Nice hack.

[Caball009]

Replicated to Zero Hour.

I think we got very lucky with how well we were able to initialize this, especially for Zero Hour. Skyaero and I will continue checking more Generals replays, so if it turns out with more replays that this fix is undesirable we'll have to readjust or revert it.

[xezon]

Merge this now?

[Caball009]

Merge this now?

Yes, as far as I'm concerned.