Source Code Project Mantis - FSSCP
View Issue Details
0003147FSSCPAIpublic2015-02-27 05:412015-03-18 00:29
Reporterniffiwan 
Assigned ToAdmiral MS 
PrioritynormalSeveritymajorReproducibilityalways
StatusfeedbackResolutionfixed 
PlatformOSOS Version
Product Version3.7.2 RC5 
Target VersionFixed in Version 
Summary0003147: AI rams stationary targets
DescriptionThe AI seems to have problems with stationary targets, when attacking it doesn't avoid them. Instead it ineffectually rolls before charging into the target.

Note: originally reported by Spoon who 1st encountered this in WoD
Steps To ReproduceSee attached mission (originally from Axem, made no-mod compatible and slightly tweaked by me). Stop moving and wait for the Perseus to attack you. Don't evade or shoot, your ship is invulnerable. Most of the time the Perseus will ram you and kill itself.

When the 1st Perseus is dead, a 2nd one will warp in and attack. This one will usually manage to avoid you completely.

The only difference between the two ships is that the 1st carries Rockeyes, the 2nd carries Harpoons.
Additional InformationThis one has wierd written all over it. In addition to the oddball change in behaviour due to carrying aspect seekers or not, you also have these gems:

1) removing primary weapons make the AI evade correctly
2) clearing/removing AIF_SEEK_LOCK from the check in avoid_ship() makes all the AI ram you
3) making it so that set_predicted_enemy_pos() isn't called makes the AI evade correctly

And some more general info from IRC:

MageKing17: At a range of approx 600 meters, it enters the SM_AVOID submode.
MageKing17: Here is where behavior diverges; with aspect-seekers, it sets AIF_SEEK_LOCK, avoids calling set_predicted_enemy_pos(), and changes direction immediately upon entering SM_AVOID.
MageKing17: SM_AVOID is a submode of AI_CHASE, so if it can still hit with its weapons, it will continue firing.
MageKing17: For instance, it can and will fire off a missile in the middle of its avoidance manuever.
MageKing17: Without aspect-seekers, it calls set_predicted_enemy_pos() and for some reason, instead of changing direction, it rolls while remaining on its course, usually colliding with the player.
MageKing17: The problem cannot lie in the aspect-seeking code, because commenting out the calls to set_predicted_enemy_pos() results in the proper behavior without aspect seekers.
MageKing17: And preventing AIF_SEEK_LOCK from being set (which results in set_predicted_enemy_pos() getting called) causes the improper behavior even with aspect seekers.
MageKing17: There are a few oddities: even with the "improper" behavior, it can still sometimes break off, but this happens seemingly at random.
MageKing17: The breaking off seems to be more likely if you increase its rotational velocities, for some reason.
MageKing17: As far as I can determine, the relevant side effects of set_predicted_enemy_pos() are to introduce a very slight randomness to the predicted enemy position, and to set a couple of globals.
MageKing17: Since G_collision_time isn't used and I just checked and setting G_predicted_pos outside of set_predicted_enemy_pos() has no effect, the relevant effect must be the slight randomness added to the predicted position.
MageKing17: It's very slight, however; in my stepping through with the debugger, it was maybe a meter or two.
TagsNo tags attached.
Attached Files? cs_suicide.fs2 (4,778) 2015-02-27 05:41
http://scp.indiegames.us/mantis/file_download.php?file_id=2656&type=bug
patch ai_no_ramming.patch (576) 2015-03-02 10:21
http://scp.indiegames.us/mantis/file_download.php?file_id=2657&type=bug
patch aicode.cpp.patch (938) 2015-03-03 19:38
http://scp.indiegames.us/mantis/file_download.php?file_id=2659&type=bug

Notes
(0016517)
niffiwan   
2015-02-27 06:02   
(Last edited: 2015-02-27 06:05)
Some more notes: the issue seems to have been introduced somewhere between 3.6.14 and 3.6.16. In 3.6.14 "Rockeye" seems to reliably avoid a collision by pulling up at the last minute.

I've also noticed that mission time seems to effect the avoid behaviour, if I let both ships attack in turn I have different behaviour from "Harpoon" to when I kill "Rockeye" at mission start by firing a missile at it.

And, there seems two types of successful avoidance behaviours:
1) pull up at the last minute
2) start avoiding much further away and make one or short short secondary passes to fire primaries at the target.

(0016518)
MageKing17   
2015-02-27 14:18   
There are a number of behaviors in the AI code that use Missiontime as a pseudorandom value, so the fact that the time affects the behavior isn't that surprising to learn (in my tests, I used two separate missions to minimize differences due to Missiontime changes).

As far as I can determine, "pull up at the last minute" is a subset of "it can still sometimes break off, but this happens seemingly at random", but heck if I know why the different behaviors.

Also, the "short secondary passes" are actually it leaving SM_AVOID mode; when it breaks off again, it has re-entered SM_AVOID.
(0016520)
Spoon   
2015-02-28 17:37   
On the subject of mission time and behavior, something that may affect testing in that regard is the $allow primary link delay flag in ai_profiles.tbl
(0016521)
niffiwan   
2015-03-01 04:22   
Thanks for the info about $allow primary link delay / $allow primary link at mission start, that explains why the Harpoon would firelink primaries and Rockeye didn't.

More testing has proved my previous statement incorrect, "pull up at last minute" seems to depend on a factor that I can't figure out, making it seem random per MageKing17's comment. I've also seen it occur as far back as 3.6.12. In any event, the AI sure as heck isn't evading when it enters SM_AVOID and it *should* be.
(0016525)
Admiral MS   
2015-03-02 10:26   
Uploaded a svn patch which should fix this bug. Kinda got off into the wrong direction until I figured that it has to be somewhere else.

(0016526)
MageKing17   
2015-03-02 15:34   
Good catch.

Further testing reveals this bug has been present since retail; along with a related issue that since the AIF_SEEK_LOCK branch doesn't set G_predicted_pos and G_fire_pos, the AI is more likely to miss with primary weapons if aspect seekers are equipped.

Time for a couple of AI profile flags?
(0016527)
niffiwan   
2015-03-03 04:03   
I can confirm the patch works for me as well.

Setting G_predicted_pos and G_fire_pos for AIs armed with aspect seekers sounds like a good idea.

As for the AI profile flags... I know the 1st rule is "don't change retail", and that AI changes can easily have unexpected side effects, however these are really _bugs_ in the AI behaviour, and they should be low impact. Heck, it's taken 15 years to notice that they even exist! So I'd probably prefer (slightly) to just fix them rather than add some more conditional statements and complexity. Thoughts?
(0016528)
The_E   
2015-03-03 13:10   
I agree with Niffiwan: While this is an undeniable change to the retail behaviour, it's such an edge case (and in so many ways an obvious bug) that the hassle of implementing an AIP flag, publicizing it, and getting people to use it is just not worth it.
(0016529)
MageKing17   
2015-03-03 13:27   
I've attached a patch to set G_predicted_pos and G_fire_pos in the AIF_SEEK_LOCK branch.
(0016530)
Admiral MS   
2015-03-03 17:13   
So the second patch for the AIF_SEEK_LOCK branch should be behind an AIP flag though, as it can affect gameplay much more than just fixing the bug. Additionally there is need for an extra condition since having the effect for (aip->target_time < 1.0f) is not desirable (no perfect aim right after targeting).
(0016531)
MageKing17   
2015-03-03 19:38   
Your point about (aip->target_time < 1.0f) is a good one, but it's not really any more drastic a change than the other fix; predicted_enemy_pos was still the point the AI would point towards, it just wasn't being subjected to the orientation-fudging code due to an oversight. Given that the only case in which it actually matters is when facing a stationary target, can anyone think of any examples where mission balance relies on the AI sometimes (and only sometimes) completely missing a stationary target because it wasn't pointed at exactly the right angle?
(0016555)
Spoon   
2015-03-14 17:50   
While I personally don't think a flag for this is necessary, I don't really mind if there is one. Anyway, I'd love to include this fix for WoD's release and have sometime to play around with it.
So would it be possible for this to get code reviewed/commited soonish?
(0016565)
MageKing17   
2015-03-17 16:47   
Fix committed to trunk@11284.
(0016568)
Goober5000   
2015-03-18 00:28   
(Last edited: 2015-03-18 00:29)
"As for the AI profile flags... I know the 1st rule is "don't change retail", and that AI changes can easily have unexpected side effects, however these are really _bugs_ in the AI behaviour, and they should be low impact. Heck, it's taken 15 years to notice that they even exist! So I'd probably prefer (slightly) to just fix them rather than add some more conditional statements and complexity. Thoughts?"


That's approaching things from the wrong perspective. The effects of a bug may not be all that noticeable, but the effects of a *fixed* bug may be very noticeable indeed. Take the example of Trebuchets: it took a long time for people to notice that enemy AI didn't fire bomber+ missiles. But when that was fixed, it made several FS2 missions utterly impossible (particularly Dunkerque) because Shivan wings started using Trebuchets that they had previously carried around as dead weight.

Please note that there are many instances of bugfixes like these scattered throughout the FSO codebase. They're quite clearly marked as bugs and yet they're still controlled by flags.


"can anyone think of any examples where mission balance relies on the AI sometimes (and only sometimes) completely missing a stationary target because it wasn't pointed at exactly the right angle?"


Destroying a cargo depot. A mission designer may assume that AI wings may take a certain amount of time to destroy all the stationary cargo. Rather than taking X time plus a random factor, it now only takes X. Another example is Sneak's attack on the Karnak reactor in ST:R.

The point is that changes in the AI code, even bugfixes, can have unpredictable effects. Some time ago, I went though the AI code and made all non-retail behavior (including bugfixes) controlled by flags. We should take care to maintain this state of affairs and not let the code drift into unpredictable territory. So yes, these fixes should be tied to AI flags.


Issue History
2015-02-27 05:41niffiwanNew Issue
2015-02-27 05:41niffiwanFile Added: cs_suicide.fs2
2015-02-27 05:42niffiwanDescription Updatedbug_revision_view_page.php?rev_id=1001#r1001
2015-02-27 06:02niffiwanNote Added: 0016517
2015-02-27 06:05niffiwanNote Edited: 0016517bug_revision_view_page.php?bugnote_id=16517#r1003
2015-02-27 06:10niffiwanAdditional Information Updatedbug_revision_view_page.php?rev_id=1005#r1005
2015-02-27 14:18MageKing17Note Added: 0016518
2015-02-28 17:37SpoonNote Added: 0016520
2015-03-01 04:22niffiwanNote Added: 0016521
2015-03-02 10:21Admiral MSFile Added: ai_no_ramming.patch
2015-03-02 10:26Admiral MSNote Added: 0016525
2015-03-02 10:26Admiral MSNote Edited: 0016525bug_revision_view_page.php?bugnote_id=16525#r1007
2015-03-02 15:34MageKing17Note Added: 0016526
2015-03-03 04:03niffiwanNote Added: 0016527
2015-03-03 13:10The_ENote Added: 0016528
2015-03-03 13:26MageKing17File Added: aicode.cpp.patch
2015-03-03 13:27MageKing17Note Added: 0016529
2015-03-03 13:27MageKing17Assigned To => Admiral MS
2015-03-03 13:27MageKing17Statusnew => code review
2015-03-03 17:13Admiral MSNote Added: 0016530
2015-03-03 19:38MageKing17Note Added: 0016531
2015-03-03 19:38MageKing17File Deleted: aicode.cpp.patch
2015-03-03 19:38MageKing17File Added: aicode.cpp.patch
2015-03-14 17:50SpoonNote Added: 0016555
2015-03-17 16:47MageKing17Changeset attached => fs2open trunk r11284
2015-03-17 16:47MageKing17Note Added: 0016565
2015-03-17 16:47MageKing17Statuscode review => resolved
2015-03-17 16:47MageKing17Resolutionopen => fixed
2015-03-18 00:28Goober5000Note Added: 0016568
2015-03-18 00:28Goober5000Statusresolved => feedback
2015-03-18 00:29Goober5000Note Edited: 0016568bug_revision_view_page.php?bugnote_id=16568#r1021
2015-03-18 00:29Goober5000Note Edited: 0016568bug_revision_view_page.php?bugnote_id=16568#r1022