Repository navigation
Conversation
…n locking distance of their target
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Attack and kill state handling Generals/Code/GameEngine/Include/GameLogic/Module/MissileAIUpdate.h, Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/MissileAIUpdate.cpp, GeneralsMD/Code/GameEngine/Include/GameLogic/Module/MissileAIUpdate.h, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/MissileAIUpdate.cpp |
In both code trees, non-retail builds return from the attack state after stopping the locomotor and removing exhaust when fuel expires without detonation. In the kill state, non-retail builds also clear braking status before returning. Retail-compatible builds retain the existing airborneTargetGone handling. The airborneTargetGone calls, implementation, and declaration are restricted to retail-compatible builds. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Suggested reviewers: bobtista
Merge Risk: 
33d27
-
Missiles may continue with incorrect movement after fuel expiration or target loss, so the behavioral fix is not ready to merge.
Pre-merge checks: 
-
Check name Status Explanation Resolution Linked Issues check Issue #1700requires that burned-out rockets stop disappearing near their target. The PR adds a fix path in MissileAIUpdate.cpp for both Generals and GeneralsMD. The fix path is compiled only when RET…Make the fix active in the default build configuration for both MissileAIUpdate implementations. This changes CRC behavior, so the Game Design Committee decision mentioned in the PR discussion must be made first. If the Committee approves t… -
Check name Status Explanation Title check The title clearly and concisely identifies the missile deletion bug addressed by the changes. Description check The description directly explains the missile disappearance issue, the intended behavior, and the linked issue. It matches the changeset. Out of Scope Changes check The change set is limited to MissileAIUpdate.cpp in GeneralsMD and the matching Generals files, plus the MissileAIUpdate.h declarations. The GeneralsMD diff shows changes to fuel expiry in doAttackSta… Full details: Out of Scope Changes check
-
Explanation
The change set is limited to MissileAIUpdate.cpp in GeneralsMD and the matching Generals files, plus the MissileAIUpdate.h declarations. The GeneralsMD diff shows changes to fuel expiry in doAttackState and doKillState, locomotor stop, exhaust removal, braking status, and gating of airborneTargetGone(). These changes directly address issue
#1700. The move of the d and curLoco declarations is a minor local reordering in the same function. No unrelated change is shown. The Generals source was not diffed directly. Its scope rests on the AI-generated summary, which describes the same changes.
-
Full details: Linked Issues check
-
Explanation
Issue
#1700requires that burned-out rockets stop disappearing near their target. The PR adds a fix path in MissileAIUpdate.cpp for both Generals and GeneralsMD. The fix path is compiled only when RETAIL_COMPATIBLE_CRC is 0. Core/GameEngine/Include/Common/GameDefines.h defines RETAIL_COMPATIBLE_CRC as 1 by default. The build option RTS_BUILD_OPTION_RETAIL_COMPATIBLE_GAME defaults to DEFAULT, which follows that define (cmake/config-retail.cmake). In the default build, doAttackState keeps the airborneTargetGone() call path, and doKillState calls airborneTargetGone(), which switches the missile to KILL_SELF. The reported behavior therefore remains in the default configuration. The fix applies only when the retail-incompatible option is set to OFF. No automated test covers this behavior.Resolution
Make the fix active in the default build configuration for both MissileAIUpdate implementations. This changes CRC behavior, so the Game Design Committee decision mentioned in the PR discussion must be made first. If the Committee approves the change, remove or change the RETAIL_COMPATIBLE_CRC gate for this fix path and set the default accordingly. If the fix stays opt-in, the issue requirement is not met by default and the PR must state that limitation. Add an automated regression test for fuel expiry and target loss near a tracked target, if the repository supports missile behavior tests.
-
- Autofix · 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.
Comment @coderabbitai help to get the list of available commands.
 [Medium impact] The change reaches missile flight behavior in both game versions. The PR still needs the previously reported target-loss fix before merging. Findings
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| #if RETAIL_COMPATIBLE_CRC | ||
| // TheSuperHackers @bugfix Stubbjax 01/10/2026 Stop missiles ungracefully deleting if their target | ||
| // dies or if fuel exhausts within locking distance of a tracked target. | ||
|
|
||
| // I would really love it if this could retarget, but all of the targeting and legality is done | ||
| // by the Weapon that fired me. The safest thing for me to do in this state is to just run out of gas. | ||
| m_fuelExpirationDate = TheGameLogic->getFrame(); | ||
| switchToState(KILL_SELF); | ||
| #endif |
There was a problem hiding this comment.
With RETAIL_COMPATIBLE_CRC=0, airborneTargetGone() now does nothing when a tracked target disappears. If fuel remains, doAttackState() keeps restoring thrust and steering, while doKillState() also keeps OBJECT_STATUS_BRAKING set. AIMoveToState::update() continues toward the saved goal without the target. The missile therefore keeps seeking the old position instead of continuing along its current trajectory under gravity.
Add a non-retail target-loss path that stops thrust and steering, removes exhaust, and clears braking. Apply it in both game copies.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
f412841e-e62a-4b04-b4e2-f0067388a01b
📒 Files selected for processing: 2
Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/MissileAIUpdate.cppGeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/MissileAIUpdate.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.
| //------------------------------------------------------------------------------------------------- | ||
| void MissileAIUpdate::airborneTargetGone() | ||
| { | ||
| #if RETAIL_COMPATIBLE_CRC |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stop propulsion when a tracked target disappears.
If a tracked target dies before fuel expires, airborneTargetGone() now does nothing in non-retail builds. doAttackState() continues to set acceleration and turn rate, so the missile does not immediately continue on its current trajectory and fall as the PR describes. Give target loss its own coasting transition without entering KILL_SELF. The same guard appears in GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/MissileAIUpdate.cpp at Line 812. (github.com)
|
This is a massive buff for missiles. Needs Committee vote :) |
| @@ -737,10 +754,15 @@ Also determines whether objects are blocked, and if so, if they are stuck. jba. | |||
| //------------------------------------------------------------------------------------------------- | |||
| void MissileAIUpdate::airborneTargetGone() | |||
There was a problem hiding this comment.
If this whole function body is now empty in non retail, then I suggest do
#if RETAIL_COMPATIBLE_CRC
void MissileAIUpdate::airborneTargetGone()
{
...
}
#endifand compile out all callsites too.
| airborneTargetGone(); | ||
|
|
||
| #if !RETAIL_COMPATIBLE_CRC | ||
| if (curLoco) |
There was a problem hiding this comment.
Actionable comments posted: 1
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
81991660-42a8-49d9-9baa-de6d04970dc4
-
Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/MissileAIUpdate.cppGeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/MissileAIUpdate.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.
-
a641731 to
33d2728
Compare
| } | ||
|
|
||
| tossExhaust(); | ||
| obj->setStatus(MAKE_OBJECT_STATUS_MASK(OBJECT_STATUS_BRAKING), false); |
There was a problem hiding this comment.
Should this also be in the doAttackState() variant of this code?
There was a problem hiding this comment.
It can only become true in the kill state and we can't return to the attack state from the kill state.
Fixes #1700 from the patch repository
This change fixes an issue where tracking missiles would be ungracefully deleted if their target died or if fuel was exhausted within locking distance of a tracked target. Missiles instead now continue along their current trajectory and let gravity do the rest.
Before
BEFORE.mp4
BEFORE.mp4
BEFORE.mp4
BEFORE.mp4
BEFORE.mp4
After
AFTER.mp4
AFTER.mp4
AFTER.mp4
AFTER.mp4
AFTER.mp4