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

["Davide Pagnin via RT" <rt@xxxxxxxxxxxxxx>]

On Sun, 2002-12-15 at 03:31, Raahul Kumar via RT wrote:
> --- 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.

If we change city_corruption() implementation, also city_waste() need
the same changes!

> 
> > 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. 

Instead I *REALLY* like the idea...

> I believe that waste and corruption are naturally joined. 

This is not the point!

> Obviously, a corrupt government *cannot* be an efficient government.

My requests are in the sense of let the modpack writer to control how
corruption and waste spread over the map.
An example:
Civ2 has a different max_distance_from_capital factor between waste and
trade, and different number for corruption_modifier and waste_modifier
for some government.

This is due to the fact that waste (production lost due to inefficient
work) is related to distance but not to corruption, and moreover, that
this hit the total amount lesser.

>  
> > > 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. 

What I mean here is that if you set corruption_level = 0 for another
government other than democracy then courthouse behavior changes also
for that government, and this is wrong, IMHO.

I agree that this is not part of this eventual patch but it is related
argument that I wanted to point out.

>  
> > > 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

Perhaps I was unclear... 

I found counter-intuitive that fixed_distance is *modified* by
distance_factor and _extra_distance.
Then something, in the explanation of fixed_distance, should say:
.. this parameter is applied BEFORE the application of distance_factor
and extra_distance ...

example:
fixed_distance=10
distance_factor=2
extra_distance=5

dist == 10 * 2 + 5 == 25!
Perhaps we don't need to change the mechanics but we need to be clearer.

> > 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. 

Please, _read_ what other people write.

I have wrote:
"But again distance_factor and extra_distance are applied to this
parameter, and again a I found this *VERY* counter-intuitive."

Then, at least, it is clear that I found counter-intuitive applying
distance_max *BEFORE* distance_factor and extra_distance.

Capping distance isn't counterintuitive, nor I have written such
things...

Example, from default ruleset:
capital at 0,0
city at 38,0
government is despotism

dist == MIN(38,36) * 1 + 5 == 41!

I found counter-intuitive that you can have modified_distances over the
max_distance_cap that you have set.

Anyway, my *strong* request is to add another ruleset variable, called
for example max_distance_cap, so that this parameter can be configured
by the modpack writer.

Per suggested instead to have this parameter depending from map
dimensions, and I found this idea interesting but less flexible.

>  
> > 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. 

No, we need to *change* 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.