Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1214,7 +1214,11 @@ Bool outOfWeaponRangePosition( State *thisState, void* userData )
}

Bool viewBlocked = false;
#if RETAIL_COMPATIBLE_CRC
if (onGround)
#else
if (onGround && !weapon->isContactWeapon())
#endif
Comment thread
xezon marked this conversation as resolved.
{
viewBlocked = TheAI->pathfinder()->isAttackViewBlockedByObstacle(obj, *obj->getPosition(), nullptr, *pos);
}
Expand Down Expand Up @@ -2654,7 +2658,11 @@ StateReturnType AIAttackApproachTargetState::updateInternal()
if (m_stopIfInRange && weapon && weapon->isWithinAttackRange(source, &m_goalPosition))
{
Bool viewBlocked = false;
#if RETAIL_COMPATIBLE_CRC
if ( ai->isDoingGroundMovement() )
#else
if ( ai->isDoingGroundMovement() && !weapon->isContactWeapon() )
#endif
Comment on lines +2661 to +2665

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Contact attacks ignore obstacle los 🐞 Bug ≡ Correctness

For attack-position commands, contact weapons now skip isAttackViewBlockedByObstacle, so the
approach/aim states can succeed and fire based only on distance even when an obstacle blocks the
target point. This can enable detonating a contact weapon at an obstacle-blocked position (e.g.,
through a thin wall/fence) from an adjacent reachable cell, which is broader than “unit intersecting
obstacle geometry.”
Agent Prompt
### Issue description
The PR skips obstacle visibility checks for contact weapons when attacking a *position*, which can allow contact detonation at obstacle-blocked points as long as the attacker is within (small) range.

### Issue Context
- `WeaponTemplate::isContactWeapon()` is range-based (< pathfind cell size), not “must be physically colliding right now”.
- `Weapon::isWithinAttackRange(source, pos)` is purely distance/min-range based and does not account for obstacles.
- The aim state explicitly treats contact weapons as “don’t aim, just go boom”, so once the state machine believes it is in range, it will fire.

### Fix Focus Areas
- Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[1216-1232]
- Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[2658-2673]
- Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[4811-4879]
- Generals/Code/GameEngine/Source/GameLogic/Object/Weapon.cpp[535-547]
- Generals/Code/GameEngine/Source/GameLogic/Object/Weapon.cpp[2056-2070]

### Suggested direction
Keep the LOS/obstacle check for contact weapons except for the specific “already overlapping the target point” case you’re trying to fix (e.g., only bypass when the attacker is essentially at the goal position within a very small epsilon, not merely within attack range).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

{
viewBlocked = TheAI->pathfinder()->isAttackViewBlockedByObstacle(source, *source->getPosition(), nullptr, m_goalPosition);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3399,7 +3399,11 @@ void AIUpdateInterface::privateAttackPosition( const Coord3D *pos, Int maxShotsT
// this fixes an obscure bug with mine-clearing: if you tell someone to clear mines and put the centerpoint
// inside a building, the dozer/worker will just go thru the building to that spot. ick. so if you find that
// this clause (below) is problematic, you'll probably have to find another way to fix this mine-clearing bug. (srj)
#if RETAIL_COMPATIBLE_CRC
if (weapon && weapon->isContactWeapon() && !isPathAvailable(&localPos))
#else
if (weapon && weapon->isContactWeapon() && !weapon->isWithinAttackRange(getObject(), &localPos) && !isPathAvailable(&localPos))
#endif
Comment on lines +3402 to +3406

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

3. Unpathable contact target preserved 🐞 Bug ≡ Correctness

privateAttackPosition now skips the “find a nearby pathable spot” adjustment for contact weapons
if isWithinAttackRange() is true, even when isPathAvailable() is false. Combined with the LOS
bypass, this lets a unit execute a contact attack on an explicitly unpathable goal point (e.g.,
inside blocking geometry) as long as it can get within range outside the obstacle.
Agent Prompt
### Issue description
The new condition in `privateAttackPosition()` bypasses the existing safety behavior for contact weapons (“must be able to path to the target pos”) whenever the attacker is merely within attack range, even if the goal point is unpathable.

### Issue Context
`Weapon::isWithinAttackRange(source, pos)` is distance-only, while the comment/behavior here is about reachability/pathing. For contact weapons, “within range” can still be on the other side of a blocking obstacle.

### Fix Focus Areas
- Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[3398-3415]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[3553-3561]
- Generals/Code/GameEngine/Source/GameLogic/Object/Weapon.cpp[2056-2070]

### Suggested direction
Only skip the `isPathAvailable()`/fallback relocation when the unit is effectively already at the target point (very small positional epsilon), rather than when it is merely within attack range.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

{
FindPositionOptions fpOptions;
fpOptions.minRadius = 0.0f;
Expand Down
8 changes: 8 additions & 0 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,11 @@ Bool outOfWeaponRangePosition( State *thisState, void* userData )
}

Bool viewBlocked = false;
#if RETAIL_COMPATIBLE_CRC
if (onGround)
#else
if (onGround && !weapon->isContactWeapon())
#endif
{
viewBlocked = TheAI->pathfinder()->isAttackViewBlockedByObstacle(obj, *obj->getPosition(), nullptr, *pos);
}
Expand Down Expand Up @@ -2740,7 +2744,11 @@ StateReturnType AIAttackApproachTargetState::updateInternal()
if (m_stopIfInRange && weapon && weapon->isWithinAttackRange(source, &m_goalPosition))
{
Bool viewBlocked = false;
#if RETAIL_COMPATIBLE_CRC
if ( ai->isDoingGroundMovement() )
#else
if ( ai->isDoingGroundMovement() && !weapon->isContactWeapon() )
#endif
{
viewBlocked = TheAI->pathfinder()->isAttackViewBlockedByObstacle(source, *source->getPosition(), nullptr, m_goalPosition);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are 8 calls to isAttackViewBlockedByObstacle in this code base but only 2 callsites are tackled in this change. Is this sufficient?

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3554,7 +3554,11 @@ void AIUpdateInterface::privateAttackPosition( const Coord3D *pos, Int maxShotsT
// this fixes an obscure bug with mine-clearing: if you tell someone to clear mines and put the centerpoint
// inside a building, the dozer/worker will just go thru the building to that spot. ick. so if you find that
// this clause (below) is problematic, you'll probably have to find another way to fix this mine-clearing bug. (srj)
#if RETAIL_COMPATIBLE_CRC
if (weapon && weapon->isContactWeapon() && !isPathAvailable(&localPos))
#else
if (weapon && weapon->isContactWeapon() && !weapon->isWithinAttackRange(getObject(), &localPos) && !isPathAvailable(&localPos))
#endif
{
FindPositionOptions fpOptions;
fpOptions.minRadius = 0.0f;
Expand Down
Loading