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

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

--- Davide Pagnin via RT <rt@xxxxxxxxxxxxxx> wrote:
> On Sun, 2002-12-15 at 03:31, Raahul Kumar via RT wrote:
> > --- Davide Pagnin via RT <rt@xxxxxxxxxxxxxx> wrote:
<snip>
> > A brand new patch should make these changes. The waste patch will stay
> almost as is.
> 
> If we change city_corruption() implementation, also city_waste() need
> the same changes!

I was thinking of applying the waste patch first, then making the changes
discussed in a subsequent follow up patch. It's much easier for me to do things
in that order.

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

OK. I didn't realise that Civ 2 did that. 
 
> > > > 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.

You want a new flag COURTHOUSE_HAPPY? I have no problems. 

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

Huh? I found the explanation in the ruleset code clear. But I have no
objections to adding docu if you feel it is confusing.

I think an example formula will probably help the readers.

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

I don't like this idea.  Extra work for minimal gain. What I suggest would be
better is instead remove the cap of 36 altogether. Use fixed_distance instead!

Fixed distance works exactly as you suggest, as a max cap on distance. Why
invent another param when the existing one can do the job ?

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

Yes, that's also true. But the fact remains that no player knows how distance
is calculated. Rather than read map_distance, I always assumed it did things
dist=max(abs(dx),abs(dy)) way. 

Help text needs to tell the players how corruption is calculated. Davide, can
you do that? I hate writing documentation.

Aloha,
RK.

An attorney was defending his client against a charge of first-degree
murder.  "Your Honor, my client is accused of stuffing his lover's
mutilated body into a suitcase and heading for the Mexican border.
Just north of Tijuana a cop spotted her hand sticking out of the
suitcase.  Now, I would like to stress that my client is *not* a
murderer.  A sloppy packer, maybe..."

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