[Freeciv-Dev] Re: Unit_move_rate 3 (PR#1347)

[Raahul Kumar <raahul_da_man@xxxxxxxxx>]

--- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, Mar 25, 2002 at 07:00:46PM -0800, Raahul Kumar wrote:
> > Author:             Raahul
> > Patch Name:         unit_move_rate3.diff
> > Dependencies:               This patch has no dependencies on other patches.
> > Supercedes:         All earlier versions .
> > CVS version:                diffed against March 22
> > Compile intruction:         The usual ./configure, make, make install
> > Savegames:          Not tested, should be no difference.
> > Compiles successfully:      Yes
> > 
> > The changes made are:
> > 
> > Removal of 
> > 
> > struct player *pplayer = unit_owner(punit);
> > and replacement of int val with int move_rate.
> > 
> > Use of a case statement to replace the barely readable if statements,
> > and insertion of debugging messages using LOG_FATAL.
> > 
> > More progress being made towards the goal of using the same variable names
> > throughtout the code. This is in contrast to the current situation where we
> > have the same thing (punit->move_rate) named move_rate and val.
> > 
> > Thanks to Per for his formatting and debugging comments and Greg for
> spotting
> > the compile error.
> > 
> > 
> > __________________________________________________
> > Do You Yahoo!?
> > Yahoo! Movies - coverage of the 74th Academy Awards®
> > http://movies.yahoo.com/
> Content-Description: unit_move_rate3.diff
> > diff -ruN -Xdiff_ignore /home/rkumar/tmp/cvs-freeciv/common/unit.c
> /home/rkumar/tmp/freeciv/common/unit.c
> > --- /home/rkumar/tmp/cvs-freeciv/common/unit.c      Sun Mar 24 18:15:02 2002
> > +++ /home/rkumar/tmp/freeciv/common/unit.c  Tue Mar 26 12:57:56 2002
> > @@ -34,30 +34,59 @@
> >  
> >  
> >  /***************************************************************
> > -...
> 
> > +Unit move rate calculates the move rate of the unit taking into 
>    ^^^^^^^^^^^^^^ It looks like this means the function
>    unit_move_rate. But if you write it this way this isn't obvious.
> 
> > +account the penalty for reduced hitpoints (affects sea and land units 
> > +only) and the effects of wonders for sea units. --RK
>                                                   ^^^^^
> Should be removed since this is a "general purpose" comment and not a
> personal FIXME one.
> 
> >  int unit_move_rate(struct unit *punit)
> >  {
> > -  int val;
> > -  struct player *pplayer = unit_owner(punit);
> > +
> > + int move_rate = unit_type(punit)->move_rate;
> > +
> > + switch (unit_type(punit)->move_type) {
> > + case LAND_MOVING:
> > +  move_rate = (move_rate * punit->hp) / unit_type(punit)->hp;
> > +  break;
> 
> The identation is correct in the patch but wrong in the applied version.

This you will have to discuss with Mike. He made some formatting changes. I am
not going to undo what one maintainer has done to please another. I'll let the
two of you fight it out. 
 
> > + default:
> > +   assert(0);
> > +   freelog(LOG_FATAL, "In common/unit.c: function unit_move_rate");
> > +   freelog(LOG_FATAL, "Illegal move type %s",unit_type(punit)->move_type);
> > +   exit(EXIT_FAILURE);
> > +   move_rate = -1;
> > + }
> 
> The assert could be moved after the freelogs.

Ok. I still don't agree with you on the underscores, but it's hardly a big
issue.



__________________________________________________
Do You Yahoo!?
Yahoo! Tax Center - online filing with TurboTax
http://taxes.yahoo.com/