Complete.Org: Mailing Lists: Archives: freeciv-dev: December 2001:
[Freeciv-Dev] Re: [PATCH] advdomestic.c cleanup (PR#1149)
Home

[Freeciv-Dev] Re: [PATCH] advdomestic.c cleanup (PR#1149)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: Raahul Kumar <raahul_da_man@xxxxxxxxx>
Cc: freeciv-dev@xxxxxxxxxxx, bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: [PATCH] advdomestic.c cleanup (PR#1149)
From: Petr Baudis <pasky@xxxxxxxxxxx>
Date: Thu, 20 Dec 2001 14:03:18 +0100

Dear diary, on Thu, Dec 20, 2001 at 12:42:28PM CET, I got a letter,
where Raahul Kumar <raahul_da_man@xxxxxxxxx> told me, that...
    -  if (bx || by)
    -     return(best);
> > -  return 0;
> > +    }
> > +  } city_map_iterate_end;
> > +  
> > +  return best;
> >  }
> >
> 
> The original function had a return 0. You got rid of it. I believe you have
> tested it and found it does not matter?
It's clear. When bx and by is zero, best is zero as well. Always. It's evident
that this code was much more complicated originally, but got reduced and
simplified.  And this test has no more mean. Even bx and by is not needed
anymore at all :).

> > -static int building_value(int max, struct city *pcity, int val)
> > +/**************************************************************************
> > +How much valuable the building would be for keeping of the city in order?
> > +
> 
> Rephrase it as, 
> The value of the building for keeping the city orderly.
I don't think the mean is same. Maybe

The value of the building (should be improvement, even; and this function
should be renamed, even ;) for keeping of order in the city.

> I wish someone would speak up as to why 16 is a good number for sad value.
> > +#define SADVAL 16
Nice wish :-). Ross? Ben?

> > +/**************************************************************************
> > +Return a weighted value for a city's Ocean tiles
> > +(i.e. based on the total number and number actively worked).
> > +**************************************************************************/
> >  static int ocean_workers(struct city *pcity)
> 
> Formatting changes only.
..?

> > -static int pollution_cost(struct player *pplayer, struct city *pcity,
> > -                     Impr_Type_id id)
> > +/**************************************************************************
> > +Pollution benefit or cost of given improvement.
> > +
> > +Positive return value: less pollution if this improvement would be built
> > +Negative return value: more pollution if this improvement would be built
> > +Higher distance from zero means higher effect, naturally.
> > +**************************************************************************/
> 
> Good comemnts. I like the work you put in here.
Thanks.

Attached revised patch. I changed building_value to impr_happy_val - it maybe
looks more unfriendly at the first sight, but I believe it's more descriptive
and less confusing.

-- 

                                Petr "Pasky" Baudis

UN*X programmer, UN*X administrator, hobbies = IPv6, IRC, FreeCiv hacking
.
  "A common mistake that people make, when trying to design
   something completely foolproof is to underestimate the
   ingenuity of complete fools."
     -- Douglas Adams in Mostly Harmless
.
Public PGP key, geekcode and stuff: http://pasky.ji.cz/~pasky/

Attachment: advdomestic.c.diff
Description: Text document


[Prev in Thread] Current Thread [Next in Thread]