Oddity in scan AI methods

[JensAyton]

Digging through AI code, I noticed that the shared implementation for a number of AI scanForFoo methods explicitly excludes ships with scan class CLASS_CARGO. The affected methods are:

• scanForNearestIncomingMissile
• scanForNearestShipWithPrimaryRole:
• scanForNearestShipHavingRole:
• scanForNearestShipWithAnyPrimaryRole:
• scanForNearestShipHavingAnyRole:
• scanForNearestShipWithScanClass:
• scanForNearestShipWithoutPrimaryRole:
• scanForNearestShipNotHavingRole:
• scanForNearestShipWithoutAnyPrimaryRole:
• scanForNearestShipNotHavingAnyRole:
• scanForNearestShipWithoutScanClass:

For scanForNearestIncomingMissile, this obviously has no effect. For scanForNearestShipWithScanClass:, it’s clearly the wrong thing. But is there a good reason for it in the other cases?

(This won’t be changing for 1.76, but I’m cleaning up the code for 2.0.)

[JensAyton]

scanForNearestShipMatchingPredicate: was missing from the list. This one strikes me as especially pointless, since you can manually filter out cargo in the JS predicate.

[Eric Walch]

But is there a good reason for it in the other cases?

The only current reason I can come up with are derelicts. You don't want to include abandoned ships in your list. But for excluding those there would be better properties to check (e.g. the isDerelict).

[JensAyton]

I’ve stuck with the CLASS_CARGO test for now, except in the three I called out above. When AIs are moved to JavaScript, these methods will probably all be JS convenience wrappers around the equivalent of scanForNearestShipWithPrimaryRole:, which will make it easier to write clean predicates than using OOEntityFilterPredicate. (Wish we could use blocks…)

The relevant commit is here, but it’s pretty messy.