bugfix(logic): Restore retail compatibility after change in CountermeasuresBehavior::calculateCountermeasureToDivertTo

[Caball009]

• Follow up for bugfix(behavior): Fix wrong and unsafe logic in CountermeasuresBehavior::calculateCountermeasureToDivertTo() #1367

#1367 did a small bug fix + refactor but inadvertently introduced a small change to the game logic with retail compatibility. The old implementation would increment the volleyFlaresCounted on each iteration, but the new implementation doesn't.

Replay 23-49-43_2v2_hitlr98_122_Alphax01_Alphax02.zip mismatches because of that change. This PR fixes the mismatch.

---

#if RETAIL_COMPATIBLE_CRC
	++volleyFlaresCounted;
#endif

This is the crucial part of this change. The other changes were not necessary to fix the mismatch.

[greptile-apps]

Greptile Summary

This PR restores retail CRC compatibility in CountermeasuresBehavior::calculateCountermeasureToDivertTo by adding ++volleyFlaresCounted outside the if(obj) block under #if RETAIL_COMPATIBLE_CRC, matching the original EA behavior where the counter incremented on every loop iteration regardless of whether a valid flare object was found.

• The core fix correctly re-adds the volleyFlaresCounted increment for null-object iterations in retail mode, resolving the replay mismatch introduced by bugfix(behavior): Fix wrong and unsafe logic in CountermeasuresBehavior::calculateCountermeasureToDivertTo() #1367.
• int → UnsignedInt for both local variables cleans up a signed/unsigned mismatch with m_volleySize, which is already declared UnsignedInt in the data struct.
• max(data->m_volleySize, 1u) is an ancillary guard not required for the mismatch fix; it subtly changes retail behavior when VolleySize is 0 (the INI default).

Confidence Score: 4/5

The retail-compatibility fix is correct and isolated to one function; the ancillary max(..., 1u) guard warrants a second look before merge.

The core change — moving ++volleyFlaresCounted outside the null-object guard under RETAIL_COMPATIBLE_CRC — directly and correctly restores the original EA loop behavior that caused the replay mismatch. The type narrowing from int to UnsignedInt is clean. The one concern is the max(data->m_volleySize, 1u) guard: m_volleySize defaults to 0, so any unit whose INI omits VolleySize will now enter the loop once in retail mode instead of skipping it entirely, which is a small but unintended deviation from true retail behavior.

The single changed file CountermeasuresBehavior.cpp deserves a second look specifically around the max(data->m_volleySize, 1u) guard and its interaction with the RETAIL_COMPATIBLE_CRC path.

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/CountermeasuresBehavior.cpp	Restores the retail-compatible volleyFlaresCounted increment outside the null-object guard; also tightens types from int to UnsignedInt and adds a max(..., 1u) guard on volleySize that slightly alters retail behavior when m_volleySize == 0.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Start: calculateCountermeasureToDivertTo] --> B[it = rbegin, volleyFlaresCounted = 0]
    B --> C{it != rend AND\nvolleyFlaresCounted < volleySize?}
    C -- No --> G[Return closestFlare or INVALID_ID]
    C -- Yes --> D[obj = findObjectByID *it++]
    D --> E{obj != null?}
    E -- Yes --> F[Update closestFlareDist if closer]
    F --> H{RETAIL_COMPATIBLE_CRC?}
    H -- Yes/Retail --> I[break — exit loop]
    H -- No/Non-Retail --> J[++volleyFlaresCounted]
    J --> C
    E -- No / obj is null --> K{RETAIL_COMPATIBLE_CRC?}
    K -- Yes/Retail --> L[++volleyFlaresCounted]
    L --> C
    K -- No/Non-Retail --> C

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[Start: calculateCountermeasureToDivertTo] --> B[it = rbegin, volleyFlaresCounted = 0]
    B --> C{it != rend AND\nvolleyFlaresCounted < volleySize?}
    C -- No --> G[Return closestFlare or INVALID_ID]
    C -- Yes --> D[obj = findObjectByID *it++]
    D --> E{obj != null?}
    E -- Yes --> F[Update closestFlareDist if closer]
    F --> H{RETAIL_COMPATIBLE_CRC?}
    H -- Yes/Retail --> I[break — exit loop]
    H -- No/Non-Retail --> J[++volleyFlaresCounted]
    J --> C
    E -- No / obj is null --> K{RETAIL_COMPATIBLE_CRC?}
    K -- Yes/Retail --> L[++volleyFlaresCounted]
    L --> C
    K -- No/Non-Retail --> C

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/CountermeasuresBehavior.cpp:162-163
**`max(..., 1u)` guard introduces a minor retail behavioral change**

`m_volleySize` is declared as `UnsignedInt` and defaults to `0` in `CountermeasuresBehaviorModuleData`. With the guard, a unit whose `VolleySize` INI field is `0` now enters the loop once instead of skipping it entirely, potentially returning a flare ID where the original retail code would have returned `INVALID_ID`. The PR description explicitly notes this change "was not necessary to fix the mismatch," so it risks introducing a small delta from true retail behavior for the zero-volley-size edge case. Consider whether to guard with `max` only under `#if !RETAIL_COMPATIBLE_CRC`.

_{Reviews (1): Last reviewed commit: "bugfix(logic): Restore retail compatibil..." | Re-trigger Greptile}

[greptile-apps]

max(..., 1u) guard introduces a minor retail behavioral change

m_volleySize is declared as UnsignedInt and defaults to 0 in CountermeasuresBehaviorModuleData. With the guard, a unit whose VolleySize INI field is 0 now enters the loop once instead of skipping it entirely, potentially returning a flare ID where the original retail code would have returned INVALID_ID. The PR description explicitly notes this change "was not necessary to fix the mismatch," so it risks introducing a small delta from true retail behavior for the zero-volley-size edge case. Consider whether to guard with max only under #if !RETAIL_COMPATIBLE_CRC.

Prompt To Fix With AI

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/CountermeasuresBehavior.cpp
Line: 162-163

Comment:
**`max(..., 1u)` guard introduces a minor retail behavioral change**

`m_volleySize` is declared as `UnsignedInt` and defaults to `0` in `CountermeasuresBehaviorModuleData`. With the guard, a unit whose `VolleySize` INI field is `0` now enters the loop once instead of skipping it entirely, potentially returning a flare ID where the original retail code would have returned `INVALID_ID`. The PR description explicitly notes this change "was not necessary to fix the mismatch," so it risks introducing a small delta from true retail behavior for the zero-volley-size edge case. Consider whether to guard with `max` only under `#if !RETAIL_COMPATIBLE_CRC`.

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

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[Caball009]

This was the original code prior to #1367

Int iteratorMax = MAX( data->m_volleySize, 1 );

while( iteratorMax-- )
{

}

[greptile-apps]

Thanks for sharing that — the MAX(data->m_volleySize, 1) guard was already in the original retail code before #1367, so this PR is correctly restoring that behavior. My concern was incorrect; the max(..., 1u) here is not a new change from retail.

[Caball009]

@greptileai re-review this pull request accordingly.

[greptile-apps]

The fix is correct and well-structured. Here's the updated review:

The core fix — moving ++volleyFlaresCounted outside the if (obj) block under #if RETAIL_COMPATIBLE_CRC — accurately restores the original retail behavior. In the original EA code, while(iteratorMax--) decremented the counter on every iteration regardless of whether the looked-up object was still alive/valid. The previous implementation in #1367 only incremented inside if (obj), meaning null/expired flare entries didn't count toward the volley limit, causing the replay CRC mismatch.

The max(data->m_volleySize, 1u) guard is confirmed to match the original retail code (MAX(data->m_volleySize, 1)), so no concerns there.

Type change from int to UnsignedInt is appropriate since m_volleySize is UnsignedInt and these values are never negative.

The RETAIL_COMPATIBLE_CRC path behavior is correct:

• Object found → compute distance, break (retail: stop at first valid flare)
• Object not found → fall through to ++volleyFlaresCounted, continue loop
• This exactly mirrors while(iteratorMax--) semantics

The change is minimal, targeted, and restores retail compatibility without touching the non-retail path. LGTM.