[freeciv-ai] Re: quizz

["Per I. Mathisen" <per@xxxxxxxxxxx>]

On Sat, 17 Aug 2002, Per I. Mathisen wrote:
> This function gets called every turn on every unit. What is fundamentally
> wrong with it?

A lot.

Let us start with the smaller issues:
 - it could assign ships and airplanes as city defenders (thanks Raahul)
 - it can lose units by making them home to a city that can't upkeep them,
   or make a city unable to produce anything/grow because of it
 - it does not allow us to defend allied cities that are in danger
 - it interferes with unit missions (thanks Tony)
 - there is no code to check if we are a better defender (thanks Tony)

But the big bad issue is, I believe:

> static bool stay_and_defend_city(struct unit *punit)
> {
> struct city *pcity = map_get_city(punit->x, punit->y);
> bool has_defense = FALSE;
>
> if (!pcity) return FALSE;
> if (pcity->id == punit->homecity) return FALSE;
> if (pcity->owner != punit->owner) return FALSE;
>
> unit_list_iterate(map_get_tile(pcity->x, pcity->y)->units, pdef) {
>   if (assess_defense_unit(pcity, punit, FALSE) >= 0
>       && pdef != punit
>       && pdef->homecity == pcity->id) {
>     has_defense = TRUE;

If the city contains a unit whose home city it is, leave it since it has a
defender. While not optimal, so far so fine.

>   }
> } unit_list_iterate_end;
>
> /* Guess I better stay / you can live at home now */
> if (!has_defense) {
>   struct packet_unit_request packet;
>
>   /* Very important, or will not stay -- Syela */
>   ai_unit_new_role(punit, AIUNIT_DEFEND_HOME);
>
>   /* change homecity to this city */
>   /* FIXME: it is stupid to change homecity if the unit has no homecity
>      in advance or the new city does not have enough shields to support it */
>   packet.unit_id = punit->id;
>   packet.city_id = pcity->id;
>   handle_unit_change_homecity(unit_owner(punit), &packet);
>   return TRUE;

If it doesn't, set it as our homecity and make us the defender of this
city. So far, still ok.

> }
> return FALSE;
> }

So all is fine? Not so fast. Let's go back to the beginning:

> static bool stay_and_defend_city(struct unit *punit)
> {
> struct city *pcity = map_get_city(punit->x, punit->y);
> bool has_defense = FALSE;
>
> if (!pcity) return FALSE;
> if (pcity->id == punit->homecity) return FALSE;

So the turn after we've made made the city our home and decide to defend
it, we automatically (no danger checks even!) decide the city doesn't need
defending anymore. Great!

Yours
Per