[Freeciv-Dev] Re: gen-impr stuff

[Ben Webb <ben@xxxxxxxxxxxxxxxxxxxxxx>]

On Fri, 4 Jan 2002, andi payn wrote:

> I downloaded the latest gen-impr combined patch right before Christmas, and 
> looked through that plus what's in FreeCiv-ac and maybe a few other places, 
> and added a bunch of new code, and I've gotten something hacked together 
> that's at least 95% workable.

        Cool! It would've been handy if you'd let me (as the author of 
the impr-gen-combined patch) know[*] though, since I wrote some code this 
afternoon which looks like it's an almost exact duplicate of your own. ;(
But it's nice to see that I'm not the only one working on generalised 
improvements.

[*] e.g. http://lists.sourceforge.net/lists/listinfo/freecivac-devel,
    or this list.

> First, I added in all of the effects that weren't handled, and changed a few 
> of them around (I don't think pollution adjustments are supposed to be 
> additive, for example), and that's mostly done.  I also added some of the 
> missing things (like reactor meltdowns).

        Well, I've taken a brief look at your patch, and have a few 
questions/concerns/problems. It may be that I'm mistaken about a few 
things, because it _is_ a rather big patch...

1. You don't actually seem to handle _all_ the effects, for example 
   Barb_Attack and Slow_Global_Warm. (Then again, the reason I didn't 
   implement them myself is that Freeciv doesn't use them, and I couldn't 
   be bothered to write code for an effect that I wasn't going to be able 
   to use. See http://arch.freeciv.org/freeciv-dev-200112/msg00967.html)

2. You've either got an old version of my patch, or you've intentionally 
   recoded some things for reasons unclear to me. For example, your 
   handling of Reveal_Map and friends is different.

3. As far as I can tell, your handling of "happy Wonders" (e.g. 
   Shakespeare's Theatre) is wrong. This is the issue discussed around 
   http://arch.freeciv.org/freeciv-dev-200112/msg00757.html, and addressed 
   by the more recent patch at the first URL (actually, it seems to have 
   vanished from Freeciv's FTP server - get it from the FreecivAC pages 
   instead).

4. You overuse is_effect_activated() horrifically. There is no need to use 
   this function in normal usage, as effects are specifically marked as 
   active by update_all_effects(). (In fact, the effect iteration 
   functions should only return activated effects anyway, so calling it 
   inside an effect_iter_iterate loop is pointless.) If you find you 
   need to use this function, then there is a bug in update_all_effects() 
   (or its calls) somewhere, most likely. Calling this function repeatedly 
   will cause CPU usage to explode for any effect that uses the cond_eff 
   field. (That said, I guess you do need to use it in the AI code, 
   because there you're considering effects that don't exist yet, and so 
   haven't been activated by update_all_effects() yet.)

5. As far as I can tell, you do not honour the "survives" field. This 
   means that the Apollo project won't work properly. (This is because my 
   code doesn't support "destroyed" effects fully yet, and this is why I 
   haven't implemented the effects of these Wonders.)

6. I don't like your Enable_Space and Enable_Nuke handling anyway. It 
   looks like it'll only work for World-range. (Doing it "properly" is a 
   bit more fiddly, which is why I haven't finished it yet. ;)

7. Looks like your handling of the various Upgrade effects (e.g. Leonardo) 
   only works for Player-range. I think mine is better. ;)

8. Easy on the effect iterations! They're rather more CPU-intensive than 
   just looking up an improvement ID in an array. This is why I've tried 
   to roll together as many of them as I can into single functions (e.g. 
   do_bonuses_pollution in common/city.c). There have already been enough 
   complaints about the extra CPU usage of "needless generalisation"!

9. You're absolutely right about the pollution modifiers. I let the 
   confusing ruleset comments confuse me, and wrote them all backwards. 
   I'm surprised nobody noticed before. (I'm more inclined to add them all 
   and subtract from 100 rather than multiply though, as this is more in 
   keeping with how the other percentage modifiers work.)

> Then I dug into the AI, and rewrote all of the code to look at effects 
> rather than improvements.

        This is very good news. I was thinking of looking at this myself, 
but the AI code just looked too scary. If you ever succeed in untangling 
the code changes, I'd love to get this into FreecivAC (and thus, hopefully 
in the not-too-distant future, Freeciv proper).

        Ben
-- 
ben@xxxxxxxxxxxxxxxxxxxxxx           http://bellatrix.pcl.ox.ac.uk/~ben/
"Use the force, Luke."
        - Obi Wan Kenobi, 'Star Wars', 1977