bugfix: Contact weapons are no longer blocked by obstacles - #3194
bugfix: Contact weapons are no longer blocked by obstacles#3194Stubbjax wants to merge 1 commit into
Conversation
PR Summary by QodoFix contact weapons being blocked by obstacle line-of-sight checks
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1. Fix compiled out by default
|
| #if RETAIL_COMPATIBLE_CRC | ||
| if ( ai->isDoingGroundMovement() ) | ||
| #else | ||
| if ( ai->isDoingGroundMovement() && !weapon->isContactWeapon() ) | ||
| #endif |
There was a problem hiding this comment.
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
| #if RETAIL_COMPATIBLE_CRC | ||
| if (weapon && weapon->isContactWeapon() && !isPathAvailable(&localPos)) | ||
| #else | ||
| if (weapon && weapon->isContactWeapon() && !weapon->isWithinAttackRange(getObject(), &localPos) && !isPathAvailable(&localPos)) | ||
| #endif |
There was a problem hiding this comment.
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
|
Do we know exactly what units this effects? Or is this only for the suicide ability? |
Also this issue with Terrorists: TERROR_DANCE.mp4 |
|
Okey this is actually great seeing this fixed. If you need testing for it feel free to share it in the testing discord. And I will see what can be done. Good work |
|
Code looks good to me. I know it basically fixes a bug, but should this go passed the game committee? Non-bugged is a lot more powerful. I don't know if the 'bug' is considered to be a feature nowadays. |
| if ( ai->isDoingGroundMovement() && !weapon->isContactWeapon() ) | ||
| #endif | ||
| { | ||
| viewBlocked = TheAI->pathfinder()->isAttackViewBlockedByObstacle(source, *source->getPosition(), nullptr, m_goalPosition); |
There was a problem hiding this comment.
There are 8 calls to isAttackViewBlockedByObstacle in this code base but only 2 callsites are tackled in this change. Is this sufficient?
This change fixes an issue where contact weapons fired on a location would be blocked by obstacles.
This was most notable when attempting to suicide any units that were intersecting obstacle geometry, where the respective unit(s) would get stuck due to the way in which the attack state machines would continuously bail out due to an obstacle being in the way, while being unable to find a new destination due to the weapon's attack range of 0 requiring no movement.
Before
A Suicide command will not detonate any units intersecting an obstacle, and they would often get stuck in a cyclical state
BEFORE.mp4
After
A Suicide command will detonate units regardless of any intersecting obstacles
AFTER.mp4