[Freeciv-Dev] Re: (PR#14923) Stupid AI Building Wants

["Benedict Adamson" <badamson@xxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=14923 >

Per I. Mathisen wrote:

> This patch is a 377k monster. It took me a while to read throug

Thanks for the effort.

...
> It must be split up as much as
> possible.

Agreed. Note, however, that applying some of the fragments might make 
the AI peculiar (or stupid) until the remainder was added. OK provided 
we bear that in mind.

> Covering every possibility and obscure twist of the rules
> makes for tons of code that is hard to follow.

I'm not sure what you have in mind here. The intention of the patch was 
for the AI to cope with a wide range of rulesets. I believe the patch 
enables the AI to cope with the default and Ancients rulesets: neither 
of these uses obscure twists, and the Ancients ruleset uses no rules 
that the default ruleset does not. The patch has the AI consider no more 
unit flags than the existing code, for example, but instead of spreading 
unit-want calculations over separate computations for conventional 
attackers, hunters, paratroopers and missiles, it does the lot in one 
section of code.

...
> It is too hard
> to maintain.

Oh dear. My intention was to produce something correct and easy to 
maintain. The current want computation code is neither.

...
> advdiplomacy.c:
> -  worth = bulbs + tech_want;
> +  worth = bulbs + tech_want / TRADE_WEIGHTING;
> 
> is this wise?

Not only wise, but necessary for dimensional correctness. The units of 
'bulbs' is 'bulbs'; the units of 'tech_want' is 'Want'. To combine them 
we must convert 'tech_want' from a 'Want' to a 'bulbs', which means 
dividing by a priority value; TRADE_WEIGHTING is a suitable value, 
although ai->science_priority would be better.

> 
> What is the point with this...
...
> +static bool is_potential_settler(const struct unit_type *settler)
...

You mean, 'what is the point of having this *in a separate function*'? 
Just an attempt to make the code more readable by abstracting 
predicates. I can in-line the code instead if you prefer.

...
> Code to second-guess what the hunter code might come up with?
 > ...
> The AI is omniscient and we should use
> that to find out what the player has and scramble to meet it as best we
> can.

My preference too, but that would mean adding more code to aidata.[hc]. 
I chose the implemented approach to reduce the size of the patch, but as 
you're keen, I'll do it the right way.

> In ai_consider_attacker(), the attack_radius() function only looks a few
> turns of unit movement outside the city. This will not do. The AI must be
> able to attack far away targets too.

That is an optimisation, to prevent the AI spending excessive time 
evaluating targets that are probably not worthwhile.

The further away the target, the less the benefit of building an 
attacker, because our gain from victory will be amortised by the travel 
time to the target. There is a diminishing return in considering ever 
more distant targets. As our desire to build attackers must compete with 
our desire to build other things, there is an effective maximum range, 
although that range might be further than the arbitrary cut-off I applied.

The computation cost of distant targets is probably excessive if we 
compute the attacker want for distant targets every turn, as the code 
does now. Perhaps the AI could instead compute the want for attacks on 
distant targets (or just cities?) less frequently, as we do for the 
computation of improvement wants.

> Err, no. A 'ruleset' is a technical term in Freeciv. It does not have a
> hyphen :) Also 'space-race' is no more correct than spacerace. The correct
> version is 'space race'. I think correcting it to be a waste of diff.

An over zealous spelling correction of the comments using Emacs, I'm 
sorry to say. I'll change them back.