bugfix(ai): Fix checkForCrateToPickup never returning the created crate

[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

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: e6eb66d3-97b4-4ff9-a3f7-3503cbce41ba

---

📥 Commits

Reviewing files that changed from the base of the PR and between 98e71bb and bcd2da6.

---

📒 Files selected for processing (2)

• Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp

---

 ________________________________________
< I'm a lean, mean, code review machine. >
 ----------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

Walkthrough

AI update logic now uses build-specific behavior for crate lookup and contact-weapon position searches in Generals and Zero Hour. Non-retail-compatible Zero Hour builds also validate explicit exit targets.

Changes

AI update behavior

Layer / File(s)	Summary
Crate lookup ordering Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp	Non-retail-compatible builds look up the crate before clearing its ID. Retail-compatible builds retain the previous order.
Contact-weapon position search Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp	Non-retail-compatible builds add an attack-range condition to the position search. In Zero Hour, the search also requires that no path exists. Retail-compatible builds retain the prior condition.
Explicit exit validation GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp	In non-retail-compatible builds, explicit exit commands return if the target has no contain module or does not contain the issuing object.

Priority: ⬇️ Low

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: caball009

Merge Risk: 🔵 Low · up to 98e71

Some contact-weapon units may fail to find a reachable attack position when obstacles block their view. Restore the fallback in both editions; this is a localized gameplay issue.

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 diff implements that order in Generals/.../AIUpdate.cpp and `GeneralsMD/…
Out of Scope Changes check		The whole-PR diff contains only the crate-processing changes in the two files required by issue #576. The preprocessor guards and comments support the requested compatibility behavior. No unrelated ch…
Title check		The title clearly identifies the AI bug fix and the affected behavior: checkForCrateToPickup fails to return the created crate.
Description check		The description directly explains the crate lookup fix, the RETAIL_COMPATIBLE_CRC behavior, reproduction results, and validation evidence.

---

---

---

• 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; it fixes the lookup order without changing retail-compatible behavior.

Summary

Fixes checkForCrateToPickup in both game versions by looking up the crate before clearing its stored ID.

• AI units find kill-created crates before their stored ID is cleared.

_{Reviews (1) · 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 98e71bb.

📒 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]

Once the retail compatible code is removed the comment is unnecessary. Better to put the comment in the retail compatibility section (3 lines up), and adjust it slightly so that it describes the retail compatible code.

[Caball009]

I think we can take advantage of the fact that the retail compatible code always returns a nullptr. Perhaps like this:

Code

Object* AIUpdateInterface::checkForCrateToPickup()
{
#if RETAIL_COMPATIBLE_CRC

	// TheSuperHackers @bugfix WebbontheWeb 07/10/2026
	// ... describe that the retail compatible behavior of this function is bugged
	m_crateCreated = INVALID_ID;
	return nullptr;

#else

	if (m_crateCreated != INVALID_ID)
	{
		Object* crate = TheGameLogic->findObjectByID(m_crateCreated);
		m_crateCreated = INVALID_ID; // we have processed it, so clear it.

		if (crate)
		{
			for (BehaviorModule** m = crate->getBehaviorModules(); *m; ++m)
			{
				CollideModuleInterface* collide = (*m)->getCollide();
				if (!collide)
					continue;

				if( collide->wouldLikeToCollideWith(getObject()))
				{
					return crate;
				}
			}
		}
	}
	return nullptr;

#endif
}

[WebbontheWeb]

I prefer my solution because it keeps the intent of the original code, even though it's bugged. I'm completely willing to update if you have strong preference though

[Caball009]

I appreciate that your version nicely shows what the bug is, but that's not worth doing unnecessary run-time work for imo.