[freeciv-ai] Re: [Freeciv-Dev] (PR#4349) New tax code

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

On Sat, 12 Jul 2003, Gregory Berkolaiko wrote:
> Let's take the maxbuycost patch and criticise it step by step:
>
> +static void adjust_buylimit(struct player *pplayer, int by_how_much)
> +{
> +if (pplayer->economic.gold - pplayer->ai.est_upkeep
> +    < pplayer->ai.maxbuycost) {
> +  pplayer->ai.maxbuycost = MAX(pplayer->ai.maxbuycost, by_how_much);
> +}
> +}
>
> Naming is bad, but the worst is I don't understand the meaning of "if"
> here. Per, please explain.  You might need to lecture me on the general
> use of maxbuycost too...

maxbuycost is the amount of gold you want to have at hand next turn to buy
stuff with. It is a gross and ugly hack that I did not give the axe since
I did not have anything else to put in its place.

The basic idea is that we may need to emergency buy _one_ thing next turn.
Of course this is crap - we may have multiple emergencies! But anyway, it
is better than not being able to handle any emergencies.

If we have enough gold already that we can buy what we need, then we
do not need to increase maxbuycost.

> -/* Disband troops that are at home but don't serve a purpose. */
> +/* Disband explorers that are at home but don't serve a purpose.
> + * FIXME: This is a clever hack, but should be removed once we
> + * learn how to ferry explorers to new land. */
>  city_list_iterate(pplayer->cities, pcity) {
>    struct tile *ptile = map_get_tile(pcity->x, pcity->y);
>    unit_list_iterate(ptile->units, punit) {
> -    if (((unit_types[punit->type].shield_cost > 0
> -          && pcity->shield_prod == 0)
> -         || unit_has_role(punit->type, L_EXPLORER))
> +    if (unit_has_role(punit->type, L_EXPLORER)
>          && pcity->id == punit->homecity
> -        && pcity->ai.urgency == 0
> -        && is_ground_unit(punit)) {
> +        && pcity->ai.urgency == 0) {
>        struct packet_unit_request packet;
>        packet.unit_id = punit->id;
>        CITY_LOG(LOG_BUY, pcity, "disbanding %s to increase production",
>
> The new code means that if an explorer walks into a city by chance, it's
> dead. Nice...

If it ends its movement in its _homecity_ by chance, yes... How often
would that happen unintentionally?

> The old code means that a production-1 cannot build itself a defender
> unless it's urgent. Nice...

This has nothing to do with building, and a prod-1 city can neither build
nor upkeep an additional unit anyway.

> I really dislike both.
>
> -      CITY_LOG(LOG_BUY, pcity, "Wonder being built in dangerous
> position!");
> +      CITY_LOG(LOG_ERROR, pcity, "Wonder being built in dangerous
> position!");
>
> Surely this is some artefact.

Perhaps. This way we will find out.

> -if (pplayer->economic.gold + cached_limit < pplayer->ai.maxbuycost) {
> -  /* We have too much gold! Don't raise taxes */
> -  pplayer->ai.maxbuycost = 0;
> -}
> -
>
> What was that supposed to mean??

The same as now done in adjust_buylimit().

  - Per