[Freeciv-Dev] Re: (PR#12833) consider_settler_action simplification (rev.)

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Brian Dunstan wrote:

> My mistake, corrected below.  My earlier patch has the
> exra still in there :)   Extra is nice also to make
> sure pollution is cleaned up.
> 
> 
>>>====================
>>>I think this would suffice:
>>>===================
>>>
>>>if (consider) {
>>>    base_value = 
>>>      MAX(0, new_tile_value - old_tile_value) +
>>>      MAX(0, extra);
>>>
>>>    discount_value = amortize(base_value, delay);
>>>    total_value = MAX(0, discount_value);
>>>
>>>  } else {
>>>    total_value = 0;
>>>  }

OK, but still we shouldn't get rid of inuse (until we have something
better to replace it with - maybe x2 instead of x4) and the MAX
shouldn't go on until the end (if at all) - a negative should be
considered as negative not as 0!

And as long as we're working with integer values (which we shouldn't be)
the multiplication-by-a-large-factor is needed to prevent rounding problems.

What about...

  if (consider) {
    value = new_tile_value - old_tile_value;

    if (!inuse) {
      value /= 2; /* rounding error, sigh... */
    }

    value += MAX(extra, 0);

    value *= 64; /* Or some other large number... */

   value = amortize(value, delay);
  }

I think the rest of your patch (MAX_WORKERS, other handling of inuse)
isn't as clearly good as these changes.  Can you make a simple patch
that just fixes the base calculation?

-jason