Fix skirmish AI buildability check passing wrong template (#2407)

[mohamedelabbas1996]

Summary

Fixes #2407.

In AISkirmishPlayer::processBaseBuilding(), the build-list scan passes the accumulated candidate template bldgPlan to TheBuildAssistant->canMakeUnit() — but at that point in the loop bldgPlan is still nullptr. It should pass the current entry's template, curPlan.

Because the check ran against the wrong (null) template, the buildability gate rejected build-list entries that rely on AutomaticallyBuild = Yes (or the default value when the field is omitted, i.e. m_automaticallyBuild = true), so the skirmish AI never selected those structures to build.

Change

canMakeUnit(dozer, bldgPlan) → canMakeUnit(dozer, curPlan) in the build-list loop, with an explanatory comment. Applied to both Generals/ and GeneralsMD/ (the issue affects both games).

Notes

This is the first of the two interacting issues described in #2407 (the wrong-template buildability gate). The second (GLA rebuild-hole bldg shadowing) is left for a separate change.

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR fixes a one-line bug in AISkirmishPlayer::processBaseBuilding() where the wrong template pointer (bldgPlan, still nullptr at that point in the loop) was passed to canMakeUnit() instead of the current iteration's template (curPlan). The null argument caused the buildability gate to always fail, so the skirmish AI never automatically selected structures that used AutomaticallyBuild=Yes (or the default). The fix is applied identically to both Generals/ and GeneralsMD/.

• Bug fix: canMakeUnit(dozer, bldgPlan) → canMakeUnit(dozer, curPlan) — passes the correct per-iteration template so the AI can evaluate actual buildability.
• Scope: Change is mirrored in both game versions (Generals/ vanilla and GeneralsMD/ Zero Hour), consistent with the dual-codebase structure of the repository.

Confidence Score: 5/5

Safe to merge — single-character variable name correction with no side effects beyond restoring intended AI behaviour.

The change swaps one variable for another in a single call site, applied consistently to both game variants. The surrounding loop logic confirms that bldgPlan is guaranteed to still be nullptr at this point in every iteration, so passing it was always wrong. curPlan is always non-null here (it is the loop iterator). No new code paths, data structures, or external interfaces are touched.

No files require special attention.

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/GameLogic/AI/AISkirmishPlayer.cpp	One-line fix replacing bldgPlan (nullptr at this point) with curPlan in the canMakeUnit call; adds an explanatory comment. Change is correct and isolated.
GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AISkirmishPlayer.cpp	Mirror of the Generals/ fix — identical one-line correction and matching explanatory comment for the Zero Hour codebase.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([processBaseBuilding loop\nforeach curPlan in build list]) --> B{bldg already exists?}
    B -- yes --> CONT1([continue])
    B -- no --> C{isLocationSafe?}
    C -- no --> CONT2([continue])
    C -- yes --> D{isPriorityBuild?}
    D -- yes --> E[bldgPlan = curPlan\nbldgInfo = info\nisPriority = true]
    E --> F{isKindOf FS_POWER?}
    D -- no --> F
    F -- yes --> G[powerPlan = curPlan]
    G --> H{isAutomaticBuild?}
    F -- no --> H
    H -- no --> CONT3([continue])
    H -- yes --> I{findDozer?}
    I -- null --> CONT4([continue])
    I -- found dozer --> J{canMakeUnit\ndozer, curPlan}
    J -- "!= CANMAKE_OK" --> CONT5([continue])
    J -- CANMAKE_OK --> K{isBuildable and\nbldgPlan == nullptr?}
    K -- yes --> L[bldgPlan = curPlan\nbldgInfo = info]
    K -- no --> CONT6([next iteration])
    L --> CONT6

    style J fill:#c8f7c5,stroke:#27ae60
    style J color:#000

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A([processBaseBuilding loop\nforeach curPlan in build list]) --> B{bldg already exists?}
    B -- yes --> CONT1([continue])
    B -- no --> C{isLocationSafe?}
    C -- no --> CONT2([continue])
    C -- yes --> D{isPriorityBuild?}
    D -- yes --> E[bldgPlan = curPlan\nbldgInfo = info\nisPriority = true]
    E --> F{isKindOf FS_POWER?}
    D -- no --> F
    F -- yes --> G[powerPlan = curPlan]
    G --> H{isAutomaticBuild?}
    F -- no --> H
    H -- no --> CONT3([continue])
    H -- yes --> I{findDozer?}
    I -- null --> CONT4([continue])
    I -- found dozer --> J{canMakeUnit\ndozer, curPlan}
    J -- "!= CANMAKE_OK" --> CONT5([continue])
    J -- CANMAKE_OK --> K{isBuildable and\nbldgPlan == nullptr?}
    K -- yes --> L[bldgPlan = curPlan\nbldgInfo = info]
    K -- no --> CONT6([next iteration])
    L --> CONT6

    style J fill:#c8f7c5,stroke:#27ae60
    style J color:#000

Loading

_{Reviews (1): Last reviewed commit: "Fix AISkirmishPlayer::processBaseBuildin..." | Re-trigger Greptile}

[Skyaero42]

Both issue and PR are generated with AI.

Can you show (e.g. screenshot/video) what the original AI behavior is and how this fix changes it.

• PR title needs updating to match Conventional Commit style.

[Skyaero42]

comment too long, maybe not even needed.
We don't reference to issues in code.

[Skyaero42]

This may cause a mismatch when using an AI in multiplayer. Has this been tested?

If it can cause a mismatch, it needs to be behind RETAIL guards.

[xezon]

I would expect that is very likely.