[Freeciv-Dev] Re: [Patch] Cleanup of defense power calculations

[Ben Webb <ben@xxxxxxxxxxxxxxxxxxxxxx>]

On Thu, 28 Feb 2002, Raahul Kumar wrote:

> --- Ben Webb <ben@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> >     Is this even true any more, anyway? It's contradicted by both the 
> > comments and the (unimplemented) effect field in buildings.ruleset.
> 
> Then in that case, this is a definate bug.

Looks like the new defense_multiplication function fixes this, anyway.

> > BTW, I'm hoping that the various AI cleanup patches will do something with 
> > the various _got_citywalls functions
> Raimar, are you going to handle the _got_citywall functions? Ben, you know
> the drill, filename, line nos and func names. Tell me exactly what 
> bothers you.

Well, it's partly code duplication, and partly misleading function names.

        First, there's city_got_citywalls in city.c. I'd like to get rid 
of this entirely for impr-gen, as it simply returns whether the city has 
city walls or not. It makes no distinction about walls that defend against 
land units and those that defend against missiles, etc. Note that the 
impr-gen code handles all the various Unit_Defend effects in a general 
way, so includes Coastal Defense, SAM Battery, and the anti non-nuclear 
missile effect of SDI. Of course, some indication of city walls is 
required for the display of enemy cities; in the FreecivAC patch 
"effect-palace-walls-v3.patch" at http://freecivac.sf.net/ I've chosen to 
display cities which have anti-land unit defences in this way.

        OK, next we have unit_ignores_citywalls in combat.c. This simply 
checks the F_IGWALL flag. Either we need to get rid of this function or 
get rid of explicit tests against F_IGWALL elsewhere in the code 
(presently in get_virtual_defense_power).

        My biggest problem is with unit_really_ignores_citywalls. I find 
this misleading because it always returns TRUE for air units. Air units 
don't "ignore" city walls; it's just that city walls don't defend against 
air units - i.e. it's a property of the walls, not the unit. This of 
course creates problems with impr-gen because it's entirely possible to 
have effects that air units _don't_ ignore - e.g. SAM. This code is 
duplicated anyway in get_virtual_defense_power.

        My partial solution for FreecivAC/impr-gen can be seen as 
effect-combat-v3.patch at the FreecivAC website. I do something similar to 
the defense_multiplication function (I call it 
get_modified_defense_power). I haven't been able to touch the 
ignores_citywalls functions though, as they're used by the AI, which I 
haven't delved into yet.

        Long term, for the AI, from my perspective I'd be looking to get 
rid of all the explicit calls to unit_really_ignores_citywalls and 
city_got_citywalls and make them use more general functions in combat.c. 
For example, the code in assess_defense_quadratic and 
assess_defense_unit (ai/advmilitary.c) seems to largely reproduce the city 
wall handling in combat.c, but introduces yet another bug, as far as I can 
see - i.e. the following code

    if (!igwall && city_got_citywalls(pcity) && is_ground_unit(punit)) {
      v *= l; v /= 10;
    }

assumes Civ2/Freeciv behaviour, so will do the wrong thing for Civ1, I 
think.

P.S. F_IGWALL seems to only be used for land units. The impr-gen code 
     allows it for air units too, so a Fighter with F_IGWALL would ignore 
     SAM. (This is partly because it seemed the logical and general 
     thing to do, and partly because SMAC's Blink Displacer can be applied 
     to all unit types, IIRC.) Is this likely to cause problems? If not, 
     I'd be tempted to rename this to F_IGDEFENSE or similar.

        Ben
-- 
ben@xxxxxxxxxxxxxxxxxxxxxx           http://bellatrix.pcl.ox.ac.uk/~ben/
"So strange, victory - 1,200 spires, the only sound, Moscow burning"