Some fixes to the Experimental AI #6783

mattsc · 2022-06-21T02:26:47Z

The incentive for this change was the fact that there is a "data leakage" bug if several sides all use the Experimental AI recruiting. Because AI candidate actions are loaded with wesnoth.require, all the information set in the old ca_recruit_rushers.lua is only loaded once and thus applied to all sides. While there are other ways of fixing this problem, I've been meaning to remove the generic recruit engine in favor of simple straight CAs for a while anyway.

There is no change in behavior due to the first (refactoring) commit. The CA functions exactly as before, and generic_recruit_engine.lua is still there, in case some UMC uses it directly.

There are, however, a couple internal parameter changes. leader_takes_village and min_turn_1_recruit were internal parameters for the interaction between the Experimental AI's castle switch and recruit rushers CAs. Now that both CAs have full access to the AI engine's data variable, they are not needed any more.

The one actual change is that score_function has now been replaced by the usual ca_score parameter. Again, this is entirely internal and there is no change of behavior. In the unlikely case that some UMC actually uses the generic recruit engine with a score function, the engine is still there, as already mentioned. And for the ExpAI itself, the behavior is exactly the same, as the score function just returned a constant value anyway.

While I'm at it, I might as well also do the following as part of this PR (but I first want to see if there are any substantial objections to what I have so far):

Deprecate the generic recruit engine
When the castle switch CA is not present, l.928 of the new ca_recruit_rushers.lua appears to go through the __cfg of the AI configuration on each call, which is very inefficient. That should be avoided.
Fix experimental_ai doesn't recruit from extra_recruit #4924. (By contrast, Duo Leader with experimental AI #4117 is more complex than I want to deal with at this time.)

mattsc · 2022-06-21T02:29:26Z

@Alarantalara I don't know how much you are around these days, but I saw that you posted some comments recently. If you have time, you may want to have a look too, since a lot of this is your code (although I may have introduced the problem with the data leakage, I don't remember).

Alarantalara · 2022-06-21T04:06:54Z

data/ai/lua/ca_recruit_rushers.lua

+
+
+local function get_best_defense(unit)
+    local terrain_archetypes = { "Wo", "Ww", "Wwr", "Ss", "Gt", "Ds", "Ft", "Hh", "Mm", "Vi", "Ch", "Uu", "At", "Qt", "^Uf", "Xt", "Tb" }


Might as well update the list of terrain codes while you're copying everything.
^Uf and Tb are the same terrain but ^Uf is deprecated, so it can go. I suggest using Tt, which is what both are aliases of.
Mine rails got a special code and while it's not useful in general, there might be some value in adding it (I left it out since it's more of a functional change in the suggestion, but worth considering).

Suggested change

local terrain_archetypes = { "Wo", "Ww", "Wwr", "Ss", "Gt", "Ds", "Ft", "Hh", "Mm", "Vi", "Ch", "Uu", "At", "Qt", "^Uf", "Xt", "Tb" }

local terrain_archetypes = { "Wo", "Ww", "Wwr", "Ss", "Gt", "Ds", "Ft", "Hh", "Mm", "Vi", "Ch", "Uu", "At", "Qt", "Xt", "Tt" }

Alarantalara · 2022-06-21T04:30:16Z

I never stopped getting mail and even read it sometimes, but hadn't been paying much attention for awhile. I started getting more involved again about a month ago, but mostly it's just been me going over all the things that changed and realizing how out of date I am. I'm having fun poking around the new features and making my unfinished ideas work again, so it's possible I might eventually do something a bit more substantial.

mattsc · 2022-06-21T12:51:18Z

Great, good to have you back, even if only partly. I made the change to the terrain codes you suggested (including the mine rails) and added a few other changes as well. I guess I didn't wait what others have to say first after all, but they were simple enough so I just went ahead.

data/ai/lua/generic_recruit_engine.lua

mattsc · 2022-06-21T15:20:33Z

This is done now, as far as I am concerned. I implemented the suggestions above, added the last task (extra_recruits) and squashed commits.

I'll let this sit here for a couple days to see if there are other comments and merge it if not.

Alarantalara · 2022-06-21T21:35:00Z

data/ai/lua/ca_recruit_rushers.lua

@@ -944,6 +944,11 @@ function ca_recruit_rushers:evaluation(cfg, data, filter_own)
        end
    end

+    recruit_data.recuit_types = wesnoth.sides[wesnoth.current.side].recruit
+    for _,unit_type in ipairs(leader.extra_recruit) do
+        table.insert(recruit_data.recuit_types, unit_type)


This might cause duplicate entries if both wesnoth.sides[wesnoth.current.side].recruit and leader.extra_recruit contain the same unit.
Apart from slowing things down, depending on the level of randomness, it could also increase the chance that unit is selected since it gets two chances to have the highest score, not one.

Whoops, indeed! How about now: 00e4b22

CelticMinstrel · 2022-06-22T02:45:04Z

data/ai/lua/ca_recruit_rushers.lua

@@ -944,6 +944,14 @@ function ca_recruit_rushers:evaluation(cfg, data, filter_own)
        end
    end

+    recruit_data.recuit_types = {}
+    for _,unit_type in ipairs(wesnoth.sides[wesnoth.current.side].recruit) do
+        recruit_data.recuit_types[unit_type] = true


Missing the R in "recruit_types" here.

Funny in that I had just noticed that myself. And there was a more substantial problem also: the code setting up that table actually had to be pulled forward about 20 lines as it was sometimes needed earlier (a situation that had not happened in my previous tests). Both is fixed now.

mattsc · 2022-06-22T03:53:23Z

By the way, I should have mentioned earlier that deprecating the generic recruit engine and the new parameter reset_cache_each_turn are for master only, but the other four commits are bug fixes or efficiency improvements and should be backported to 1.16.

mattsc · 2022-06-22T17:29:01Z

data/ai/lua/ca_recruit_rushers.lua

+--    high_level_fraction: the approximate fraction of units of level 2 or higher to be recruited
+--      (default = 0)
+--     enemy_types: array of default enemy unit types to consider if there are no enemies on the map
+--      and no enemy sides exist or have recruit lists


I just noticed that specifying enemy_types as a Lua array does not work (and has never worked) if this parameter is to be supplied by an [args] tag. We could convert it to a comma-separated list instead, but I suggest to simply drop this parameter altogether. It is an undocumented parameter and, as is obvious from the comment above, it is essentially never needed, and even in the rare scenario when that might happen, there is a default list of types provided that will be used instead. This is such a rare edge case that it does not seem worth it to me to keep this parameter.

Assigning an array of strings as a config values does do an implicit join (so it's a comma-separated list), but if you're later reading it from a config you'd need to manually split it.

Probably easier to remove it, like you said. If someone needs the AI to be able to play a scenario that doesn't have any enemies, they should probably use a different type of recruitment algorithm anyway. (Or no recruitment, just use pre-placed units.)

As I recall, I added it with maps like Dark Forecast in mind, where there are lots of enemies of known types, but they're all placed via scripting. Humans can play it fine after the first few games, but the AI has no way of knowing what's coming and this would help with the first round of enemies.

Of course, I never actually tried making an AI for Dark Forecast, so I never discovered the issue. I suspect I would have ended up either providing the parameter through Lua and not noticed anyway or modifiing it further to support changing spawn lists.

At any rate, unless you plan on writing a Dark Forecast AI, it seems completely reasonable to drop it.

Thanks for the comments. I am not planning to write an AI specifically for Dark Forecast either, so I am going to stick with dropping this for now. We can always add something (which would likely end up being more complex than this) later, either in this CA or a custom setup, if the need arises.

This removes the need for the Generic Recruit Engine and makes Recruit Rushers a "clean" candidate action. In the process, it fixes a bug of data leakage between the Experimental AIs of different sides.

While most data in recruit_data can stay persistent from turn to turn, some of recruit_data.recruit, such as enemy_types, needs to be updated each turn.

Many of the operations of the recruit rushers CA are expensive and are therefore cached. For the most part, this cache can be kept from turn to turn, but in case there are events that change, for example, the map or the recruit list, this parameter can be used to force a reset of the cache each turn.

This fixes wesnoth#4924. Note, however, that the recruit rushers CA is set up for single-leader sides and does not work with multiple leaders, so this is of limited use.

mattsc · 2022-06-22T23:35:13Z

Update: I got rid of the enemy_types config parameter, as discussed above. I squashed this into the first commit, which already modifies some of the other internal parameters.

github-actions bot added the AI Issues with the AI engine, including micro AIs. label Jun 21, 2022

Alarantalara reviewed Jun 21, 2022

View reviewed changes

CelticMinstrel reviewed Jun 21, 2022

View reviewed changes

data/ai/lua/generic_recruit_engine.lua Outdated Show resolved Hide resolved

mattsc force-pushed the expai_updates branch from 62bba8e to aa2a5ee Compare June 21, 2022 15:16

Alarantalara reviewed Jun 21, 2022

View reviewed changes

mattsc force-pushed the expai_updates branch from aa2a5ee to 00e4b22 Compare June 21, 2022 22:25

CelticMinstrel reviewed Jun 22, 2022

View reviewed changes

mattsc force-pushed the expai_updates branch from 00e4b22 to 0ed6bd0 Compare June 22, 2022 02:55

mattsc commented Jun 22, 2022

View reviewed changes

mattsc added 6 commits June 22, 2022 16:31

ExpAI: do not use generic_recruit_engine

e4b776c

This removes the need for the Generic Recruit Engine and makes Recruit Rushers a "clean" candidate action. In the process, it fixes a bug of data leakage between the Experimental AIs of different sides.

ExpAI recruit rushers: reset recruit_data.recruit each turn

dfde09c

While most data in recruit_data can stay persistent from turn to turn, some of recruit_data.recruit, such as enemy_types, needs to be updated each turn.

ExpAI recruit rushers: do not parse side __cfg each eval

561314b

Deprecate generic_recruit_engine.lua

7877218

ExpAI recruit rushers: include unit types from extra_recruit

c1d1ff5

This fixes wesnoth#4924. Note, however, that the recruit rushers CA is set up for single-leader sides and does not work with multiple leaders, so this is of limited use.

mattsc force-pushed the expai_updates branch from 0ed6bd0 to c1d1ff5 Compare June 22, 2022 23:33

mattsc merged commit a70ecf1 into wesnoth:master Jun 24, 2022

mattsc deleted the expai_updates branch June 24, 2022 23:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Some fixes to the Experimental AI #6783

Some fixes to the Experimental AI #6783

mattsc commented Jun 21, 2022 •

edited

mattsc commented Jun 21, 2022

Alarantalara Jun 21, 2022

Alarantalara commented Jun 21, 2022

mattsc commented Jun 21, 2022

mattsc commented Jun 21, 2022

Alarantalara Jun 21, 2022

mattsc Jun 21, 2022

Alarantalara Jun 21, 2022

CelticMinstrel Jun 22, 2022

mattsc Jun 22, 2022

mattsc commented Jun 22, 2022 •

edited

mattsc Jun 22, 2022

CelticMinstrel Jun 22, 2022 •

edited

Alarantalara Jun 23, 2022

mattsc Jun 23, 2022

mattsc commented Jun 22, 2022



		local function get_best_defense(unit)
		local terrain_archetypes = { "Wo", "Ww", "Wwr", "Ss", "Gt", "Ds", "Ft", "Hh", "Mm", "Vi", "Ch", "Uu", "At", "Qt", "^Uf", "Xt", "Tb" }

Some fixes to the Experimental AI #6783

Some fixes to the Experimental AI #6783

Conversation

mattsc commented Jun 21, 2022 • edited

mattsc commented Jun 21, 2022

Choose a reason for hiding this comment

Alarantalara commented Jun 21, 2022

mattsc commented Jun 21, 2022

mattsc commented Jun 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattsc commented Jun 22, 2022 • edited

Choose a reason for hiding this comment

CelticMinstrel Jun 22, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattsc commented Jun 22, 2022

mattsc commented Jun 21, 2022 •

edited

mattsc commented Jun 22, 2022 •

edited

CelticMinstrel Jun 22, 2022 •

edited