Remove Formula AI uses

[mattsc]

Formula AI will probably be deprecated in Wesnoth 1.17. As preparation for that, this PR removes uses of FAI in mainline, but does not yet remove or deprecate the existing FAI candidate actions or tools that can be used for UMC.

Instances to be removed:

• Legend of Wesmere: S16 The Chief Must Die: replace the patrol FAI code with the Patrol Micro AI. In order for the behavior to remain unchanged, two new optional parameters need to be added to the Patrol MAI (which will be generally useful as well):
  • attack_range: the patrol interrupts its route to attack enemies within this distance, not just enemies that it happens to end up next to
  • attack_invisible_enemies: include invisible enemies when considering whether there are enemies within attack_range
• Remove FAI lurkers from Lurkers MAI test scenario
• Remove FAI home guardian from Guardians MAI test scenario
• Remove FAI dev AIs from MP computer player menu (available in debug mode only)
• Macros using FAI: nothing to be done here; they are not used in mainline anymore and won't be deprecated until 1.17

[mattsc]

With that last commit (4086758) I think this is done. I'll let it sit for a few days in case anybody has comments, and then merge it.

[CelticMinstrel]

Hmm? Is this actually necessary? Does this end up adding the stage twice? I think the whole [ai] block could just be deleted, right?

[mattsc]

It does not add the stage twice. The AI either adds the default stage, or whatever is given, not both. The point here is that we want an empty AI stage, except for the patrol MAI (there are other units on this side that are supposed to do nothing). So we can either add the default stage and delete every single CA, or add an empty stage. I decided that the latter is easier and more robust to possible future changes to the default AI.

[CelticMinstrel]

Ah, okay, so this is basically equivalent to setting idle AI… actually… does that still exist?

[mattsc]

Yes, it still exists and is, in fact, used for some of the other sides in the same scenario. However, it is somehow handled differently internally (I forgot the details) and trying to add the MAI to the Idle AI still results in idle behavior. That's actually the first thing I tried.

[CelticMinstrel]

Home Guardian is just removed, not converted to Lua?

[mattsc]

Home Guardian is just removed, not converted to Lua?

That would be my preference, yes. It's not a "general capability" that's provided in some sense or the other, you'd have to copy the code directly from the test scenario into your own scenario to use it. So nothing will change for anybody who has done so already. (I also never really liked that guardian all that much, so in my opinion this is not a loss.)

[mattsc]

I could add a Home Guardian Micro AI though, if that's something people would like to have.

[CelticMinstrel]

The description sounds vaguely like something that would be nice to have; I don't know if the FormulaAI-based implementation actually made sense though. I'm also not sure if it would make sense for it to be an entirely separate AI though? Maybe it could be an option in the Return Guardian? Something like a "max distance" maybe. Unless that already exists.

[mattsc]

Hmm... No, that setting does not currently exist. But I think that shouldn't be too hard to do and could indeed be useful. So, if the distance is greater than "max_distance", return no matter what. If it is smaller, attack if there is an enemy to be attacked, otherwise return. Does that sound right?

Oh, and I guess that should be the distance from "home" to the enemy, not the current distance of the unit, right?

[CelticMinstrel]

Yes and yes.

[mattsc]

I looked at the Return Guardian code and it is not quite a simple as I thought. The reason is that this MAI leaves attacks to the default AI combat CA. This means that if there are two enemies within the guardian's attack range, one within max_distance of "home", and the other farther away, the default AI may choose to attack the latter.

The solution is to add the attack action to the guardian code. If max_distance is set, and enemies are within that range, use that action to attack. If it is not set, leave it to the default AI (in order to preserve the previous behavior). But then there are two distinct attack behaviors (there is currently no mechanism to exactly replicate the default attack evaluation in Lua) within the same MAI and it might make more sense to set up a separate Home Guardian MAI instead.

Any opinion on that?

There are other ways of accompishing this that I'd rather not get into:

• Add the behavior to the Stationed Guardian MAI: that one is already complex enough the way it is
• Add another instance of the default combat CA that only controls the guardian via [filter_own]: that comes with a whole rat's tail of potential problems
• Add a Lua function which replicates the default combat CA evaluation that can be called by any Lua AI: that one is on my long-term wish list, but it's a lot of work and not going to happen any time soon.

[mattsc]

The other question is, of course, still whether we need this at all. Does it add enough value to be worth it? As I said before, we did not lose any generally provided capability by removing the home guardian from the test scenario.

[CelticMinstrel]

Hmm… it is a fairly minor addition from an external standpoint, so perhaps it's not worth it. It doesn't seem like it fits the Stationed Guardian, either, though that one is a bit confusing.

Add a Lua function which replicates the default combat CA evaluation that can be called by any Lua AI: that one is on my long-term wish list, but it's a lot of work and not going to happen any time soon.

I think this actually exists already as part of debug_ai, doesn't it? So exposing it it a less debug sense might not be that hard.

[mattsc]

Hmm… it is a fairly minor addition from an external standpoint, so perhaps it's not worth it. It doesn't seem like it fits the Stationed Guardian, either, though that one is a bit confusing.

Yeah ... I'd suggest that we don't make this PR dependent on that, but I am always happy to add features to the existing MAIs, if people have use cases for them.

Add a Lua function which replicates the default combat CA evaluation that can be called by any Lua AI: that one is on my long-term wish list, but it's a lot of work and not going to happen any time soon.

I think this actually exists already as part of debug_ai, doesn't it? So exposing it it a less debug sense might not be that hard.

Oh, right, I'd forgotten about that. I'd even started looking into that at some point, but there was some limitation that didn't work for what I wanted back then. I think it had something to do with attack combinations of multiple units (it only returns what it considers the best combination, not all of them; or something like that). But it can still be used in many other situations, I think, and there is probably even a way to "trick" it into doing all the combinations if we really want that. I'll put that back on the list.

Is there anything else that should be done on this PR then?

[CelticMinstrel]

I did some grepping and uncovered a few more things:

data/ai/scenarios/scenario-formula-recruitment.cfg:                engine=fai
data/ai/scenarios/scenario-formula.cfg:                engine=fai
data/ai/scenarios/scenario-formula.cfg:                engine=fai
data/ai/scenarios/scenario-formula.cfg:                    engine=fai
data/ai/scenarios/scenario-formula.cfg:                    engine=fai
data/ai/scenarios/scenario-formula.cfg:                name=unit_formulas
data/schema/ai/stage.cfg:                                       value=unit_formulas

Not sure if it needs to be removed from the schema; deprecating it is possibly okay too. Note that that one line in the schema certainly isn't the only one that would need to change if it's being removed; there's gotta be something somewhere that accepts engine=fai, at least.

Anything else I could grep for?

[mattsc]

@CelticMinstrel Thanks for checking. I had found those also, and here's the relevant discussion from the irc log:

20210321 21:40:32<+wesdiscordbot> <mattsc> @Pentarctagon I have started on removing Formula AI uses from mainline.  There isn't a whole lot to do, mostly removing them from LoW:S16, and from the Micro AI test scenarios.  There are also a couple of macros, but I think they are not used in mainline any more (will check later).
20210321 21:40:46<+wesdiscordbot> <mattsc> In addition, there are a couple FAI test scenarios here: https://github.com/wesnoth/wesnoth/tree/master/data/ai/scenarios
20210321 21:41:27<+wesdiscordbot> <mattsc> Do I keep those for removal when FAI gets removed completely, or do I delete them at this time?
20210321 21:42:00<+wesdiscordbot> <mattsc> And just to confirm, the plan is to deprecate FAI in 1.17, not now, right?

20210321 21:44:18<+wesdiscordbot> <Pentarctagon> correct
20210321 21:46:07<+wesdiscordbot> <mattsc> @Pentarctagon Thanks.  What about my question about the FAI test scenarios.
20210321 21:46:58<+wesdiscordbot> <Pentarctagon> I guess keep them for now since it's technically not deprecated quite yet.
20210321 21:47:25<+wesdiscordbot> <mattsc> Okay
20210321 21:49:08<+wesdiscordbot> <mattsc> I'll still throw out the Lurkers FAI from the lurkers MAI test scenario — even though that was the first ever custom AI I wrote.  😢

So I think we are good. Nothing is going to be deprecated until 1.17, and those test scenarios can stay.

[mattsc]

Oh, I forgot this:

Anything else I could grep for?

I think I have grep-ed for everything relevant, so nothing I can come up with.

[CelticMinstrel]

The reason I brought it up was because you did delete two other FAI test scenarios, so it seemed inconsistent.

[mattsc]

The reason I brought it up was because you did delete two other FAI test scenarios, so it seemed inconsistent.

Well, no, technically I removed two uses of FAI from Micro AI test scenarios. It's not quite the same. I am aware of the inconsistency, but I am fine with that. I am also fine with removing the other test scenarios.

[CelticMinstrel]

Ah, I didn't click "Load Diff" and got confused, looks like those weren't scenario files that were deleted. Carry on then, I guess.

[mattsc]

Okay, thanks for checking things out. I don't really have an opinion on whether to remove those FAI test scenarios. Let me know if you think that should be done for consistency, and I'll take care of it.

Otherwise I think I am done. Anything else that should be done in your opinion?