Complete.Org: Mailing Lists: Archives: freeciv-dev: April 2002:
[Freeciv-Dev] Re: Unit_move_rate 3 (PR#1347)
Home

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

[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: Unit_move_rate 3 (PR#1347)
From: Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>
Date: Wed, 3 Apr 2002 18:03:28 +0200
Reply-to: rf13@xxxxxxxxxxxxxxxxxxxxxx

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.

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

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Life is too short for reboots."


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