[Freeciv-Dev] Re: Railroad Movement Patch

[Thue <thue@xxxxxxx>]

On Wed, 08 Nov 2000 05:51:07 Raahul Kumar wrote:
> 
> 
> This patch updates the AI and movement code(it was patched against
> freeciv-cvs-Oct31, replacing a lot of those
> magic numbers
> based on 3 with constants defined below. This should make it easier to
> support
> more civ variants and make the AI code a little bit easier to follow. This
> was mostly based on Artur Biesiadowski's patch for 1.8.1.

A worthy cause.

> Will someone who understands the AI code please rename those variables? It
> is difficult to understand where those numbers come from in particular:
>
> -3 for sea movement cost appears to be a special number(Does it depend on
> the movement cost of 1 square or not?)

It is just a convention; a movecost of -3 means a boat can move between tiles
in one move.

> maxcost =  72 ( apparently this is computed as a result of threshold * 6)
> 
> How was this number derived?

This constant is arbitrary. It exists to limit the amount of calculations the
AI does.

> In general, whoever is currently maintaining the AI code

aheh. Who is that? :)

> needs to insert a lot of comments.

He truly does!

> It would be nice if the AI was moved away from relying on so many hardcoded
numbers.
> 
> Missing:
> 
> some funcs in server/gotohand.c
> common/map.c:tile_move_cost_ai
> some funcs in ai/advmilitary.c
> some funcs in ai/advunit.c

No way those in the patch plus these are all that need to be changed.

> I've added a few additonal  /* MOVEFIXME */ comments in places where I
> wasn't sure of
> meaning of code (3 or 4 places) - somebody should review those places
> closely.

I think I can do that.

> The plan is to to use 30 instead of 3 as a base - 1/3 for road/river, 1/10
> for rail. It would also allow C:CTP costs (1/2 for river, 1/3 for road,
> 1/5 for rail, 1/10 for maglev). It should also make it easier for SMAC and
> Civ III. There is some additional code currently commented out that allows
> for
> a fixed railroad movement - i.e if you stuck a howitzer or a mech infantry
> on a railroad they would both move the same distance.

Note that you will also have to convert the warmap from char to short to avoid
it overflowing. (or else it would only allow warmaps with routes of ~25 moves).
Some places the number 255 is used; this should be replaced with MAXCOST,
defined in server/gotohand.c currently.

> For now it is defined in unit.h 
> #define SINGLE_MOVE 3
> Also there are definitions for
> #define MOVE_COST_RIVER 1
> #define MOVE_COST_ROAD 1
> #define MOVE_COST_RAIL 0 
> 
> They are
> used only in once place, so it should be easy for grep for them and
> change (or even replace with
> #define MOVE_COST_RIVER game.river_movement_cost
> or anything).
>
> Somebody has to work hard on goto code - there is way too much places
> where magic numbers are used, which probably are dependent on 3. 

Worthy goal, as I said. But not close to finished. The entire code base will
have to be audited. I don't have the time to do that now, but I am willing to
apply the incomplete patch, as it makes the modified code more readable.

-Thue