Attack Command: keep moving towards target if static obstruction blocks shot - #3241
Attack Command: keep moving towards target if static obstruction blocks shot#3241eun-ice wants to merge 6 commits into
Conversation
|
Is there any profiling of the before/after for both the aim check change and the optimisation? |
yes, but in my test scenarios the differences are immeasurably small, the check is only run all 15 frames as part of SlowUpdate anyways. The second optimization commit should (TM) restore at least the performance from before the correctness fix. How can I A/B benchmark a behavior change? |
ce257ec to
1645c48
Compare
|
I downgraded this PR to draft. The original PR tried to treat this as a bug and did solve the issue in my test cases but was insufficient. I tried several ideas: Use the strafe mechanic, make the unit walk orthogonal to the target line, etc. once the unit gets in a "stopped" state, i tried caching that there is no firing solution - none of them solved the test case. This draft PR is now a simple "detect blocked state" => "continue moving to the target" logic. But that causes the sheldons in the back to walk to the front, blocking other sheldons in the process, who then want to walk to the front etc. My original PR here did none of that and worked in all my manual test cases. Maybe someone with a better grasp of the unit movement can attempt a fix? The problem is real. I preserved the previous attempt here: |
Is that not what maneuver does? E.g. Grunts on maneuver will sidestep to get a shot if their target is blocked by a friendly grunt. |
So you are suggesting I trigger the "maneuver" code (or something similar to it) when a blocked state is detected? |
305b90b to
81eb4f4
Compare
|
I am in over my head here, the movement AI is too complex for a newcomer to modify (at least for me). However the problem is real and I want to help fix it, within my capabilities. @sprunk do you have guidance on how I should proceed? Is my original solution really so problematic? From all my trial and error attempts, it consistently produced the best results. Is there a way for me to reproduce the issue you mentioned with my original PR (that a target becomes invalid after rotation and the unit would freeze)? Don't you think that would be less bad than the current state of the game? Would anyone be interested in solving this? |
|
this also happens with ground attacks 2026-08-26.00-25-12.-.00m20s-00m40s.mp4 |
|
@sprunk I did port solution 2) and solution 4) to ground attacks. I do not claim these are ready to be used, as I do not have enough knowledge of the movement AI code. However, I hope my reproducer and solution attempts might help someone more experienced to fix this. IMO this is a valid issue that should be addressed. |
7f78d1b to
261ed1f
Compare
eun-ice
changed the title
|
working on a better solution. |
eun-ice
changed the title
CGround::LineGroundCol returns a hit distance of 0 when the ray origin is underground, but TraceRay only accepted ground hits with a distance > 0 and CWeapon::HaveFreeLineOfFire applied the same filter to the result. A ray from an underground origin was therefore reported as unobstructed, so a weapon whose muzzle (or aim-from piece) sat inside a cliff passed the line-of-fire test, stopped, and could never fire (beyond-all-reason#3242), and Spring.GetUnitWeaponHaveFreeLineOfFire told game code the same (beyond-all-reason#3301). Accept 0 as a hit in both places: the ray is blocked at its origin. CCannon::HaveFreeLineOfFire had the same pattern with TrajectoryGroundCol, which also reports 0 for an origin below the terrain, but tests against GetApproximateHeight; on rough ground that can lie above a muzzle that is clear of the interpolated surface. Reject an origin below GetHeightReal explicitly instead, the test the fire-time check already applies to the muzzle, and keep ignoring the coarse 0 from the trajectory scan. The underground test of LineGroundCol itself compared the origin against the corner vertex of its heightmap square. Next to a steep cliff that vertex can be far above an origin that is well clear of the ground, which skipped the whole ground trace. Compare against the interpolated surface instead, and treat an origin exactly on the surface as above ground; LineGroundSquareCol still reports a hit at distance 0 when such a ray points into the ground. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Since fb385e4 (beyond-all-reason#2814) CallAimingScript decomposes the full wantedDir in the unit's frame and passes atan2(d.rightdir, d.frontdir) as the AimWeapon heading. For a steep launch vector (high-trajectory cannons aim 70-80 degrees up) the horizontal part of wantedDir is small, so a few degrees of hull tilt contribute more to d.frontdir than the target's azimuth and the yaw swings by tens of degrees: on the beyond-all-reason#3324 Wolverine reproducer the target is 71 degrees off the hull but the script received -108 degrees. Scripts with a yaw arc check (BAR's Wolverine and Mart refuse headings beyond +-90 degrees) then return 0, angleGood is never set, and because TestRange/CheckTargetAngleConstraint still consider the target in arc the CommandAI neither turns the hull nor moves, so the unit stands still without firing. Take the heading from the horizontal part of wantedDir (falling back to the target's azimuth, then the hull's front, for vertical shots), still in the unit's frame, and keep the unit-frame pitch. Identical to the previous behaviour for horizontal shots. Fixes beyond-all-reason#3324. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…al extra value The heading passed to AimWeapon is the azimuth again (previous commit), which is what arc checks need. The exact unit-frame yaw of the full shot direction, the value passed as heading since 2026.06, is still the right angle to turn the turret by on tilted ground, so offer it alongside without changing the meaning of the existing arguments: * LUS: optional fourth argument, AimWeapon(weaponNum, heading, pitch, launchHeading), radians. The base-content unit_script gadget wrapper passes it through. Existing scripts ignore it. * COB: get WEAPON_LAUNCH_HEADING(weaponNum) (141), COB angle units with the same convention as the heading argument. A third positional argument would shift the function's locals, so COB gets a getter instead. CWeapon stores the value at each AimWeapon callin (creg member, so it survives save/load). Changelog entry added, and its release reference fixed: the unit-frame heading shipped in 2026.06, not 2026.02. Syntax-checked the touched translation units; not run in game. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…re tests
TryTarget(preFire=false), used for target selection and by
SlowUpdate/HoldIfTargetInvalid, traced line of fire from the AimFromWeapon
piece (or, for Cannon/MissileLauncher/StarburstLauncher, the current muzzle),
while the shot itself is tested from the muzzle after the turret has turned.
Where the two disagree a unit either aims and then never fires, or drops a
target it could have hit.
Add an optional unit def weapon tag
aimFromEstimate = {pivotX, pivotY, pivotZ, lateral, forward, barrelForward, barrelUp}
describing the muzzle as a yaw pivot plus offsets turning with yaw only and
with yaw and pitch (AimFromEstimate.h; one sqrt, no trig, linear in the
offsets so it can be fitted offline). With the tag, GetAimFromPos(tgtPos)
returns the predicted muzzle for that target using the direction the aiming
script would be asked for (CalcWantedDir for cannons, trajectoryHeight for
missiles). Without it the AimFromWeapon piece is used as before; the three
weapon classes that overrode GetAimFromPos to the current muzzle now follow
the same rule.
Weapons that alternate between barrels use one estimate for the mean muzzle:
which barrel fires next cannot be predicted from the QueryWeapon piece the
engine holds at aiming time (measured on BAR's Storm, AK and Pawn the relation
differs per script and burst state), so per-barrel entries were dropped.
Also add Spring.GetUnitWeaponAimFromPos(unitID, weaponNum, x, y, z) and fix
Spring.GetUnitWeaponTryTarget(unitID, weaponNum, x, y, z), which built
SWeaponTarget(nullptr, pos) and therefore always tested (0, 0, 0).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…fire A mobile unit inside 0.9 * maxRange of its attack target whose weapons all fail the preliminary line-of-fire test used to plain-return (unit target) or stop (ground target) and retry from the same spot every SlowUpdate. With the test now traced from the predicted muzzle (beyond-all-reason#3327) the answer is honest, but a unit parked behind a rock or a crest still never fires (beyond-all-reason#3242). The obvious fix, treating "in range but no solution" like "out of range" and closing in, has a known failure mode: in a group the rear units are blocked by the front units, walk past them, and thereby block the units they passed, so the whole ball creeps forward. The CommandAI cannot tell the two cases apart because the line-of-fire test only returns a bool. The plain return has the same failure mode already: the goal set with the attack order is the target itself, so a unit that enters range without a solution just keeps walking, which the false positives of the old pre-aim test used to hide. * TraceRay: add Collision::NOMOBILEFRIENDLIES, a trace-only flag (never part of a weapon's avoidFlags) that skips allied units that are able to move. Allied structures, features and terrain are still scanned. Honoured by TraceRay, TestCone, TestTrajectoryCone and the MissileLauncher trajectory scan. * CWeapon::TryTargetRotate / TryTargetHeading take optional extra avoid flags for one test. * CMobileCAI: when no weapon has a solution although the target is in range, test again with mobile allies ignored. If that still fails the blocker is static (terrain, feature, allied wall or building) and the unit keeps closing in like an out-of-range unit would, for unit and ground targets. If it passes, only an allied unit is in the way and it may move out of it, so the unit stops and waits instead of walking past it. The approach is bounded: it stops at 20% of maxRange (so artillery does not walk up to a target that sits behind a wall), an approach that fails (unreachable goal) is not retried for 10 seconds, stopToAttack units always wait, and the strafeToAttack, gunship, very-close and owner-rotation branches keep their old behaviour. Hold-position units follow the same logic; an explicit attack order already makes them approach an out-of-range target and the temp-order case is cancelled earlier as before. Fixes beyond-all-reason#3242.
…errain TryTarget rejected an aim position below the ground only at fire time, for the real muzzle. The pre-aim test traced from the predicted muzzle (aimFromEstimate) did not, so a Sheldon standing against a cliff with its muzzle 60 elmo inside the rock passed the pre-aim test, stopped, and could never fire; the CommandAI saw no reason to move it. Apply the check to the predicted muzzle as well. The AimFromWeapon piece is deliberately left alone, it may legitimately sit inside the hull below the surface on slopes.
261ed1f to
d307d77
Compare
4 participants
Fixes #3242.
Stacked on #3328 (rays that start below the terrain), #3325 (AimWeapon heading) and #3327 (
aimFromEstimate): the first four commits are theirs and disappear from this diff once they land. The fix itself is the last two commits.Problem
In current game, units with an attack command that are within 90% of their weapons range will stop moving towards the target, even if terrain obstructs the shot.
This results in units disobeying a players attack command and just cowardly hiding behind a rock.
CMobileCAIdecides whether an attacker stops or keeps moving with the preliminary line-of-fire test (TryTargetRotatefor unit targets,TryTargetHeadingfor ground targets). When the target is inside0.9 * maxRangeand no weapon passes that test, the CAI does nothing useful: it plain-returns (unit target) or stops and keeps pointing (ground target), and retries from the same spot every SlowUpdate.Video of the issue on master (five Sheldons, two stuck behind the rock, one not shooting behind another Sheldon):
attack-obstacle-original-master.mp4
Earlier attempts (kept for reference)
strafeToAttackcode when there is no solution — best results at the time, preserved inarchive/attack-obstacle-close-in-original. @sprunk pointed out that a solution found by rotating can become invalid again after the rotation.FindBlockedAttackSidestepGoal): works in the reproducers but a lot of code. Preserved inarchive/attack-obstacle-close-in-sidestep.Idea
The current AI does nothing at 90% of weapons range if there's no shot, causing units to stay behind rocks or blindly continue walking depending on the unit state.
If we tell the unit to continue closing in when the shot is blocked, it causes the creep behavior in groups of units, where the back of the pack tries to move to the front and then the other units get block and they try to get to the front.
So the simple boolean test, blocked or not, is not enough. A unit in firing range should continue moving if terrain or buildings obstruct a shot but they should stop moving if a friendly unit is in the way.
The core idea of this PR is to differentiate between the two obstruction types.
Implementation
Collision::NOMOBILEFRIENDLIES, a trace-only flag (never part of a weapon'savoidFlags) that skips allied units that are able to move. Allied structures, features and terrain are still scanned. Honoured byTraceRay,TestCone,TestTrajectoryConeand the MissileLauncher trajectory scan.TryTargetRotate/TryTargetHeadingtake optional extra avoid flags for one test.StopMove+KeepPointingTo) instead of walking past it.maxRange(artillery does not walk up to a target that sits behind a wall); an approach that fails (unreachable goal, e.g. the rock itself) is not retried for 10 seconds;stopToAttackunits always wait; thestrafeToAttacksidestep and the gunship / very-close / owner-rotation branches are unchanged. Hold-position units follow the same logic: an explicit attack order already makes them approach an out-of-range target, and the temp-order case is cancelled earlier as before.aimFromEstimate). Without it a Sheldon standing against a cliff (muzzle 60 elmo inside the rock) passes the pre-aim test, stops, and can never fire; with it the CAI sees a static blocker and moves the unit out. TheAimFromWeaponpiece is deliberately not checked: it may legitimately sit inside the hull below the surface on slopes. With Weapons: Fix muzzle below terrain reporting free line of fire #3328 an underground predicted muzzle is also caught byHaveFreeLineOfFireitself; this check stays because it covers weapons that skip ground checks (avoidGround = false) and mirrors the fire-time test.Why stopping matters: the plain
returnof the old code has the creep problem of attempt 3 already. The goal set with the attack order is the target itself, so a unit that enters range without a solution simply keeps walking; the false positives of the old pre-aim test hid that. With the honest test a column of five Sheldons ordered to attack a Fatboy 600 elmo away walks all the way up to it.Performance consideration
Cost: For every units that are within 90% range of their target and have no firing solution we make one extra
TryTargetRotateper weapon per SlowUpdate.Validation
Headless runs of BAR overlays (gadgets, start scripts and runner: https://github.com/eun-ice/bar-attack-reproducers), BAR
feature/aim-from-estimate(beyond-all-reason/Beyond-All-Reason#9090). "baseline" = #3325 + #3327 without the fix commits. Five Sheldons per run,targetHitsandmoved(elmos) per unit at frame 800:movedLast60Frames=0)Two reproducers built from player reports, plus one more wall variant, run against master (not the #3325 + #3327 baseline) and the current branch stacked on #3328:
cormship1396–1630 elmo from an advanced geo on the 304 elmo plateau, ground attack on itmoveState=done),tryTarget=false, 0 rockets in 60 sThe commander case is the unit-target sibling of #3242 but this change cannot see it: the commander's sea laser (never aims on land) passes
TryTargetRotatefrom its forward muzzle piece while the land laser is out of range, so the CAI is told there is a solution and stops. Only a "no weapon has a solution" state triggers the re-test added here. It needs its own fix (a weapon the script refuses to aim should not count as a solution); reproducer and analysis are in the repo above.The two waiting units in the static-wall row are the price of the "wait for allies" rule: the allies in the way are their own group mates, which now stand and fire. A sidestep for that case (lateral goal instead of waiting) is a possible follow-up; it is a different problem from #3242 and was left out on purpose.
Videos
Same scenarios as above, recorded fullscreen at 1x; baseline = #3325 + #3327 without the fix commits. Each video opens with a two-second still of the end state, then runs from game start to the result at frame 800.
Rock, maneuver: baseline. Two Sheldons stay behind the rock, one of them with its muzzle inside the cliff.
rock-maneuver-before-preview.mp4
Rock, maneuver: this branch. All five walk out and stop once they have a shot. (Re-recorded with the branch stacked on #3328.)
rock-maneuver-after-preview.mp4
Rock, hold position: baseline.
rock-holdpos-before-preview.mp4
Rock, hold position: this branch.
rock-holdpos-after-preview.mp4
Bantha row (mobile allies): baseline. The Sheldons push into the row of allied Banthas that blocks their shot.
wall-banthas-before-preview.mp4
Bantha row (mobile allies): this branch. Nobody moves; they wait for the Banthas.
wall-banthas-after-preview.mp4
Fortification row (allied structures): baseline.
wall-fortifications-before-preview.mp4
Fortification row (allied structures): this branch. Three walk around the wall and fire, two wait behind the Sheldons that went first.
wall-fortifications-after-preview.mp4
Rocko on the crest, plain attack order: this branch. Fires from where it stands (#3325 + #3327 already fixed this; unchanged here).
rocko-attack-after-preview.mp4
Missile cruisers vs. plateau geo, far layout: master. All four sit at 1396–1630 elmo, aim at the geo, and never fire (cut at 32 s, nothing happens afterwards).
mship-geo-far-before-preview.mp4
Missile cruisers, far layout: this branch. All four close in and shell the geo (cut at 32 s).
mship-geo-far-after-preview.mp4
Missile cruisers, close layout: master. The two nearest fire, the two farthest never do.
mship-geo-close-before-preview.mp4
Missile cruisers, close layout: this branch. The two farthest move up and all four fire.
mship-geo-close-after-preview.mp4
Commander vs. Ticks: master. Stops 320 elmo from the Ticks with the attack order and never fires.
corcom-ticks-before-preview.mp4
Commander vs. Ticks: this branch. Unchanged, see above.
corcom-ticks-after-preview.mp4
Known residual visible in the rock videos: a Sheldon can stop at a spot where the pre-aim test from the estimated muzzle says "clear" but the real muzzle is a few elmo above the terrain and the fire-time test says "blocked". The CommandAI is then told there is a solution and stays; that is an estimate-accuracy question for #3327 / the BAR values, not something this change can see.
AI disclosure
OpenAI Codex assisted with the earlier attempts; Claude Code (Fable 5.1) assisted with diagnosis, implementation, the reproducers and this description of the current approach. I directed the iterations and reviewed the diff and in-game behaviour.