View Issue Details

IDProjectCategoryView StatusLast Update
0000804FSSCPgameplaypublic2020-12-06 15:48
ReporterGoober5000 Assigned ToGoober5000  
PriorityhighSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Summary0000804: Need to investigate AI in aiturret.cpp
DescriptionWhen I went through the AI code, I got every file except for WMC's turret code. I still need to do that (though if WMC chipped in, I wouldn't complain ;)). The AI should behave identically to retail, and any changes should be tied to a flag in ai_profiles.
TagsNo tags attached.

Activities

taylor

2006-09-09 07:04

administrator   ~0006570

* BUMP *

Do you still need help with this? I know that the turret code already has some ai_profile checks in it, so I wasn't sure if this is just a stale reminder or what. I don't mind doing an aiturret.cpp comparison against retail code this weekend if you still need it done. I may not have a chance to get through it 100%, but I'll get through most of it.

Goober5000

2006-09-09 17:00

administrator   ~0006586

It's basically a stale reminder. If you wouldn't mind doing that, it would be amazingly helpful. :)

taylor

2006-09-09 18:04

administrator   ~0006588

K, I'll start going through it later tonight.

taylor

2006-09-11 02:33

administrator   ~0006593

I didn't find anything really bad, with only one questionable spot. Another ??? or so, but not something that should actually affect AI handling much, if at all.

The one "spot":
  turret_should_pick_new_target() has uncommented the check that will pick a new target if there currently is no target. Because of how the code works, it could decide to not have a target, then try to pick a new one, possibly giving it a greater chance at firing at something. This was done in the move from aicode.cpp to aiturret.cpp, since there it was never a change in the old aicode.cpp and was in the inital commit of aiturret.cpp. This needs to be fixed to retain retail target selection behavior, regardless of how often this would actually be an issue.

The main ??? spot (the other probably isn't even worth a mention):
  MAX_AIFFT_TURRETS was bumped to 200, from 60. I don't think that this would affects AI behavior really, but I do question the change. Despite it's name it's the max size of an array of target subsystems to possibly target. Unless we actually support 200 ship subsystems this is just wasting some memory. It also just picks out a random fraction of the list to try and validate as a target. Where it might affect behavior is that it's able to try against slightly more subsystems, with slightly more turrets, per target now. Although I can see the initial draw of this, it will eventually get to all of the subsystems so it's not like anything really gets skipped or avoided in the end.

The only other possible issue is just the differences in ai_fire_from_turret(). Nothing really looks wrong, but it's different enough from the retail function that it's a little more difficult to determine any behavior change unless the function was totally dissected into small blocks of functionality first. It may need another pair of eyes on that one function just in case, but like I said, nothing really jumped out at me when I was going over it.

taylor

2006-09-11 19:06

administrator   ~0006595

Ok, the one "spot" is fixed in CVS, and I fixed an out-of-bounds issue too.

I'll leave the MAX_AIFFT_TURRETS thing as is until I hear from you, though I have half a mind to go ahead and change it back to the retail value.

Goober5000

2006-09-11 22:28

administrator   ~0006596

"This was done in the move from aicode.cpp to aiturret.cpp"
Crud. I hope this was the only thing that got changed. File renaming should happen independently of code changing. :(

As for reverting MAX_AIFFT_TURRETS: go for it. Even if there was a valid reason for the change (which from your description there doesn't appear to be) I'd favor changing it and tying the new behavior to aip* somehow.

I'll need to find some time to go through ai_fire_from_turret().

taylor

2006-09-13 02:09

administrator   ~0006615

Ok, the MAX_AIFFT_TURRETS thing is in CVS now. I added a comment about it to in order to try and disuade future change attempts. I guess about the only thing left is ai_fire_from_turret().


I will go ahead and mention the other "???" thing I had though, and that's the flags situation. It will check that every weapon on a turret has a particular flag before considering it a beam, that it attacks captital ships, or whatever. This shouldn't affect retail behavior in any way (that I can think of), but it could allow for some misuse or strange bugs for some mods. I think that the code is needed (the retail stuff was just too basic to be of much use) but in the future we may want to add some better error checking to it all to help guard against unforseen problems.

taylor

2006-10-21 04:59

administrator   ~0006957

* BUMP *

Any progress here?

I've got numerous changes to aiturret which need to go in soon but I don't want to do that until this review is done with, since there it's a little counterproductive adding new/changed code to the mix at this point. The new/changed code is already "behavior safe", but I don't want to cause a set-back with the review process already most of the way done.

Goober5000

2006-10-21 10:50

administrator   ~0006961

Oof, sorry. I'll need to go through and check, but I don't think I'll have time this weekend or next. :( I'll see what I can do.

chief1983

2008-10-08 22:58

administrator   ~0009840

Not sure what the difficulty on this is, but I'm marking it high in the hopes that it can be completed (or closed if it has already been done) for 3.6.10.

The_E

2010-10-22 13:25

administrator   ~0012427

Can this be closed?

Goober5000

2010-10-24 05:31

administrator   ~0012435

I still want to look at it. Though it's probably fine at this point.

Echelon9

2012-02-24 09:24

developer   ~0013356

Seeing as it wasn't an issue before the 3.6.10 release, it likely won't be for 3.6.14 as well.

Closing.

Goober5000

2012-02-25 02:56

administrator   ~0013359

It's not an "issue", but I still want to review it, since any bugs that were introduced back then would likely still affect things now. There are still balance bugs we haven't detected -- witness the change in play of "A Game of TAG" between retail and FSO.

Goober5000

2020-05-05 05:42

administrator   ~0016991

Turns out there is at least one issue:
https://github.com/scp-fs2open/fs2open.github.com/pull/2367

There may be more, unfortunately.

Goober5000

2020-12-06 15:48

administrator   ~0017043

Additional PR posted with other issues.
https://github.com/scp-fs2open/fs2open.github.com/pull/2959

Goober5000

2020-12-06 15:48

administrator   ~0017044

With the merging of the PR, this can finally be marked resolved.

Issue History

Date Modified Username Field Change
2006-02-13 05:28 Goober5000 New Issue
2006-02-13 05:28 Goober5000 Status new => assigned
2006-02-13 05:28 Goober5000 Assigned To => Goober5000
2006-09-09 07:04 taylor Note Added: 0006570
2006-09-09 17:00 Goober5000 Note Added: 0006586
2006-09-09 18:04 taylor Note Added: 0006588
2006-09-11 02:33 taylor Note Added: 0006593
2006-09-11 19:06 taylor Note Added: 0006595
2006-09-11 22:28 Goober5000 Note Added: 0006596
2006-09-13 02:09 taylor Note Added: 0006615
2006-10-21 04:59 taylor Note Added: 0006957
2006-10-21 10:50 Goober5000 Note Added: 0006961
2008-10-08 22:58 chief1983 Note Added: 0009840
2008-10-08 22:58 chief1983 Priority normal => high
2010-10-22 13:25 The_E Note Added: 0012427
2010-10-24 05:31 Goober5000 Note Added: 0012435
2012-02-24 09:24 Echelon9 Note Added: 0013356
2012-02-24 09:24 Echelon9 Status assigned => closed
2012-02-24 09:24 Echelon9 Resolution open => fixed
2012-02-25 02:56 Goober5000 Note Added: 0013359
2012-02-25 02:56 Goober5000 Status closed => assigned
2012-02-25 02:56 Goober5000 Resolution fixed => open
2020-05-05 05:42 Goober5000 Note Added: 0016991
2020-12-06 15:48 Goober5000 Note Added: 0017043
2020-12-06 15:48 Goober5000 Status assigned => resolved
2020-12-06 15:48 Goober5000 Resolution open => fixed
2020-12-06 15:48 Goober5000 Note Added: 0017044