[Freeciv-Dev] Re: (PR#2574) RFC: (PR# 1762) corruption revisited

["Raahul Kumar via RT" <rt@xxxxxxxxxxxxxx>]

--- Davide Pagnin via RT <rt@xxxxxxxxxxxxxx> wrote:

We need an agreement on *who* is going to do the implementing. I assume I am.
I like some of your ideas. I still think you and Per suffer from feauturitis.

A brand new patch should makie these changes. The waste patch will stay almost
as is.

> btw, we need a little more flexibility to let waste and corruption
> differentiate totally (the max_distance_from_capital variable should be
> game.ruleset defined *AND* should be different between waste and
> corruption).

I *really* don't like this idea. I believe that waste and corruption
are naturally joined. Obviously, a corrupt government *cannot*
be an efficient government.
 
> > if (g->corruption_level == 0) {
> >   return trade_penalty;
> > }
> 
> fine with this, but we need to have courthouse behavior not depending on
> this parameter

Hmm? That particular courthouse behaviour does not occur in this snippet of
code. Beyond the scope of this patch. 
 
> > if (g->fixed_corruption_distance != 0) {
> >   dist = g->fixed_corruption_distance;
> > } else {
> >   capital = find_palace(city_owner(pcity));
> >   if (!capital)
> >     dist = 36;
> >   else {
> >     int tmp = map_distance(capital->x, capital->y, pcity->x, pcity->y);
> >     dist = MIN(36, tmp);
> >   }
> > }
> > dist =
> >     dist * g->corruption_distance_factor + g->extra_corruption_distance;
> 
> This whole distance calculation, is badly flawed, IMHO.
> 
> 1. fixed_corruption_distance, I found counter-intuitive the fact that
> distance_factor and extra_distance are applied to this parameter, and at
> least this should be documented in the appropriate ruleset

It is documented. 

_fixed_distance  = if non-zero, used instead of actual calculation of 
;                    distance from Palace; also used for distances in 
;                    unit and city bribe cost calculations
; _distance_factor = multiply distance by this factor for corruption 
;                    (but not bribe costs)
; _extra_distance  = add this to distance after applying distance factor

> 2. We have a magical number (36), that is as always a bad thing, and who
> nobody knows where it came from. 
> NOTE: I have checked civ2 and civ2 uses 32.
> This number is a cap for distance from the capital or in the event that
> the capital isn't present.
> But again distance_factor and extra_distance are applied to this
> parameter, and again a I found this *VERY* counter-intuitive.

Why? Distance has a max and min limit (0 and 36). Why is that 
counterintuitive? If you'd prefer comments, add them. Waste itself
has a max and min limit. 
 
> 3. I've looked at map_distance, to understand which "distance" is used
> by the code and found that map_distance = abs(dx) + abs(dy).
> An example:
> capital in 0,0
> 
> city in 10,0
> map_distance --> 10
> 
> city in 5,5
> map_distance --> 10!!!
> 
> This is clearly wrong! That city is 5 diagonal square away from your
> capital and suffer same corruption of a 10 square away city!
> We have 2 choices here, IMHO:
> 
> a) dist=sqrt(dx*dx+dy*dy)
> b) dist=max(abs(dx),abs(dy))
> 
> The first one is used by civ2, but don't take into account that you can
> move diagonally with the same cost of moving horizontally or vertically.
> Then the second one is more appropriate for the move calculation we
> adopted.

I agree with you on this. Major buglet.  Someone needs to add a comment
explaining how distance is calculated. 
 
> > val = trade * dist / g->corruption_modifier;
> >
> > if (city_got_building(pcity, B_COURTHOUSE) ||
> >     city_got_building(pcity, B_PALACE)) val /= 2;
> 
> I know that Per isn't convinced of this, but I think that this check
> should be moved down, after the CLIP function.
> This is civ2 behavior and moreover increases the value of the
> courthouse.
> When this 2 choice differ??
> 
> Then you are very distant from your capital and there is a lot of
> corruption!
> 
> Example:
> despotism, city 32 square distant from the capital,
> the city without corruption has 20 trade.
> 
> Without courthouse, in default ruleset, you get:
> val = 20 * (32+5) / 27 = 27
> val = CLIP(0, 20, 27) = 20
> you end up with 20/20 trade lost to corruption
> 
> with courthouse, actually, you get:
> val = 20 * (32+5) / 27 = 27
> val = val / 2 = 13
> val = CLIP(0, 20, 13) = 13
> you end up with 13/20 trade lost to corruption
> 
> with courthouse effect applied *AFTER* clipping, you get:
> val = 20 * (32+5) / 27 = 27
> val = CLIP(0, 20, 27) = 20
> val = val / 2 = 10
> you end up with 10/20 trade lost to corruption
> 
> This change will imply that having a courthouse in a city will save *AT
> LEAST* 50% of your total trade, this is IMHO the behavior you expect
> from the courthouse.

Agreed. Per, I think Davide is right on this one. I intuitively expect a
courthouse to save 50% of total trade lost. 

Aloha,
RK.

I went on to test the program in every way I could devise.  I strained
it to expose its weaknesses.  I ran it for high-mass stars and low-mass
stars, for stars born exceedingly hot and those born relatively cold.
I ran it assuming the superfluid currents beneath the crust to be
absent -- not because I wanted to know the answer, but because I had
developed an intuitive feel for the answer in this particular case.
Finally I got a run in which the computer showed the pulsar's
temperature to be less than absolute zero.  I had found an error.  I
chased down the error and fixed it.  Now I had improved the program to
the point where it would not run at all.
-- George Greenstein, "Frozen Star: Of Pulsars, Black
   Holes and the Fate of Stars"

__________________________________________________
Do you Yahoo!?
New DSL Internet Access from SBC & Yahoo!
http://sbc.yahoo.com