[Freeciv-Dev] (PR#7236) removal of most direct references to build_cost

["James Canete" <use_less@xxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=7236 >

>  - am I correct that
>        notify_player_ex(pplayer, pcity->x, pcity->y, E_IMP_SOLD,
>                        _("Game: %s is selling %s (not needed) for
> %d."),
>                        pcity->name, get_improvement_name(i),
> -                      improvement_value(i)/2);
> +                      impr_sell_gold(i));
> 
>    removes a bug in the original code?
> 

Yeah, I'm pretty sure that was a bug.

>  - the *_buy_gold_cost. I think you got side-tracked here with the
>  factor of 2. I have restored some of the users you added for these to
>  use the *_build_shield_cost one and apply the factor of 2 by
>  themself. The reason here is that these users doesn't want to know
>  the real buy cost but some estimate.

Here are the 3 locations I chose *_buy_gold_cost that differ from your
patch and why:

ai/aicity.c, line ~452:

    if (bestchoice.type == CT_ATTACKER 
        && buycost > unit_types[bestchoice.choice].build_cost * 2) {
       /* Too expensive for an offensive unit */
       continue;
    }

I chose unit_buy_gold_cost here because buycost is a gold cost.  Later
on it appears in the expression pplayer->economic.gold -
pplayer->ai.est_upkeep >= buycost on line ~462.


server/cityturn.c, line ~1200

  unit_list_iterate(map_get_tile(pcity->x,pcity->y)->units, punit) {
    cost += unit_type(punit)->build_cost * game.incite_cost.unit_factor;
  } unit_list_iterate_end;

  /* Buildings */
  built_impr_iterate(pcity, i) {
    if (!is_wonder(i)) {
      cost += improvement_value(i) * game.incite_cost.improvement_factor;
    } else {
      cost += improvement_value(i) * 2 *
game.incite_cost.improvement_factor;
    }
  } built_impr_iterate_end;

I set these to buy costs because I wanted to make sure the calculation
for inciting the city was independent of the number of shields and more
a reflection of the buying cost.  If my accelerated production patch
were applied here, the price of inciting a city would drop.  If I recall
correctly, Civ 3 kept buy prices and diplomat action prices the same
regardless of shield cost under accelerated production.  I could be
wrong though.  Perhaps another function, *_bribe_gold_cost() or
*_spy_action_base_gold_cost(), is in order.


server/diplomats.c, line ~1431

  cost=(cost/(dist+2))*(unit_type(punit)->build_cost/10);

Same reasoning as above, just for bribing units.

>  For the user which wants the exact numbers (city_buy_cost) I have
>  added a new parameter to the *_buy_gold_cost functions. So eventually
>  the estimation callers _could_ be converted.
> 
>  - in unit_upgrade_price() I have replaced
> 
> const int diff = unit_build_shield_cost(to) -
> unit_build_shield_cost(from) / 2;
> 
>    with
> 
> const int diff = unit_build_shield_cost(to) -
> unit_disband_shields(from);
> 
>    I think this is more exact. Later I have converted it to a call of
>    unit_buy_gold_cost()
> 

Nice change, I like it.

Anyway, thanks for cleaning up the patch.  It's much nicer looking and
readable now than the too wide lines and too close together operators I
had in there before.

-James Canete