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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Feb 28, 2002 at 08:09:50AM -0800, Raahul Kumar wrote:
> 
> --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> <snip>
> 
> > > I assume you going to get rid of unit_vulnerability_virtual. 
> > 
> > I don't know how to do this without behavior change.
> >
> 
> I'm not clear on this. I did a search, and did not find any code in the patch
> using unit_vulnerability_virtual. It all seems to be using
> unit_vulnerability_virtual2.

$ grep -Irn unit_vulnerability_virtual .|grep -v unit_vulnerability_virtual2
./ai/aihand.c:323:              if (unit_vulnerability_virtual(punit) <
./ai/aihand.c:324:                  unit_vulnerability_virtual(defender)) {
./ai/aiunit.c:596:int unit_vulnerability_virtual(struct unit *punit)
./ai/aiunit.c:1243:  u = unit_vulnerability_virtual(punit);
./ai/aiunit.c:1251:    def = (u - unit_vulnerability_virtual(buddy))>>d;
./ai/aiunit.c:1299:        if (!unit_vulnerability_virtual(punit)) q = 0; /* 
thanks, JMT, Paul */
./ai/aiunit.c:1324:    if ((aunit && aunit->ai.bodyguard && 
unit_vulnerability_virtual(punit) >
./ai/aiunit.c:1325:         unit_vulnerability_virtual(aunit)) ||
./ai/aiunit.c:1351:    q /= unit_vulnerability_virtual(punit);
./ai/aiunit.h:50:int unit_vulnerability_virtual(struct unit *punit);
./server/unittools.c:1369:  d = unit_vulnerability_virtual(punit) * db;


> > > That is precisely the problem. 
> > 
> > > Are you willing to guarantee that functions like
> > > defence_multiplication are never called directly?
> > 
> > This makes sense if s/never called directly/never changed/. But I
> > don't understand the original question.
> > 
> 
> You've added a lot of checks to defence_multiplication for example.

Lot of checks?

> If Randon function A calls defence_multiplication directly, your
> change has just caused a change in behavior.

> If no function ever calls defence_multi except through
> get_virtual_defense_power, then we know your change is correct. I can sleep
> the restful peace that only the truly evil can attain.

Someone has to make defence_multiplication first non-static. So there
is no way to call it by incident. If defence_multiplication is made
non-static this isn't my (as the patch author) problem but my problem
(as a maintainer).

> If on the other hand Random function A calls defence_multi expecting terrain
> and fortress bonus only, it will be in for a rude surprise.
> 

> Correct? I'm asking if you have checked the code to see that for example that
> defence_multiplication is only ever used by two functions.

This is a new function and 
$ grep -Irn defence_multiplication .
./common/combat.c:317:static int defence_multiplication(Unit_Type_id att_type,
./common/combat.c:396:  return defence_multiplication(att_type, def_type, x, y, 
defensepower,
./common/combat.c:407:  return defence_multiplication(attacker->type, 
defender->type,

> There are other
> functions for which this might be a problem, but defence_multi is the big one.
> It seems the only other direct caller is get_total_defense_power. There is no
> behaviour change for get_total_defense_power. I'm just asking if these are the
> only two functions that ever call defence_multiplication directly.

Use grep the next time.

> > This isn't POWER_FACTOR. Just a random factor which is local to
> > find_a_good_partisan_spot.
> 
> Interesting. How is 10 a random factor? Don't tell me you belong to the 
> heathen
> church of blasphemous idolators called "We can do random number generation 
> with
> software" ;-).

s/random/ENGLISH(Wald und Wiesen)/ ;)

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Of course, someone who knows more about this will correct me if I'm
  wrong, and someone who knows less will correct me if I'm right."
    -- David Palmer (palmer@xxxxxxxxxxxxxxxxxx)