bugfix: Contact weapons are no longer blocked by obstacles - #3194
bugfix: Contact weapons are no longer blocked by obstacles#3194Stubbjax wants to merge 2 commits 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.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| #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?
|
Maybe it would be better to check within // srj sez: this is a good start at taking terrain into account for attacks, but findAttackPath needs to be smartened also
#define LOS_TERRAIN
#ifdef LOS_TERRAIN
const Weapon* w = attacker->getCurrentWeapon();
if (attacker->isKindOf(KINDOF_IMMOBILE)) {
// Don't take terrain blockage into account, since we can't move around it. jba.
w = nullptr;
}
if (w)
{
if (w->isContactWeapon())
return false;
Bool viewBlocked;
if (victim)
viewBlocked = !w->isClearGoalFiringLineOfSightTerrain(attacker, attackerPos, victim);
else
viewBlocked = !w->isClearGoalFiringLineOfSightTerrain(attacker, attackerPos, victimPos);
if (viewBlocked)
{
//CRCDEBUG_LOG(("Pathfinder::isAttackViewBlockedByObstacle() 3"));
return true;
}
}
#endif |
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Source/GameLogic/AI/AIPathfind.cpp | Bypasses obstacle-based attack-view rejection for contact weapons in non-retail-compatible builds. |
| Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp | Preserves an in-range contact weapon's requested attack position while retaining reachable-position fallback when movement is needed. |
| GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp | Mirrors the Generals contact-weapon attack-position fix for Zero Hour. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Contact weapon attacks position] --> B{Already within attack range?}
B -- Yes --> C[Keep requested attack position]
B -- No --> D{Path available?}
D -- Yes --> C
D -- No --> E[Find nearby reachable position]
C --> F[Obstacle view check]
F --> G[Contact weapon bypasses obstacle blocking]
Reviews (1): Last reviewed commit: "chore: Expand condition coverage" | Re-trigger Greptile
5 participants
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