[WebbontheWeb]

• Fixes Bug in AIUpdate::checkForCrateToPickup() #576

Updates AIUpdateInterface::checkForCrateToPickup to search for crates before clearing their id, allowing units to correctly pickup crates. Since this updates AI behavior, it's turned off with RETAIL_COMPATIBLE_CRC.

Before update - both guard and idle units ignore the crate

beforesalvagefix.mp4

After update - both guard and idle units persue the crate

aftersalvagefix.mp4

Map:
SalvagePickup.zip

All replays ran successfully.

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

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: 6ddf8a82-2554-48be-b7d7-bd74136b2dca

📥 Commits

Reviewing files that changed from the base of the PR and between bcd2da6927b5f7e9ee1090f224bbd0b87fe4a781 and d1014b0.

📒 Files selected for processing (2)

• Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.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

Generals and Zero Hour now use build-specific ordering for the recorded crate ID reset and crate lookup in checkForCrateToPickup. Retail-compatible CRC builds clear the ID before lookup. Other builds look up the crate before clearing the ID.

Changes

Crate pickup handling

Layer / File(s)	Summary
Crate lookup handling Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp	Both game editions clear m_crateCreated before lookup in retail-compatible CRC builds. In other builds, they look up the crate before clearing the ID.

Priority: ⬇️ Low

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: caball009

Merge Risk: ⚪ Minimal · up to d1014

The change is limited to the intended crate pickup correction and does not introduce an established merge-blocking risk.

Pre-merge checks: 4

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Issue #576 requires the crate lookup to occur before m_crateCreated is cleared in both Generals and Zero Hour. The whole-PR summary confirms this order in both AIUpdate.cpp files for non-retail-co…
Out of Scope Changes check		The changes are limited to checkForCrateToPickup in the Generals and Zero Hour AIUpdate.cpp files. The compatibility guards support the requested behavior change. No unrelated change is identified…
Description check		The description directly explains the crate lookup fix, the affected AI behavior, the RETAIL_COMPATIBLE_CRC condition, and the test results.
Title check		The title clearly identifies the AI update bug and states that checkForCrateToPickup failed to return a crate.

• 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Fixes checkForCrateToPickup in both game versions.

• Idle and guard units can find and pursue the crates they create.

No actionable issues were found.

_{Reviews (3) · Last reviewed commit: "bugfix(ai): Fix AIUpdateInterface::check..." · Reviewed by Greptile}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Retain the fallback for blocked in-range positions. · AIUpdate.cpp:3418-3422

Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp:3418-3422
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Retain the fallback for blocked in-range positions.

When the attack position is in range but its attack view is blocked, the current non-retail guard skips findPositionAround. computeAttackPath then tries to path to the exact unreachable position and can fail. A nearby position returned by findPositionAround can provide a reachable, clear attack position.

Keep the range optimization only when the attack view is clear in both editions.

Suggested fix

-if (weapon && weapon->isContactWeapon() && !weapon->isWithinAttackRange(getObject(), &localPos) && !isPathAvailable(&localPos))
+if (weapon && weapon->isContactWeapon() &&
+	(!weapon->isWithinAttackRange(getObject(), &localPos) ||
+	 (isDoingGroundMovement() &&
+	  TheAI->pathfinder()->isAttackViewBlockedByObstacle(
+		  getObject(), *getObject()->getPosition(), nullptr, localPos))) &&
+	!isPathAvailable(&localPos))

Apply the same change in GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp.

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: c535d213-a6b4-4625-a0b9-d1f75c8165ad

📥 Commits

Reviewing files that changed from the base of the PR and between 0e6a06c and 98e71bb0789d0a23921f95abb2b66c741fc749ef.

📒 Files selected for processing (2)

• Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.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.

[Caball009]

Looks good to me.

EDIT: I think the title could mention that this makes AI GLA players pick up the scrap / salvage crates. I'll leave it to xezon, though, since they regularly adjust PR titles before merging anyway.