bugfix: Prevent cases where weapons would partially fire and require reloading without actually firing a shot

[Stubbjax]

Fixes #113

This change fixes an issue where weapons could partially fire and trigger their time and clip reloads without actually firing a shot. This could happen with all weapons, but those with long reload times are the most conspicuous, such as Jarmen Kell's sniper rifle or a Scorpion's rocket.

This occurred because the target-is-in-range checks are done prior to deciding to fire a weapon, but the actual firing of the weapon is done on the next frame. The weapon-firing logic has a few range guards which abort firing the weapon, but these checks come after the clip/reload/effect handling. Putting another range check before actually firing the weapon in the AIAttackFireWeaponState::update is the simplest/safest solution while maintaining retail compatibility.

Before

Jarmen Kell fires upon the Humvee, but the shot does nothing

BEFORE.mp4

After

Jarmen Kell does not fire upon the Humvee

AFTER.mp4

[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: ac21bf88-5ed7-4448-b4bc-8d8181e250c8

📥 Commits

Reviewing files that changed from the base of the PR and between f8ba7eb and 352445c.

📒 Files selected for processing (2)

• Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp

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

---

Walkthrough

AIAttackFireWeaponState::update now checks attack range before firing in both game variants. When the applicable CRC compatibility condition is disabled, out-of-range attacks fail unless the weapon has leech range.

Changes

Attack range validation

Layer / File(s)	Summary
Pre-fire target range check Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp	Both variants check the object target or goal position before firing. The state returns STATE_FAILURE when the target is out of range. Leech-range weapons bypass the check. CRC-compatible builds skip the check.

Priority: ⬇️ Low

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: xezon

Merge Risk: ⚪ Minimal · up to 35244

The reported linked-turret ammunition loss is not established for current weapon configurations. No actionable merge-blocking risk remains after normal checks.

Architecture Summary

Architecture risk: 🔵 Low · up to 35244

The change affects 2 systems.

Changed systems: Generals, GeneralsMD

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — Generals (service) was modified; 1 changed file maps to changed impact.
• observed — GeneralsMD (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp: AIAttackFireWeaponState::update adds a non-CRC-compatible pre-fire check: weapons without leech range must still be in range of the object or position target, or the state fails before firing. The check is omitted for leech-range weapons.
• observed — Modified behavior in GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp: AIAttackFireWeaponState::update adds a range check in non-retail-compatible CRC builds before firing: weapons without leech range fail if the object target or goal position is no longer within attack range. Retail-compatible CRC builds skip the check.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: preventing weapons from consuming reload time or ammunition without firing.
Description check	✅ Passed	The description explains the out-of-range firing issue and the proposed fix, so it is directly related to the changeset.
Linked Issues check	✅ Passed	Issue #113 reports that Jarmen Kell can consume the snipe action when a vehicle moves out of range as the shot fires. In both Generals and GeneralsMD, AIAttackFireWeaponState::update now checks …
Out of Scope Changes check	✅ Passed	The changes are limited to the firing-state range check in AIStates.cpp for Generals and GeneralsMD. Both changes directly address issue #113. No unrelated changes appear in the whole-PR diff.

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[macroscopeapp]

🟠 High AI/AIStates.cpp:5246

The range guard can still let a linked turret call fireWeapon out of range, consuming that weapon's clip/reload state without producing a shot. It validates only the currently selected weapon at line 5246, while the linked position-attack path fires every weapon slot in the loop at lines 5326–5340; validate each slot's weapon (including its own hasLeechRange() exemption) before calling fireWeapon.

🤖 Copy this AI Prompt to have your agent fix this:

In file @GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp around line 5246:

The range guard can still let a linked turret call `fireWeapon` out of range, consuming that weapon's clip/reload state without producing a shot. It validates only the currently selected `weapon` at line 5246, while the linked position-attack path fires every weapon slot in the loop at lines 5326–5340; validate each slot's weapon (including its own `hasLeechRange()` exemption) before calling `fireWeapon`.

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Adds range validation to weapon firing logic.

The PR should not merge until the Zero Hour range check preserves valid linked-turret shots.

Findings

1.  Linked turrets lose valid shots ▶

Summary

The PR adds a range check immediately before weapon firing in both Generals and Zero Hour to avoid consuming a shot when a target has moved out of range.

• The Zero Hour check also gates linked-turret position attacks using only the current weapon’s range.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Ready to fire at position] --> B{Current weapon in range?}
  B -- No --> C[Return failure]
  B -- Yes --> D[Fire every linked weapon slot]
  C -. Prevents .-> E[Other linked weapon's valid shot]

Loading

_{Reviews (1) · Last reviewed commit: "bugfix: Prevent cases where weapons woul..."}

[greptile-apps]

Linked turrets lose valid shots When linked turrets attack a position, this check tests only the current weapon, but the firing branch fires every linked weapon slot. If the target moves beyond the current weapon’s range while remaining within another linked weapon’s range, STATE_FAILURE prevents that other weapon from firing a valid shot. Check the weapons that will fire rather than using one slot to gate all of them.

[Caball009]

Is this a valid concern?

[greptile-apps]

Yes, this is a valid concern. weapon is only the weapon selected when the state is entered, but the multi-turret path below this check can fire all linked weapon slots. If that selected weapon is out of range while another linked weapon is still in range, this early STATE_FAILURE aborts the entire state before the valid linked weapon gets a chance to fire. The range guard should be applied per weapon in the linked-weapon firing path (or otherwise determine whether any weapon that will fire is in range), rather than gating the whole state using only weapon.

[xezon]

So the complaint is that this piece only checks one weapon, but further down below there are more weapons being considered.

[greptile-apps]

Yes—that is the concern. This guard evaluates only the single weapon pointer captured before the multi-turret handling. The code below can then process/fire the linked weapon slots as well. Therefore, if the selected weapon is now out of range but another linked weapon is still in range, this early STATE_FAILURE aborts the state and suppresses a valid shot from that other weapon. The range decision should account for each weapon that the subsequent branch will actually fire (or allow the state to continue when at least one such weapon remains in range).

[xezon]

Is there an alternative way to deal with it by calling the update immediately instead of next frame?

[Stubbjax]

This was my initial consideration too but it seems like a much riskier/consequential change and there's seemingly no precedent for it.

[xezon]

I understand it likely will be more complicated. Judging from the video the current fix addresses the issue only half way. The Jarmen no longer wastes a snipe with no effect, but it also stops moving and is unable to perform the shot when arguably it should be able to take the shot with no frame delay, which is the reason it stops moving in the first place. What happens now is that he needs to stand still for 1 frame which may or may not be enough to take the shot. Is the outcome different in 30 vs higher logic tick rates?

The fix seems ok for now, but maybe it should go further in the future. It is controversial, because it will be a buff for chasing weapons. It likely will be very good for all factions vs USA because USA relies on the evading Humvee playstyle.