Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Caball009 wants to merge 1 commit into
base: main
Choose a base branch
from
+7 −3
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/CountermeasuresBehavior.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ ObjectID CountermeasuresBehavior::calculateCountermeasureToDivertTo( const Objec
Real closestFlareDist = 1e15f;
Object *closestFlare = nullptr;

const int volleySize = data->m_volleySize;
int volleyFlaresCounted = 0;
const UnsignedInt volleySize = max(data->m_volleySize, 1u);
UnsignedInt volleyFlaresCounted = 0;
Comment on lines +162 to +163

@greptile-apps greptile-apps Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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 Caball009 Jun 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the original code prior to #1367

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

while( iteratorMax-- )
{

}

@greptile-apps greptile-apps Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Caball009 Jun 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@greptileai re-review this pull request accordingly.

@greptile-apps greptile-apps Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


//Flares are pushed to the front of the list, but we only want to acquire the "newest" of the flares, therefore
//Start at the end of the list and go towards the beginning.
Expand All @@ -182,9 +182,13 @@ ObjectID CountermeasuresBehavior::calculateCountermeasureToDivertTo( const Objec
// The non retail behaviour corrects the code to iterate through all flares in the volley
break;
#else
volleyFlaresCounted++;
++volleyFlaresCounted;
#endif
}

#if RETAIL_COMPATIBLE_CRC
++volleyFlaresCounted;
#endif
}

if( closestFlare )
Expand Down
Loading